Voice: client-driven narration via explicit requestNarration API#325682
Open
meganrogge wants to merge 14 commits into
Open
Voice: client-driven narration via explicit requestNarration API#325682meganrogge wants to merge 14 commits into
meganrogge wants to merge 14 commits into
Conversation
Deferral keyed off `_getActiveSessionId()`, which prioritizes the sticky
input `_targetSession` over the session the user is actually viewing. In the
floating voice window, browsing a session via `openSession` changes what is
shown but not `_targetSession`, so with target=A while viewing B:
- A's background narration played live over the unrelated shown session B
("a different session's response plays in an unrelated session"); and
- B's own reply was deferred with no focus change to ever flush it, leaving
the controller idle/listening ("listening mode when a pending reply should
have been read out on focus").
Add `_shownSessionId()` (the viewed session, ignoring `_targetSession`) and
key `_shouldDeferResponse` off it plus `_awaitingReplyForSession` (so the
awaited answer to the user's own utterance still plays live even if they
glanced away). No behavior change in the agents/sessions window: external
mode never sets `_targetSession`, so the resolved shown session is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review: the 5s safety-net flush in _checkSessionStateChanges still used _getActiveSessionId() (target-priority), so a background session's reply could be flushed and played over the shown session on the next poll. Flush _shownSessionId() instead. Also trim the _shownSessionId / _shouldDeferResponse JSDoc and inline comment to the VS Code one/two-line convention and correct the "targeted or focused" wording to the new shown-session contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A new response from a session whose previous response is still playing was fast-pathed straight into playback. The TTS service holds a single playback turn, and once the prior response's final chunk scheduled node.stop() at that response's boundary, the new response's audio was appended into the same buffer past the stop point and never heard. Track whether the current response has been finalized and serialize a subsequent same-session response through the audio queue so it gets a fresh turn. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The backend narrates one thing at a time and drops a session's completion narration when another is in flight, so a reply that completes while its session is in the background is never sent as audio - nothing to buffer, and returning to the session finds it silent in listening mode. Track background completions and, on focus, re-elicit narration via a forced thinking->idle transition (mirroring the confirmation two-phase). Cleared when the reply's audio actually arrives (played or deferred), on a new turn, or once re-elicited, so a reply is never read twice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Responses now re-assert once if their re-narration is preempted by another session's narration, matching the bulletproof two-mechanism approach used for confirmations (two-phase transition + preemption recovery). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The on-focus re-narration of a backend-dropped reply was gated to non-external mode, so it never ran in the embedder-driven agents window where the bug was actually reported. Confirmations run their two-phase unconditionally in both modes; do the same for responses. The unheard marker is already populated in both modes and is cleared on audio receipt, so a genuinely heard reply is never re-narrated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The unheard-reply marker was only set for completions of sessions NOT shown at debounced-emit time. On a fast switch, a background session's completion could arrive after the user already switched to it, so it was treated as 'narrates live', never marked, and — when the backend dropped its narration — silently lost (no pending indicator, no re-narration). Now mark every idle+summary completion as a candidate unheard reply and clear the marker the moment its audio is actually heard (tagged, or the active session's untagged live audio). A reply that narrates live is never re-read; only a dropped one survives. Add a response re-narrate watchdog (mirroring the confirmation flush watchdog) so a reply completing on the on-screen session is re-checked and re-narrated if still unheard, covering the focus/emit ordering race. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…drop Fix several voice-mode narration reliability bugs on fast session switches, address Copilot review feedback, and stop a passive backend query from killing a just-started auto-listen. - Hold a background completion as `thinking` (summary withheld) until its session is viewed, so the backend's first `->idle` sighting happens on focus and reliably narrates, instead of being deduped. Applied across all three `_buildSessionContext` branches (resident/no-model agent sessions and regular chat sessions), so both the agents window and the main window are covered. - Decide "already narrated?" by summary identity (`_narratedSummaryById`, recorded when audio is actually heard) rather than the racy recently-read timing, so an eager-reload context push that leaks a raw idle+summary can no longer drop the on-focus re-narration. - Route re-narration by the SHOWN session, not the sticky input target: add `_narrationActiveOverride` so `_buildSessionContext` marks the viewed session active during its two-phase, and gate phase-2 / preemption recovery / watchdog on `_shownSessionId()`. - Serialize same-session responses: queue entries carry a `finalized` flag; every first chunk starts a fresh entry and continuation chunks attach only to an unfinished entry, so a later response can't be merged past a scheduled node.stop() and dropped. - Share unheard-reply bookkeeping (`_recordCompletionForRenarration`) between the coalesced autorun path and `_checkSessionStateChanges`, so completions surfaced only by the direct path (remote/unloaded sessions) are recoverable. - Don't finish PTT / churn voice state for passive backend queries (`get_session_info`/`get_session_changes`/`get_session_thread`); dispatching them like an action tool was ending auto-listen the instant the backend probed the session on connect. Also clear toggle mode in `_finishPtt` so no turn-end path leaves a stale toggle that self-kills the next listen. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the client-side machinery that tricked the backend into
narrating (two-phase re-narration, reconcilers, renarrate watchdogs,
background-idle holding, and summary-identity dedup) with a single
explicit requestNarration(codingSessionId, kind, text) call. The
client now decides when a reply or confirmation is spoken: live when
an item lands on the shown session, and on focus for an item that
arrived in the background. A per-session last-narrated-text guard makes
both paths idempotent, replacing the removed dedup/holding logic.
Backend contract: on receiving {type:'request_narration',
coding_session_id, kind, text} the backend speaks the given text and
echoes audio tagged with coding_session_id; it must stop auto-narrating
idle / waiting_for_confirmation transitions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces inferred voice narration with an explicit client-driven backend request.
Changes:
- Adds the
requestNarrationAPI and WebSocket message. - Narrates shown-session responses and confirmations with per-session deduplication.
- Refines audio queuing, session routing, and passive tool handling.
Show a summary per file
| File | Description |
|---|---|
voiceClientService.ts (common) |
Defines the narration API contract. |
voiceClientService.ts (browser) |
Sends narration requests over WebSocket. |
voiceSessionController.ts |
Coordinates narration, deduplication, routing, and playback. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 4
- Review effort level: Medium
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…clear, doc fixes - requestNarration returns whether it sent; _narrate records the dedup entry only on a successful send and retries the shown item on session_init, so a narration dropped while the socket was down is no longer permanently suppressed. - Clear the per-session dedup on a real thinking transition before coalescing (skipping eager-reload wobble), so a fast idle->thinking->idle burst that collapses to net-zero still lets an identical next-turn reply narrate. - Update stale comments that described backend auto-narration via session_context; context is now state-tracking only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vijayupadya
previously approved these changes
Jul 13, 2026
Collaborator
Author
|
@copilot resolve the merge conflicts in this pull request |
Copilot stopped work on behalf of
meganrogge due to an error
July 13, 2026 20:59
…d-narrate-api # Conflicts: # src/vs/workbench/contrib/chat/browser/voiceClient/voiceSessionController.ts
pwang347
approved these changes
Jul 13, 2026
roblourens
approved these changes
Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes https://github.com/microsoft/vscode-internalbacklog/issues/8430
What
Replaces the client-side machinery that tricked the backend into narrating voice replies/confirmations with a single explicit
requestNarrationAPI. The client now decides when narration happens; the backend only speaks when asked.Why
The prior approach inferred narration from
session_contextstate transitions and used a growing pile of workarounds to make the backend narrate the right thing at the right time — two-phase re-narration, reconcilers, renarrate watchdogs, background-idleholding (reportingthinkinguntil focus), and summary-identity dedup. This was fragile: it depended on precisesession_contextdelta timing, raced against eager model reloads, and dropped or double-read replies in several reproducible cases.How
New API:
Sends
{ type: 'request_narration', coding_session_id, kind, text }over the websocket.The client fires it in exactly two places:
_handleNarratableStateChange)._activateShownSession→_currentNarratable).A per-session last-narrated-text guard (
_lastNarratedText) makes both paths idempotent — a reply already read live is not re-read on refocus — replacing all of the removed dedup/holding logic. The guard is cleared on a new turn (thinking) so a later identical reply still narrates.Text is passed inline so the backend never depends on the latest
session_contextdelta having arrived first.Removed
_narrateResponseViaTwoPhase,_narrateConfirmationViaTwoPhase,_reconcile*Narration,_armResponseRenarrateWatchdog,_recordCompletionForRenarration_maybeHoldBackgroundIdle(+ thebgHold/forceThinking/_narrationActiveOverridecontext plumbing)_narratedSummaryById,_unheardResponseSessions,_responseRenarrateWatchdogs,_forceThinkingOnce, and the associated timers/constsNet: ~486 lines removed.
Backend contract (required for this to work)
This is a coordinated client+backend change. The backend must:
{type:'request_narration', coding_session_id, kind, text}, speaktext(phrasing perkind) and echo the audio asaudio_responsetagged withcoding_session_id.idle/waiting_for_confirmationtransitions inferred fromsession_context. If it still auto-narrates, replies will be read twice.Follow-up
The audio-deferral subsystem (
_deferredResponses/_shouldDeferResponse/_flushDeferredResponse/_enqueueAudio/_isRenarration) is left intact but is now largely inert (background sessions are never narrated). It can be removed in a follow-up to further shrink this file.Testing
Not runtime-testable until the backend implements
request_narration;npm run typecheck-clientis clean.