Repository navigation
quest(m1): plan five refactors from the 2026-10-22 audit - #6338
Conversation
Catalog phantom generics, JSON encoder options without a downcast, one Annex-B exporter, one CLI duration type, and a shared @moq/ui package. 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 25 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. |
|
Ready to merge: five m1 quests (catalog phantom generics, JSON encoder options, one Annex-B exporter, one CLI duration type, shared (Written by Claude Opus 5.5) |
#6054 landed and deleted quest/m1/refresh/json.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Dropped out of the queue because #6054 finished and deleted (Written by Claude Opus 5.5) |
![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: 21b9cc08e3
ℹ️ 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".
| Decided 2026-10-22 (maintainer): drop `E` only where it is phantom. It stays | ||
| on `Reserved<E>`, `Timebase<E>`, the catalog `Producer<E>` and capture | ||
| drivers that own a typed catalog, because those store it. JSON's `T` stays. |
There was a problem hiding this comment.
moq_audio::encode::producer::Reserved<E> does not store a typed catalog: it contains only the erased track plus _ext: PhantomData, and its constructor consumes catalog before building the reservation. Exempting Reserved<E> therefore contradicts the stated “drop E only where it is phantom” rule and leaves another instance of the same unnecessary generic behind; include this private reservation and its capture-state callers in the quest scope instead of claiming it stores E.
AGENTS.md reference: AGENTS.md:L23-L25
Useful? React with 👍 / 👎.
Plans the findings from a 2026-10-22 simplification audit (main at d44813c) that the maintainer agreed with and that no quest already owns. All five join m1.
New quests
quest/m1/catalog-phantom.md[M]: moq-video, moq-audio and moq-mux producers drop anE: CatalogExtheld only inPhantomData. Breaking Rust API.quest/m1/json-encoder-options.md[S]: moq-muxjson_snapshotgetsdeltaswithout anAnydowncast. Requires refresh/json (moq-dev/moq#6054) and the phantom quest.quest/m1/annexb-export.md[M]: one private Annex-B lifecycle for H.264/H.265 export, which also follows a rebound rendition. Requires moq-dev/moq#6276's quest.quest/m1/cli-duration.md[S]: moq-cli, moq-bench and moq-relay reuse a publicmoq_tokio::cli::Duration.quest/m1/js-ui.md[M]: new@moq/uipackage for the graph, buttons and support banner duplicated across@moq/watchand@moq/publish.Not planned
GroupServe, IETFstart()halves, and the adapter queue are already in the m1 simplification audit's "Check first" list (moq-dev/moq#6108).Decisions
Which audit items should get new quests?
The two small CLI/config items (#8 duration, #9 usage-rs dual declarations)?
Items #4-#6 are already in the m1 simplification audit. Split any out?
Which milestone for the non-breaking refactors?
#3: how should
deltasreach json_snapshot without the Any downcast?#7: where should the shared watch/publish UI live?
#8: how should the four duration wrappers consolidate?
#2: how should the Annex-B refactor relate to the rebind PR?
Public API and wire impact: none in this PR (quest files only).
quest checkpasses.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)