feat(voice): the talk button, and voice as a mode on the chat surface (C-D) - #2397
Conversation
The first user-visible piece of the audio-native path. Everything beneath it was merged and untested by a human; this is the part a person actually touches. THE SHAPE, as settled: - ONE trigger, in the nav bar, on every route. Nothing connects until it is pressed. It doubles as the live indicator and as the way back to a minimized call — and it NEVER hangs up, because a toggle there means pressing the live indicator hides the call you pressed it to see. Ending a call is the End button on the call. - It binds to whatever assistant is in view: the sidebar's selected agent on a page route, the GlobalAssistantView conversation on the dashboard (where the sidebar has no chat tab at all). There is no second target picker anywhere. - Voice is a MODE on the chat surface, not a fourth tab and not an overlay: the same message list, with a live call header above it. Spoken turns arrive in that list as ordinary messages — the realtime server writes them through messageRepository, so they ride the `conversation:*` events every surface already subscribes to. No new client wiring, and no second history. - Closing the sidebar minimizes; navigating updates locationContext and does NOT rebind; the sidebar's agent switcher DOES rebind. Every decision is a pure module beside the wiring — `resolveVoiceBinding`, `rebindAction`, `decideReveal`, `describeCall`, `toVoiceLocationContext` — so the rules are testable without a browser, a peer connection or a microphone. `rebindAction` is the load-bearing one. Navigating and switching agent arrive as the SAME change to the derived target, so a "watch the target and call start" effect cannot tell them apart and would hang up the user's call every time they opened a page. A rebind is therefore driven by the switch EVENT: the switcher records an intent, and the intent is applied once the newly chosen agent's conversation has resolved. Navigation records nothing. TWO THINGS THE MERGED SEAM WAS MISSING, added at the provider rather than worked around in the UI: - `failure` — the classified reason beside `error`. A sentence cannot be branched on, and a denied prompt and an absent capture device need opposite affordances: one offers Try again, the other must not, because a retry that cannot conjure hardware teaches the user that voice is broken. - `muted`/`setMuted` — mute has to live with the microphone. Held by the chrome it would return un-muted every time the sidebar was collapsed. It also fixes a latent bug: the swap hardcoded `setMicrophoneEnabled(true)`, so a chain at the duration ceiling un-muted a deliberately muted call. `messages.source` now reaches the UI (conversion AND broadcast, so the glyph does not appear only after a refresh) and both renderers mark a spoken turn. The browser reads its own copy of the value with a drift guard, since client components here cannot import @pagespace/db/schema. Also: the right sidebar's page-context tab moved from panel-local state into the layout store, because a button in the header cannot reach a `useState` inside a panel that is unmounted while collapsed. Unrelated to this chunk, but the branch's knip gate was already red for it: two per-member request types in voice-bridge-contract.ts (from #2395) were exported and never imported. Removed — consumers narrow on the union's `kind`. Gate: `bun run typecheck` 17/17, `bun run lint` 15/15, `bun run knip:check` within baseline, web tests 17085 passed with only the 17 known Postgres-requiring integration files failing (no test DB in this worktree). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PyxC1FeYap8bU98cLzcXhH
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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: 2db59d583a
ℹ️ 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".
| const busy = chrome.state === 'connecting' || isStarting; | ||
| // Disabled ONLY while the target is still resolving with nothing running. | ||
| // Never disabled during a live call: that press is the way back to it. | ||
| const disabled = !live && (binding.kind !== 'ready' || isStarting); |
There was a problem hiding this comment.
On a fresh page-route load with the right sidebar closed and a previously selected agent restored from local storage, only selectedAgent is persisted while conversationId remains null; the hook that resolves that conversation lives inside the unmounted SidebarChatTab. This condition disables the trigger indefinitely, so it cannot reveal the sidebar that would mount the resolver, and voice remains unavailable until the user manually opens the assistant panel.
Useful? React with 👍 / 👎.
| const handlePress = useCallback(async () => { | ||
| // Already up: this press is "show me the call", never "end it". | ||
| if (live) { | ||
| onReveal(binding.surface); |
There was a problem hiding this comment.
When a call is started with a sidebar page agent and the user navigates to the dashboard, the session intentionally remains bound to the old sidebar conversation, but binding.surface changes to dashboard. Pressing the live indicator therefore invokes the dashboard no-op reveal while GlobalAssistantView usually shows a different conversation, so no call bar or End button appears and the user must navigate back manually to hang up.
Useful? React with 👍 / 👎.
| * unified reader (`unifiedColumns`); carried out to the UI here so a thread | ||
| * can be honest about how it was made. | ||
| */ | ||
| source?: string | null; |
There was a problem hiding this comment.
For global-assistant conversations, the new source field is added only to DatabaseMessage; GlobalAssistantMessage and convertGlobalAssistantMessageToUIMessage still omit it, and the global messages route does not pass msg.source. A spoken turn consequently has its microphone glyph when received through the realtime event but loses it after a refresh or history reload.
Useful? React with 👍 / 👎.
The audio-native path is merged end to end — server call plane (#2393), server behaviour (#2395), browser + lifecycle (#2396), UI (#2397) — so the old pipeline is dead weight on the conversational path. This removes exactly that much and nothing else. WHAT WENT, and why each piece could not stay: - `useVoiceMode` + `/api/voice/transcribe`: the loop itself. Whisper existed to turn audio into text before inference; the realtime session hears the audio directly, so there is nothing left for it to do. Verified with a repo-wide search that no other caller reaches the route. - `useVoiceModeStore` and everything reading it — `VoiceCallPanel`, `VoiceModeSettings`, `VoiceModeBorder`, and the mic button in the chat box's footer. The store's only writer was that button. Left in place, the border would be UI that can never render and the button an affordance that toggles a mode nothing implements. The way into voice is the nav-bar trigger. - `selectVoiceStreamText`, `selectVoiceActivationBaseline` and `selectPostBaselineAssistantMessage`: three pure selectors whose only job was deciding which written reply the old path should speak. Spoken turns now arrive as ordinary messages, so nothing derives a "what to say out loud" from the message list any more. - The Whisper rate in `voice-pricing`, and `VOICE_HOLD_ESTIMATE_CENTS` — the flat hold that existed because STT could not know its own cost until the provider answered. Both had exactly one caller, the deleted route. WHAT DELIBERATELY STAYED. `/api/voice/synthesize`, the tts-1/tts-1-hd rates, `estimateVoiceHoldCents`, `VOICE_MAX_INFLIGHT` and `chunkForTts` all back Read Aloud, which is an open PR (#2173) and a genuinely different feature: an audio-native conversation does not replace "read this to me". `mic-errors` stays because the realtime path is now its only consumer. `chunkForTts` is kept despite having no in-tree caller on this branch — its last one went with `VoiceCallPanel` — because `useReadAloud` imports `flushForTts` from it on #2173. Deleting it would break work in flight. knip does not report it, so it needed no ignore. The one knip.json line added is for `@radix-ui/react-slider`: deleting VoiceModeSettings left `components/ui/slider.tsx` as its only importer, and `src/components/ui/**` is already ignored. TESTS DELETED WITH THEIR SUBJECTS, never to make the gate pass: `useVoiceModeStore.test.ts`, `transcribe/route.test.ts`, and the three stream selector tests. `voice-pricing.test.ts` loses its Whisper describe block; the unknown-model and 1¢-floor assertions are kept, retargeted off `whisper-1`. `whisper-1` survives as a fixture in the admin billing-coverage tests, where it stands for historical usage rows that still exist in the database. Gate: monorepo `bun run typecheck` 17/17, `bun run lint` 15/15, knip ratchet green with an unchanged baseline. Unit suites pass (lib 9175, web 17057); the only red files are the DB-backed integration tests, which need a Postgres this worktree has no access to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYjZgYpebAVgBq9f5oVt81
1 participant
The first part of the audio-native voice path a human actually touches. Everything beneath it — the pure core (#2387), the tool registry (#2388), pricing (#2391), the server joining the call (#2393), what it does while it's there (#2395), and the browser holding the mic (#2396) — is merged, tested, and has never been driven by a real person.
The shape
One trigger, in the nav bar, on every route. Nothing connects until it is pressed — no microphone, no negotiation, no permission prompt on page load. It doubles as the live indicator and as the way back to a minimized call, and it never hangs up: a toggle there would mean pressing the live indicator hides the call you pressed it to see. Ending a call is the End button on the call itself.
It binds to whatever assistant is in view. On a page route: the right sidebar opens in voice mode, bound to its selected agent. On the dashboard — where
showChatTab = !isDashboardContextmeans the sidebar has no chat tab at all — it binds to theGlobalAssistantViewconversation already on screen and renders there. There is no second target picker anywhere.Voice is a mode on the chat surface, not a fourth tab and not an overlay: the same message list, with a live call header above it (level meter, speaking state, mute, end). Spoken turns arrive in that list as ordinary messages — the realtime server writes them through
messageRepository, so they ride theconversation:*events every surface already subscribes to. No new client wiring, and no second history.Closing the sidebar minimizes, it does not hang up. The session lives above
RightPanel; the nav mic stays lit.The decision that everything turns on
Navigating and switching agent arrive as the same change to the derived target. A "watch the target and call
start" effect cannot tell them apart, and would hang the user's call up every time they opened a page. So a rebind is driven by the switch event: the switcher records an intent, andrebindActionapplies it once the newly chosen agent's conversation has resolved (selectAgentclears the id and re-resolves it asynchronously, so there is nothing to bind to at the moment of the click). Navigation records nothing, so navigation can never rebind.Every decision is a pure module beside the wiring —
resolveVoiceBinding,rebindAction,decideReveal,describeCall,toVoiceLocationContext— testable without a browser, a peer connection or a microphone.Two things the merged seam was one field short on
Added at the provider rather than worked around in the UI:
failure— the classified reason besideerror. A sentence cannot be branched on, and a denied prompt and an absent capture device need opposite affordances.classifyMicFailurealready draws the line andconnect.tsalready carries it; this is where it reaches the screen, as the difference between a Try again button and no Try again button. A retry offered for a missing microphone is guaranteed to fail, and teaches the user that voice is broken rather than that they have no device.muted/setMuted— mute has to live with the microphone. Held by the chrome it would come back un-muted every time the sidebar was collapsed. This also fixes a latent bug: the chain swap hardcodedsetMicrophoneEnabled(true), so reaching the duration ceiling un-muted a deliberately muted call, mid-conversation, with no control touched.Also
messages.sourcenow reaches the UI — through both the DB conversion and the broadcast, so the mic glyph does not appear only after a refresh. Both renderers mark a spoken turn. The browser reads its own copy of the value (client components here cannot import@pagespace/db/schema) with a drift guard test.useStateinside a panel that is unmounted while collapsed. Same default, same lifetime, not persisted.Acceptance criteria
VoiceNavTrigger.test.tsxVoiceNavTrigger.test.tsx,voice-binding.test.tsVoiceNavTrigger.test.tsx,Layout.voice-reveal.test.tsxVoiceSessionBridge.test.tsxVoiceSessionBridge.test.tsxSpokenTurnGlyph.test.tsx,message-utils.test.tscall-chrome.test.ts,VoiceNavTrigger.test.tsxAssert structure, not arrangement
Every guarantee was verified by breaking it and watching a test go red, then restoring — 22 mutations, all caught:
crossing the two surfaces' agent/conversation pairs · rebinding on any derived-target change ·
starton navigation instead ofsetLocationContext· resolving location while idle · reveal as a toggle · reveal not raising the chat tab · the bridge deleted fromLayout· the trigger hanging up while live · retry offered unconditionally · the failure reason dropped at the provider · the chain un-muting · hangup keeping the mute · the call bar keyed on "any live call" instead of the conversation · the glyph reading anything-non-null · the glyph dropped from one renderer only · the drift-guard constant diverging ·RightPanelreverting to panel-local tab state · and more.Two structural files exist only to hold guarantees nothing else could:
Layout.voice-reveal.test.tsx(the bridge renders nothing, so deleting it is otherwise invisible) andTopBar.voice-trigger.test.tsx(move the trigger into the sidebar and every behavioural test still passes while voice quietly becomes a feature of a panel).The chat surfaces are ~1000-line files the repo does not render in tests, so the "which surface shows the call" decision was extracted into
VoiceCallBarForConversationrather than left as an&&in each — one line, one place, exercised against a real session.Not from this chunk
The branch's knip gate was already red: two per-member request types in
voice-bridge-contract.ts(added by #2395) were exported and never imported anywhere. Removed — consumers narrow on the union'skind. Nothing else underpackages/is touched.Gate
bun run typecheck(monorepo) — 17/17bun run lint— 15/15bun run knip:check— within baselineOld voice mode is untouched:
useVoiceMode.tsand/api/voice/*stay, and Read Aloud's/api/voice/synthesizeis not being retired at all.🤖 Generated with Claude Code
https://claude.ai/code/session_01PyxC1FeYap8bU98cLzcXhH