Repository navigation
feat(net)!: carry group ranges on track::Subscription - #5732
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oq-c Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…relay A relative range on an empty track now resolves against the first group to arrive rather than the latest one at the first read, so a live subscriber that reads late still starts at the first group. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Folding each subscriber re-merged the growing aggregate, so N disjoint ranges cost O(N^2 log N): 150ms for 4096. Collect every subscriber's ranges and union them once instead: 110us. track_subscription_ranges sweeps range count and span, and span stays flat. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…apper Python, Go, Kotlin, Dart, C++ and Swift replace group_start/group_end with ranges and order, alias Range, Anchor and Order, and test that an ascending subscription reads a cached backlog oldest first. Swift nests the aliases under Subscription, since a top-level Range would shadow Swift.Range. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The model now carries ranges. Range requests on Dynamic, per-subscriber range filtering and drop accounting move to quest/m1/subscribe-ranges/dynamic.md. The lite-05 mixed relay case moves to the lite quest, and the draft-20+ widening gap is recorded on the moq-transport update quest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A Subscription holding a Vec of ranges keeps the producer under the lint's threshold on 32-bit targets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
#5689 merged: (Written by Claude Opus 5.5) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe subscription API replaces optional group-start and group-end bounds with range requests and an order preference. The core model aggregates ranges and derives subscriber read floors from produced groups. FFI and language bindings expose the new types and options. Network code and callers adopt range-based subscriptions. Tests cover range aggregation, cached-group ordering, and subscribers requesting different ranges. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0612ca207
ℹ️ 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".
| subscription.as_ref().and_then(|s| s.start()), | ||
| subscription.as_ref().and_then(|s| s.end()), |
There was a problem hiding this comment.
Do not collapse relative anchors into an IETF live join
When an IETF subscriber requests Anchor::Relative(n), start() returns None and end() also returns None for a relative end, so subscribe_join silently converts a relative-history start into a live join and a relative end into an unbounded subscription. For example, Relative(2).. receives only the current group instead of the two preceding groups, which the local cursor cannot recover because they were never requested upstream. Translate these anchors to the IETF relative filter/FETCH forms, or reject the unsupported request rather than changing its meaning.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining the wire translation here. This PR keeps every published wire on the covering span (wire impact: none), and a relative start already maps to no floor, the same request the default Range::oldest() sends, so Relative(n).. over moq-transport behaves exactly like a default subscription does today. Refusing it instead would also refuse every merged aggregate that holds a live subscriber, since Range::live() is relative.
Mapping relative anchors onto moq-transport (draft-20 Relative(n + 1), and SUBSCRIBE plus a relative joining FETCH n groups back on older drafts) is planned in quest/m1/subscribe-ranges/lite.md. The Anchor doc claimed a relay forwards a relative anchor as is, which was wrong today; f717aac now says what goes upstream, including that a moq-transport upstream starts at the live group.
(Written by Claude Opus 5.5)
| /// cursor; [`Position::group_end`] translates this field into its bound. Setting one | ||
| /// does not imply the other. | ||
| pub end: Option<Position>, | ||
| pub ranges: Vec<Range>, |
There was a problem hiding this comment.
Mirror subscription ranges in js/net
A repo-wide search shows that js/net/src/track.ts still exposes the old single groups span and combineSubscriptions still merges only one floor/end pair, so browser consumers cannot construct relative or disjoint ranges and browser relays cannot preserve the new union semantics. Add the matching Anchor/Range model and aggregation behavior to js/net as required for an rs/moq-net API change.
AGENTS.md reference: AGENTS.md:L100-L105
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining for this PR. The JS mirror is planned as its own quest, quest/m1/subscribe-ranges/js.md, which follows the lite-07 wire (lite.md) so @moq/net takes ranges and drops fetchGroup in one step. Nothing on the wire changes here, so a browser relay still interoperates: every Rust hop encodes the same covering span JS already reads.
(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. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Counts absolute ranges as signed boundaries in a treap that keeps each subtree's sum and lowest running sum, so a subscriber's range change costs O(log n) whatever the audience holds. Relative and mixed ranges are counted by value. The union is cached until a change moves it, and the reference fold is test-only. track_range_demand sweeps readers over distinct ranges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The producer crosses the lint's threshold on 32-bit targets too since the NEW_GROUP request joined the subscription. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7aae3fec5
ℹ️ 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".
| state | ||
| .arrival | ||
| .front() | ||
| .map(|(sequence, _)| *sequence) | ||
| .or(state.max_sequence) |
There was a problem hiding this comment.
Preserve the first group used to resolve relative floors
When a relative subscription is created while the track is empty but its first group is evicted before the subscriber polls, arrival.front() identifies the oldest retained group rather than the first group that arrived. For example, if Relative(2) first sees group 10, then cache pressure evicts 10 before polling and leaves 11 at the front, the floor becomes 9 instead of 8; a fresh late arrival of group 8 is then incorrectly filtered out. Record the first post-subscription group independently of the evictable arrival queue, or resolve the anchor when that group arrives.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 374e453. The track now records the first sequence it is given (TrackState::first_sequence, set once in advance), and a pending relative floor resolves against that instead of the front of the evictable arrival queue. a_relative_start_on_an_empty_track_survives_its_first_group_leaving reproduces your case: it failed with [11] before the fix and now delivers [8, 11].
(Written by Claude Opus 5.5)
… evicted A relative start taken on an empty track resolved against the oldest cached group, so evicting the first one before the subscriber read moved its floor up. The track now keeps the first sequence it was given. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 374e453bcf
ℹ️ 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".
| let preferences = subscription.read(); | ||
| let state = state.read(); | ||
| let min_sequence = floor_of(&preferences); | ||
| let floor = preferences.floor(state.max_sequence); |
There was a problem hiding this comment.
Resolve relative anchors against the latest group
On tracks that interleave groups and datagrams, max_sequence can be ahead of latest_group because datagrams share the sequence namespace. For example, after group 0 and datagram 1, a new Range::live() subscriber resolves its floor to 1, skips the actual live group 0, and waits for the next group. Resolve group-relative anchors from the latest group rather than the latest sequence of either delivery type.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7d44715. A new cursor now resolves relative starts against latest_group, not max_sequence, so datagrams that share the sequence space don't move them. The empty-track anchor is now first_group, recorded with the first cached group, so a datagram can't set it either. a_live_range_ignores_a_datagram_past_the_newest_group reproduces your case: before the fix it delivered nothing, and now it delivers [0].
(Written by Claude Opus 5.5)
Datagrams share the sequence space, so a datagram past the newest group moved a live subscriber's floor beyond the group it should start on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The union already drops them, so an empty span below a real one no longer starts the cursor early. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c189a163ae
ℹ️ 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".
| let state = state.read(); | ||
| let min_sequence = floor_of(&preferences); | ||
| // Relative starts count groups, so datagrams sharing the sequences don't move them. | ||
| let floor = preferences.floor(state.latest_group); |
There was a problem hiding this comment.
Resolve relative starts from live groups only
When a one-shot fetch caches group N above the current live group M, insert_group_request inserts that backfill with visible = false, but insert_group still advances latest_group. After the datagram fix, this is fresh evidence of another non-live source feeding this floor: a Range::live() subscriber starts at N even though N is absent from the arrival queue, so it skips visible groups M through N-1 until the live feed reaches N. Track the latest publisher-produced group separately, and likewise keep fetched backfill out of first_group.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 405e94b. The track now keeps first_produced and newest_produced, which only publisher-produced (visible) groups set. Relative starts count back from those, so fetched backfill moves neither the anchor for a new cursor nor the one for a cursor taken on an empty track. latest_group stays the live edge for eviction and resume. a_live_range_ignores_a_fetched_group_past_the_newest_group covers both cases with a fetched group 5 and a produced group 0: before the fix the early reader got [], and now both readers get [0].
(Written by Claude Opus 5.5)
…kfill A fetch caching a group past the live edge advanced latest_group, so a live subscriber skipped the produced groups below it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 405e94bf85
ℹ️ 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".
| return Poll::Ready(Ok(None)); | ||
| }; | ||
| self.datagram_index = found_index + 1; | ||
| self.resolve_floor(); |
There was a problem hiding this comment.
Wait for relative floors before returning datagrams
When Range::live() or another relative-start range subscribes before any group is produced, a datagram can arrive first. resolve_floor() then returns false because first_produced is still None, but that result is ignored; min_sequence remains 0, so the datagram is returned and its cursor is irreversibly advanced. For example, a datagram at sequence 0 preceding the first group at sequence 1 is delivered even though the eventual live floor is 1. Keep the datagram pending until the relative floor resolves.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining this one. Holding datagrams until a relative floor resolves would starve a datagram-only track: no group is ever produced, so first_produced never resolves and a Range::live() subscriber would receive nothing at all. A datagram that arrives after subscribe on a track with no groups is live content, so delivering it matches what live() asks for. Relative starts place the group floor once a produced group exists, and datagrams already behind that floor when it resolves are filtered from then on. f102ff4 adds a_live_range_on_a_track_without_groups_delivers_datagrams to pin this.
(Written by Claude Opus 5.5)
…oups Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f102ff4853
ℹ️ 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".
| let start = subscription.start.map(|start| start.group).or_else(|| track.latest()); | ||
| // Reads start where the lowest range does. Without a floor of its own (the oldest | ||
| // range, the default) a raw reader joins at the latest group. | ||
| let latest = track.latest(); |
There was a problem hiding this comment.
Resolve raw relative floors from produced groups
When a raw FFI subscriber uses a relative anchor on a track containing both groups and datagrams, this new calculation uses Ordered::latest(), which reflects the shared group/datagram sequence namespace, and then permanently raises the local group cursor. For example, after group 0 and datagram 1, Relative(0) starts at 1 and skips group 0 even though the model cursor correctly resolves against produced group 0; Relative(u64::MAX) is likewise mapped to the latest sequence instead of group 0. Fresh evidence beyond the earlier core-model finding is this newly added FFI-only re-resolution path; preserve the model cursor's resolved floor or resolve against the produced-group edge instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Deferred, and the maintainer agrees: quest/m1/subscribe-ranges/dynamic.md (added in this PR) owns resolving relative starts outside the model, and its Plan now names this call site and the datagram skew in Ordered::latest(). Nothing on main regresses. Relative starts are new in this PR, and the default Range::oldest() still joins at latest() as it did on main. The fix moves resolution into the model, which already counts back from produced groups only.
(Written by Claude Opus 5.5)
| .map(|range| match range.start { | ||
| moq_net::track::Anchor::Absolute(start) => Some(start.group), | ||
| moq_net::track::Anchor::Relative(_) => None, | ||
| }) |
There was a problem hiding this comment.
Preserve relative floors during container startup
When a container subscription requests Relative(n) and the publisher sends its backlog newest-first, as the default descending order permits, treating every relative start as no floor puts the consumer into startup. poll_event then adopts the first completed arrival, which can be the newest group, and discards the lower-sequence groups that the relative range requested. Preserve the relative floor until it can be resolved rather than treating this as an unfloored live join.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Deferred, and the maintainer agrees: quest/m1/subscribe-ranges/dynamic.md (added in this PR) owns resolving relative starts outside the model, and its Plan now names this call site. Nothing on main regresses. Relative starts are new in this PR, and a subscription without an absolute floor joins from the first arrivals, as it did on main. The fix lets the container use the model's resolved floor instead of guessing it from arrivals.
(Written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Marks the covering-span helpers pending on serve.md for the new model-markers lint, and narrows floor and clamped to the model. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts: - cpp/moq/include/moq/moq.hpp: take main's #5727, which generates unprefixed names, dropping the alias list (and this branch's Anchor/Order/Range aliases, now generated directly). - rs/moq-net/src/model/track.rs: keep both appended test sets (subscription ranges here, rewind refusal from #5803). Semantic follow-ups for callers main added against the old API: - rs/moq-net/tests/integration/received_rewind.rs: with_start(group 0) becomes with_groups(0..), as this branch migrated other callers. - py/moq-rs/tests/test_testing_clock.py: group_start=0 becomes an absolute group-0 range. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds #5732's subscription-ranges entry to moq-net, moq-ffi, moq-c, and each binding guide. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merge main (#5732 landed model ranges): delete model.md and move the Time(pts) anchor into Dynamic range requests. Review fixes: - consumer: refresh with a cursor per track plus a delimiter LIST for new tracks, instead of one prefix-wide StartAfter. - producer/consumer: drop the source max age from .info; served tracks advertise no Publisher Max Age limit. - producer: resume at the frame after the last stored one, per track. - producer: keep the canonical percent encoding so a track name is one key segment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Quest:
quest/m1/subscribe-ranges/model.md(deleted here; the rest is split intoquest/m1/subscribe-ranges/dynamic.md).Summary
track::Subscriptioncarries a list of group ranges, in place of a single start and end. Subscribers are aggregated by the union of their ranges, so a range above the live edge no longer floors a merged live subscriber.Range { start: Anchor, end: Option<Anchor> }, whereAnchorisAbsolute(Position)orRelative(n)(n groups before the latest, clamped to group 0).Range::live()isRelative(0)...Range::oldest()isRelative(u64::MAX)..and is the default, so the default still means everythingmax_delayallows. A subscriber's cursor starts at its lowest resolved range start. Relative anchors count produced groups, not datagrams or fetched backfill. On a track without groups, a relative start resolves against the first group to arrive, even after that group is evicted.model/track/demand/ranges.rs). Absolute ranges are signed boundaries in a treap, +1 at a start and -1 at an end, and each subtree keeps its sum and lowest running sum. A change touches two boundaries and asks one range-minimum question, so it costs O(log n) and reports exactly whether the union moved. Open relative ranges keep only the farthest, holders ofRange::oldest()short-circuit to it, and the remaining relative or mixed ranges are counted by value. The union is cached until a change moves it.subscription::Combinedandunionare the test-only oracle for a randomized demand test and a randomized check that every add and remove reports a change exactly when the union moves.quest/m1/subscribe-ranges/lite.md.MoqSubscription { ranges: Option<Vec<MoqRange>>, order: Option<MoqOrder> }withMoqRange,MoqAnchor,MoqOrder. In C,moq_subscription { ranges, ranges_len, order }withmoq_range,moq_anchor,moq_anchor_kind,moq_order. Bad discriminants returnInvalidCode.group_start/group_endand aliasRange,AnchorandOrder. Each wrapper gains a test that an ascending subscription reads a cached backlog oldest first.Relative(k)..merged with a future range, a frame-precise start merged with live, and an untimedAbsolute(0)...tests/integration/subscribe_ranges.rsruns the merged live case in-process and over lite-05/06/07 and moq-transport 14/17, both directly and through a relay.Public API and wire impact
Subscription::{start, end}fields are gone;ranges: Vec<Range>replaces them.with_start/with_endare now crate-private. Usewith_groups(range)orwith_ranges(..)instead.track:Anchor,Range(live,oldest,groups).PositionderivesHash.group_start/group_endbecomerangesandorder, as listed above.Decisions
Picked by an unattended iterate session as the recommended option (✅). Each is open to the maintainer.
Default range. ✅
Range::oldest(), which keeps today's "no floor" meaning and the fix(net): deliver a fresh group below the first served group when the floor allows it #5000 split between explicit 0 and unfloored. Alternative:Relative(0).., which would change in-process replay.Anchornaming. ✅ keepAnchor. Alternatives:BoundorEdge.Local filtering. ✅ ranges set where a subscriber's reads start but don't filter later reads, until
dynamic.mdlands per-subscriber serving.FFI optionals. ✅
rangesandorderareOptionbecause uniffi can't default an enum literal;Nonemeans the moq-net default.Swift naming. ✅ nested aliases (
Subscription.Range), since a top-levelRangewould shadowSwift.Range. Alternative: a prefixedTrackRangein every language.Distinct-range demand cost. ✅ Incremental (RLE) range demand, chosen by the maintainer. Alternatives were accepting the slope with a follow-up, or restricting ranges. Bench medians, before (cached union redone per reshaping change) and after:
track_range_demand/10000r/1track_range_demand/10000r/1000track_range_demand/10000r/10000track_range_demand/1000r/1000track_subscriber_preferences/update/100/1track_subscriber_preferences/update/10000/1track_subscriber_preferences/extreme_leave/10000track_subscription_ranges/span_64/4096track_range_demandmoves one reader between two private ranges (two updates and two polls per iteration) over readers holding a given number of distinct overlapping ranges, swept 1 to 10k on both axes; it now stays flat on both. A single reader pays about 0.1 µs more per iteration for the treap.Relative starts outside the model. ✅ Deferred to
dynamic.md, chosen by the maintainer. Known limitation until then: two consumers re-resolve relative starts on their own. moq-c's raw reader (apply_raw_subscription) counts back fromOrdered::latest(), which includes datagram sequences, so a datagram past the newest group skews the start. moq-mux'scontainer::Consumer::newtreats a relative start as an unfloored join and adopts the first arrival. Neither regresses main, where relative starts did not exist. Alternatives were a public produced-group or resolved-floor accessor here, or redefininglatest().Follow-ups
quest/m1/subscribe-ranges/dynamic.md(new): range requests onDynamic, replacingrequested_group/fetch_group, per-subscriber filtering, and drop accounting. Quest links that pointed atmodel.mdfor theDynamicrange request now point here.lite.md. It needs an explicit group 0 on pre-06.ietf-update.md.Checks
just checkafter the latest main merge (Rust workspace tests, wasm clippy, py, go, dart, kt, cpp, lint)sh/rs/model-markers.py. The covering-span helpers (Subscription::{start, end, set_bounds, with_start, with_end}) are marked pending onpublic-model/serve.md, andfloor/clampedare narrowed to the model.cargo test -p moq-net --all-targetsapart from the origin bench belowquest checkTS validation(an mp2 hole in the full multiplex) fails the same way on most open PRs; fix(mux): put a live export's budget on its first SUBSCRIBE #5854 is the fix.cargo test -p moq-net --bench originpanics atannounce_duplicate("announce delivered"), deterministically, after merging main; this PR does not touch announcements.just test interop --allReviews
An earlier Codex review of f0612ca had two findings, both declined in their threads: relative anchors on moq-transport (owned by
lite.md; theAnchordoc now says what goes upstream today) and the JS mirror (owned byjs.md).Codex on later heads:
live()would starve.a_live_range_on_a_track_without_groups_delivers_datagramspins the behavior (f102ff4).dynamic.md(decision 7).Each fix has a regression test that fails without it.
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code