[Frando]

Problem

Every origin::RouteEvent variant carries an Announce, but there is no way to get it without matching all five variants. Code that only wants the prefix or route, or only whether the route ended, repeats the or-pattern across moq-net, moq-relay, moq-stats, tests, benches, and downstream relays reading origin::Routes.

Approach

Add RouteEvent::announce(&self) -> &Announce, RouteEvent::into_announce(self) -> Announce, and RouteEvent::is_end(&self) -> bool, and use them wherever a match only read the route or split on End: moq-net's production code (follow.rs, origin.rs, lite/replay.rs, both publishers), moq-relay's web.rs listing, and the tests, benches, and test helpers in moq-net, moq-net-tests, moq-relay, moq-stats, moq-gst, and moq-tokio. The second main merge also converts the new memory.rs relay test helper and drops the next_update helper main deleted from goaway_cluster.rs.

Matches that tell Start, Update, Restart, or Takeover apart, or bind Takeover's predecessor, stay as they are.

This PR originally targeted announce::Event. After moq-dev/moq#7678 that name is the two-variant Present/Absent listing, and the five-variant route event lives on as origin::RouteEvent, so the accessors moved with it.

Decisions

Decided 2026-11-07 by the iterate; the maintainer delegated the API choice to it.

• ✅ Keep all three accessors, announce, into_announce, and is_end, on origin::RouteEvent (rejected: dropping is_end for matches!, dropping into_announce for announce().clone(), a combined (Announce, bool) accessor). Each has production callers: announce in Follow::fold, take_prefix, Requesting, and Routes::hand_out; into_announce and is_end in both publishers' snapshots, replay::initial, Discovery::local, and moq-relay's web.rs. into_announce avoids cloning the route's captures; is_end pairs with it, since the event is consumed after. main's RouteEvent is unchanged since the PR was opened, and the final API audit (moq-dev/moq#8088) does not touch it. The name announce matches the payload type and Takeover's field; route() was rejected since Announce already has a route field.
• ✅ No accessors on the two-variant announce::Event (rejected: adding announce()/into_announce()), confirming Frando's comment. Every reader in the repo branches on the variant: hang's subscribe example, moq-bench, moq-c, moq-cli (announced.rs, archive.rs, report.rs, publish.rs), moq-ffi, moq-room, moq-tokio (origin.rs, server.rs, client.rs, clock.rs), and moq-webrtc. Most build (announce, present) straight from a two-arm match, which an accessor would not shorten.
• ✅ Switch the remaining mechanical or-pattern callers here (rejected: leaving tests, benches, and other crates for later), so every match that treats the live variants alike reads the same way.

Impact

• New: origin::RouteEvent::{announce, into_announce, is_end}. Additive, no migration. No wire change.

Alternatives

Leaving the pattern to callers. The match is easy to get wrong when a variant is added, and Takeover's struct form makes it noisy.

Caused by

None: an API addition, not a fix.

Validation

• just ci via quest gate: passed on 91aba4a, and again on 81dffbc after merging main

Models

Size: S
Implementer: Claude Code, claude-opus-5-5, unknown effort
Implementer: Claude Code, claude-opus-5-5, medium effort

Review

• Reviewer: Codex, gpt-6.1-sol, medium effort (quest review)
• Session 01a1b844-959d-7aa3-8181-24ca98746455, round 2
• Head 81dffbc against base origin/main 3ce26d1
• Score 9/10: the merge keeps the reviewed behavior, and the newly converted test callers add no actionable regressions.
• Findings: none. Round 1 on 91aba4a also had none.

Record written by hand: quest review printed record skipped for this fork branch.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5 with medium)

[pullfrog]

ℹ️ Minor suggestions only.

Reviewed changes

• announce::Event accessors: adds announce(), into_announce(), and is_end() to AnnounceEvent, each an exhaustive match, so a new variant fails to compile rather than silently falling through.
• Adoption in moq-net: lite/replay.rs derives the active flag and the update from the accessors, and Follow::fold replaces its five-variant or-pattern with announce().
• Changelog: the additive API is listed under ## Unreleased.

ℹ️ The adoption claim stops short of the rest of moq-net

The body says the accessors are used "where moq-net's own source spelled out the pattern", but three production sites still spell it out, two of them in the origin.rs this PR edits. That leaves the claim slightly overstated; converting them here or rewording the body would keep it accurate.

Technical details

# Remaining or-patterns in moq-net production code

