Add minimal stack-chan-ai Realtime voice adapter - #635
Conversation
|
Warning Review limit reached
Next review available in: 1 minute You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR adds a server-backed realtime voice adapter for conversation modules. It introduces a WebSocket transport worker, an OpenAI Realtime-style model, chat configuration for worker selection and endpoint override, platform wiring for CoreS3, and tests for session, audio, and function-call flows. ChangesServer realtime conversation adapter
Estimated code review effort: 4 (Complex) | ~50 minutes Sequence Diagram(s)sequenceDiagram
participant ChatService
participant ServerOpenAIRealtimeModel
participant RealtimeServer
participant FunctionHandler
ChatService->>ServerOpenAIRealtimeModel: construct and connect with endpoint, tools, and instructions
ServerOpenAIRealtimeModel->>RealtimeServer: session.update
ServerOpenAIRealtimeModel->>RealtimeServer: input_audio_buffer.append / commit
RealtimeServer-->>ServerOpenAIRealtimeModel: response.function_call_arguments.done
ServerOpenAIRealtimeModel->>FunctionHandler: post function call and arguments
FunctionHandler-->>ServerOpenAIRealtimeModel: function result
ServerOpenAIRealtimeModel->>RealtimeServer: conversation.item.create
ServerOpenAIRealtimeModel->>RealtimeServer: response.create
RealtimeServer-->>ServerOpenAIRealtimeModel: transcript and audio events
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![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: 1cadfb28aa
ℹ️ 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".
Cloudflare PR previewThis pull request is closed. Its preview has been replaced with a closed page at https://pr-635.stack-chan-pr-preview.pages.dev. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
firmware/host/modules/conversation/__tests__/server-realtime-model/server-realtime-model.test.ts (1)
30-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the configuration failure path.
The tests cover only the success path. The failure path decides whether a session starts at all. Add cases that call
configurewith a non-WebSocket endpoint and withapiKeyoverws://, then assert thatconnectpostsfailed. Add a case that reconfigures with a valid endpoint afterwards; that case detects the staleconfigurationErrorreported onfirmware/host/modules/conversation/chat-audioio/server-openai-realtime-model.jslines 25-28.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@firmware/host/modules/conversation/__tests__/server-realtime-model/server-realtime-model.test.ts` around lines 30 - 37, Add failure-path coverage in the tests around model.configure and the session.created flow: verify non-WebSocket providerID and apiKey used with ws:// cause connect to post failed, then reconfigure with a valid endpoint and verify the session succeeds without retaining the prior configurationError. Use the existing tool, connect mock, and valid configuration symbols.firmware/host/modules/conversation/__tests__/server-realtime-model/audio-codecs.js (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winModel the 2:1 sample reduction in the codec double.
The double copies bytes one to one. A-law encoding produces one byte per 16-bit sample. Because of this, the assertion in
firmware/host/modules/conversation/__tests__/server-realtime-model/server-realtime-model.test.tsline 60 confirms only the model's ownmessage.size >>= 1, and it would still pass if the encode step were removed.Write one byte per input sample in the double, and assert the byte content that reaches
sendAudioBuffer.♻️ Proposed refactor
export const Encode = { toAlaw(source, target) { - target.set(source.subarray(0, target.length)) + const samples = source.byteLength >> 1 + for (let i = 0; i < samples; i++) { + // Deterministic stand-in for A-law: one output byte per 16-bit sample. + target[i] = source[i * 2 + 1] ^ 0x55 + } }, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@firmware/host/modules/conversation/__tests__/server-realtime-model/audio-codecs.js` around lines 1 - 5, Update Encode.toAlaw in the codec double to reduce 16-bit input samples to one output byte per sample rather than copying bytes one-to-one. Extend the server-realtime-model test assertion around sendAudioBuffer to verify the transmitted byte content, ensuring the encoding step is exercised beyond the message.size reduction.firmware/host/modules/conversation/chat-audioio/server-chat-websocket-worker.js (1)
98-106: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRead the close opcode from the same io class that
connectselected.
connectselectsdevice.network.wss.iofor secure sessions.disconnectreads the constant fromdevice.network.ws.io. Store the selected class in a field and reuse it, so the two paths cannot diverge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@firmware/host/modules/conversation/chat-audioio/server-chat-websocket-worker.js` around lines 98 - 106, Update the WebSocket worker’s connect/disconnect state by storing the selected io class, including the secure wss path, in an instance field during connect. In disconnect(), reuse that stored class for the close opcode instead of always reading device.network.ws.io, while preserving the existing close frame and state transition.firmware/host/modules/conversation/chat.ts (1)
163-166: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winVerify the two forwarded contracts: type-as-specifier and endpoint-in-
providerID.Line 163 now forwards the chat type directly as a worker module specifier. Line 166 overloads
providerIDwith a WebSocket URL. The server model reads the endpoint frommessage.providerID, so the overload works for that worker. Other workers may interpretproviderIDas a provider identifier, and the module load fails at runtime if aChatTypevalue has no matching manifest specifier.Confirm both contracts across the repository. If the base
ChatAudioIOaccepts only a fixed option set, document theproviderIDoverload next to line 166.#!/bin/bash # Map ChatType values and manifest specifiers, then find other providerID consumers. set -euo pipefail rg -n -C3 'ChatType' --glob '*.ts' firmware/host/modules/conversation fd -e json . firmware/host/modules/conversation --exec rg -n 'modules|preload|stackchan|openAI' {} rg -n -C4 'providerID' firmware🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@firmware/host/modules/conversation/chat.ts` around lines 163 - 166, Verify the forwarding contracts for config.specifier and providerID across ChatAudioIO, worker manifests, and providerID consumers. Ensure each ChatType resolves to a valid worker module specifier rather than relying on an unmapped value, and preserve endpoint forwarding only where the receiving worker expects message.providerID to contain the WebSocket URL. If ChatAudioIO permits this overload, document it beside the providerID forwarding.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@firmware/host/modules/conversation/chat-audioio/server-chat-websocket-worker.js`:
- Around line 116-119: Update onJSON to dispatch remote messages only through an
explicit declared-handler allowlist, rather than using `type in this`, so
inherited transport methods cannot be invoked. Add the handler table to the
relevant subclass or base dispatch design and preserve invocation of accepted
event handlers with the JSON payload.
- Around line 81-92: Update the websocket worker’s onReadable callback so data
consumed by this.ws.read(count) is processed even before onWritable runs; remove
the `#state` === 1 read gate or buffer and replay early frames. Keep the existing
onWritable behavior for setting `#writable` and performing the open transition,
while ensuring the initial server session frame cannot be discarded.
In
`@firmware/host/modules/conversation/chat-audioio/server-openai-realtime-model.js`:
- Around line 37-44: Extend the cleartext validation in the model constructor
around the existing apiKey guard to reject any credential-bearing endpoint path
or query when endpoint.secure is false, including the token query used by the
test endpoint. Preserve insecure ws:// only when the endpoint contains no
credentials, or explicitly enforce the documented trusted-LAN disposable-token
policy.
- Around line 25-28: Update the configure method in
server-openai-realtime-model.js to clear configurationError at the start before
parsing the new message, so a previous failure does not leak into later valid
configurations. Keep the existing error-setting behavior inside configure and
the connect flow unchanged, but ensure the current configuration fully replaces
any stale state.
---
Nitpick comments:
In
`@firmware/host/modules/conversation/__tests__/server-realtime-model/audio-codecs.js`:
- Around line 1-5: Update Encode.toAlaw in the codec double to reduce 16-bit
input samples to one output byte per sample rather than copying bytes
one-to-one. Extend the server-realtime-model test assertion around
sendAudioBuffer to verify the transmitted byte content, ensuring the encoding
step is exercised beyond the message.size reduction.
In
`@firmware/host/modules/conversation/__tests__/server-realtime-model/server-realtime-model.test.ts`:
- Around line 30-37: Add failure-path coverage in the tests around
model.configure and the session.created flow: verify non-WebSocket providerID
and apiKey used with ws:// cause connect to post failed, then reconfigure with a
valid endpoint and verify the session succeeds without retaining the prior
configurationError. Use the existing tool, connect mock, and valid configuration
symbols.
In
`@firmware/host/modules/conversation/chat-audioio/server-chat-websocket-worker.js`:
- Around line 98-106: Update the WebSocket worker’s connect/disconnect state by
storing the selected io class, including the secure wss path, in an instance
field during connect. In disconnect(), reuse that stored class for the close
opcode instead of always reading device.network.ws.io, while preserving the
existing close frame and state transition.
In `@firmware/host/modules/conversation/chat.ts`:
- Around line 163-166: Verify the forwarding contracts for config.specifier and
providerID across ChatAudioIO, worker manifests, and providerID consumers.
Ensure each ChatType resolves to a valid worker module specifier rather than
relying on an unmapped value, and preserve endpoint forwarding only where the
receiving worker expects message.providerID to contain the WebSocket URL. If
ChatAudioIO permits this overload, document it beside the providerID forwarding.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 095fe67a-3db3-44a1-9c5b-0011de85529b
📒 Files selected for processing (12)
.changeset/stackchan-ai-realtime.mdfirmware/host/modules/conversation/__tests__/chat-service/chat-service.test.tsfirmware/host/modules/conversation/__tests__/server-realtime-model/audio-codecs.jsfirmware/host/modules/conversation/__tests__/server-realtime-model/manifest.test.jsonfirmware/host/modules/conversation/__tests__/server-realtime-model/server-chat-websocket-worker.jsfirmware/host/modules/conversation/__tests__/server-realtime-model/server-realtime-model.test.tsfirmware/host/modules/conversation/chat-audioio/server-chat-websocket-worker.jsfirmware/host/modules/conversation/chat-audioio/server-openai-realtime-model.jsfirmware/host/modules/conversation/chat-audioio/server.manifest.jsonfirmware/host/modules/conversation/chat-audioio/stackchan-server-openai-realtime.jsfirmware/host/modules/conversation/chat.tsfirmware/host/modules/conversation/manifest.json
1 participant
Summary
ChatServicecallers to select a worker specifier and server endpoint explicitlyChatAudioIOimplementation and leave all other hardware targets unchangedWhy
stack-chan-ai supplies its own
/device/v1/realtimeWebSocket endpoint. The direct OpenAI worker used by the MOD hardcodesapi.openai.com, so passing the stack-chan-ai endpoint to that worker does not route audio through the server.PR #634 contains the server adapter, but also includes duplex/AEC, renderer, diagnostics, and benchmark work that is not required for stack-chan-ai connectivity. This PR extracts only the integration boundary needed by the existing half-duplex pipeline.
Impact
esp32/m5stackchan_cores3.Validation
npm run formatnpm run lint(passes; one pre-existing informational finding)npm run test:unitnpm run check:architecture(73 passed)npm run check:manifest(6 targets passed)chat-serviceandserver-realtime-modelnpm run build:m5stackchan_cores344:1b:f6:e2:99:a4: Wi-Fi, control WebSocket, server-backed Realtime worker, transcription/history persistence, and TTS verified through the fourth voice turn without a reboot or debugger exceptionSummary by CodeRabbit