Repository navigation
quest: subscribe ranges replace FETCH in lite-07 - #4565
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Recommendation: MERGEReviewed head Positive improvement? Yes. This captures the #4558 decisions cleanly: lite-07 SUBSCRIBE carries past+live group ranges and Worth the complexity? Yes for a planning PR. ~140 lines of quest markdown, lean Required chain (model → lite → ietf → js), and the real complexity (breaking published moq-net APIs, lite-07 draft churn) is correctly parked on Better approach? Not for this stage. Putting ranges on Minor watch-items for the follow-on work (not blockers here): write down the aggregation rule before coding; keep datagram past-range DROPs (#4551) consistent with subscribe-drop's live best-effort story; update the lite draft in the same PR as the wire change as Verdict: MERGE — coherent questline, decisions well captured, scoped correctly to This is an automated review, not the maintainer's decision |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
(Written by OpenAI)
Reviewed SHA 019a9f955385f7d68b3ec97e435f82f1c989b958. This is a planning-only PR: no implementation, public API, or wire behavior changes yet.
Finding [P2]: distinguish the live group's existing prefix from future objects (quest/m1/subscribe-ranges/README.md:35-38, also ietf.md:5-7). moq-transport does not forbid fetching the live group; it caps FETCH at the Largest Object snapshot. Draft-20 §10.13 permits that existing prefix. The current bridge already depends on it: rs/moq-net/src/ietf/subscriber.rs:5602-5611 uses a fill/joining FETCH for the current group's head and SUBSCRIBE for its tail. Applying this plan's blanket exclusion to that path would lose the decodable prefix. Keep older-group standalone backfill separate if desired, but explicitly preserve current-group fill/joining FETCH and correct the claimed protocol prohibition. Add a boundary case where SUBSCRIBE starts mid-group and its prefix is absent from the relay cache.
Direction / complexity: Range-based demand and one FETCH per missing run are worthwhile for sparse tracks, and sharing a model is preferable to maintaining two independent retrieval paths. Keep the representation as normalized ranges of the existing frame-precise Position, with whole-group shorthand: today's model and wire support partial-group resume and an object-level Largest Object cap, which a group-only replacement cannot express. Record that preservation in the model/wire plan before implementation. I would not abandon the quest or add a parallel request_groups API; correct the boundary above and retain the current frame precision.
Verification: Read all eight changed files, repository review instructions, related DROP plan, current subscription model and IETF join/fill paths, and the relevant transport specification. Rechecked open/non-draft status and unchanged head; no prior review from this reviewer on this SHA. No tests run: this changes quest documents only, and the quest CLI is unavailable here. Implementation and interop correctness remain to be verified in the follow-on PRs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Re the automated P2 on the live group (review 5360160866):
(Written by Claude Opus 5.5) |
…stays Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plans
quest/m1/subscribe-ranges/, a questline ondev. A moq-lite-07 SUBSCRIBE asks for past and live group ranges in either order and replaces FETCH. Holes are reported by SUBSCRIBE_DROP, and relays fill misses by range, including over moq-transport.model.md[L],lite.md[L],ietf.md[M],js.md[M]. The existingsubscribe-drop.mdis Required.track-priority-scope.mdandquic/scheduler.md: group order within a track now follows the subscription instead of being "never inverted".Public API: none (quest files only). Wire: none in this PR; lite-07, which is unpublished, will change.
Decision paper trail
Maintainer's proposal (from #4558's open decision): "the next version of moq-lite could use SUBSCRIBE in the past if SUBSCRIBE_DROP is a thing? No need for a separate FETCH method? ... there should be an order: asc/desc enum ... use SUBSCRIBE_DROP to learn about holes ... maybe we could actually have multiple subscribe ranges, and let the publisher avoid duplicating groups ... the api should have a request_groups(start..end) or something."
Goal (SUBSCRIBE replaces FETCH, with ranges, order, and DROP holes)?
Lite FETCH
#4558
Order
Ranges
Holes
moq-transport mapping
IETF range FETCH cut
Model API
Structure
Branch
Max Age
fetch-span relay half
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code