feat(net): start and end subscriptions and fetches at a specific frame

[kixelated]

Closes #2517.

Targets dev. The frame-precise work itself is additive, but it leaves Subscription with four loosely-coupled bound fields where the model wants two positions, and closing that properly is a semver break. Rather than ship the half-measure and reshape later, the reshape lands here. See Follow-up below for what it involves.

Summary

Route migration splices per-session tracks at group boundaries, so it never resumes a track whose current group can stay open indefinitely. moq-json::stream keeps its whole append log in one group, and a quiet catalog may not roll for a long time. A route dying partway through such a group stalled the logical subscriber until a group that might never come.

Bounds are frame-precise now, end to end.

The governing rule is that a partial group only ever goes to a subscriber that asked for one. A publisher that cannot serve a group from the frame the subscription names skips it and resolves to a later group, rather than delivering the part it holds. A group is the unit of decodability: dropping leading groups leaves a stream the application can still decode, whereas dropping leading frames often does not, whether because the group opens with a keyframe the rest depends on or because its compression state lives in the frames that went missing. Only the subscriber knows whether a partial group is any use to it. That also keeps every subscriber which does not opt in on exactly the pre-lite-06 behavior.

Wire (moq-lite-06)

• SUBSCRIBE / SUBSCRIBE_UPDATE carry Frame Start and Frame End, qualifying the start and end group. Frame Start is a plain index; Frame End is the index + 1, matching Group End. Two abutting subscriptions therefore carry the same numbers on the wire, which is what makes a splice exact.
• FETCH carries the same pair, bounding the returned frames within the group. A publisher that cannot serve the range in full resets, since the response has no header to report a different start.
• GROUP carries Frame Start, so a stream holding only part of a group is self-describing. It is redundant in the steady state, and carried anyway because a SUBSCRIBE_UPDATE that moves the bound races the group streams already in flight: separate streams, no ordering between them.
• SUBSCRIBE_OK is unchanged. It needs no frame field, because the start frame follows from Group plus the subscriber's own request. Worth stating explicitly since it is easy to get backwards: a subscriber that requested group 5 frame 15 and receives Group = 6 starts at frame 0 of group 6, not frame 15.

Model

• group::Producer::start_at starts a group at a later frame, reusing the eviction tombstone: a reader below the offset gets Lagged either way. A group can be short at its front or its back, never in the middle, so this and finish are the only two ways to trim one.
• group::Consumer gains index / start_at / end_at, mirroring how track::Subscriber bounds group sequences. start_at clamps up to the first frame the group still holds, which is what lets both the publisher and the spliced reader detect a missing head instead of serving one group's frames under another's numbering.
• Subscription gains frame_start / frame_end. The aggregate folds whole (group, frame) positions; folding the group and the frame independently would invent a bound nobody asked for.
• group::Fetch gains frame_start only. A fetch always runs to the end of the group, and a caller wanting less caps the returned consumer. Stopping the fetch short would put a group in the cache indistinguishable from a complete one, so a later fetch of the whole group would resolve from it and come up short. The wire keeps Frame End, applied by the serving publisher as a read cap, so a downstream peer still saves the transfer.
• The fetch cache is coverage-aware. A group cached from a frame-bounded subscription starts partway in, so answering a whole-group fetch from it would silently hand back a tail; it is a miss instead, and a too-narrow entry is replaced rather than colliding on its sequence. fetch_group also returns a consumer positioned where the caller asked rather than at the group's own start.
• resume segments are bounded by position rather than sequence, and a group straddling a boundary is itself spliced. The reader keeps one handle and pulls each route's copy in turn rather than being fed, so a reader that stops polling the subscription (exactly the moq-json case) still gets its continuation. A copy that dies, or that cannot cover the seam, stalls the group instead of erroring it, matching how a dead segment stalls the track.
• A group tracks its committed frame count alongside its written one. create_frame advances the written count when a chunked frame is opened, so a route dying midway through one used to hand the replacement a boundary past a frame nobody ever received in full, permanently stalling any whole-frame reader. It now resumes at that frame.

Public API changes

All additive; nothing renamed, removed, or signature-changed.

• moq_net::track::Subscription: new pub frame_start: u64, pub frame_end: Option<u64>, with_frame_start, with_frame_end. #[non_exhaustive], so external struct literals were never possible.
• moq_net::group::Fetch: new pub frame_start: u64, with_frame_start. Same reasoning.
• moq_net::group::Producer::start_at; moq_net::group::Consumer::{index, start_at, end_at}; moq_net::track::GroupRequest::frame_start.

Not public despite appearances:

