[kixelated]

Works on quest/m2/unbounded-groups/run-fill-pacing.md: lands the moq-transport half and rewrites the quest to the lite-07 remainder. Follow-up from moq-dev/moq#8017.

Problem

A relay filling a run of groups with one moq-transport FETCH wrote the whole run at the fetch stream's speed. On a cache smaller than the run, the run evicted its own groups before a slow reader took them. Reproduced with a 40-group run on a 4-group pool and a reader taking one group per stream poll: the reader got only groups 36 to 39.

Approach

• Evidence (model/track/fill.rs). Fill::poll_run_paced is ready at once until the track's eviction count (TrackState::evictions, bumped when a reader loses a group it judged cached) is nonzero, then waits as Fill::poll_paced (feat(net)!: pace a batch fill's handler by its fastest reader #8017) does: until a reader picked up every group the fill accepted. An unpaced run still moves at the source's speed while readers keep up.
• FETCH run (ietf/subscriber.rs, GroupFetchObjects). At each group boundary the run finishes the group before, then holds the fetch stream unread on that gate, keeping the next group's first object until it may open the group. Flow control then holds the publisher back.
• Pause budget (coding/reader.rs). The hold goes through Reader::poll_pace, now pub(crate), like within-group pacing: it reserves the stream's receive credit from the session's pause budget, watches for a reset while paused, and reads on unpaced once the budget is full.
• Test transport. ScriptedSession::with_receive_window, and GroupFetchRun opens its stream through the subscriber's transport so it pauses like a session's stream.
• Quest. Rewritten to the lite-07 remainder, with the findings below and a decision for the maintainer. The epic's Required entry follows.

Impact

• Public API: none.
• Wire: none.

Alternatives

These were prototyped for lite-07 range fills and dropped. The rewritten quest records them:

• Holding each arriving lite-07 group stream unread until the fill's readers picked up the one before. It admits streams in arrival order, not the fill's order, so the groups a reader takes last evict each other first: 10 of 40 groups.
• Fetching a paced lite-07 fill a group at a time (FillServeRun): 26 of 40, but it costs a round trip per group, which feat(net)!: pace a batch fill's handler by its fastest reader #8017 and moq-dev/moq#7339 rejected. It also leaves the subscription's own history range unpaced.
• Pacing every run from the start, with no evidence gate. The quest asked for pacing on evidence, so an unpaced run keeps prefetching at full speed while readers keep up.

Choices made without asking

• The evidence is the existing eviction count. It already counts a reader losing a group it judged cached, so no new state was added. Once set it stays set for the track copy's life.
• The moq-transport half lands alone. Our own publisher refuses multi-group FETCH, so FETCH runs only happen against third-party upstreams. The change is small and matches the plan. The lite-07 half needs a mechanism decision, recorded in the quest's ## Waiting.

Validation

• a_fetch_run_waits_for_its_reader_once_one_lost_a_group (new): with evidence, a slow reader of a 40-group FETCH run on a 4-group pool gets every group, with the stream paused through a session that grants one group of credit. It covers an ordered ascending reader and a descending poll_recv_group reader. Without evidence the same run outruns both: the descending one reads only [39, 38]. With the gate disabled, the ascending paced half reads only [36, 37, 38, 39].
• just ci on 872e887f3a: passes (11021 Rust tests).
• just check on 17b6c4d0f9 (after merging main): passes.

Follow-ups

• The quest stays, rewritten to lite-07, waiting on the maintainer's pick of mechanism. Its findings: lite-07 history arrives unpaced three ways (the subscription's own history range, RangeFills, and Recovery), and feat(net)!: pace a batch fill's handler by its fastest reader #8017's per-fill pacing lets several fills of one reader outrun it together.

Review

Codex (quest review), gpt-6.1-sol, medium effort.

• Round 1 on 872e887f3ab5ce4a66a64646258c3cf5fe0bc743: no findings.
• Round 2 on 17b6c4d0f9544912448ff7d87638a6871052c114: no findings.

Online: pullfrog's thread said a descending reader is not held back. Declined with evidence in the reply: the descending case is now in the test and passes. Gating on hand-out instead deadlocks a_fill_against_its_readers_order_still_finishes.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 872e887
Base: ce0a4f0

No actionable findings in the reviewed changes. The FETCH loop retains the already-decoded next object across a pending pace gate, finishes the previous group before waiting, and resumes through the existing receive-credit pause budget and reset watcher. The eviction evidence selects the existing fill pickup gate without adding a wait to uneventful runs. The regression covers a run larger than the cache with and without prior eviction evidence and opens its receive stream through the session so the pause budget is exercised.

