[kixelated]

Problem

moq-dev/moq#7838 found that a subscriber can drop a group without recv_group ever returning it. This happens when the group's stream is stopped or evicted before its header is read, or when the group is evicted and so is the FETCH that refetched it. No public API reports that group, so the receive-windows benchmark's overcommitted cells stall instead of counting the loss. Lite's SUBSCRIBE_DROP already names groups that fail at the publisher, but not groups a subscriber stops. track::Subscriber::poll_drop, which the ordered reader uses to skip gaps, is crate-private.

Approach

A new m1 quest, A group a subscriber gives up on is always reported:

• Wire: the subscriber always learns each abandoned group's sequence. The quest compares "the publisher answers STOP_SENDING with SUBSCRIBE_DROP" with "read the header before stopping or evicting", starting from why a subscriber stops before the header at all. Doing both is allowed.
• API: the drop signal becomes public in Rust, @moq/net and the bindings.

The receive-windows benchmark's overcommitted cells, which moq-dev/moq#7838 skips, wait on it.

Decisions (maintainer, 2026-11-04, recorded in the quest):

• ✅ m1.
• ✅ the quest picks the wire mechanism with numbers.

Public API: none in this PR (plan only). Wire: none.

Validation

• quest check --base origin/main --branch quest-plan/group-drops: 609 documents ok.
• sh/markdown.sh check: passes.

Review

• Round 1 (Codex gpt-6.1-sol, medium) on 77a18486a4: 0 findings.
• Pullfrog's five findings fixed in 5a81676712: the Goal is scoped to moq-lite; the measurement is named; the quest is ordered after the drop set; JS and the bindings mirror the read only for a named consumer; the m0 Required edge is dropped.
• Round 2 (Codex) on 5a81676712: 0 findings.
• Pullfrog's API-impact note applied in 8d233f18ff (additive for a separate read, breaking if folded into recv_group); a one-line wording fix, so no new round.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 7 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

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: f8839cf4-b7d8-41fb-8bdc-ff9780f489d2

Commits

Reviewing files that changed from the base of the PR and between 08ab033 and 8d233f1.

Files selected for processing: 2

• quest/m1/README.md
• quest/m1/group-drops.md

• 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

[kixelated]

Automated review by review (OpenAI)

Reviewed head 77a1848 against base 7697e2d.

• [P2] Distinguish STOPs already reported from the missing case (quest/m1/group-drops.md:27–32). The plan says a subscriber's STOP_SENDING does not produce SUBSCRIBE_DROP, but an in-flight Rust GroupServe sees peer closure through writer.poll_closed, returns Cancel, and settle_failed queues a named SUBSCRIBE_DROP on lite-05+ (rs/moq-net/src/lite/publisher.rs:5458–5460, 5657–5672); the serving loop writes those outcomes at 4734–4738. Please trace which benchmark loss escapes that existing path before planning a new response, and test that boundary. In particular, a group whose FIN was already acknowledged can have its writer retired before a later receiver-side eviction, so that case needs separate treatment rather than assuming every STOP is observed by an active writer.

The goal of reporting abandoned groups remains useful; the correction narrows the mechanism investigation.

Verification limits: GitHub-only static review of the quest, publisher and transport closure paths; I did not reproduce the benchmark or run tests.

(Written by review (OpenAI))

[pullfrog]

ℹ️ The plan is clear and the failure mode is real. The main approach question is the public API's premise: the only named consumer is the receive-windows benchmark's overcommitted cells (m0), while "applications" is unnamed. The JS/bindings mirror may have no consumer, and a Rust test hook or a crate-private accessor behind a test feature could cover the benchmark. Also, a Gap/Drop variant on recv_group might merge into an existing read instead of adding a second one.

Inline comments cover the API consumer and shape, ordering against the drop set's Delivered rework, the m0 Required cross-milestone dependency, the Goal's "every" vs. the open moq-transport decision, and the missing measurement for "with numbers".

None blocks the plan; they are decisions the quest should record.

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

[pullfrog]

ℹ️ No critical issues — one minor suggestion inline.

Reviewed changes

Re-reviewed the delta since the prior pullfrog review (77a18486) at 5a816767.

• Scoped the Goal to moq-lite and recorded moq-transport's partial coverage.
• Named the wire comparison's measurement: the receive-windows benchmark's overcommitted cells, with delivered_frames plus reported drops accounting for every group sent.
• Left the public drop signal's shape to this quest, preferring a fold into an existing read, and gated the @moq/net and binding mirrors on a named consumer.
• Ordered the API publication after the drop set, against its final disposition record.
• Dropped the m0 Required edge; the receive-windows quest now waits on this as an outside condition via #7838.

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

[kixelated]

Automated review by review (OpenAI)

Reviewed head 5a81676712b9c8298612b6b62421e317aa699743 against base 08ab0338600af36bc7063c10f91d9bc68206f649. The update narrows the goal to moq-lite, names the benchmark measurements, conditions JS and binding mirrors on a named consumer, orders API work after the drop set, and replaces the cross-milestone dependency with an outside condition. No new actionable finding in those changes.

The previously reported STOP_SENDING distinction remains: an in-flight Rust group can already yield a named SUBSCRIBE_DROP, while the post-FIN retirement boundary needs separate investigation.

Verification: GitHub-only static review; I did not reproduce the benchmark or run tests.

(Written by review (OpenAI))

[pullfrog]

✅ No new issues found.

Reviewed changes

Re-reviewed the delta since the prior pullfrog review (5a816767) at 8d233f18.

• Split the Public API impact line into "additive for a separate read" versus "breaking if folded into recv_group", noting MIGRATE entries and keeping the @moq/net/bindings mirror gated on a named consumer.

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