Repository navigation
fix(net): widening an interest ends no live claim - #8540
Conversation
Quest-Claim: ca7377368cd58350
…d, even if it changed show() treated a vanished claim as moved only when the claim above it had the same item, so a claim that changed while the client could not hear it read as withdrawn once the interest widened, and the reader saw END. A shown claim now records whether it sits at a presentation root; such a claim moves up whenever anything above it is still shown. MOQ_ROUTE_SCENARIOS sets how many scenarios the handoff sweeps run, and a `sweep` nextest profile lets a long run finish. Co-Authored-By: Claude <noreply@anthropic.com>
kixelated
changed the title
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 22 minutes. View limit details
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. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the full PR at 62e812c (both commits, 7 files) and re-ran the routing suite locally.
presentedonShown— records whether the row sits at the root of a floor region of the merged view, so a row that may stand for whichever claim covers that region can be told apart from a claim at its own path. Recording it on the stored entry is what makes the classification possible after the region itself is gone.- Relaxed move rule —
movednow acceptswas.presented || now.item == was.item, so a presented row that vanished while any strict ancestor is still shown counts as moved up instead of ending. Any other row keeps item equality, so a real withdrawal under a live claim still ends. - Regression tests —
widening_past_a_changed_claim_moves_it_upshrinks the scoped sweep failure to seven steps on three nodes, anda_claim_withdrawn_under_a_live_one_endspins the item-equality side that no test covered before. - Scenario knob and
sweepprofile —MOQ_ROUTE_SCENARIOSlets both handoff sweeps run past the gate's 150 from the same seeded stream, and[profile.sweep]gives that opt-in run a bounded hour instead of the tight default timeout. - Quest completion —
widen-false-end.mdis deleted with both inbound links, leaving no stale references (the slug grep is clean).
The trade-off in Impact is sound: a presented row can be withdrawn itself while an ancestor still announces its path, so the batch cannot prove a publisher finish, and Offline is the conservative side given that a clean end means only a deliberate finish. I traced the staleness question too: every floors-key change, view swap, prune, retire, or ctx change marks the id's claims all-dirty, and the #[cfg(test)] incremental-view assert compares the full recompute including presented, so the stored flag cannot drift.
Verification: cargo test -p moq-net --lib route::tests → 89 passed, 2 ignored (8.1s); MOQ_ROUTE_SCENARIOS=1000 on sweep_scoped_handoff → clean in 9.1s, which covers scenario 750, the reported first failure (the 20,000-scenario run was started here but the sandbox cut the background process off). Both new tests fail under mutation: removing was.presented || makes the widening test report a false end, and replacing the predicate with a bare is_some() makes the withdrawal test report Offline instead of End. rustfmt --edition 2024 --check is clean on the changed Rust files.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
…00 nightly Scenario 16316 of sweep_handoff failed its false-END assertion, not the revival one, at 688312b. #8540 fixed it: undoing that fix's one line brings the failure back. The shrunk case is now a plain test, and nightly runs the handoff sweeps over 20,000 scenarios. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Finishes quest/m2/cluster-routing/routes/widen-false-end.md.
Problem
A client that widens its interest can see END for a claim its publisher still has.
sweep_scoped_handoffhits it in 25 of 20,000 scenarios (first at 750).Cause:
showreports a vanished claim as moved (offline), not ended, only when a claim above it is shown with the same item. A claim presented at a narrow interest stands for whichever claim covers it. If that claim changed attributes while the client could not hear it (here the origin re-announcedlive/while the client was cut off), the widened stream's first batch removes the stalelive/b/zand showslive/with the new item in the same recompute. The items differ, so the move reads as a withdrawal. Scopes, scenario order and session-close order are incidental: three nodes and no grant reproduce it.Approach
Shownrecordspresented: the claim sits at the root of a region its merged view was presented at, so it stands for whichever claim covers that region. A presented claim that vanishes counts as moved when anything above it is still shown, whatever its item. Any other claim keeps the item-equality rule, so a real withdrawal under a live claim still ends.widening_past_a_changed_claim_moves_it_upis the case shrunk from scenario 750's 13 steps to 7 on three nodes.a_claim_withdrawn_under_a_live_one_endspins the other side, which no test covered (dropping the item check entirely passed all 124 route tests).MOQ_ROUTE_SCENARIOSsets how many scenariossweep_scoped_handoffandsweep_handoffrun (default: the gate's 150). Each sweep draws from one seeded stream, so a larger count replays the gate's scenarios and continues past them. handoff-revival now names it.sweepnextest profile (just rs test -P sweep ...) bounds a test at an hour instead of the default 60s, which kills a 20,000-scenario sweep at ~79s.show), not in the Babel machinery it deletes, so this is a fix, not aresidual_pin.Sweeps with the fix (
MOQ_ROUTE_SCENARIOS=N just rs test -P sweep -p moq-net -E 'test(=route::tests::handoff::<sweep>)'):sweep_scoped_handoff: 20,000 scenarios clean (79s). Before: 25 false ENDs.sweep_handoff: first 16,316 clean (127s), which ends just before the known revival at 16,316 that handoff-revival owns.Impact
js/nethas no counterpart of this reader logic to change.Alternatives
Claim::instance) instead of the whole item: the router is generic over the item and has no identity notion, and same-origin claims can share attributes.Choices made without asking
MOQ_ROUTE_SCENARIOS.MOQ_RESUME_CASESinmodel/resume_model.rs.sweepnextest profile rather than raising the default timeout, which.config/nextest.tomlasks to keep tight.Caused by
Follow-ups
None.
Models
Size: M
Implementer: Claude Code, claude-opus-5-5, medium effort
Review
Rewritten by
quest reviewafter each round.01a1ceab-e24a-7f20-93b4-b5bc77baf175, round 1The targeted fix preserves ordinary withdrawal handling and adds focused regression coverage, though builds, tests, and model checks were not run as instructed.(Written by Claude Opus 5.5 with medium)
🤖 Generated with Claude Code