## Affected sites
- `rs/moq-net/src/model/origin.rs:6236` — `Requesting::poll_ok` splits `AnnounceEvent::End(_)` from the other four variants and only reads `announce.route.instance`, so it can branch on `event.is_end()` and then take `event.announce()`.
- `rs/moq-net/src/model/origin.rs:7353` — `AnnounceConsumer::hand_out` splits `End` from the rest and only reads `announce.prefix`, same shape.
- `rs/moq-net/src/model/follow.rs:97` — `self.idle = matches!(event, AnnounceEvent::End(_))` is exactly `event.is_end()`.

## Required outcome
- Either migrate these three sites to the new accessors in this PR, or name them in the body's Follow-ups so "where moq-net's own source spelled out the pattern" matches what landed.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

AnnounceEvent adds methods to borrow or consume its Announce and to check whether an event is End. Replay initialization, Follow::fold, and take_prefix use these methods instead of matching event variants. The Unreleased changelog documents the helpers.

Merge Risk: Minimal · up to 1789b

The accessor additions and consumer updates show no identified merge-blocking risk. Mergeable subject to normal build and test checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Title check		The title names the main change, which adds route accessors to origin::RouteEvent. The changeset adds RouteEvent-style methods to AnnounceEvent in origin.rs, and the title's type name differs from the…	Rename the title to match the code. The changeset adds announce, into_announce, and is_end to AnnounceEvent in rs/moq-net/src/model/origin.rs, and the changelog refers to announce::Event. Confirm the correct type name, then update the title…

Passed checks: 4 passed

Check name	Status	Explanation
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description is related to the changeset. It explains that the PR adds RouteEvent::announce, into_announce, and is_end accessors to remove repeated five-variant matches across moq-net, moq-re…

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files. (2 skipped: 1 unsupported, 1 too large.)

Full details: Description check

Explanation

The description is related to the changeset. It explains that the PR adds RouteEvent::announce, into_announce, and is_end accessors to remove repeated five-variant matches across moq-net, moq-relay, and test code. It also gives the reason for the change, the decisions behind it, the validation run, and the impact (additive, no wire change). The raw summary describes the changelog, replay, follow, and origin edits, which match this description. Note that the description names RouteEvent where the summary names AnnounceEvent, and it describes a wider set of call sites than the summary lists.

Full details: Title check

Explanation

The title names the main change, which adds route accessors to origin::RouteEvent. The changeset adds RouteEvent-style methods to AnnounceEvent in origin.rs, and the title's type name differs from the code's AnnounceEvent. The PR description says the target was renamed after #7678, so the title is outdated and does not match the code's type name.

Resolution

Rename the title to match the code. The changeset adds announce, into_announce, and is_end to AnnounceEvent in rs/moq-net/src/model/origin.rs, and the changelog refers to announce::Event. Confirm the correct type name, then update the title to match, for example: feat(net): add announce accessors to origin event type.

Finishing Touches

Simplify code

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3ed1dc79-ded3-4672-9773-e6da18dd6747

Commits

Reviewing files that changed from the base of the PR and between b50557d and 1789b5c.

Files selected for processing: 4

• rs/moq-net/CHANGELOG.md
• rs/moq-net/src/lite/replay.rs
• rs/moq-net/src/model/follow.rs
• rs/moq-net/src/model/origin.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[Frando]

Rebased onto main. #7678 turned the five-variant event into origin::RouteEvent, so the accessors now live there. The three remaining sites from the review (Requesting::poll_ok, AnnounceConsumer::hand_out, Follow's idle flag) use them now, as do routed_now in the ietf subscriber and the snapshot loops in both publishers. Body updated to match.

[Frando]

On scope: I looked at adding announce() / into_announce() to the new two-variant announce::Event as well, and left them out.

Every reader of announce::Event I could find branches on the variant. In this repo that's hang's subscribe example, moq-bench, moq-boy, moq-c, moq-cli, and moq-ffi, and the same holds downstream in iroh-live. Most of them end up with (announce, present), which a two-arm match already gives as cheaply as an accessor would. The only place that wants just the Announce is a single test helper. The five-way or-pattern was the noisy case, and that one is now RouteEvent, which this PR covers. Relays that care about route-level changes such as source, takeover or restart read routes(), and they're the ones the accessors are for.

If a real announce()-only reader turns up, adding the accessor later is a small additive change.

🤖 Generated with Claude Code

[kixelated]

