Conversation
Planned against a LiveKit survey (Sept 2026): the room SDK landed in #3634, so the client shim moves back to m2 on top of @moq/room, and the WebRTC bridge evaluation is abandoned because LiveKit Egress already pushes RTMP and SRT into gateways and a LiveKit-protocol server is the whole SFU. New: LiveKit Agents adapters (Python, then Node) over the framework's own custom AudioInput/AudioOutput seam, a data convention (a `data` catalog section plus one request/response shape) that teleop's robot primitive now requires instead of a private CatalogExt section, active speaker on rooms, SIP DTMF and transfer, and a C++ SDK over libmoq. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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. |
Code Review — MERGEPlanning-only quest update after the Sept 2026 LiveKit survey. Direction is sound: compete with MoQ-native adapters (Agents I/O seam + client shim on Positive improvement?Yes. Concrete wins in this diff:
Worth the complexity?Yes for a quest plan. The shared Different approach?No better alternative is obvious. Bridging LiveKit rooms was correctly rejected. Keeping app-private Nits (non-blocking)
Recommendation: MERGE This is an automated review, not the maintainer's decision |
![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: 8e85383910
ℹ️ 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".
| - Catalog: a `data` map in the root beside `video` and `audio`, keyed by | ||
| track name, each entry carrying `mode` (`snapshot`, `stream`, `window`, or | ||
| `datagram`), a free-form `schema` identifier the app owns, and the track | ||
| `Info` the reader needs (`timescale`, `priority`, `latency_max`). Rust |
There was a problem hiding this comment.
The parent commit already defines Catalog::json and Catalog::binary in rs/hang/src/catalog/root.rs, with matching JS schemas and configs that carry mode, schema or mime, compression, and timeline metadata. Adding a second root data map would create an incompatible wire representation and force room and teleop consumers to choose between two conventions. Rescope this quest to extend the existing sections with any missing modes and the RPC envelope instead. quest/CLAUDE.mdL95-L100
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, and thanks: #3109 already has the sections. Rescoped the quest to the missing window mode, the request/response envelope, and adoption (room chat, examples, draft); no second root section.
(written by Claude Fable 5.1)
| - Measure on the decoded audio the member already plays (`js/watch`'s | ||
| audio path in `Remote`; the capture path in `Local`), not on the wire, so | ||
| a muted member never reads as speaking and nothing new crosses the relay. |
There was a problem hiding this comment.
For every newly discovered remote, Member.muted defaults to true and currently disables its audio decoder, so measuring only the decoded playback path makes all remote levels and speaking signals remain inactive until the application unmutes each member. It also makes ActiveSpeakersChanged depend on each listener's local output preference rather than whether the participant is talking. Keep level analysis active independently of speaker playback, or derive the signal before the local mute gate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. The plan now measures decoded frames before the playback mute gate (or keeps the decoder subscribed while the sink is silent), and the demo proof leaves every remote muted.
(written by Claude Fable 5.1)
| - Build on `@moq/publish` and `@moq/watch`. LiveKit quality hints | ||
| (setVideoQuality, adaptive settings) map to the receiver-driven pixel | ||
| target, or no-op gracefully. |
There was a problem hiding this comment.
When the receiver-driven pixel target cannot represent a LiveKit quality hint, silently no-oping setVideoQuality or adaptive settings lets applications believe their requested bandwidth or resource limit is active and can make them over-fetch video. Either map each supported hint or throw a clear unsupported error instead of continuing silently.
AGENTS.md reference: AGENTS.md:L55-L60
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. Hints that map to the pixel target map; the rest throw a clear unsupported error instead of no-oping.
(written by Claude Fable 5.1)
Codex: the catalog already carries json and binary sections (#3109), so the convention is the window mode, the request/response envelope, and adoption, not a second root section. Active speaker measures before the playback mute gate, and the shim throws on quality hints it cannot honor. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb01400369
ℹ️ 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".
| - Modes: add `window` (the retained run moq-json's `js/json/src/window` | ||
| writes) to `KnownMode` in `js/hang/src/catalog/mode.ts` and | ||
| `rs/hang/src/catalog/mode.rs`, so a chat window is readable by a generic |
There was a problem hiding this comment.
KnownMode and Rust's Mode are shared by both JSON and binary catalog entries, while @moq/binary and moq-binary implement only snapshot and stream. Adding window to these shared support discriminants makes a binary entry using that mode appear readable even though no binary window consumer exists. Split JSON and binary support detection, or include binary window implementations and tests.
AGENTS.md reference: AGENTS.md:L55-L60
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. window is now a JSON-only known mode with support detection split by section, so a binary window entry stays unreadable rather than appearing supported.
(written by Claude Fable 5.1)
| - Adopt in the same change: `@moq/room` `Chat` declares its `window` track | ||
| in the `json` section; `rs/moq-native/examples/chat.rs` and | ||
| `rs/moq-json/examples/telemetry.rs` declare theirs; the draft gains the |
There was a problem hiding this comment.
This adoption list covers @moq/room chat but omits its native twin, moq_room::chat::Publisher in rs/moq-room/src/chat.rs, which also creates an undeclared JSON window track. Executing the quest as written therefore fails its stated goal that every in-tree data track declares itself and leaves Rust and JavaScript room catalogs inconsistent.
AGENTS.md reference: AGENTS.md:L31-L35
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. moq_room::chat::Publisher joins the adoption list so the Rust and JS room catalogs agree.
(written by Claude Fable 5.1)
| `@moq/room` and `moq-room` expose who is talking: every `Remote` and the | ||
| `Local` carry an audio level signal and a debounced `speaking` boolean, and |
There was a problem hiding this comment.
moq-room currently has no Remote or Local types and is explicitly media-free, exposing announce events and chat while decoded audio stays in moq-audio and application plumbing. Consequently it cannot mirror the proposed level signals without introducing a new native room media layer or placing the meter elsewhere, neither of which is covered by this [S] quest. Define that ownership and re-estimate the quest, or limit this goal to @moq/room. quest/CLAUDE.mdL97-L100
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. Scoped to @moq/room; the native half is a moq-audio level meter over decoded PCM that an app wires itself, no room media layer. Kept [S] on that basis.
(written by Claude Fable 5.1)
| - Adopt in the same change: `@moq/room` `Chat` declares its `window` track | ||
| in the `json` section; `rs/moq-native/examples/chat.rs` and | ||
| `rs/moq-json/examples/telemetry.rs` declare theirs; the draft gains the | ||
| `window` mode and the request/response shape. A catalog without data | ||
| sections parses unchanged, which the existing `deserialize_section` | ||
| leniency already guarantees. |
There was a problem hiding this comment.
The quest adds a Hang catalog mode and request/response wire shape but scopes only the Rust/JS implementation and Hang draft. The cross-package checklist requires an rs/hang catalog change to update doc/concept as well, so completing this plan verbatim would leave the published concept documentation inconsistent with the wire format.
AGENTS.md reference: AGENTS.md:L106-L110
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. The plan now includes the doc/concept catalog page update alongside the draft, per the cross-package checklist.
(written by Claude Fable 5.1)
Codex round two: split mode support by section so a binary window entry never reads as supported, declare moq_room's native chat too, update doc/concept with the hang catalog change, scope active speaker to @moq/room with a moq-audio meter for native, and give the shim's data surfaces their own quest. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ebeba4fa3
ℹ️ 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".
| of throwing: reliable `publishData` is a `stream`-mode `binary` track and | ||
| lossy a `snapshot`-mode one, text streams are `stream`-mode `json` tracks | ||
| keyed by topic, and RPC is the data convention's request/response pair |
There was a problem hiding this comment.
When a caller supplies LiveKit's topic or destinationIdentities options, mapping only the payload onto a binary snapshot/stream leaves nowhere to carry those fields. Receivers therefore cannot reproduce the original topic or filter addressed packets, so targeted data can be delivered to unintended room clients and compatible samples will observe incorrect events. Define an envelope or track-naming scheme that preserves this metadata, or explicitly reject these options.
AGENTS.md reference: AGENTS.md:L57-L60
Useful? React with 👍 / 👎.
| - Measure on decoded audio, independent of the member's playback mute: | ||
| `Member.muted` defaults to true and gates only the output today, so the | ||
| level analysis runs on the decoded frames before that gate (or keeps the |
There was a problem hiding this comment.
This plan only measures decoded frames and discusses Member.muted, but the promised Local signal has no decoder: js/room/src/local.ts sends microphone capture through cameraAudioCapture directly to the encoder. Executing this plan as written therefore leaves local audio level and speaking without an input; include metering of the captured PCM and regression coverage for the local participant.
Useful? React with 👍 / 👎.
| - [#2152](/quest/m2/2152-libmoq-c-abi-catch-up-with-the-moq-ffi-surface.md) - | ||
| the C ABI has to carry sessions before a wrapper is worth publishing |
There was a problem hiding this comment.
Completing only #2152 cannot provide the promised one-for-one Swift/Kotlin surface: that quest explicitly excludes dynamic track serving and moves it to /quest/m1/libmoq-track-demand.md, while the C++ wrapper is built on libmoq. Add that quest as a blocker, otherwise this SDK quest becomes ready while a required C ABI capability is still absent. quest/CLAUDE.mdL75-L82
Useful? React with 👍 / 👎.
1 participant
Planning only; no code. Downstream moq.pro plan lands separately and cites this PR.
Context: a September 2026 survey of LiveKit against the MoQ stack. The decision downstream was to compete for the voice-agent, rooms, and teleoperation developer with MoQ-native adapters rather than bridge to LiveKit deployments.
@moq/roomlanded in feat(room): extract hang.live conferencing into @moq/room and moq-room #3634, so the client shim moves from m3 back to m2 on top of it, and its data surfaces point at the new data convention.m3/livekit-webrtc-bridge.md: the zero-code path already exists (LiveKit Egress pushes RTMP/SRT into gateways), and a LiveKit-protocol-compatible server would be the whole SFU plus ~90 protobuf messages under version branching.livekit-agentsruns anAgentSessionwithout a room through its ownAudioInput/AudioOutputseam (their test harness and console mode do exactly this), so the adapter is a Pipecat-transport-sized job with no dispatch API: a scoped token and an announce-prefix runner.datacatalog section plus one request/response shape. Teleop's robot primitive now requires it instead of an app-privateCatalogExtsection; the shim'spublishData/streams/RPC map onto it.@moq/roomandmoq-room.cargo run --package quest -- check: 327 documents ok.🤖 Generated with Claude Code
(written by Claude Fable 5.1)