docs(quest): record four findings from the m1 quest wave - #3424
Conversation
Each came out of a quest PR that could not fix it in scope, and none had a tracker: - resume-info-newest: `resume::Consumer::poll_info` resolves track info from segment zero while data reads route to the newest segment, so a replaced broadcast serves the predecessor's timescale. Isolated while fixing the js/net half, which alone was not enough against a real relay. - smoke-go-client: the interop matrix has no Go client, so a passing `smoke-full` says nothing about the Go wrapper even though moq-ffi changes are required to run it. - uring-all-features-build: `moq-uring` does not compile with `--all-features --all-targets`; `quinn_proto` is ambiguous once `noq` and `quinn` are both on, so the nightly features gate fails on it. - go-native-context: the cancellation token added to moq-ffi exists only because uniffi-bindgen-go emits no cancellation handle. A fork that fixes that upstream is held behind a plain-text Required bullet, since publishing and pinning it is a decision rather than work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DLtoPv9B3779kGxLFTUGHR
|
Warning Review limit reachedNext included review available in 2 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (6)
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 445ae057e1
ℹ️ 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".
| `resume::Consumer::poll_info` (`rs/moq-net/src/model/resume.rs`) resolves info | ||
| from **segment zero**, while every data read routes to the **newest** segment. | ||
| When a broadcast is replaced on a path, those are different generations, so a | ||
| subscriber can be handed the predecessor's metadata alongside the successor's | ||
| frames. |
There was a problem hiding this comment.
Account for metadata across every retained segment
resume::Subscriber::poll_recv_group iterates every retained SegmentSub, not only the newest one, because the segments partition the sequence space across a takeover. Resolving poll_info from the successor would therefore make predecessor frames use the successor's timescale, while existing subscribers would retain the predecessor's already-sent TRACK_INFO for successor frames. Define and enforce a stable-info invariant across splices, or introduce a real generation boundary, rather than basing the fix on newest-only reads.
AGENTS.md reference: AGENTS.md:L148-L149
Useful? React with 👍 / 👎.
| ## Required | ||
|
|
||
| - The `context.Context` support in `kixelated/uniffi-bindgen-go` is published and pinned in `flake.nix` |
There was a problem hiding this comment.
Create a separate gate quest for the generator pin
This prerequisite combines publishing the fork with changing flake.nix, but no ready quest owns that work. The quest rules require a release or pin bump that unblocks repository work to have its own gate quest, with the dependent quest requiring that link. As written, this quest cannot start until the repository pin has already changed, while no quest is available to claim and perform that change.
AGENTS.md reference: quest/AGENTS.md:L95-L99
Useful? React with 👍 / 👎.
* docs(quest): settle scope narrowing in place, and mark pre-media sidecar placement (moq-dev#3427) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * fix(moq-video): pick the V4L2 mode nearest the requested resolution (moq-dev#3355) Co-authored-by: Luke Curley <kixelated@gmail.com> Co-authored-by: Codex <codex@openai.com> * feat(moq-video): add the Android MediaCodec encoder and decoder (moq-dev#3354) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Luke Curley <kixelated@gmail.com> * docs(quest): import the post-grooming issues as quests (moq-dev#3431) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * docs(quest): apply the Codex findings on the issue import (moq-dev#3432) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * fix(claude): adopt a quest branch at the remote tip that was inspected (moq-dev#3421) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * docs(quest): record four findings from the m1 quest wave (moq-dev#3424) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: reorganize the site around what a reader can do (moq-dev#3426) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * chore: ignore Claude Code's scratch directories (moq-dev#3428) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(moq-audio,moq-cli): assert publish_capture stays Send off macOS (moq-dev#3433) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * docs: correct claims found during merge review (moq-dev#3435) Co-authored-by: GPT-5 <noreply@openai.com> * docs(quest): import the open issues that had no quest, and gate the dev merge (moq-dev#3434) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> * docs(moq-audio): scope the local-task guidance to macOS (moq-dev#3436) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * docs: track deferred review findings (moq-dev#3438) Co-authored-by: GPT-5 <noreply@openai.com> * chore: remove redundant packaging work and plan relay ownership fixes (moq-dev#3440) Co-authored-by: GPT-6 <noreply@openai.com> * perf(net): avoid redundant chunk copies and plan performance investigations (moq-dev#3443) Co-authored-by: GPT-6 <noreply@openai.com> * fix(transcode): follow a source resolution change with the ladder (moq-dev#3381) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: GPT-5 <noreply@openai.com> * feat(watch): share one AudioContext across audio decoders Spatial playback needs every remote in the same Web Audio graph. Injected contexts are never closed. Co-Authored-By: Cursor Grok 4.6 <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Luke Curley <kixelated@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Franz Heinzmann <frando@unbiskant.org> Co-authored-by: Codex <codex@openai.com> Co-authored-by: GPT-5 <noreply@openai.com> Co-authored-by: Cursor Grok 4.6 <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Four findings surfaced by the m1 quest wave (#3408, #3410, #3412, #3414, #3415, #3418, #3419, #3420, #3422). Each was found by a PR that correctly declined to fix it in scope, and none had an issue or quest tracking it.
m0
Resume info
[M]—resume::Consumer::poll_inforesolves track info from segment zero, while every data read routes to the newest segment. A broadcast replaced on a live path therefore serves the predecessor'spriority,maxAge, andtimescale.timescaleis the damaging one: the reader converts frame timestamps with it, so a mismatched generation silently rescales rather than failing.Found while fixing the
js/nethalf (#3422). Against a real relay, a publisher wrote 1234 ticks on a successor's MICRO grid and the subscriber got1000us— the JS fix alone was not enough, because the relay's own stale metadata did the rescaling. Ranked with the othermoq-netdefect. Not covered by #2991 (sequence continuity only), and #2610's epoch remedy was removed from the draft by #3225.uring all-features
[S]—cargo clippy -p moq-uring --all-features --all-targetsdoes not compile:quinn_protois ambiguous oncenoqandquinnare both on, andquinn/connection.rs's test module refers to it unqualified.--libis clean, so only--all-targetstrips it. Reproduced on a stashed tree at the merge base while working #3420, so it predates that work and fails the nightlyjust rs featuresgate today.Go smoke client
[M]—just test smoke-fullruns 21 cells and Go is in neither axis.moq-ffichanges are required to runsmoke-fullby Cross-Package Sync, so #3412 ran it, passed 21/21, and thereby proved nothing about the Go code it changed.Both close the list, per m0's own Plan ("hardening, tooling, and test debt close the list").
m1
Native Go context
[M]— #3412'sMoqCanceltoken exists only becauseuniffi-bindgen-gorenders a Rustasync fnas a blocking Go call with no cancellation handle. A fork adding nativecontext.Contextsupport exists on branchcodex/context-cancel-3188; landing it retires the token from themoq-ffisurface.Filed with a plain-text
## Requiredbullet rather than as ready work, per the quest model's rule for an external release that unblocks repository work. Keeping the token is a supported outcome — it already ships, costs nothing outside Go, and a pinned generator fork is an obligation on every UniFFI bump. The quest says so explicitly so nobody starts it before that call is made.Verification
cargo run --package quest -- check: 238 documents ok — links resolve, the index matches the file tree, headings stay in the allowed set, andRequiredstays acyclic.Markdown only.
🤖 Generated with Claude Code
https://claude.ai/code/session_01DLtoPv9B3779kGxLFTUGHR
(written by claude-opus-5[1m])