• The lite::{Subscribe, SubscribeUpdate, SubscribeStart, Fetch, Group} field additions are internal: mod lite is private in moq-net/src/lib.rs and only re-exports Role.
• group::Consumer was split into Plain / Spliced variants behind the existing surface. All six public read methods were relocated; each signature is byte-identical to main.
• The js/net lite/ changes are internal: js/net/src/index.ts does not export lite, and package.json exports only . and ./zod.

Test plan

• nix develop --command just check — clean (rust + js + drafts + shell/md/toml/gh workflow lints).
• nix develop --command just ci — clean; 2439 tests run, 2439 passed.
• After round 3: just fix and just check clean, just rs test green (2252 passed, 0 failed).
• After the round 4 lint fix: cargo clippy --all-targets -- -D warnings clean, cargo test -p moq-net --lib green (634 passed, 0 failed).
• After round 4, pre-rebase: cargo test -p moq-net --lib green (631 passed, 0 failed), covering the paired setters and the seam fix. Post-rebase the Rust is byte-identical and the only conflict was the changelog above, so the rebased tree is verified by CI rather than re-run locally.
• RUSTDOCFLAGS="-D warnings" cargo doc — clean.
• just drafts check — parses.
• just test smoke — passes.
• just test smoke-full — passes; 21/21 across the matrix (rust, python, and js publishers against rust / python / js / js-native-node / js-native-bun / c / gst subscribers), no failures or skips. The root guide asks for this on a wire change and only the rust-only smoke had been run. Needed SMOKE_PORT to dodge a dev relay on 4443.
• nix develop --command just rs loom — passes (9 + 5 model checks, no deadlock or leaked Arc), re-run on the current head after round 4 changed poll_current. rs/CLAUDE.md makes this a mandatory manual gate for any change under moq-net/src/model/, and this PR rewrites group.rs, track.rs, and resume.rs. It was not run at all before round 3.

New coverage: mid-group takeover splicing frames seamlessly; the old route racing past its frame cap being filtered; rolling past a finished group; a dead copy stalling until its continuation; a copy missing the head stalling rather than serving a tail as a head; a chunked frame that never completed being redelivered; position_group skipping a missing head and capping the end group; start_at / end_at / clamping on the group model; position-folding in the subscription aggregate; and wire roundtrips including a lite-05 byte-compatibility check.

Round 3 adds: frame_bounds_widen_for_an_older_peer, frame_bounds_widen_on_update, and frame_bounds_survive_on_a_lite06_peer on the wire side, plus takeover_splices_a_replacement_that_resends_the_head on the model side, which pins the invariant the widening relies on: a replacement serving the whole group is spliced at the seam, with the re-sent head filtered rather than replayed.

Load-bearing assertions were mutation-checked rather than assumed, since a rebase onto #2526's cache rewrite could easily have made one vacuous. Each of these reverts a fix and fails:

Mutation	Result
resume_position uses the written frame count	1 test fails
fetch cache coverage check removed	2 tests fail
claim_sequence coverage check removed	1 test fails
js publisher ignores startFrame	2 tests fail
js GROUP frameStart hardcoded to 0	2 tests fail
js publisher ignores endFrame	1 test fails

One that does not fail is documented under the coalescing note below.

Review follow-ups

An adversarial review pass raised three things. Two are fixed here; the third I traced and disagree with as stated.

Fixed: range-unaware fetch cache. Reachable, and introduced by this PR's resume path. A relay that subscribes upstream with a frame offset caches a partial group; a later whole-group fetch matched it on sequence alone and resolved to the tail. Now a cached group that starts above the request is a miss, a too-narrow entry is replaced rather than colliding on its sequence, and the end dimension is gone by construction (see group::Fetch above). Regression tests: fetch_ignores_a_group_that_starts_too_late, fetch_widens_or_fails_cleanly.

Fixed: js/net publisher ignored frame bounds. Both the FETCH and the SUBSCRIBE path served the whole group, so a peer asking for a tail would have frame 0's payload land under index N. The publisher now threads the bounds from SUBSCRIBE (and any SUBSCRIBE_UPDATE) through to each group, filters on the frame's real index via readFrameSequence, and puts the actual start in the GROUP header. A group that ends before the requested start resets rather than FINning, since FINning would claim an empty group at that index when the truth is this publisher cannot serve the range.

On severity: the review called the SUBSCRIBE half corrupting, and for a Rust peer it wasn't — JS labelled its frames from 0 and a Rust splice read the ones it needed at their true indices, so it was wasted transfer rather than wrong data. Fixed regardless, because it violated the partial-group rule above and was a trap for any peer that trusted the bound.

Note on the coalescing guard