The moq-transport-only scope is clear. Lite-07 history and pacing across multiple fills remain explicitly tracked rather than being represented as solved; the proposed mechanism still needs the maintainer's decision.

Verification: GitHub-only static inspection of the changed FETCH state machine, RangeWriter lifecycle, fill pacing, Reader::poll_pace, test transport and regression. No Cargo builds, tests, interop runs or runtime reproduction. Check run 42331635546 was in progress when inspected; the PR's reported validation was not independently reproduced.

(Written by review (OpenAI))

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The change adds eviction-aware pacing at group boundaries in multi-group FETCH runs. When pacing is pending, the FETCH reader retains the next object for a later poll. A new test compares paced and unpaced reads with a slow reader and a limited cache. The test transport can now configure a receive window. Quest documentation now describes lite-07 history delivery paths and pacing approaches.

Merge Risk: Minimal · up to 872e8

No actionable merge-blocking issue is established for the FETCH pacing change. It is mergeable subject to normal checks.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title names the main change: pacing a moq-transport FETCH run after a reader lost a group to eviction. The raw summary shows the subscriber.rs FETCH pacing, the fill.rs poll_run_paced gate, and th…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
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 description is directly related to the changeset. It names the quest file it works on and explains the moq-transport FETCH-run pacing fix, including the evidence gate via the track eviction count,…

Full details: Title check

Explanation

The title names the main change: pacing a moq-transport FETCH run after a reader lost a group to eviction. The raw summary shows the subscriber.rs FETCH pacing, the fill.rs poll_run_paced gate, and the eviction-based trigger, so the title matches the changeset. The title is a single conventional-commit line of 80 characters, which is slightly long but readable.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 3 files. (3 skipped: 2 unsupported, 1 too large.)

Full details: Description check

Explanation

The description is directly related to the changeset. It names the quest file it works on and explains the moq-transport FETCH-run pacing fix, including the evidence gate via the track eviction count, the pause-budget hold through Reader::poll_pace, the ScriptedSession::with_receive_window test helper, and the new test a_fetch_run_waits_for_its_reader_once_one_lost_a_group. It also covers the quest rewrite to the lite-07 remainder, the rejected alternatives, and validation results. These match the file summaries for fill.rs, subscriber.rs, reader.rs, test_transport.rs, and the quest files.

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: a31edae4-8bda-48e1-973b-7d09932083d2

Commits

Reviewing files that changed from the base of the PR and between ce0a4f0 and 872e887.

Files selected for processing: 6

• quest/m2/unbounded-groups/README.md
• quest/m2/unbounded-groups/run-fill-pacing.md
• rs/moq-net/src/coding/reader.rs
• rs/moq-net/src/ietf/subscriber.rs
• rs/moq-net/src/lite/test_transport.rs
• rs/moq-net/src/model/track/fill.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

[pullfrog]

ℹ️ No critical issues — one minor suggestion inline.

Reviewed changes

• Pacing gate — Fill::poll_run_paced is ready until the track's eviction count is nonzero, then waits as Fill::poll_paced does.
• FETCH run — GroupFetchObjects finishes each group, then holds the stream unread on the gate, stashing the next group's first object in paced.
• Pause budget — Reader::poll_pace is now pub(crate) and used for the run gate.
• Test transport — ScriptedSession::with_receive_window, and GroupFetchRun opens through the subscriber's transport.
• Quest — rewritten to the lite-07 remainder, with the epic README link retitled.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Follow-up on 17b6c4d since 872e887, accounting for base 75c0847. No new actionable findings.

The substantive update extends the existing regression to a descending subscriber using poll_recv_group, while retaining ordered ascending coverage. Both modes require every group with pacing and demonstrate loss without eviction-triggered pacing. This is focused coverage of the disputed reader mode; the PR's production changes remain unchanged relative to the base. The explicitly deferred lite-07 and multiple-fill work remains outside this test's claim.

Verification: GitHub-only comparison and static inspection of the changed test and its harness. No Cargo tests, interop execution, or quest context run; the reported passing results were not independently reproduced.

(Written by review (OpenAI))

[pullfrog]

✅ No new issues found.

Reviewed changes

• Descending reader case — a_fetch_run_waits_for_its_reader_once_one_lost_a_group now runs both Order::Ascending (via ordered()) and Order::Descending (via poll_recv_group), asserting the paced run hands a slow reader all 40 groups and the unpaced run does not. The fill follows the subscription order, so the descending case exercises the descending fill's cursor; the test passes locally.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}