voice: make passive ptt_start the canonical hands-free narration fix#326405
Draft
accnops wants to merge 9 commits into
Draft
voice: make passive ptt_start the canonical hands-free narration fix#326405accnops wants to merge 9 commits into
accnops wants to merge 9 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Improves voice narration recovery during hands-free listening and reconnects.
Changes:
- Marks passive barge-in PTT sessions.
- Restores session and deferred narration state after reconnects.
- Adds narration defer/drop telemetry and tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
voiceClientService.test.ts |
Tests PTT serialization and session recovery. |
common/voiceClientService.ts |
Extends the PTT API with passive. |
voiceTelemetry.ts |
Defines narration lifecycle telemetry. |
voiceSessionController.ts |
Replays and revalidates deferred narrations. |
browser/voiceClientService.ts |
Serializes passive PTT and adopts server session IDs. |
micCaptureService.ts |
Preserves passive state through microphone acquisition. |
accnops
force-pushed
the
arthurcnops/voice-narration-handsfree-fix
branch
from
July 17, 2026 22:18
c06bdc7 to
fb29d8c
Compare
accnops
force-pushed
the
arthurcnops/voice-narration-handsfree-fix
branch
4 times, most recently
from
July 18, 2026 08:34
a85eb07 to
3a98729
Compare
… user speaking Thread a `passive` flag immutably through mic-start into the ptt_start frame instead of reading controller state at emit time, which could race with the async getUserMedia acquire in the cold-capture path. - micCaptureService: pttDown(turnId, passive = false) now captures passive per-press and fires it on onPttStart (Emitter<boolean>) at both the warm and post-acquire fire sites. - voiceClientService: sendPttStart(turnId, passive = false) serializes `passive: true` only when true, keeping the wire frame byte-identical for real presses / old clients. - voiceSessionController: onPttStart forwards the captured passive to sendPttStart; _startBargeInListen is the only caller passing passive=true (verified single true caller). Safe because barge-in promotion (a real press during a passive listen) does not re-send ptt_start, so the passive flag captured at listen-open time remains correct for the whole turn. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 803dc35b-1aee-4354-85c7-df6076293a49
`_isResuming` was set on reconnect but never cleared, and a failed resume refused to adopt the server's fresh session_init id, leaving the client believing it still owned a dead session and stalling reconnect. On both `session_init` and `session_resumed`, always adopt `msg.session_id` and clear `_isResuming`; also reset `_isResuming` in `_cleanup()` for terminal disconnects. Adopting a fresh id on a failed resume is strictly more correct than the previous refusal. Extended the test createTestWindow harness to bind setInterval and clearInterval to the real window target (native timer methods are branded and throw Illegal invocation when called with a Proxy as this), which was needed to drive ws.onopen in the new tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 803dc35b-1aee-4354-85c7-df6076293a49
`_deferredNarrations` (busy/interrupted narrations awaiting a narration_unblocked nudge) was never replayed on reconnect - only _pendingNarrationRetries was. The narration_unblocked nudge that would otherwise trigger a retry is itself lost with the dropped socket, so a narration deferred right before a disconnect could be silently lost forever. On session_init/session_resumed, now also replay _deferredNarrations by reusing _retryDeferredNarration for each entry - the same revalidation already used for the live narration_unblocked path: it re-checks _currentNarratable before firing and reuses the same narration_id when the text is unchanged, so backend idempotency returns accepted without re-synth for anything already spoken. Entries no longer warranted or no longer the shown session are dropped without speaking, same as before. _retryDeferredNarration now returns whether it actually sent a retry (mirrors _narrate's return) so the reconnect handler can gate entering auto-listen the same way it already does for _pendingNarrationRetries - a narration about to play should not be torn down by a listen that starts moments later. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 803dc35b-1aee-4354-85c7-df6076293a49
`_retryDeferredNarration` ignored the `narration_id` carried by `narration_unblocked` and always retried whatever was currently deferred for the session key. `_deferredNarrations` is latest-wins per session key, so if a second narration for the same session went busy before the first one's `narration_unblocked` arrived, the stale nudge would retry the newer (possibly still-busy) entry early instead of waiting for its own unblock. `_retryDeferredNarration` now takes an optional `unblockedNarrationId`; when it does not match the currently deferred entry's id, the retry is skipped - the correct nudge for that entry will arrive separately once its own guard clears. The reconnect replay path (no unblock event, no id to compare against) is unaffected and keeps retrying whatever is currently deferred. Low risk: retrying a superseded id was already an idempotent no-op server-side, so this only removes an unnecessary/early retry attempt. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 803dc35b-1aee-4354-85c7-df6076293a49
…ent-side Add voiceNarrationDeferred/voiceNarrationDropped telemetry events (voiceTelemetry.ts) following the existing publicLog2 pattern used elsewhere in voiceSessionController.ts (owner meganrogge, SystemMetaData/ FeatureInsight classification). Only `kind` (response|confirmation) and a `reason` enum are logged - never narration text. Wired into the four client-side branches where a narration is deferred or dropped without ever playing: - _handleNarrationAck: 'invalid' disposition -> dropped (reason: invalid); 'busy' disposition -> deferred (reason: busy) - _handleNarrationInterrupted -> deferred (reason: interrupted) - _retryDeferredNarration: no longer warranted -> dropped (reason: stale); session no longer shown -> dropped (reason: session_changed) No dedicated unit test: voiceSessionController has no unit harness (per task scope, not building a new one). Verified via typecheck/eslint on the touched files and the unaffected voiceClientService.test.ts suite (15/15 passing). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 803dc35b-1aee-4354-85c7-df6076293a49
Extend the passive open-mic flag to the auto-listen path (not just barge-in) so every hands-free turn opened without a deliberate user press sends ptt_start with passive:true. The backend then leaves user_is_speaking clear on those silent turns, so a client-driven narration request is accepted instead of being NACK'd busy: user_speaking and stranded. This supersedes the force-ptt_end workaround from microsoft#326378: a passive turn never latches in the first place, so there is no latch to force-clear. Remove that mechanism (_userSpeechActive, _isActivelyDictating, the _finishPtt 'immediate' variant, _prepareForPlayback(endOpenTurn), and canRequestNarration) and restore _narrate to unconditionally prepare for playback on a sent request. Depends on backend passive handling (yolo#56284): must be live in GA before this lands, or silent hands-free turns latch again on the old backend. Adds controller tests: auto-listen opens a passive mic turn; a deliberate press stays non-passive.
…playback
A deliberate (non-passive) ptt_start latches the backend's user_is_speaking,
so its narration request is NACK'd busy and deferred. _prepareForPlayback
previously aborted any held turn via _finishPtt('auto'), which sends no
ptt_end and strands that latch (the later physical release no-ops because
_pttHeld is already false). Track the current turn's passiveness and skip the
abort for a held deliberate press, leaving its natural release to send
ptt_end and clear the guard. Passive open-mic turns (auto-listen / barge-in),
which never latch, are still torn down to free the mic for narration audio.
When a deliberate (non-passive) press is held, its natural release drives the narration_unblocked retry, so the playback slot must not be torn down under it (that sends no ptt_end and strands the backend latch). Three changes keep a held press intact without dropping or duplicating its buffered reply: - _flushDeferredResponse peeks the slot via _prepareForPlayback() (now returns a boolean) and only deletes/enqueues the buffered chunks once the slot is actually free; a held deliberate press leaves the buffer intact and reports `retained` plus the buffered reply's transcript. Passive open-mic turns (which never latched) are still torn down. - _activateShownSession skips a fresh narration only for the SAME reply the retained buffer will replay on release (matched by normalized transcript); otherwise it would double up with that buffer. An unrelated confirmation or a newer, different response for the session still narrates, because a stale buffered response can outlive its turn (thinking clears _deferredNarrations, not _deferredResponses). The pending indicator stays until the buffer plays. - Clear _pttCurrentTurnPassive when a passive barge-in listen is promoted to a deliberate press, so playback prep preserves the promoted press.
accnops
force-pushed
the
arthurcnops/voice-narration-handsfree-fix
branch
from
July 18, 2026 08:54
3a98729 to
5bdc624
Compare
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.
Problem
In hands-free mode the client opens a passive barge-in mic that keeps streaming while it waits for the user to actually speak. Auto-listen does the same. Those
ptt_startevents made the backend latch the session as "user is speaking" for the whole open-mic window, and the window ends silently (the mic is aborted with noptt_end), so the latch never cleared. Client-driven narration requested during that window was NACK'dbusyand stranded. A few reconnect and unblock paths also lost or failed to replay narrations that were deferred while the session was busy.#326378fixed the same symptom from the frontend by force-ending the open turn (synthesizing aptt_end) before narrating. That works, but it overloadsptt_endto mean "stop for playback" and can cut a live barge-in short.Solution
Address the root cause instead of forcing an end. The client tags every silent open-mic turn (barge-in and auto-listen) with
passive: trueonptt_start; the backend setsuser_is_speaking = not passive, so a passive turn never latches. Deliberate presses stay non-passive and still latch, which correctly defers narration while the user is actually speaking.Because this makes the passive flag the canonical fix, it reverts the
#326378frontend workaround (now on main):_userSpeechActive,_isActivelyDictating(),_finishPtt('immediate'), the_prepareForPlayback(endOpenTurn)parameter,canRequestNarration, and theendPassiveTurnFirstblock in_narrate.Supporting changes on the branch:
narration_unblockedinstead of the latest._prepareForPlayback: a non-passive press latcheduser_is_speaking, so its narration was deferred; aborting it here would send noptt_endand strand the latch. Its natural release sendsptt_end, clears the guard, and drives thenarration_unblockedretry. Passive open-mic turns (which never latched) are still torn down to free the mic for narration audio._prepareForPlaybacknow returns whether the slot is ready._flushDeferredResponsepeeks the buffered response, prepares the slot, and only deletes/enqueues it when the slot is actually ready; a preserved press keeps the buffer intact and reportsretainedplus the buffered reply's transcript._activateShownSessionthen skips a fresh narration only for the SAME reply that buffer will replay on release (matched by normalized transcript). An unrelated confirmation or a newer, different response for the session still narrates, because a stale buffered response can outlive its turn (athinkingtransition clears_deferredNarrations, not_deferredResponses). The buffer plays on release via the periodic safety-net; the pending indicator stays until then, so nothing is lost.Tests
voiceSessionController.test.tsandvoiceClientService.test.ts:Full voiceClient suites pass (controller 19, client service 17, glow 3). ESLint clean; typecheck clean (only the 8 pre-existing native-module errors unrelated to this change).
Risks
passive(yolo#56284). This must reach GA before this lands. If it merges first, the backend ignorespassiveand, with#326378reverted, hands-free narration would strand again. Kept as a draft for that reason.#326378; anyone relying on the forced-ptt_endbehavior would see it change to the passive model.Non-goals
ptt_startwas already sent aspassive: true, so the backend still treats it as passive and does not latch. With VAD, real speech during the press cancels any narration via the speech edge. Without VAD there is no speech edge, so a narration requested during that brief promoted-press window can be accepted and play over the user. A wire-level promotion that latches the backend for the same turn is deferred (it needs a protocol change and carries regression risk on the working deliberate-press path).