join hands back &mut to an entry a handler may already have snapshotted into an immutable GroupRequest, so widening a range after hand-off cannot reach the wire. This PR originally guarded the widen behind "still queued", and mutation testing said the guard changed no behavior: restoring the unconditional widen still passed everything, because the write was always a no-op that merely read like a promise. The guard (and Requests::is_queued with it) is gone in round 3 below. What protects the late caller is the coverage check: it refuses a group that starts above what it asked for, so the caller fails cleanly and its retry queues a fresh attempt.

Remaining gap

The js/net subscriber ignores GROUP.Frame Start. Its publisher now honors frame bounds on both subscriptions and fetches, but its group model has no frame-offset concept (no equivalent of group::Producer::start_at), so a received partial group is numbered from 0. A browser therefore cannot yet initiate a frame-precise resume. Not reachable today: a JS subscriber never requests a frame offset, and under the partial-group rule a publisher only sends one to a subscriber that asked.

Review round 2 (CodeRabbit)

• JS served groups outside the requested group range. frameRange handed back a whole-group range for any sequence, and the publisher never had group bounds to begin with, so SUBSCRIBE_START could name a group below the requested start. It now returns nothing outside [startGroup, endGroup] and those groups are skipped. Regression tests on both boundaries.
• The draft forbade a range both implementations accept. It called a Frame End "at or below Frame Start" a protocol violation, but Rust (end < start_frame) and JS (endFrame - 1 < startFrame) both allow equal bounds, which is a legal single-frame range. The spec was wrong, not the code. (The review's stated rationale — that the draft says "through the start of the group" — did not match the text; the contradiction below it was the real defect.)
• budget overflow. end - index + 1 panics in debug when end == usize::MAX. Unreachable from the wire, since varints cap at 2^62-1, but Consumer::end_at is public. Now saturating_add.
• Stale send_update doc and positional-parameter creep in the JS publisher (#runGroup 5 args to 2, #runTrack 6 to 3, via a ServeGroup options type) both applied.

Review round 3

A fresh review pass found one real defect, one piece of dead weight, and three doc errors.

Fixed: no degradation path for peers that predate lite-06

Version::has_frame_bounds was consulted only inside the wire codecs, so a frame-precise demand reached an older peer's encoder unchanged and came back EncodeError::Version. That fails the SUBSCRIBE, which handle_subscription turns into Teardown::GiveBack and the origin turns into a re-splice. The splice itself keeps succeeding, so fails resets on every attempt and MAX_TRACK_RETRIES never trips: the route retries in a hot loop instead of being retired.

Two triggers, and the second needs no route change on our side:

• a mid-group takeover whose replacement upstream negotiated lite-05, which is exactly the mixed-mesh upgrade case (lite-06 is WIP; lite-05 is what is released);
• a lite-06 subscriber downstream, whose frame offset propagates through combine / slice into the relay's aggregate demand and reaches an already-serving older upstream as a SUBSCRIBE_UPDATE.

TrackServe::widen_frame_bounds now widens the request to whole groups for such a peer. Widening is safe there and not in the codec: the codec cannot know who the caller is, so widening at the message layer would hand the caller frames it excluded, and refusing is right. At the session layer we are the caller and we want the wider range, because the read side positions and caps every group copy again (group::Consumer::{start_at, end_at}, driven by resume::Group), so the extra frames are filtered locally and never reach a subscriber. The only cost is transferring frames we already hold.

FETCH takes the same treatment and skips the matching Producer::start_at, so the response is numbered from 0. That is strictly better than the tail: covering_group accepts any cached group starting at or below the request, so the wider group answers the original fetch and stays reusable for anyone else.

Regression tests: frame_bounds_widen_for_an_older_peer, frame_bounds_widen_on_update, and frame_bounds_survive_on_a_lite06_peer to pin that the widening is version-gated rather than unconditional.

Removed: the is_queued coalescing guard

The note above already recorded that mutation testing found this guard behavior-neutral. It is: once the request is popped, GroupRequest holds its own copy of the range, so the write the guard suppresses cannot reach the wire either way. It cost a new pub(crate) API and an O(n) scan of Requests::order on every fetch_group in exchange for nothing. Both are gone. The coverage check is what actually protects the late caller, and fetch_widens_or_fails_cleanly (renamed from fetch_widens_only_while_queued, which was named for the guard) still pins it.

Doc fixes

• hasFrameBounds in js/net listed SUBSCRIBE_OK among the messages carrying a frame index, which is what the Positions section spends a paragraph explaining it does not do.
• The Publisher class's JSDoc had been orphaned above type FrameBounds, leaving the exported class undocumented.
• SUBSCRIBE's Frame Start said "a value of 0 means the whole group", which is the end bound's semantics. FETCH words the same field correctly.

Review round 4

Targets dev: the frame bound is paired with its group

Subscription exposed with_frame_start / with_frame_end as standalone setters, so Subscription::default().with_frame_start(5) was expressible even though a frame index means nothing without the group it counts from. Nothing in the model rejected it. The remote decoder did, as InvalidSubscribeLocation.

with_start(group, frame) and with_end(group, frame) now set the pair together, so the builder cannot express the invalid state. The fields stay pub for reading, so assigning one alone still can, and the encode path reads both halves back through Subscription::start, which drops a frame that has no group rather than emitting one the peer must reject (a_frame_bound_without_its_group_is_dropped).

That change alone is additive against main: both replaced setters were added earlier in this same branch and have never been released. What is not additive is the shape this is halfway to, and that is why the PR now targets dev. See the follow-up section below.

Fixed: a spliced group whose seam no route can serve

Group::poll_current parked until the whole logical track ended, so a group whose missing frames no route could supply stayed open forever. On a relay that is a downstream GROUP stream held open for the life of the track; for a single-group track (a JSON append log, the case this PR exists for) it is the whole track.

takeover derives every boundary from resume_position, which only moves forward, so once it is past a position no future segment will ever cover it and nobody will be asked for those frames again. That is now terminal for the group, and the loss already buried in dead is reported instead of swallowed.

Only once a route has been given up on, though. A live route that has not delivered the group yet may still do so out of order, and its own progress is what moved the resume point past us, so being ahead is not evidence the frames are gone. live_route_ahead_of_the_seam_still_parks pins that gate; without it the test errors instead.

give_up also logs the loss with the group and frame. It was silent, which is why a wedged seam presented as a mysteriously stuck group rather than anything diagnosable.

copy_missing_the_head_stalls becomes copy_missing_the_head_is_lost and now asserts the error. Its old assertion encoded the parking as correct; every future takeover starts above the missing frames, so nobody will ever be asked for them again and saying so beats parking forever.

Known limitation: a live route that silently skips the group

Round 4 closes the case where a route has been given up on. One case remains: a route that is alive, covers the seam, and simply never delivers that group, because it skipped it under the partial-group rule (its own copy was missing the head) and resolved SUBSCRIBE_START to a later group. poll_peek_group parks, nothing marks the route dead, and the reader holds the group open.

The fix is not a timeout. latency_max defaults to Duration::ZERO, which would make every seam give up instantly and destroy the legitimate park the splice design rests on; it is also a cache-retention budget, not a wait budget. The exact signal is already on the wire and already parsed: the draft defines a resolved Group above the requested start as an implicit drop of that leading range, and the lite subscriber currently just logs it. Recording it on the per-session producer would make poll_peek_group return Ready(None) below that group, which marks the route dead, which the round 4 rule then resolves. Left for a follow-up because it reaches into the subscribe-response path and is independently useful.

Follow-up: fold the four bound fields into Position {#follow-up}

Subscription exposes group_start / frame_start / group_end / frame_end as four independently-settable public fields, while the model works in whole Positions. Round 4 closes the builder path, but sub.frame_start = 5 with no group_start is still writable: the invalid state is defanged, not unrepresentable.

Making the fields start: Option<Position> / end: Option<Position> fixes that and is net-negative in code, since set_start / set_end / start() / end() exist only to adapt between the four-field public shape and the two-position internal one. It breaks group_start / group_end, which are released, hence dev.

One piece needs designing rather than renaming: the end bound has three states, not two. Unbounded track, whole end group, and capped frame. Today end() fakes the middle one by mapping frame_end: None to u64::MAX so it sorts above any capped frame in the max_unbounded fold. A plain Option<Position> either leaks that sentinel to consumers or needs an inner Option<u64>, which is two fields again.

Rebase note

Rebased three times: onto #2526, then #2473, then dev.

dev

One conflict, in the lite-06 changelog. dev carries a newer revision of #2473's advertisement bullet ("the later replacing the earlier" rather than "the earlier is served until it ends"), with prose explaining why: holding the path until the transport notices a dead publisher is exactly the reconnect case. Took dev's wording and re-applied only this PR's edit, dropping "at a Group boundary". The ANNOUNCE prose auto-merged into dev's replacement semantics plus this PR's Position sentence.

#2526

Rebased onto main, which includes #2526 (global LRU pool replaced by per-track write-time eviction). That rewrote the structures this PR builds on: groups: VecDeque<Option<(Producer, Instant)>> became lookup: HashMap<u64, Slot>, replace_evicted was deleted in favour of claim_sequence, and Drop moved into an Alive refcount. Resolved by taking main's structure and re-deriving these semantics on top of it, most notably folding the coverage rule into claim_sequence rather than keeping a parallel function. The mutation table above is the evidence that survived the move.

#2473

The lighter of the two: one textual conflict (the lite-06 changelog, additive on both sides) plus one semantic conflict git could not see. #2473 added normative prose saying a relay may splice redundant routes "at a Group boundary", which this PR supersedes; left alone the draft would contradict its own Positions section. Updated to say the splice point is a Position, and dropped the phrase from #2473's changelog bullet so the two entries agree.

That rebase also surfaced a real defect. #2473 added two standby tests that produce a group without writing to it, and they failed: resume_position returned (G, 0) for a group created but never written, so a replacement pointed at frame 0 re-served a sequence the reader already held. The boundary now rolls to the next group unless the copy actually wrote a frame (committed_frames() > first_frame()), which is also why this PR no longer touches origin.rs at all: an earlier round had me adding finish() to tests I do not own to accommodate the old behavior, and those edits are gone.

Cross-Package Sync

drafts/, js/net, and doc/concept are updated. rs/moq-ffi is deliberately untouched: exposing the frame bounds there would ripple through four language wrappers for a primitive whose only consumer today is moq-net's own route resumption, which is transparent to FFI callers.

(Written by Opus 5)

[sourcery-ai]

Sorry @kixelated, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Walkthrough

The change adds frame-precise subscription and fetch bounds for Lite-06 and later. Wire codecs, group state, consumers, publishers, subscribers, caching, and route splicing now carry (group, frame) positions, including partial GROUP streams and bounded FETCH behavior. Older protocol versions retain group-only encoding with validation for unsupported frame bounds. Documentation and tests describe and verify the new semantics.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: frame-specific subscription and fetch bounds in net code.
Description check	✅ Passed	The description is directly related to the changeset and explains the frame-bound protocol and model updates.

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch claude/frame-specific-subs-fetches-6d6a0b

---

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

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

js/net/src/lite/publisher.ts (1)

402-409: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Bundle the added serving parameters into options objects.

The touched private methods now take five or six positional parameters. Group timescale and frame-bound values should be named fields to prevent argument-order mistakes.

As per coding guidelines, replace functions with four or more arguments or repeated groups of values with appropriate structs or abstractions.

Also applies to: 550-556, 580-580

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/net/src/lite/publisher.ts` around lines 402 - 409, Update the touched
private methods, including `#runTrack` and the methods at the referenced
locations, to replace four-or-more positional arguments with an options object.
Bundle the related serving parameters, including timescale and frame-bound
values, as clearly named fields and update every call site to pass and consume
the object while preserving behavior.

Source: Coding guidelines

rs/moq-net/src/model/requests.rs (1)

108-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Match the surrounding style: use the already-imported Borrow/Hash and the same trait-bound ordering as the sibling methods.

Every other method here writes K: Borrow<Q>, Q: Eq + Hash + ?Sized. Visibility also differs (pub(crate) vs pub); intentional narrowing is fine, just flagging the inconsistency.

♻️ Proposed tidy-up

-	pub(crate) fn is_queued<Q>(&self, key: &Q) -> bool
-	where
-		K: std::borrow::Borrow<Q>,
-		Q: std::hash::Hash + Eq + ?Sized,
-	{
+	pub(crate) fn is_queued<Q>(&self, key: &Q) -> bool
+	where
+		K: Borrow<Q>,
+		Q: Eq + Hash + ?Sized,
+	{

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rs/moq-net/src/model/requests.rs` around lines 108 - 113, Update is_queued’s
trait bounds to use the already-imported Borrow and Hash types, matching the
sibling method ordering of K: Borrow<Q> and Q: Eq + Hash + ?Sized. Preserve its
existing pub(crate) visibility.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@drafts/draft-lcurley-moq-lite.md`:
- Around line 1141-1149: Update the FETCH “Frame End” documentation to preserve
0 as the unbounded-through-end default and describe non-default values using
decoded inclusive semantics: the encoded value is the absolute inclusive frame
index plus 1. Keep the existing protocol-violation rule for decoded values at or
below “Frame Start.”

In `@js/net/src/lite/publisher.ts`:
- Around line 73-77: Update frameRange to return no range for sequences before
startGroup or after endGroup, while preserving the existing startFrame/endFrame
behavior within the requested interval. In `#runTrack`, skip consumers whose
frameRange is absent, and ensure runSubscribe passes the bounded ranges to
broadcast.subscribe so SUBSCRIBE_START and SUBSCRIBE_END reflect the request.
Add a regression test covering groups on both sides of each boundary.

In `@rs/moq-net/src/lite/subscriber.rs`:
- Around line 1396-1413: Update the documentation comment for `send_update` to
state that it varies both `end_group` and `end_frame`, matching the parameters
forwarded into `lite::SubscribeUpdate`; leave the implementation unchanged.

In `@rs/moq-net/src/model/group.rs`:
- Around line 977-979: Update the budget calculation in the consumer buffering
logic around self.end and index to avoid overflowing when the inclusive end
equals usize::MAX. Preserve the existing usize::MAX/unbounded behavior and
ensure the computed budget remains valid for end == usize::MAX, including index
== 0, so buffering does not stall.

---

Nitpick comments:
In `@js/net/src/lite/publisher.ts`:
- Around line 402-409: Update the touched private methods, including `#runTrack`
and the methods at the referenced locations, to replace four-or-more positional
arguments with an options object. Bundle the related serving parameters,
including timescale and frame-bound values, as clearly named fields and update
every call site to pass and consume the object while preserving behavior.

In `@rs/moq-net/src/model/requests.rs`:
- Around line 108-113: Update is_queued’s trait bounds to use the
already-imported Borrow and Hash types, matching the sibling method ordering of
K: Borrow<Q> and Q: Eq + Hash + ?Sized. Preserve its existing pub(crate)
visibility.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 908cbad6-94e6-47b8-8ae4-a1207e8a7664

📥 Commits

Reviewing files that changed from the base of the PR and between 67720fa and 9458d7f.

📒 Files selected for processing (21)

• doc/concept/layer/moq-lite.md
• drafts/draft-lcurley-moq-lite.md
• js/net/src/lite/connection.ts
• js/net/src/lite/fetch.ts
• js/net/src/lite/group.ts
• js/net/src/lite/publisher.test.ts
• js/net/src/lite/publisher.ts
• js/net/src/lite/subscribe.ts
• js/net/src/lite/version.ts
• rs/moq-net/src/lite/fetch.rs
• rs/moq-net/src/lite/group.rs
• rs/moq-net/src/lite/publisher.rs
• rs/moq-net/src/lite/subscribe.rs
• rs/moq-net/src/lite/subscriber.rs
• rs/moq-net/src/lite/version.rs
• rs/moq-net/src/model/group.rs
• rs/moq-net/src/model/origin.rs
• rs/moq-net/src/model/requests.rs
• rs/moq-net/src/model/resume.rs
• rs/moq-net/src/model/subscription.rs
• rs/moq-net/src/model/track.rs

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@js/net/src/lite/publisher.test.ts`:
- Around line 144-149: Extend the bounded subscription tests around serveBounded
with a case spanning multiple groups, using distinct start and end groups.
Assert that startFrame limits only the start group, inclusive endFrame limits
only the end group, and intermediate groups are fully served; preserve the
existing single-group test.
- Around line 193-210: Update the incoming stream read loop around reader.read()
to retain the pending read and its timeout handle, clear the timer when either
settles, and on timeout await reader.cancel() before releasing the reader lock.
Preserve normal stream processing and ensure cleanup handles the pending read
without an unhandled rejection.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ee9445e8-17b6-4707-96a9-21c2304b7a33

📥 Commits

Reviewing files that changed from the base of the PR and between 9458d7f and 36ad19d.

📒 Files selected for processing (20)

• doc/concept/layer/moq-lite.md
• drafts/draft-lcurley-moq-lite.md
• js/net/src/lite/connection.ts
• js/net/src/lite/fetch.ts
• js/net/src/lite/group.ts
• js/net/src/lite/publisher.test.ts
• js/net/src/lite/publisher.ts
• js/net/src/lite/subscribe.ts
• js/net/src/lite/version.ts
• rs/moq-net/src/lite/fetch.rs
• rs/moq-net/src/lite/group.rs
• rs/moq-net/src/lite/publisher.rs
• rs/moq-net/src/lite/subscribe.rs
• rs/moq-net/src/lite/subscriber.rs
• rs/moq-net/src/lite/version.rs
• rs/moq-net/src/model/group.rs
• rs/moq-net/src/model/requests.rs
• rs/moq-net/src/model/resume.rs
• rs/moq-net/src/model/subscription.rs
• rs/moq-net/src/model/track.rs

🚧 Files skipped from review as they are similar to previous changes (18)

• rs/moq-net/src/model/requests.rs
• rs/moq-net/src/lite/version.rs
• rs/moq-net/src/lite/group.rs
• rs/moq-net/src/lite/fetch.rs
• js/net/src/lite/connection.ts
• js/net/src/lite/version.ts
• doc/concept/layer/moq-lite.md
• rs/moq-net/src/lite/subscribe.rs
• js/net/src/lite/group.ts
• js/net/src/lite/subscribe.ts
• js/net/src/lite/publisher.ts
• js/net/src/lite/fetch.ts
• rs/moq-net/src/lite/publisher.rs
• rs/moq-net/src/model/subscription.rs
• rs/moq-net/src/lite/subscriber.rs
• rs/moq-net/src/model/track.rs
• rs/moq-net/src/model/group.rs
• rs/moq-net/src/model/resume.rs

[coderabbitai]

🧹 Nitpick comments (1)

rs/moq-net/src/model/requests.rs (1)

104-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a direct test for is_queued's queued→popped transition.

is_queued gates whether a coalesced fetch's frame_start may still be widened (see track.rs::fetch_group), so it directly protects frame-precise coverage correctness. The existing tests exercise pop/join/drain_queued but never assert on is_queued itself.

✅ Proposed test

 	fn popped_stays_joinable_until_removed() {
 		let mut requests = Requests::<u64, &str>::default();
 		requests.add_handler();
 		assert!(requests.insert(1, "a").is_ok());
+		assert!(requests.is_queued(&1));
 
 		assert_eq!(requests.pop(), Some(1));
 		assert_eq!(requests.join(&1), Some(&mut "a"));
 		assert!(!requests.has_queued());
+		assert!(!requests.is_queued(&1));
 
 		assert_eq!(requests.remove(&1), Some("a"));
 		assert!(requests.is_empty());
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rs/moq-net/src/model/requests.rs` around lines 104 - 115, Add a focused test
for Requests::is_queued that inserts or queues a request, asserts it returns
true, pops the request, then asserts it returns false while preserving the
existing ability to join the popped request. Place the test alongside the
existing pop/join/drain_queued tests and exercise the queued-to-popped
transition directly.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@rs/moq-net/src/model/requests.rs`:
- Around line 104-115: Add a focused test for Requests::is_queued that inserts
or queues a request, asserts it returns true, pops the request, then asserts it
returns false while preserving the existing ability to join the popped request.
Place the test alongside the existing pop/join/drain_queued tests and exercise
the queued-to-popped transition directly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ce52cef5-a393-49f3-85e4-b52049e4bec6

📥 Commits

Reviewing files that changed from the base of the PR and between 36ad19d and 42094bc.

📒 Files selected for processing (20)

• doc/concept/layer/moq-lite.md
• drafts/draft-lcurley-moq-lite.md
• js/net/src/lite/connection.ts
• js/net/src/lite/fetch.ts
• js/net/src/lite/group.ts
• js/net/src/lite/publisher.test.ts
• js/net/src/lite/publisher.ts
• js/net/src/lite/subscribe.ts
• js/net/src/lite/version.ts
• rs/moq-net/src/lite/fetch.rs
• rs/moq-net/src/lite/group.rs
• rs/moq-net/src/lite/publisher.rs
• rs/moq-net/src/lite/subscribe.rs
• rs/moq-net/src/lite/subscriber.rs
• rs/moq-net/src/lite/version.rs
• rs/moq-net/src/model/group.rs
• rs/moq-net/src/model/requests.rs
• rs/moq-net/src/model/resume.rs
• rs/moq-net/src/model/subscription.rs
• rs/moq-net/src/model/track.rs

🚧 Files skipped from review as they are similar to previous changes (17)

• rs/moq-net/src/lite/fetch.rs
• js/net/src/lite/fetch.ts
• rs/moq-net/src/lite/version.rs
• js/net/src/lite/connection.ts
• rs/moq-net/src/lite/group.rs
• js/net/src/lite/group.ts
• js/net/src/lite/subscribe.ts
• js/net/src/lite/version.ts
• rs/moq-net/src/lite/subscriber.rs
• doc/concept/layer/moq-lite.md
• js/net/src/lite/publisher.ts
• rs/moq-net/src/model/subscription.rs
• rs/moq-net/src/lite/publisher.rs
• rs/moq-net/src/lite/subscribe.rs
• rs/moq-net/src/model/resume.rs
• rs/moq-net/src/model/track.rs
• rs/moq-net/src/model/group.rs

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

rs/moq-net/src/lite/subscriber.rs (1)

1610-1618: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Store the paired start frame, not the raw field.

start_group comes from subscription.group_start while start_frame comes from subscription.frame_start directly, so a subscription with a frame but no group leaves SubStream holding (None, 3) even though the SUBSCRIBE encoded (None, 0). handle_subscription currently overwrites both before every send_update, so nothing breaks today, but deriving both halves from subscription.start() here keeps the stored pair consistent with what went on the wire.

♻️ Proposed change

 		*sub = Sub::Active(SubStream {
 			stream,
 			id,
 			ordered: subscription.ordered,
 			max_latency: subscription.latency_max,
-			start_group: subscription.group_start,
-			start_frame: subscription.frame_start,
+			start_group: start.map(|start| start.group),
+			start_frame: start.map_or(0, |start| start.frame),
 			priority: subscription.priority,
 		});

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rs/moq-net/src/lite/subscriber.rs` around lines 1610 - 1618, Update the
SubStream initialization in handle_subscription to derive start_group and
start_frame from subscription.start() rather than reading group_start and
frame_start separately. Store the paired values exactly as returned by
subscription.start(), preserving consistency with the encoded subscription.

js/net/src/lite/fetch.ts (1)

22-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Positional constructors growing with new options; both should switch to a props object. Fetch and Group both use positional constructors that just grew (to 6 and 3 params respectively) to carry the new frame-bound fields, while Subscribe/SubscribeUpdate in this same PR already use a props object for exactly this reason.

• js/net/src/lite/fetch.ts#L22-L45: replace the 6-arg positional constructor with a props: { broadcast, track, priority, group, startFrame?, endFrame? } object, mirroring Subscribe.
• js/net/src/lite/group.ts#L5-L23: replace the 3-arg positional constructor with a props object, mirroring the same pattern.

As per coding guidelines: "Use an options object or interface instead of positional parameters for TypeScript/JavaScript APIs that could gain additional options."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/net/src/lite/fetch.ts` around lines 22 - 45, Replace the positional
constructor in Fetch with a props object containing broadcast, track, priority,
group, and optional startFrame/endFrame, preserving the current defaults and
assignments; update all Fetch call sites accordingly. In
js/net/src/lite/group.ts lines 5-23, make the same constructor API change for
Group and update its call sites, mirroring the existing Subscribe props-object
pattern.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rs/moq-net/src/lite/subscribe.rs`:
- Around line 87-131: Mirror the decode invariant in encode_frame_bounds so
nonzero frame bounds require matching group bounds, returning the existing
protocol/version error before encoding invalid data. Apply the corresponding
guard in js/net/src/lite/subscribe.ts lines 6-21 as well as
rs/moq-net/src/lite/subscribe.rs lines 87-131, preserving the existing
unsupported-version validation.

In `@rs/moq-net/src/model/track.rs`:
- Around line 829-862: Update resume_position to derive the latest resumable
group from latest_group rather than max_sequence, so datagram-only sequence
advances cannot skip an open group's remaining frames or intervening groups.
Return None when no group has produced frames, while preserving the existing
committed-frame handling for an open group. Add a regression test covering an
open group followed by a later datagram and asserting the resume position
remains in that group.

---

Nitpick comments:
In `@js/net/src/lite/fetch.ts`:
- Around line 22-45: Replace the positional constructor in Fetch with a props
object containing broadcast, track, priority, group, and optional
startFrame/endFrame, preserving the current defaults and assignments; update all
Fetch call sites accordingly. In js/net/src/lite/group.ts lines 5-23, make the
same constructor API change for Group and update its call sites, mirroring the
existing Subscribe props-object pattern.

In `@rs/moq-net/src/lite/subscriber.rs`:
- Around line 1610-1618: Update the SubStream initialization in
handle_subscription to derive start_group and start_frame from
subscription.start() rather than reading group_start and frame_start separately.
Store the paired values exactly as returned by subscription.start(), preserving
consistency with the encoded subscription.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2775df85-9a65-4ae9-af25-2a69b67efc2d

📥 Commits

Reviewing files that changed from the base of the PR and between 42094bc and e5fb8b7.

📒 Files selected for processing (20)

• doc/concept/layer/moq-lite.md
• drafts/draft-lcurley-moq-lite.md
• js/net/src/lite/connection.ts
• js/net/src/lite/fetch.ts
• js/net/src/lite/group.ts
• js/net/src/lite/publisher.test.ts
• js/net/src/lite/publisher.ts
• js/net/src/lite/subscribe.ts
• js/net/src/lite/version.ts
• rs/moq-net/src/lite/fetch.rs
• rs/moq-net/src/lite/group.rs
• rs/moq-net/src/lite/publisher.rs
• rs/moq-net/src/lite/subscribe.rs
• rs/moq-net/src/lite/subscriber.rs
• rs/moq-net/src/lite/version.rs
• rs/moq-net/src/model/group.rs
• rs/moq-net/src/model/requests.rs
• rs/moq-net/src/model/resume.rs
• rs/moq-net/src/model/subscription.rs
• rs/moq-net/src/model/track.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• rs/moq-net/src/model/requests.rs
• doc/concept/layer/moq-lite.md