[kixelated]

Finishes quest/m0/ci/nightly-relay-session-hour.md.

Problem

Nightly (Relay session benchmarks) ran out of its hour: each pass spent about 22 minutes in the teardown of relay_session_delivery_prefixes/moq-lite-07-wip/broadcasts=1025/follow=1024. A lite-07 viewer holding 1024 announce prefixes has 1024 route streams, each its own router neighbor. Closing each one ran prune, whose View::restrict_all merged one restriction per remaining region at claims x floors per merge, so a session's teardown was cubic or worse. A relay pays the same whenever a lite-07 peer holding many prefixes leaves, so a client controlled the CPU.

Approach

As decided on moq-dev/moq#8536: batch disconnect and single-pass restrict_all first, indexing only where they left it super-linear.

• Batch disconnect. Router::disconnect takes many neighbors and recomputes each identity once. route::Node::closing() returns a guard that holds back each dropped stream's disconnect, then disconnects them together. The lite subscriber and publisher drivers hold it while their announce tasks drop.
• Single-pass restrict_all. It takes the widest regions only (a region within another adds nothing the wider one's restriction lacks), unions their restrictions, and drops covered floors once.
• Per-segment lookups instead of floor scans. as_of, narrow, covering, the merge's stale check and floor settling, retire, Heard::resync, prune's lost floors, and the reader's certified regions look up each segment above a path in the Path-keyed map, instead of scanning every floor or region. certified returns a set. The node's home looks up the routes whose asked prefix lies above a claim instead of checking every route. The rebuild in routes/scope.md deletes most of this; it was needed because the first two steps alone left refollow at 3.4 s per op.
• js/net. Floors.leave and Floors.floor reshow only the routes whose prefix meets the changed route's. A route's claims lie within its prefix, so no other route's can change.

Measured (routes bench, cargo bench -p moq-net --features fuzz --bench routes -- --quick, this host):

Cell	Before	After
routes/teardown/1024p_1c	unbounded (per-stream prune)	1.6 ms
routes/teardown/1024p_16c		22 ms
routes/refollow/1024p_1c	3.44 s	7.6 ms
routes/refollow/1024p_16c		63 ms
relay prefixes/moq-lite-07-wip/follow=1024, whole cell	over 120 s locally, 22 min on CI	3 s
js/net Floors teardown, 1024 prefixes	2345 ms	15 ms

One relay session pass with Nightly's screening args (--quick --warm-up-time 0.05 --measurement-time 0.2) took 139 s here, excluding the lite-06 join cells below. The 11-11 Nightly's pass took 25 minutes, 22 of them in that one teardown.

Choices made without asking:

• Per-segment lookups and the home route index go beyond the decision's preferred minimum; the batch and single pass alone left refollow super-linear.
• restrict_all over nested regions now ignores the narrower one. The old fold was order-dependent there and could leave the narrower region's presented covering claim as a phantom claim at its own path (restrict_all_ignores_a_region_within_another). /verify-route's model (RouteHandoff.tla) describes the path-per-scope prototype in route/tests/paths.rs, not route::Router, View::restrict_all, or the floors this PR changes, so its spec and result are unchanged.
• When every stream closes at once, the merged view stays whole until the route returns or is collected, where closing in turn pruned all but the last region; readers see the same either way (closing_streams_together_matches_closing_them_in_turn).

Impact

• No public API or wire change. moq_net::fuzz (bench-only, fuzz feature) gains RouteFollower.

Caused by

• moq-dev/moq#7676 - restrict_all merged one restriction per region, each scanning every floor.
• moq-dev/moq#8023 - routed lite-07 announce streams through the route layer, one neighbor per stream, each closing with its own prune (the first Nightly after it, 11-08, carried the cost; not bisected).
• moq-dev/moq#8443 - js/net reshowed every route on each route's close or floor.

Alternatives

• Drop the lite-07 follow=1024 Nightly cell: rejected on quest: the relay session hour is lost to a super-linear lite-07 route teardown #8536, it hides a relay cost a client controls.
• An index of floors by region: per-segment lookups into the existing Path-keyed maps give the same bound with no new structure.
• Defer disconnects lazily until the node's next operation: leaves readers stale with nothing to trigger it.

Validation

• just check: pass (12955 Rust tests, js checks).
• bun test in js/net: 3202 pass.
• Route unit tests, including the random view equivalence test against a full scan and the merge fold.
• The relay session group still cannot finish on main: relay_session_join_broadcasts and relay_session_join_stale hang in their moq-lite-06 cells since moq-dev/moq#8476 (bisected; c916855d16 passes, f3edb0b8d9 hangs, reproduced on main 9122f9b6b7 with none of this PR). Follow-up below.

Follow-ups

• A lite-06 viewer joining a relay reads its first group again - the lite-06 join hang above, which keeps Nightly's relay group past its hour until fixed. No open decisions.

Models

Size: M
Implementer: Claude Code, Opus 5.5, medium effort

🤖 Generated with Claude Code

(Written by Claude Opus 5.5 with medium)

Review

Rewritten by quest review after each round.

• Reviewer: Codex, gpt-6.1-sol, medium effort
• Session: 01a1d02f-c5ae-77e2-b71e-7f818e6c641c, round 2
• Head: fb2a9ad
• Base: origin/main at 9122f9b
• Score: 8/10, The update merges base-branch changes with no actionable defects identified in the merge resolution or its interaction with the previously reviewed code; tests and model checks were not run as instructed.
• Findings: none

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Rust route handling now uses region-based lookups and supports grouped stream teardown. New route benchmarks measure teardown and refollow across prefix and claim counts. JavaScript flooring and leaving now refresh only carriers whose prefixes meet the changed carrier’s prefix. The benchmark scope note changed, the relay session-hour note was removed, and a lite-06 join-hang report was added.

Merge Risk: Minimal · up to fb2a9

This change reduces route teardown and refollow cost for sessions with many announce prefixes, with no public API or wire change reported. No actionable merge-blocking risk was identified in the review.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: the route teardown cost for a lite-07 session, which the changeset addresses through batch disconnect and a single-pass restrict_all. It is specific and matches the ob…
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 pull request description is directly related to the changeset. It explains the teardown cost of the nightly relay-session benchmark, the batch disconnect and single-pass restrict_all changes, the …

Full details: Title check

Explanation

The title names the main change: the route teardown cost for a lite-07 session, which the changeset addresses through batch disconnect and a single-pass restrict_all. It is specific and matches the objectives. Minor issue: the title is 80 characters, slightly long but within normal limits.

Full details: Description check

Explanation

The pull request description is directly related to the changeset. It explains the teardown cost of the nightly relay-session benchmark, the batch disconnect and single-pass restrict_all changes, the per-segment lookups, the js/net Floors reshow scoping, the benchmark numbers, and the validation performed. It also names the files and functions that the summary lists, so it is not off-topic or vague.

Finishing Touches

Simplify code

• Commit to this branch
• Create a new PR

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

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: a89a8ddc-e5ab-42d7-87ff-8744466e4cbf

Commits

Reviewing files that changed from the base of the PR and between 709799c and fb2a9ad.

Files selected for processing: 2

• quest/m2/cluster-routing/routes/scope.md
• rs/moq-net/src/route/mod.rs

Files skipped from review as they are similar to previous changes: 1

• quest/m2/cluster-routing/routes/scope.md

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

[pullfrog]

✅ No new issues found.

Reviewed changes

• Single-pass restrict_all — keeps only the widest regions (a region inside another adds nothing the wider restriction lacks), unions their restrictions, and settles the floors once, replacing a per-region merge fold that could leave a narrower region's presented covering claim behind as a phantom.
• Per-segment lookups — new above/within_any/settle helpers replace full floor and region scans in as_of, narrow, covering, the merge stale check, retire, Heard::resync, prune's lost floors, and certified, which now returns a set.
• Batch disconnect — Router::disconnect takes many neighbors and recomputes each identity once; route::Node::closing() defers each dropped stream's disconnect to a Closing guard, held by the lite subscriber and publisher drivers over their announce-task teardown, and State::close is now the single close path.
• home route index — routes are bucketed by the prefix their stream asked for, so each shown claim looks up its carriers per segment instead of scanning every route; the carries filter (including the link scope) still runs, so the indexed set stays exact.
• js/net Floors — leave/floor reshow only routes whose prefix meets the changed one, which is sound because certifies requires the view's prefix to be an ancestor of the queried path and a route's claims lie within its own prefix.
• Bench and quests — fuzz::RouteFollower plus routes/teardown and routes/refollow cells swept over prefixes × claims, the teardown quest retired, and the lite-06 join hang split into its own follow-up.

Checks run here: cargo test -p moq-net --lib --features async route:: (126 passed, 2 ignored, including restrict_all_ignores_a_region_within_another, lookups_match_a_scan_of_every_floor, and closing_streams_together_matches_closing_them_in_turn) and bun test src/lite/floors.test.ts (6 pass). RouteHandoff.tla models only the root interest, so the claim that the model's spec is unchanged holds, and no inbound link to the retired quest slug remains.

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

[pullfrog]

ℹ️ No blocking issues — the merge changed nothing in the code this PR owns. One sentence of the PR body went stale against the new TLA+ model that landed with it.

Reviewed changes

• Delta since the prior review — fb2a9ad is a merge of origin/main (9122f9b) into the branch. The range-diff shows only main's own comment and doc wording arriving alongside; the PR's patch is byte-identical (same per-file diff hashes), so nothing about the teardown fix, the batch disconnect, or the Floors narrowing changed.
• Merge resolution checked in the code it touches — cargo check -p moq-net --lib --features async is clean at fb2a9ad and cargo test -p moq-net --lib --features async route:: passes (128 passed, 2 ignored), so the merged certified set, within_any lookups, and State::close still compile and hold against main's new Shown::presented work.
• Previously reviewed at 709799c — single-pass restrict_all over the widest regions, per-segment above/within_any/settle lookups, batch Router::disconnect with Node::closing() guards held by the lite subscriber and publisher drivers, the home route index, the js/net reshow narrowing, and the new routes/teardown/routes/refollow cells.

ℹ️ The PR body's reason for skipping model work no longer describes the model

The body justifies "/verify-route's spec is unchanged and its result with it" by saying the TLA+ model "only covers the root interest". Main's #8541, merged here, replaced that model with one over two prefixes (root and a) whose whole subject is routes per region. The conclusion still holds, so this is a wording fix only, but the stated reason now reads as a claim about a model that no longer exists.

Technical details

# Restate the model-coverage reason in the PR body

## Affected sites
- PR body, "Choices made without asking" bullet 2: "The TLA+ model only covers the
  root interest, so `/verify-route`'s spec is unchanged and its result with it."
- `rs/moq-net/verify/route/RouteHandoff.tla` (at `fb2a9ad`) models the prototype in
  `rs/moq-net/src/route/tests/paths.rs`, two prefixes `root`/`a`
  (`Keys`, `Covers`, `Restrict`), one floor per view key (`s.view[k].floor`), and has
  no `prune`.
- `.claude/skills/verify-route/SKILL.md` `## Coverage`: "The model describes the
  prototype ... `route::Router` does not run this rule yet."

## Required outcome
- The sentence should say why this change is outside the model, e.g. that the model
  keeps one floor per view key and has no `prune`, so `restrict_all`'s merging of a
  view's several floors (and nested-region choice) is not modelled — rather than that
  the model is root-only.

## Open questions for the human
- Whether the model, which now has two nested prefixes, should eventually cover the
  multi-floor merge; nothing in this PR needs it, since it does not model `prune`.

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