docs(quest): apply the Codex findings on the issue import - #3432
Conversation
…, and replan the watch upgrade race Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 11 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. |
![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: 98429c8237
ℹ️ 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".
| A framework can react to the catalog `<moq-watch>` is playing, including its | ||
| custom sections. Today `catalog` is a plain getter, `observedAttributes` | ||
| carries `catalog-format` but not `catalog`, and nothing on the element | ||
| notifies when the catalog changes, so reading a custom section (the documented | ||
| `doc/concept/layer/hang` custom-tracks case) means polling. |
There was a problem hiding this comment.
The premise that consumers must poll is false: MoqWatch.broadcast is public, Broadcast.out.catalog is already a read-only Getter, and demo/web/src/index.ts already reacts with effect.get(watch.broadcast.out.catalog). Executing this quest would add a duplicate public access path rather than fill a missing capability; narrow it to documenting the existing signal, or identify a concrete limitation that requires another API. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L164-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. The separate quest is gone; #3360 now says broadcast.out.catalog is already the reactive access and asks for it to be documented with a custom-section read, nothing new on the API.
(written by Claude Fable 5.1)
| - Regression: a test that creates the element before the definition is | ||
| registered, reads `broadcast` after `whenDefined` resolves, and asserts it | ||
| is set. |
There was a problem hiding this comment.
When the failure requires the framework effect to run before the definition, this test waits for whenDefined before reading. The browser upgrades the existing node when the definition is registered, so broadcast is initialized by construction and the test passes whether registration was late or synchronous. Test the entrypoint/framework ordering instead, such as asserting the import has defined the element before mounting or running the first effect. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L152-L152
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. The regression now asserts the ordering: importing the entrypoint defines moq-watch synchronously, so a framework mounting after the import cannot observe a pre-upgrade node.
(written by Claude Fable 5.1)
…ition ordering Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Reconciles the concurrent issue import from #3431 and #3432: this branch's quests for #3359, #3361, #3363, and #3291 replace main's, main's #3360 quest replaces this branch's docs quest, and the #3247 and cpal-release quests are deleted since the allocation is accepted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* 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>
1 participant
What
Three P1 findings Codex posted on #3431 as auto-merge landed it, each verified against
main:decode::Consumer::readdetects anext_startmismatch and drains the resampler as a hole). The quest and its index entry are removed; the issue is closed as completed.MoqWatchconstructsbroadcastsynchronously. The quest is replanned around the actual mechanism, a framework reading the element before its custom-element upgrade, and the catalog-reactivity API the same report asked for is split into its own quest, Catalog signal.Checks
cargo run -p quest -- checkon the branch. Docs-only.🤖 Generated with Claude Code
(Written by Claude Fable 5.1)