fix(voice): keep the draining transcript after a manual dictation stop - #2469
Conversation
Design Review (Fable 5) — 🟡 CONCERNSAdvisory design-level review of Design-Verdict: CONCERNS Sound fix aimed at the real cause, but the dictation state machine has now outgrown ChatPage — an implicit protocol spread over eight refs and five reset sites. Watch
Suggestions
[DESIGN-REVIEWED] 61070c7 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsI've thoroughly traced the state machine across all call sites ( Key falsification checks that held up:
The change is confined to one file, has no security/data-integrity surface, and every branch is covered by a red-before-green test whose expectations I verified against the code. No findings. [OPUS-REVIEWED] 61070c7 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — ✅ PASSAdvisory UX-level review of All the evidence is in: this PR changes no strings, no visuals, no controls — it's purely which dictation text lands in the composer after a manual stop, fully covered by behavioral tests. My assessment: UX-Verdict: PASS Post-release corrections now land where the user was speaking, preserve their typing and caret, and never auto-send — the composer converges on what was actually said. Suggestions
[UX-REVIEWED] 61070c7 |
13d402f to
1fdd231
Compare
|
Prior reviewed SHA:
The finding is correct and it is mine: keeping I did not apply the prescribed fix — What the code now does. Two new tests, both red-before-green against the specific hunk:
The PR body previously listed this clobber as an accepted tradeoff. That paragraph is now wrong, so I rewrote the description rather than leave a fixed problem documented as a known one.
Verified on |
1fdd231 to
92a42ca
Compare
|
Prior reviewed SHA:
The finding is correct and the prescribed fix is the right one. Anchoring only through New test, red-before-green against this hunk alone:
The PR body's description of the anchor was written for the old whole-value form and is now updated to describe the prefix anchor and why mid-draft dictation forces it. Verified on |
92a42ca to
6d8643c
Compare
|
Prior reviewed SHA:
Fixed with the first of the two prescribed shapes: New test, red-before-green:
Verified first that this is pre-existing, not introduced here: on Worth recording how the test landed, because the first version of it passed vacuously: with an empty composer Verified on |
6d8643c to
c0042b8
Compare
ba92566 to
47a3902
Compare
|
Prior reviewed SHA:
I did not seed What is seeded at a cold-stream stop is the caret and the value fingerprint: The text still comes from the live composer at the first partial, so post-release typing is inside the snapshot and nothing is lost; the caret is the one frozen at the release, so the transcript inserts where the user was speaking. Two supporting changes make that hold:
New test, red-before-green: (Its first draft also failed with the fix in place: I re-used the same string for the interleaving change, so React never fired
Verified on |
47a3902 to
4591179
Compare
|
Prior reviewed SHA:
The collapse is gated on the signal already seeded at stop time — An untouched selection is left alone: it is still a legitimate replacement target, which is the behaviour dictating-over-a-selection is supposed to have. Applied at both splice sites, per "before any cold-stream splice": New test, red-before-green:
Prior round's Verified on |
4591179 to
b3cd0e6
Compare
|
Prior reviewed SHA:
The caret is now re-armed at the same logical position, shifted by how much the region ahead of it changed:
Two sub-cases stay deliberate: with no typing the caret still goes to the end of the dictated region, which is the ordinary "we own the composer" behaviour; and on a cold-stream first write with typing already done there is no old anchor to measure a shift against, so it is left unarmed — there the value commit leaves the caret at the end, which is past their text and a sane place to be. New test, red-before-green: Worth recording: the first version of this test was vacuous. I asserted on an end-of-draft scenario where the browser's own reset lands on exactly the offset the shift computes, so both behaviours agreed. Only a mid-draft case (with a tail after the typed text) distinguishes them — the assertion had to be moved there before it could fail. Verified on |
Stopping a streaming dictation by hand -- mic button or push-to-talk
release -- dropped everything Transcribe stabilised afterwards, leaving
the composer holding the last UNSTABLE hypothesis. On a short
push-to-talk hold that is the common case: the tail of the utterance is
exactly the part still unstable at release.
One flag was gating two routes that need opposite treatment.
applyVoiceText APPENDS, so the close-time final landing on a composer
that already holds the hypothesis duplicates the utterance. onPartial
REPLACES the region at the frozen boundary, and the hook re-emits
finals.join(' ') through it as the drain stabilises each segment -- that
is the authoritative text.
Split the flag: sttDisarmedRef keeps suppressing every route (cancel,
send, slot-switch), and the new sttAppendDisarmedRef suppresses only the
append, which is what a manual stop now sets. onEndpoint honours both, so
an endpoint verdict during the drain still cannot turn a stop into an
unrequested send.
b3cd0e6 to
61070c7
Compare
|
Prior reviewed SHA:
I did not adopt "use the live caret when the composer differs", because that is the exact inverse of the previous round's fix and just swaps which case breaks: the live caret is right when the user typed before the frozen point and wrong when they typed after it (the transcript then lands behind text they wrote later — the reordering that seed was added to prevent). Neither position is correct unconditionally. The frozen caret is now rebased onto the current text.
This also subsumes last round's New test, red-before-green:
Verified on |
Problem
Stop a streaming dictation by hand — the mic button, or releasing the push-to-talk key — and whatever Transcribe stabilises after that moment never reaches the composer. The user is left holding the last unstable hypothesis instead of the real transcript: a missing tail, an uncorrected word.
On a push-to-talk hold this is the common case rather than a corner. A hold is short, so the tail of the utterance is exactly the part still unstable at release. Say "remind me to call Ana", let go, and the composer can read
remind me to.Why it matters
Dictation is only worth using if what lands is what you said. Silently keeping a half-finished hypothesis is worse than a visible failure, because nothing tells the user to look — they send the truncated text.
Fix (symptom → root cause → change)
Symptom: the composer keeps a truncated hypothesis after a manual stop.
Root cause: one boolean,
sttDisarmedRef, was doing two different jobs, and a manual stop only wants one of them. The two delivery routes need opposite treatment:applyVoiceText(close-time)base + ' ' + text"hello hello")onPartial(drain-time)stop()deliberately leaves the socket draining, and the hook re-emitsfinals.join(' ')through this route as each segment stabilisesBecause both were gated by the same flag, suppressing the duplicate append also threw away every correction the drain produced.
Change, in two parts.
1. Split the flag.
sttDisarmedRefkeeps its meaning — suppress every route. Still what cancel (Esc),send(), and slot-switch set: the user discarded, already sent, or left.sttAppendDisarmedRefis new and narrower — suppress the append only. This is what a manual stop sets, and only oncefrozenInputRefis non-null, i.e. once the composer actually holds a copy of the speech.The pre-existing conditional is preserved exactly: with
frozenInputRefstill null no partial has landed, the close-time final is the only copy of the utterance, and it must land. That is the cold-stream case where the release beats the server's first partial.onEndpointgates on all the suppression flags, including a third one,sttEndpointDisarmedRef, which every streaming manual stop sets unconditionally. A manual stop means "stop capturing", not "send". The append flag cannot carry that property, because the cold-stream case deliberately leaves it unset — so without a separate flag a short press against a cold stream left the endpointer armed, and a trailing final's verdict submitted whatever draft was already in the composer.2. Preserve what the user types after the release.
Keeping
onPartialarmed is not enough on its own: it rebuilds from the frozen snapshot, so a correction arriving while the user is already typing would delete that typing. After a release the user has every reason to believe dictation is over, so their text has to survive.lastDictationAnchorRefrecords the composer content up to and including the regiononPartiallast inserted. A drain-time update verifiescur.startsWith(anchor)and rebuilds asnewAnchor + cur.slice(anchor.length)— the corrected region, then everything that now follows it, verbatim.The prefix (rather than the whole value) is the anchor because dictation splices at the caret, so it can sit mid-draft with an existing tail after it, and typing after the release lands at the restored caret — between the dictated words and that tail. Anchoring on the whole value fails its own
startsWithcheck in exactly that case and drops the correction; anchoring on the prefix keeps the typed text and the original tail both intact.This branch is gated on the endpoint flag, not the append flag: a cold-stream stop deliberately leaves the append armed, so keying off it would skip the preservation in exactly the case that still needs it. A null anchor (no partial has landed — the cold-stream stop) falls through to a plain write rather than returning, since that write IS the first one and returning would drop the utterance.
A cold-stream stop also freezes the caret (not the text) at the release, plus the value fingerprint. Without that, a drain partial arriving after the user has started typing would snapshot the live caret and insert the transcript after their new text instead of where they were speaking. The text is deliberately left un-pinned: with no partial landed,
applyVoiceTextmust splice into the LIVE composer, and a pinned snapshot there would delete post-release typing — a worse failure than a wrong insertion point.A frozen caret is only valid against the text it was taken from, so before either splice it is rebased onto the current composer. Using the release-time fingerprint, the user's edit is bounded between the longest common prefix and suffix: an edit after the offset leaves it alone, an edit before it shifts it by the length delta, and an edit straddling it falls back to the user's live caret. This covers both ways a frozen position goes stale — a range whose selection they have since typed over (which would otherwise delete that replacement) and an offset whose meaning moved because they edited earlier text (which would otherwise cut mid-word). An untouched composer keeps the range intact, because replacing a selection is the intended behaviour.
Entering this branch also sets the append flag.
stopVoicecould not decide that: withfrozenInputRefnull the close-time final really was the only copy of the utterance, so the append had to stay armed. That premise expires the moment a drain partial lands — and on the streaming pathapplyVoiceTextoverwrites (it re-splices fromfrozenInputRef) rather than appending, so letting the final through afterwards would delete post-release typing. Deciding it here keeps the original invariant intact: the final is allowed through exactly while it is the only copy.When the anchor cannot be verified the user edited inside the dictated region, and the update leaves the composer untouched rather than guessing; a heuristic there deletes user-authored text. This is the same verify-then-slice shape, and the same leave-it-alone policy, that
cancelVoicealready uses to roll a region back. The caret is handled the same way. While the composer is still exactly what we wrote it goes to the end of the dictated region; once the user has typed it is re-armed at their own LOGICAL position, shifted by how much the region ahead of it grew. Simply not arming it is not the same as leaving it alone — React replaces the textarea value and the browser resets the DOM caret to the end, which mid-draft is past the tail. That edited state is sticky for the whole drain (postStopEditedRef): a drain emits several corrections, and the first one rewrites the composer to include the typed suffix — so a per-update "was it edited?" test would say no on the second correction and pull the caret back in front of the user's text.This applies to the post-stop drain only. During recording the region is still being actively rewritten and that behaviour is unchanged.
Tests
website/src/test/ChatPage.sendDuringDictation.test.tsx, 10 existing → 22. Each new one is red-before-green against the specific hunk it covers:lets a drain-time correction replace the hypothesis after a manual stop— dictateremind me to, stop, deliver the stabilisedremind me to call Anathrough the drain route, assert the composer takes it; then fire the close-time route with the same text and assert no duplication. Reverting the split fails withexpected 'remind me to' to be 'remind me to call Ana'.keeps text typed after the release when a drain correction lands— type— urgentafter releasing, then let the correction arrive; the composer must readremind me to call Ana — urgent. Reverting the suffix preservation fails withexpected 'remind me to call Ana' to be 'remind me to call Ana — urgent'— the typing is gone, which is the defect this half fixes.keeps a mid-draft correction and everything after it— dictate into the middle ofhello world, stop, typeNOWat the restored caret, then let the correction arrive; the composer must readhello remind me to call Ana NOW world— correction applied, typed text and original tail both intact. Anchoring on the whole value instead of the prefix fails withexpected 'hello remind me NOW world' to be 'hello remind me to call Ana NOW world', i.e. the correction is silently dropped.leaves the composer alone when the dictated region was edited— replace the dictation withnever mind, then let a correction arrive; the composer must staynever mind.does not reclaim the caret on a later correction in the same drain— type a suffix, then deliver two successive corrections; the caret must not end up at the end of the dictated region (offset 21), which sits in front of the typed text. Making the edited state per-update instead of sticky fails withexpected 21 not to be 21.does not auto-send on a drain-time endpoint verdict after a manual stop— an endpoint verdict after a manual stop must not callapi.sendChat.does not auto-send on an endpoint verdict after a cold-stream stop— hold, type a draft, release before any partial arrives, then let an endpoint verdict fire; the draft must neither be sent nor altered. DroppingsttEndpointDisarmedReffails withexpected "vi.fn()" to not be called at all, but actually been called 1 times— the user's draft sent for them.The three tests pinning behaviour this change must not alter pass unmodified: the cold-stream case where the draining final is the only copy, the close-time drop once partials populated the composer, and hand-typed text surviving the drain.
Manual verification
N/A — no microphone in this environment, and the change is entirely in which of two already-mocked delivery routes is gated and how the composer value is reassembled. The tests drive both routes directly, which is the actual contract.
Screenshots
N/A — no user-visible surface changed; the only difference is which text ends up in the composer, covered by the tests above. The path-based
Screenshot Evidencegate flags anywebsite/srcchange, so this PR carries theno-screenshotslabel the gate itself prescribes for a change with no visual delta.