@Frando: an API review of the open PRs and quests (moq-dev/moq#8214) proposes a different shape that removes the need for these accessors. Once moq-dev/moq#8063 deletes Takeover, every remaining variant carries only an Announce, so the event can be a struct:

pub struct RouteEvent { pub kind: RouteKind, pub announce: Announce }
pub enum RouteKind { Start, Update, Restart, End }

This mirrors JS's { kind } & Routed exactly, and callers need no or-pattern and no announce()/into_announce()/is_end(). The plan (quest/m2/api-shape/net.md) records that the reshape lands in this PR, so you keep the credit, if you'd like to take it after #8063 merges. Otherwise it lands with the api-shape work, and this PR can close.

(Written by Claude Opus 5.5 with high)

[kixelated]

@Frando I took this over at the maintainer's request and settled the API (recorded under ## Decisions in the body):

• Kept origin::RouteEvent::{announce, into_announce, is_end} as you had them. Each has production callers in moq-net, and moq-relay's web.rs listing now uses is_end and into_announce too. main's RouteEvent hasn't changed, and the API audit in moq-dev/moq#8088 doesn't touch it.
• Confirmed your call to leave announce::Event alone. Every reader in the repo branches on Present/Absent, mostly into (announce, present).
• Changed: merged main, then switched the remaining or-patterns that treat every live variant the same way. That covers moq-relay, moq-stats, moq-gst, moq-tokio and moq-net tests, the benches, and 13 copies of the next_update test helper. Matches that tell Start, Update, Restart, or Takeover apart are unchanged.

just ci passes locally and a Codex review came back with no findings. The PR is now ready for review.

(Written by Claude Opus 5.5 with medium)

[kixelated]

Automated review by review (OpenAI)

Reviewed head 91aba4a.

No actionable findings. The borrowed and consuming accessors preserve every variant's Announce, including End's last advertised route, while is_end preserves the existing live/retracted split. The migrated snapshot, discovery, request and egress-accounting paths retain their previous behavior. This also addresses the earlier review's incomplete adoption concern without replacing matches that need variant-specific semantics.

Direction is sound for this additive API: centralize payload extraction and avoid cloning when the event can be consumed. Verification limits: GitHub-only static review of the full 34-file diff and surrounding RouteEvent definitions; no tests, builds or benchmarks run. GitHub currently reports merge conflicts, so this is not a claim that the branch is ready to merge. Rechecked open/non-draft state, full head/base and published reviews immediately before posting.

[pullfrog]

✅ No new issues found.

Reviewed changes

• origin::RouteEvent accessors: adds announce(&self) -> &Announce, into_announce(self) -> Announce, and is_end(&self) -> bool, each an exhaustive match, so a new variant fails to compile instead of silently falling through.
• Production adoption: Follow::fold, OriginConsumerState::take_prefix, Routes::hand_out, Requesting::poll_ok, OwnClaims::local, lite/replay.rs, and both publishers now read the route and the ended state through the accessors, with the per-variant behavior unchanged.
• Test and benchmark adoption: converts the repeated next_update/await_event helpers and assertions across moq-net, moq-net-tests, moq-relay, moq-stats, moq-gst, and moq-tokio, including the End-first short-circuits that must not read route on a retraction.
• Changelog: the additive API is listed under ## Unreleased / ### Added, and the body's Impact section covers it.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

@Frando quick update: main moved and conflicted in goaway_cluster.rs, where it deleted a helper I had converted. I merged main again and converted the one new or-pattern it added, in memory.rs. just ci passes again and the review of the new head (81dffbc) came back clean.

(Written by Claude Opus 5.5 with medium)

[pullfrog]

✅ No new issues found.

Reviewed changes

This incremental review covers the delta since the prior pullfrog review at 91aba4a: head 81dffbc merges main (3ce26d1e4) into the branch and resolves the resulting test conflicts.

• Merge conflict in goaway_cluster.rs: adopted the accessors in main's rewritten loop (!event.is_end() && event.announce().route.cost() != Cost::DRAIN) and dropped the next_update helper main deleted, with no dangling callers left behind.
• memory::record adoption: converted main's newly added helper's explicit five-variant match to let active = !event.is_end(); let update = event.into_announce();, keeping the same active flag and prefix recorded.

Both conversions are behavior-identical to the code they replace, no conflict markers remain, and the only prior review thread (Follow::fold) is resolved.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Thanks for this, @Frando. Closing it for now. The final API audit is reshaping origin::RouteEvent underneath it. moq-dev/moq#8063, which is queued, removes the Takeover variant. After that, quest/m2/api-shape/net.md un-hides origin::Consumer::routes, Routes and RouteEvent as the relay-level stream. Accessors on today's hidden five-variant enum would conflict with that and land on a shape that's about to change. If the or-pattern still hurts once RouteEvent is public with four variants, a fresh PR against that shape is welcome.

(Written by Claude Opus 5.5 with high)