Voice: answer question carousels by voice - #327899
Merged
meganrogge merged 22 commits intoJul 29, 2026
Merged
Conversation
The widget's private getOptionsWithDefaultsFirst defined the displayed order of a question's options, but the voice path needs the same order to number them for the user: the ordinal someone hears has to be the ordinal they see. Move it to a common module (implementation unchanged) and add the two voice helpers that hang off it - formatQuestionPrompt, which mirrors the backend's format_question_prompt so the client and the backend speak a form the same way, and resolveQuestionAnswers, which turns backend-resolved answers into carousel answers by exact option-value match only. resolveQuestionAnswers rejects the whole set on any unresolvable entry rather than applying what it can. The backend resolved these values against its own mirror of this schema, so a miss means the mirror was stale, and half-filling a form is a failure the user cannot see. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
A session's pending state reaches the backend only as agent_state_detail - a
sentence. That is enough to say out loud but not enough to act on, which is why
a spoken answer to a question form had nowhere to land. Add IVoiceSessionPending
alongside it, carrying the routing ids and the numbered options.
Also widen the narration kind to include 'question', and let sendToolResult
carry {ok, reason} instead of a bare string. The backend acknowledges a
dispatch only after seeing that result, so it has to be able to say no.
Types only; nothing populates or reads them yet.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
_getAgentStateInfo reduces a whole question form to "questions: <titles>", destroying the options, their values, the question ids and the request id before the backend ever sees them. Build the typed payload alongside it. pending_id is the part's index within its request. Response parts carry no identity of their own, and that list is append-only - parts are pushed and adjacent markdown is merged in place, never spliced - so an index keeps naming the same part. It is derived through one exported function because the inbound lookup has to reproduce it exactly; if the two ever disagree, every spoken answer comes back stale and the feature stops working quietly. Scans newest-first so an already-answered earlier form can't shadow the live one, and ships even when agent_state_detail is held back: withholding it would leave a form the user can see with no way to answer it by voice. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
Replaces approve_confirmation/reject_confirmation with one respond_to_session
path that can reach every pending construct, not just a tool invocation.
Routing is by pending_id + request_id with no fallback. The old path searched
only for a toolInvocation part -- a question carousel, a confirmation and an
elicitation were all invisible to it -- and when it could not resolve the
target session it fell back to whichever session happened to be focused, so a
spoken "yes" could approve a prompt the user was not looking at.
The result is now structured. The controller used to discard the dispatch
outcome and send a bare 'ok' for approve/reject, which is what let the backend
announce "Okay, approved." for something that never happened. It now reports
{ok, reason}, and the approval telemetry carries the observed outcome rather
than the assumption.
derivePendingId moves to common/ so the controller (which mints the id) and
the dispatch service (which reverses it) share one definition. If those two
ever disagreed every spoken answer would come back stale_pending and the
feature would fail silently, so its properties are pinned by a test.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
Narration of a question form spoke `agent_state_detail`, which flattens the whole form to "questions: <titles>". The user heard what they were being asked and none of the options they were meant to pick from, so there was nothing for them to say back. The narratable is now built from the structured pending payload, which carries the options in the order the widget renders them. Only ever question 1. Questions 2..N are spoken by the backend as it replies to each answer, so at first sighting the form is by definition untouched and the client needs no view of the backend's draft. The pending/question ids ride alongside the text rather than inside it. Every dedup, retry-reuse and interruption guard in the controller keys on text identity, so burying a per-occurrence id in the text would defeat all of them at once. Ordering: all three trigger paths narrated before sending the context that justifies the narration, and context is debounced 500ms on top. The backend validates a narration request against its mirror of the session context, so a first-sighting question would have been judged against a mirror that did not yet contain the form. Each site now invalidates, sends, flushes, then narrates. The spoken rendering moves to common/voiceClient/ and renders the wire shape. It used to renumber the options itself from the carousel; the ordinals already exist on the payload, and numbering them a second time from a second source is the one way this could read out a number that selects something else. Doing so also surfaced a real divergence from the Python it mirrors: the "you can also give your own answer" hint belongs inside the has-options branch, since a text question is freeform by definition and the hint is noise there. _stopConfirmationNarration becomes _stopPendingNarration and covers questions too: a form answered with the mouse while its narration is in flight would otherwise keep reading out a form that is no longer on screen. The refocus "already heard" guard is widened the same way, so a still-open form is not re-read on every refocus. Responses stay exempt -- a completed reply is still worth hearing after the thing it describes has been dealt with. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
A carousel answered by voice is resolved directly on the model, but the rendered part keeps its own skip/ignore handlers, which fire when the next request is submitted. Without this guard they overwrite the real answers with defaults. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
…on ref Four fixes from a review of the dispatch path: - "Skip" on an untouched form dispatched an `answer` with zero answers, which the client cannot distinguish from a backend that resolved nothing and correctly refuses as invalid_answer. Skipping is now its own `skip` response type carrying whatever was answered first, so the empty case is meaningful. - A just-loaded session's only reference was released before the response was applied, so the model could be disposed mid-dispatch. It is now held across the whole operation. - A confirmation answered by voice continued in built-in Agent mode with every tool enabled, regardless of the mode and model the user had selected. It now mirrors the widget's own button handler and only falls back to agent-mode options when the session has no open widget. - A confirmation was marked used only after its follow-up request was sent, so a second response arriving during that await could submit a contradictory answer. It is marked before the send and restored if the send fails. Also stop reporting success for a carousel that has neither a deferred completion nor a resolve id: nothing was resolved, so the assistant would have claimed an answer landed while the form was still on screen. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
…s by id Three narration bugs, all found by reviewing the question-narration path: A request named the question it wanted spoken, but the client can only ever see question 1 — the draft of answers given so far lives in the backend. Once question 1 was answered, every request the client could make was refused as stale, so a partially answered form went permanently silent after a refocus or a reconnect. A request now names only the form and the backend renders whichever question it is waiting on. Dedup keyed on spoken text, and text is not an identity: two forms can ask the same questions and two tools can raise the same prompt. A second form arriving while the first was open produced no transition, and was suppressed on refocus as "already heard". Every narratable now carries an occurrence identity — its pending id where it has one, its text otherwise — which is what the heard marker, the in-flight guard, the deferred-retry id reuse and state-transition detection all compare. Replacing one pending item with another without leaving waiting_for_confirmation now releases the heard marker and stops the narration it replaced. Narrations queued while the socket was down were replayed verbatim on reconnect. A form answered or dismissed during the disconnect would be read out regardless. They are now re-derived from the session as it is at replay time, the same way the busy-retry path already did. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
…omes `derivePendingId` used the part's index in `response.value`, documented as safe because that list is append-only. It is not: `Response.clear` and `clearToPreviousToolInvocation` both splice it, so retrying a request seats a new part at an index already published to the backend. The backend keys partial answers and "already narrated" markers off that id, so a reused one lets a draft written for one form be submitted against another. Ids are now minted per part object and never reissued. Lookups use `peekPendingId`, which never mints, so a part that was never offered as pending cannot be reached by a stale id at all. The elicitation branch reported success as soon as `accept()` resolved, but the handler decides the outcome -- an authorization elicitation that fails to open its URL settles as Rejected. That produced "Okay, approved" for something the agent received as a decline, which is the class of bug this whole change exists to remove. It now reports the state the request actually settled in. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
`accept` and `reject` both ran their handler and then wrote the result unconditionally, so whichever finished last owned the outcome. A decline by mouse could therefore be overwritten by a slower accept that was already in flight -- and worse, the accept handler still ran, opening the URL or granting the authorization the user had just refused. Claim the settlement synchronously before awaiting, in accept, reject and hide. JavaScript is single-threaded, so the claim genuinely makes the first caller the winner. This matches the confirmation and question-carousel parts, which are already first-wins. Also rewrite the pending-id dispatch test so it exercises the case a positional id gets wrong: an id minted for a part that was later spliced out must not resolve to whatever now occupies its position. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
The previous commit made settlement first-wins with a permanent flag, which was too coarse in two ways. `ElicitationState.Pending` is a legitimate handler result: the sandbox password prompt's "Focus Terminal" hands the user off to the terminal and deliberately leaves the request open, expecting the later `hide()` to settle it. A permanent claim meant that `hide()` could no longer do so, leaving a hidden request Pending forever -- which keeps the session reporting "awaiting confirmation" (chatModel `_pendingInfo`) and keeps voice advertising it. Ownership is now released when a handler answers Pending, and a `hide()` that arrived meanwhile is applied on the way out. An outcome recorded elsewhere also has to be able to supersede an accept that is still in flight. The agent host wrote `part.state` directly for a server-reported completion, and dispatched Cancel on cancellation and disposal; in both cases a slow `open()` could still resolve afterwards and overwrite the result, so voice would report an approval the server was given as Cancel. Those paths now go through `settle`, which invalidates the in-flight owner via a token. Disposal only overrides when it was the one that told the server, so an accept that already completed keeps its own result. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
Contributor
There was a problem hiding this comment.
Pull request overview
Updates voice sessions to safely narrate and answer pending question forms.
Changes:
- Adds structured pending-form context, stable IDs, and option narration.
- Dispatches validated answers with explicit success/failure results.
- Makes elicitation settlement first-wins and expands regression coverage.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
test/common/voiceClient/voicePendingNarration.test.ts |
Tests spoken question formatting. |
test/common/voiceClient/voicePendingId.test.ts |
Tests stable pending identities. |
test/common/model/chatElicitationRequestPart.test.ts |
Tests settlement races. |
test/common/chatQuestionCarouselHelpers.test.ts |
Tests option ordering and answer resolution. |
test/browser/widget/chatContentParts/chatQuestionCarouselPart.test.ts |
Tests stale skip/ignore behavior. |
test/browser/voiceClient/voiceToolDispatchService.test.ts |
Tests response dispatch outcomes. |
common/voiceClient/voicePendingNarration.ts |
Formats question narration. |
common/voiceClient/voiceClientService.ts |
Defines pending and dispatch protocol types. |
common/model/chatProgressTypes/chatElicitationRequestPart.ts |
Implements first-wins settlement. |
common/chatService/chatQuestionCarouselHelpers.ts |
Shares ordering and answer conversion. |
browser/widget/chatContentParts/chatQuestionCarouselPart.ts |
Uses shared ordering and stale guards. |
browser/voiceClient/voiceToolDispatchService.ts |
Applies structured session responses. |
browser/voiceClient/voiceTelemetry.ts |
Extends response telemetry. |
browser/voiceClient/voiceSessionController.ts |
Sends context before structured narration. |
browser/voiceClient/voiceClientService.ts |
Serializes structured results and narration IDs. |
browser/agentSessions/agentHost/agentHostSessionHandler.ts |
Coordinates authoritative elicitation settlement. |
Comments suppressed due to low confidence (2)
src/vs/workbench/contrib/chat/browser/voiceClient/voiceSessionController.ts:5617
- The carousel displays and announces
question.message ?? question.title, but the voice payload always sends the title. Built-inaskQuestionsdeliberately stores the short header intitleand the actual question inmessage, so voice users hear only the header plus options and can miss what is being asked.
title: question.title ?? '',
src/vs/workbench/contrib/chat/browser/voiceClient/voiceToolDispatchService.ts:495
sendRequestcan reject (for example, it throws for an unknown/disposed session). BecauseisUsedis set first and only restored for a non-sent return value, that exception permanently disables the confirmation even though no follow-up was sent. RestoreisUsedin a catch path before propagating the error.
confirmation.isUsed = true;
const result = await this.chatService.sendRequest(model.sessionResource, `${label}: "${confirmation.title}"`, options);
if (!ChatSendResult.isSent(result)) {
confirmation.isUsed = false;
return { ok: false, reason: 'stale_pending' };
…ents The elicitation settlement state machine touched shared chat model code used by MCP elicitation and the terminal tools, which is a much wider blast radius than a voice questionnaire change needs. Revert it here and keep it for a follow-up; voice elicitation support still works because the dispatcher already refuses a part that is not Pending and reports the state it actually settled to. A rebase had also stacked three JSDoc blocks onto _pendingIdFor, leaving _narratableIdentity and _questionNarratable undocumented. Reattach each. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
…esolution out The built-in askQuestions tool puts the question in `message` and a short header in `title`; the widget renders `message ?? title` in four places. The voice path sent `title` alone, so it narrated headers. Extract `getDisplayedQuestionText` so the widget and voice cannot drift, and use it in all five sites. Move `IBackendQuestionAnswer` and `resolveQuestionAnswers` into common/voiceClient/. They are voice websocket types, snake_case, with one caller, and did not belong in the generic carousel helper. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
`voiceToolDispatchService.test.ts` covered elicitation and stale identity but not the reported bug itself. Add six tests over the carousel path: a spoken answer submits the form and completes the deferred, an unmatched value is refused, an approval spoken at a question form is refused, skip is gated on `allowSkip`, and an already-used form rejects a late answer. Drop `resolve_id` and `approval_kind` from the pending payload. Nothing read them: the dispatch service finds the part by `pending_id` and uses the part's own `resolveId`. Fix the telemetry comment, which documented a `skip_remaining` value this client never emits, and collapse the duplicated URI parse in `_pendingIdFor`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
…guard double submit Three fixes found reviewing the answer path: A form replaced by another asking the same things has identical `detail`, so the burst coalescer dropped it as a non-change and never narrated the new form. The per-session path already distinguishes them by `pendingId`; carry that id through the queued changes so both paths agree. The widget shows the freeform input unless `allowFreeformInput` is explicitly `false`, but voice serialized an omitted field as `false`. A question that omitted it offered an "Other" box on screen that voice would refuse. `handleSubmit` set `data`, `isUsed` and notified the extension unconditionally. Voice is a second submitter, so a queued click could land afterwards, replace the spoken answer and notify twice. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
…fault Elicitations and confirmation parts were never part of the reported carousel bug and are not handled on main, so supporting them here widened the change without a caller asking for one. `_answerConfirmation` in particular resolved a part by sending a whole new chat request, which is a different workflow from the carousel and toolInvocation paths this PR exists to fix. `resolveQuestionAnswers` read an omitted `allowFreeformInput` as disabled while the widget and the pending payload both read it as enabled, so a custom answer the user was invited to give came back as `invalid_answer`. An unresolvable carousel is now refused before it is marked used, instead of being left answered on screen while the assistant reports that it did not land. Adds wire-level coverage of `request_narration` serialization and drops the `required` field, which was sent and never read. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
The approval branch probed four tool parameter names (`command`, `input`, `explanation`, `goal`) to build its own description. That changes approval content for every tool, which is nothing the question-carousel fix needs; it now uses `invocationMessage`, the description the part already carries. Option order is displayed order and both sides number by position, so transmitting an `ordinal` only restated the index. The pending `title` had no producer. The narration retry map stored text the replay path never reads. Adds the missing controller test: a carousel model in, the structured pending payload out, including displayed option order and the question text rather than its header. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
Three findings from review: - A present but non-array `answers` field was coerced to an empty list, so a malformed skip succeeded while discarding what the call carried. It is now rejected as an invalid answer. - `respond_to_session` copied the model-generated `response.type` verbatim into classified telemetry. It is now clamped to the known response types. - A spoken freeform answer bypassed the length, range and format rules the widget enforces on submit. The rules move into a shared pure helper that both paths use; the widget keeps ownership of the wording. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
`respond_to_session` replaced two tools that only ever approved or rejected, and `voiceToolApproval` had grown three fields to describe answers and skips too. That redefines what an existing classified event counts, which this change does not need. The event keeps its original shape and is emitted only for an approve or a reject; the response type stays in the trace log. This also drops the clamping needed to keep model output out of it. Also trims comments that restate the code or the review history. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
The widget will not submit a form while a question marked required is unanswered. The voice path enforced the per-answer validation rules but not completeness, so it could submit and dismiss a form a click could not. The backend only dispatches once every question is filled, so this is a backstop rather than a behaviour change. Skip is unaffected: declining a form is not an incomplete submission. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1c4672e9-182f-4a2f-8a79-bd8b770961a1
Collaborator
|
@copilot resolve the merge conflicts in this pull request |
1 similar comment
Collaborator
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 440f5109-5c2e-4b98-b74f-36c122d4b43a
meganrogge
marked this pull request as ready for review
July 29, 2026 17:48
meganrogge
enabled auto-merge (squash)
July 29, 2026 17:48
roblourens
approved these changes
Jul 29, 2026
amunger
approved these changes
Jul 29, 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.
Problem
Speaking an answer to a question carousel did nothing. The assistant replied "Okay, approved." and the form stayed open.
The voice backend could describe a session's state as prose, but nothing it sent could be acted on. A form arrived as
questions: <titles>, which loses the options, their values and the question ids, so there was no way to turn a spoken answer into a selection. The only session-response tools wereapprove_confirmationandreject_confirmation, neither of which a carousel can accept.Solution
Serialize what a session is waiting on into a typed
pendingpayload alongside the existing prose, carrying the pending id, request id, questions, and each option's label and value in displayed order.Answer it through one
respondToSessionentry point. Routing is by exact pending id plus request id with no fallback, so a form that was replaced, dismissed or already answered is refused rather than guessed at. Answers match by exact option value, and one unresolvable entry rejects the whole batch, since a half-filled form is not something the user can see went wrong. The result is returned as{ok, reason}so the backend can acknowledge truthfully instead of assuming.Question narration is requested from the client and rendered by a formatter that mirrors the backend's, against the same fixtures. The client speaks a form's first question; the backend, which owns the draft, speaks the rest.
A spoken freeform answer now goes through the same length, range and format rules the widget enforces on submit. Those rules moved into a shared helper; the widget still owns the wording.
Tests
185 tests across 10 suites. New coverage for the pending payload, pending id derivation, narration formatting, answer resolution and its rejections, dispatch routing including stale and malformed input, carousel submission, and that only a genuine approve or reject is reported as an approval.
Risks
The wire contract must ship with the matching backend. It fails closed: an unknown response is refused with a reason rather than silently succeeding.
Non-goals
Moving the visible carousel as a partial answer is collected; that needs a new wire operation. Elicitation and generic confirmations are unchanged. The
voiceToolApprovaltelemetry event keeps its existing shape and meaning.Fixes https://github.com/microsoft/vscode-internalbacklog/issues/8623