Repository navigation
quest: a path is the only content identity - #4761
Conversation
Record the 2026-10-03 decisions that follow #4741 dropping first-hop identity: broadcast epochs move to m0 and gate the next release, --hop removal joins that line (with the hop stamp and NO_CAPACITY removed), same-epoch importers re-key to --epoch, wildcard double claims are settled by interchangeable output, and a new quest makes transcoders start at group boundaries. Stale reply-Origin, first-hop, and resume.rs references across the routing, handover, and untimed quests now match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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 4 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (37)
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. Comment |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit dfa7b7a.
P2: The new group-boundary rule needs to survive caching relays, not just requests reaching the transcoder (quest/m1/transcode-group-start.md:5-10). See the inline finding.
Direction: making epochs the content identity and gating the release on publisher migration is a useful simplification. Removing route identity is only safe once the replacement content contract holds end to end; deterministic catalogs and matching group numbers alone do not establish that. Make the cached-relay failover case a prerequisite of the wildcard double-claim change.
Verification: static review of the quest changes, current stats/transcoder publishing paths, and #4741's pump/cache implementation. No tests or live failover were run; this is a planning-contract finding, not a claim that an implementation test failed.
(Written by OpenAI)
| two encoders' output mid-group. moq-transcode (`rs/moq-transcode`, and | ||
| `moq transcode` in moq-cli) resolves a subscription that starts at (N, M>0) | ||
| to (N+1, 0), and refuses a FETCH that starts mid-group. Output groups mirror |
There was a problem hiding this comment.
P2: Preserve the group-boundary restriction through cached relays. Suppose worker A and worker B produce different encoded bytes for group N at the same derived path, each behind a relay. A downstream has consumed A's frames 0..M, then switches to B's relay, which already cached B's whole group for another viewer. That relay can serve (N,M+1) directly without any request reaching the transcoder, so the rule here never runs. #4741's pump continues the existing group (even a partial frame) by index and assumes duplicate bytes are identical; its track cache also satisfies a covering mid-group FETCH directly. The result can combine A's keyframe/reference state with B's predictive frames. Require an end-to-end group-only resume contract that caches preserve, or byte-identical output / distinct identities, and add a two-caching-relay failover test rather than only direct transcoder SUBSCRIBE/FETCH tests.
Review (dfa7b7a)Moves broadcast epochs to m0 as a release gate for #4741's path-only identity, folds BlockingNone. Non-blocking
Verdict: MERGECross-check: #4741 still open (planning companion; prior Grok MERGE with NBs that epochs are still required — this PR is that plan). moq.pro#2165's This is an automated review, not the maintainer's decision |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merge summaryMerged
#4741 stays open; nothing here depends on it merging. (Written by Claude Opus 5.5) |
Re-review (7a78d36)Follow-up after push from Fixed since last review
Still open (non-blocking)
BlockingNone. Verdict: MERGECross-check: #4741 still open (head This is an automated review, not the maintainer's decision |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit 7a78d36; follow-up to dfa7b7a, excluding changes inherited from main.
The two new notes usefully expose the wildcard ship-order risk and the stats exception to the release gate. No new actionable bugs found in this delta.
P2 remains: the existing cached-relay finding is not addressed. wildcard/README.md:172–174 now limits the risk to “until that m1 quest lands,” but transcode-group-start.md:7–21 is unchanged and constrains only transcoder requests. A replacement relay with the other worker's group cached can still serve its suffix without contacting that worker, mixing encoder outputs within a group. I independently rechecked #4741's current resume.rs:833–859 and cached-fetch path; they still permit this.
Direction: keep the simpler path/epoch identity, but make an end-to-end group-boundary contract that survives caches, plus a two-caching-relay failover test, a prerequisite of treating claim workers as interchangeable. Otherwise require byte-identical output or distinct paths. The new warning alone does not close this correctness gap.
Verification: static delta and targeted contract/code review only; no tests or live failover run. Existing finding referenced rather than duplicated inline.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Main's #4741 makes the path the only content identity, and the wildcard README (#4761) drops the reply Origin. Take main's code for everything the line built on Origin: SUBSCRIBE_OK/FETCH_OK Origin, track provenance, front Identity and Pin (incl. Stay), the datagram hold behind SUBSCRIBE_START, the JS origin hop, and their tests (pool_failover.rs, lite/origin.test.ts). Re-applied on main's structure: - route_order keys its hash on the requested path, so an equal-cost pool spreads its paths (Rust and JS spreadHash, pinned to match), plus the Spread Hash tie-break in the lite draft and relay cluster docs. - NO_CAPACITY removed from the lite and cluster drafts and the JS StreamCode; a closed dynamic route rejects with Unroutable. - The equal-cost pool churn benchmark, now with fronts following the newcomer as main's front does. The pool resolve bench is dropped: it relied on a refusal ending the front. - js/watch counts hidden service claims when gating renditions. Main's front still skips a refuser and re-selects (#4741), while the README decides a refusal is final; the README now records that gap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
Summary
Planning for #4741, which makes a path, with its
@<uuidv7>epoch, the only content identity. Quest changes only; nothing outsidequest/.quest/m0/broadcast-epoch/), ranked after Wildcard as a new identity track, and gate the next release: since fix(net)!: resume route changes by reading the routes' copies; a path is one broadcast #4741, a publisher that restarts its groups under the same name resumes into the old broadcast and stalls viewers. fix(net)!: resume route changes by reading the routes' copies; a path is one broadcast #4741 can merge to main; no release ships until first-party publishers mint epochs. moq.pro pinsrelease, so the gate covers it too.--hopjoins that line (quest/m0/broadcast-epoch/hop-removal.md): an optional--epochreplaces--hop/MOQ_HOP/--originand is minted per run when omitted. A plain publisher declares a random Hop ID per process, and--cluster-idstops falling back to--hop. Folded in: drop the per-session hop stamp, and remove NO_CAPACITY (every refusal is terminal) from code, js/net, and the in-progress lite-07 and cluster-02 drafts.Originand the "never splice across Origins" rule are dropped. The line branch must remove its Origin/Identity/Pincode and feat(net): re-resolve once on a NO_CAPACITY refusal #4050's NO_CAPACITY re-resolution when it next merges main.The moq.pro half is moq-dev/moq.pro#2165.
Open for review
route-wakesfollows the wildcard line's decision that a serving front stays put, while main (and fix(net)!: resume route changes by reading the routes' copies; a path is one broadcast #4741's tests) still moves a serving front to a cheaper route. That holds until the wildcard line lands; confirm Stay is still the plan.quest/m1/qos/final-lag-sample.md, with open PR quest(qos): block the line on the #4298 splice lag findings #4381) samples the splicer's segments, which fix(net)!: resume route changes by reading the routes' copies; a path is one broadcast #4741 deletes; left unchanged.Decisions (2026-10-03 planning interview)
Goal. Proposed: "Nothing in moq or moq.pro treats who published a route as what it carries: a path (with its @epoch) is the only content identity, and every first-party publisher that can restart mints a fresh epoch, so dropping identity in #4741 stalls nobody."
Scope (multi-select)
--hopbecomes--epochPriority of the broadcast-epoch line
Wildcard double claims
A plain publisher's Hop ID after
--hopgoesNO_CAPACITY re-resolution
What "deterministic" transcoders means
Stamping (first asked; answered "It means we would not advertise a route back to the original publisher"; checked: split horizon rests on
viaand own-hop checks, and the stamp never equals any id, so it doesn't)moq.pro recording key
moq.pro 0.16 adoption
Transcoder rule placement
moq.pro milestone
Epoch line rank in upstream m0
Wildcard lite-07 reply
OriginfieldStamping and NO_CAPACITY removal
--hopRemove
--hopplacementmoq.pro derived services (overlay, voice response,
.dash)(Written by Claude Opus 5.5)
🤖 Generated with Claude Code