Repository navigation
refactor(net)!: one with_range cursor setter - #6138
Conversation
One consuming with_range builder on track::Subscriber, track::Ordered,
group::Consumer, and Subscription assigns the floor and cap. poll_start,
poll_drop, group::Consumer::poll_expired, Position::final_sequence, and
Subscription::{start, end, with_start, with_end} are public. poll_live and
latest_sequence wait on the track-live quest; Consumer::final_sequence on
the deferred quest.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mirror the Rust with_range in @moq/net: withRange replaces setGroups,
withGroups, and hooks.replaceGroups, assigning both bounds and re-offering
unread cached groups below a lowered floor. tryRecvGroup returns the Recv
union with a public groupChanged wake, Ordered gains tryNextGroup, and
finalSequence and Track.Consumer.info({ hold }) are public, so the lite and
IETF publishers drop their hooks. The json and flate snapshot consumers
drain held groups instead of raising the floor, keeping the caller's cap.
Narrow the serve quest to the JS group expiry decision, move the live-edge
answers to track-live, and add MIGRATE entries for both packages.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dba67edc132 # Conflicts: # js/net/MIGRATE.md # js/net/src/track.ts # quest/m1/egress-held-state.md # quest/m1/subscribe-ranges/frame-bounds.md # rs/moq-hls/src/export/archive_tests.rs # rs/moq-net/MIGRATE.md # rs/moq-net/src/ietf/publisher.rs # rs/moq-net/src/lite/publisher.rs # rs/moq-net/src/model/group.rs # rs/moq-net/src/model/resume.rs # rs/moq-net/src/model/subscription.rs
Group.Consumer.expired settles with the Old error once the subscription's budget gives up on a group, even after every frame was read, mirroring the Rust poll_expired. It watches the budget only while raced, so a race per write leaks nothing. The lite and IETF publishers race their writes against it, and hooks.guardGroup, hooks.readGroupFrame, and ReadGroupFrame go away. A read no longer holds the consumer's closed open while the frame is written: expired already judges a group read whole, and holding closed on every read would hang a reader that stops at the last frame without a trailing read. serve.md keeps the one serving item left, the crate-private Rust poll_abandoned that #6140 added. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly identifies the primary change: replacing multiple cursor setters with one consuming with_range API. |
| Docstring Coverage | |
Docstring coverage is 82.81% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 128 functions across 48 files. (4 skipped: … |
| 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 directly explains the with_range API changes, related bug fixes, affected Rust and JavaScript components, tests, compatibility impact, and validation results. |
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 82.81% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 128 functions across 48 files. (4 skipped: 1 unsupported, 3 too large.)
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.
Comment @coderabbitai help to get the list of available commands.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b9a1c1141
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # js/net/MIGRATE.md # js/net/src/internal.ts # js/net/src/track.test.ts # js/net/src/track.ts # quest/m1/subscribe-drop-errors.md # rs/moq-net/MIGRATE.md # rs/moq-net/src/lite/publisher.rs # rs/moq-net/src/model/resume.rs # rs/moq-net/src/model/track.rs
A cut group is handed out with its error since #6143, so lowering the floor re-offers it like any other cached group. A reader that closed itself stays closed: withRange no longer refills its buffer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged Merge resolution (against moq-dev/moq#6143 and moq-dev/moq#6183):
Codex findings, both fixed in 08a0c34:
Checks: (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08a0c34c0c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Resolve against #6206 and #6214: - The lite publisher keeps #6206's held first group and pre-START skipped drops, reading through the public `tryRecvGroup({ drops: emitRange })` and raising the floor to the resolved `startSequence`. - `hooks.sourceStart` (#6206) stays private, pending serve.md, as the JS counterpart of the now-public `poll_start`. - `hooks.guardGroup` returns for #6214's parked group opens only: `Group.Consumer.expired` covers the budget, not eviction. Its public shape joins `poll_abandoned` in serve.md. - model-markers follows a `hooks` read split across lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged Merge resolution (against moq-dev/moq#6206 and moq-dev/moq#6214):
Checks: Open: the Codex P2 on @codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d7bdf39d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Not re-queued. The fresh Codex review of Together with the earlier P2 (recovery lease released before a blocked write finishes), both findings reopen decision 6 and its amendment, so the maintainer decides how to proceed. (Written by Claude Opus 5.5) |
Keep #6209's datagram identity set beside the reader's `with_range` bounds, and its `MAX_DATAGRAMS` in internal.ts, where `finalSequence` already left for the public `track.ts` export. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A publisher writing a group now stops once `expired` settles or `closed` fails (Rust: `poll_expired`, or `poll_finished` erring), so an eviction of what it has yet to read resets a stream blocked on flow control. Racing `expired` alone missed it. - Delete JS `hooks.guardGroup` and Rust `group::Consumer::poll_abandoned`. Both publishers share a session-side helper over the public calls. - Rust `poll_finished` ends cleanly at a `with_range` frame cap, as reads do, so a served range is not reset when frames past it fail upstream. - A closed JS subscriber keeps its groups' arrival times, so a group read from a copy the resumed reader let go of still expires at its budget. - serve.md drops the settled abandonment decision, leaving JS `sourceStart`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Implemented amendment 2 to decision 6 in
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fa2d1734f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # js/net/src/lite/publisher.ts # js/net/src/track.test.ts # rs/moq-net/MIGRATE.md
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's #6493 keeps a held group's recorded successor so its wall clock outlives the successor's eviction, and drops #isStale's timeline identity check. This branch keeps the arrivals a closed subscriber held. Both stay: a closed subscriber judges by its kept arrivals, and a valid recorded successor still wins over a later kept one. Main's two new guardGroup tests race group.expired instead, as hooks.guardGroup is gone here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…xpired Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@coderabbitai review Please focus on the merge resolution at (Written by Claude Opus 5.5) |
|
|
Codex adversarial review (local (Written by Claude Opus 5.5) |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # js/net/src/lite/publisher.ts # js/net/src/track.ts
One range setter for a reader's cursor, plus the bug fixes found while building it. Reshaped after the public-model goal was dropped (moq-dev/moq#6581): an item goes public only with a named consumer outside the sessions, so the serving calls this PR first made public are internal again.
What changed
with_range(JSwithRange). One consumingwith_range(self, range) -> Selfontrack::Subscriber,track::Ordered,track::Subscription, andgroup::Consumerreplacesset_groups,with_groups,start_at,raise_start_to,end_at,subscription::Cap, andgroup::Consumer::set_frames. Both bounds assign, in either direction. Lowering the floor offers the cached groups below the old floor that the reader never read, in every cursor mode (Rust Plain rescans, Rust Resume re-offers on the serving copy, JS re-mirrors from the producer cache). A group still goes out once. Sessions that move a live reader use a crate-privateset_range.Consumers: js/json and js/flate snapshots, js/room chat, moq-hls, moq-mux, moq-archive, moq-json, moq-flate, moq-room, moq-stats, moq-cli, moq-audio, and moq-video, plus the lite and IETF publishers. (The moq-c
RawReaderthe original plan named no longer exists onmain.)Bug fixes, each with a regression test:
latest()drain the groups the reader already holds and keep the newest, as the Rust consumers do (moq-dev/moq#5677). The old code raised the floor, which dropped the caller's cap, and a datagram past every group left the read waiting forever. This needs a non-blocking ordered read, soTrack.Ordered.tryNextGroup(): Group.Consumer | undefinedis public, mirroring Rustpoll_next_groupand the existingSubscriber.tryRecvGroup().Lagged; capping the end no longer moves the start past the missing frames.group::Consumer::poll_finishedends cleanly at awith_rangeframe cap, as reads do. Otherwise a publisher reset a range it had fully served when frames past the cap failed upstream.Track.Subscriberkeeps when each group it held arrived, so a group read from a copy the resumed reader has let go of still expires at its budget.closedopen while the frame is written (the pending hold is gone). JS publishers race their writes against an internalexpiredand againstclosedsettling with an error, replacinghooks.guardGroup; Rust drops the crate-privatepoll_abandonedthe same way.Internal, as before this PR: Rust
track::Subscriber::{poll_start, poll_drop},group::Consumer::poll_expired(now folding inpoll_expired_while_pending),subscription::Position::final_sequence, andSubscription::{start, end, with_start, with_end}arepub(crate). JSRecv,finalSequence,hooks.tryRecvGroupandhooks.groupChangedstay ininternal.ts, andGroup.Consumer.expiredis@internal.Track.Consumer.info()andTrack.Subscriber.tryRecvGroup()keep theirmainshapes.Quests: finishes
quest/m1/api-audit/serve.md(planned by moq-dev/moq#6581). That plan listedOrdered.tryNextGroupas internal, but the maintainer kept it public: the js/json and js/flate fixes need a non-blocking ordered read, and it mirrors Rusttrack::Ordered::poll_next_groupand JSSubscriber.tryRecvGroup(), both already public. Adds JS resume reports drops and Lowered resume floor, follow-ups Codex found here.Public API and wire impact
moq-net:track::Subscriber::set_groups,track::Ordered::set_groups, andtrack::Subscription::with_groupsare replaced by the consumingwith_range(range) -> Self, andgroup::Consumer::set_framesbygroup::Consumer::with_range. To migrate, rebind the reader:subscriber.set_groups(5..)becomeslet subscriber = subscriber.with_range(5..);. Both bounds now assign: an omitted start is group 0, whereset_groupskept the floor, so pass the floor you want to keep. Lowering the floor offers the cached groups below the old one that the reader never read; a group is still handed out once. To lift a capped reader's cap and read the track's edge, usewith_range(floor..)(wasset_groups(..)).@moq/net:Track.Subscriber.setGroups,withGroups, and theirTrack.Orderedtwins are replaced bywithRange(groups), which returns the reader. To migrate,subscriber.setGroups({ end: { excluded: 10 } })becomessubscriber.withRange({ start: { included: floor }, end: { excluded: 10 } }): an omitted start is group 0, so pass the floor you want to keep. To lift a cap, usewithRange({ start: { included: floor } })(wassetGroups({})).MIGRATE.mdentries that still nameset_groups,with_groups, orsetGroups(thelargest()bounds entries, the subscriber ranges entry, and "Subscriptions carry group ranges") now meanwith_range/withRange; this PR no longer editsMIGRATE.md, so the release cut should carry that rename.group::Consumer::finishedends at awith_rangeframe cap.Track.Ordered.tryNextGroup()in@moq/net.Checks
On
4869c701ec9d65fdeeca701a4edba42483bd241e: the queue droppedc269942914for amainconflict with moq-dev/moq#6470 (export trim). The merge keeps main'sPositionreturn type onTrack.Ordered.largest()with this PR'swithRangedoc link, and drops thelimits.tsimport main removed fromlite/publisher.tswhile keeping this PR'suntilGivenUp.just checkpassed every module (includingtest rsandtest js) on the merge before main's last two commits (quests and CI dispatch only);quest checkpasses on the head.On
c26994291428cd08e1113acfa953b7854bee25b6(merged withmainateaf2422f1d): the queue dropped the previous head because moq-dev/moq#6519 onmainadded afloor_honors_set_groupstest and two doc links namingset_groups. They now usewith_range(test renamedfloor_honors_with_range).just check: every module passed, includingtest rsandtest js.On
decadbead6ac5cde813c145ed2832e1a6b1d0df6(merged withmainat5155321baf):just checkpassed every module on the merge withmainate24ac5a193(full workspacetest rs), thencargo check --workspace --testsandquest checkafter5155321baf. A local Codex adversarial review of thetrack.tsresolution approved it.On
bffc24817f300f61d1f44c0452d614837ec27e41(merged withmainatf52848d634):MIGRATE.mdhunks (main now writesMIGRATE.mdat the release cut); the migration notes are under Public API and wire impact above.mainkeeps a held group's recorded successor so its wall clock outlives the successor's eviction. This PR's closed-subscriber arrivals stay beside it: a closedTrack.Subscriberjudges a handed-out group by the arrivals kept at close, and a valid recorded successor still wins over a later kept one (#keptSupersededAt). Main's two new tests racegroup.expiredinstead of the removedhooks.guardGroup, and its Rust tests call the foldedpoll_expired.just check: every module passed, includingtest rsandtest js.Earlier:
On
5f0380f10e(merged withmainat60ba42adc4):just fix: clean.just check: every module passed (js, rs, quest, drill, markdown, shell, toml, nix, justfile, gh), includingtest rs(8404, 519 and 226 passed) andtest js(@moq/net2717).quest check --base origin/main: clean.tryNextGroup abandons the frame helpers' group when it passes it(@moq/net) anda_group_missing_its_prefix_fails(moq-net IETF publisher).just test interop --allnot run: no wire ormoq-ffichange.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)