Repository navigation
quest: plan follow-ups from the 2026-10-13 merge pass - #5419
Conversation
- m1 js-track-status: split out of js-fetch; the JS publisher answers TRACK_STATUS like Rust (from #5409). - m1 js-forget-bench: sweep cancellation over competing dynamic routes (from #5411). - m2 ietf-driver-double-poll: IETF drivers still poll twice per fired deadline (from #5393). - m0 audio-quality-budgets: every row stays budgeted; measure across scheduled nightlies, never a branch dispatch (from #5394). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 31 minutes. View limit details
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 |
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. |
![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: 1fa789abff
ℹ️ 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".
| #5393 added `kio::Waiter::batch`, which holds a `Tasks` set's wake of its own | ||
| owner during that owner's poll. IETF sessions still poll twice: the driver is | ||
| a boxed future, so its children's sets are woken inside nested polls that the |
There was a problem hiding this comment.
The reviewed parent contains no kio::Waiter::batch, and rs/moq-net/tests/idle_session.rs:71-75 still describes and permits two driver polls per refresh for the lite versions. Consequently, the premise that #5393 already fixed lite and left only a nested IETF batching gap is not true in this tree, so this quest cannot apply its proposed remedy independently. Land/rebase #5393 first or record it as a required dependency and remeasure before prescribing the IETF-only fix.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed: #5393 is still open, so the premise was stated as if it had landed. Fixed in bc387ed: the quest now starts after #5393 merges and remeasures on top of it, and the Goal and the m2 README entry say "as #5393 makes moq-lite's". Required only lists quests, and #5393 is a PR, so the dependency lives in the Plan.
(Written by Claude Opus 5.5)
| Verify with a native IETF subscriber against a browser publisher on every | ||
| supported draft: a fetch-only request gets TRACK_STATUS_OK with no SUBSCRIBE | ||
| fallback, a cancelled request releases origin demand, and a request outside |
There was a problem hiding this comment.
every supported draft includes draft 17, but TrackStatusOk::describes_track deliberately returns false there because its REQUEST_OK cannot carry the track's timescale; ietf/subscriber.rs:1994-1999 therefore learns the track through SUBSCRIBE instead. Requiring TRACK_STATUS_OK with no fallback on that draft is unachievable without regressing timed draft-17 tracks. Specify drafts 14-16 and 18-22 for the no-fallback case and retain the draft-17 fallback.
AGENTS.md reference: AGENTS.md:L78-L80
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, confirmed against TrackStatusOk::describes_track and ietf/subscriber.rs. Fixed in bc387ed: the publisher answers on every draft (the Rust publisher has no version gate), but the no-fallback check covers drafts 14-16 and 18-22, and draft 17 keeps subscribing. The Goal's "drafts 18 and later" was also wrong, since the Rust reader sends TRACK_STATUS on 14-16 too, so that's fixed as well.
(Written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merge summary:
(Written by Claude Opus 5.5) |
1 participant
Problem
PRs merged on 2026-10-13 surfaced four follow-ups with no owner, or with an owner that is blocked.
Approach
quest/m1/js-track-status.md[S] (from fix(net): an IETF TRACK_STATUS refused as NOT_SUPPORTED falls back to SUBSCRIBE #5409): split out ofjs-fetch.md, which is blocked on JS ranges. The@moq/netpublisher answers TRACK_STATUS by mirroring the Rust publisher, so a native fetch-only request no longer needs the SUBSCRIBE fallback (draft 17 still subscribes by design).js-fetch.mdnow links it.quest/m1/js-forget-bench.md[XS] (from fix(net): release origin demand when requests are cancelled #5411):bench/requests.tssweeps cancellation over competingdynamic()routes, which settles Codex's unresolvedforgetP2 with measurements.quest/m2/ietf-driver-double-poll.md[S] (from fix(net): a fired deadline costs its driver one poll, not two #5393): moq-transport drivers still poll twice per fired deadline, because the boxed driver future wakes child task sets inside nested polls. It starts once fix(net): a fired deadline costs its driver one poll, not two #5393 merges.quest/m0/audio-quality-budgets.md(from quest(m0): plan the nightly release-link, fuzz LTO, and audio budget fixes #5394's review): every row stays budgeted, and ceilings are measured across scheduled nightly runs, never a branch dispatch.Decisions
forgetbench go?Impact
Quest files only. No code, API, or wire change.
quest checkpasses.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.