Skip to content

Commit feb9927

Browse files
PierrunoYTampagentanandh8x
authored
fix(tui): bound live dictation regions (#985)
* fix(tui): bound live dictation regions Amp-Thread-ID: https://ampcode.com/threads/T-01a0448f-5860-721c-8a47-5119fc57f685 Co-authored-by: Amp <amp@ampcode.com> * fix(tui): preserve user edits across dictation partials * fix(tui): reanchor stale dictation regions Amp-Thread-ID: https://ampcode.com/threads/T-01a04c92-2d1d-7508-91bc-416341b7e8b0 Co-authored-by: Amp <amp@ampcode.com> * fix(tui): bound live dictation regions Amp-Thread-ID: https://ampcode.com/threads/T-01a0448f-5860-721c-8a47-5119fc57f685 Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch> * fix(tui): preserve user edits across dictation partials Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch> * fix(tui): reanchor stale dictation regions Amp-Thread-ID: https://ampcode.com/threads/T-01a04c92-2d1d-7508-91bc-416341b7e8b0 Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch> * fix(tui): preserve dictation separators after reanchoring Amp-Thread-ID: https://ampcode.com/threads/T-01a07cfc-4f4e-773c-b42e-6229ef568a01 Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch> * fix(acp): reject Unicode whitespace in browser title origins Cover encoded whitespace before permission translation and through wire payloads. Pin parent reasoning-effort forwarding at the specialist resume call site. Amp-Thread-ID: https://ampcode.com/threads/T-01a097c8-242d-732f-bf6e-7321d8cf732c Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch> --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Anandan <anandan.8x@gmail.com>
1 parent 23c27e0 commit feb9927

6 files changed

Lines changed: 187 additions & 45 deletions

File tree

‎internal/acp/translate.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,15 +139,15 @@ func browserToolTitle(command, rawArgs string) string {
139139
}
140140

141141
// browserTitleTextSafe validates text after URL parsing has decoded escaped
142-
// UTF-8 in the host. Valid UTF-8 alone is not presentation-safe: control,
142+
// UTF-8 in the host. Valid UTF-8 alone is not presentation-safe: whitespace, control,
143143
// format/bidi, and line/paragraph separator runes can reorder or split the
144144
// permission label shown to a user. The execution URL remains unchanged.
145145
func browserTitleTextSafe(text string) bool {
146146
if !utf8.ValidString(text) {
147147
return false
148148
}
149149
for _, r := range text {
150-
if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || unicode.Is(unicode.Zl, r) || unicode.Is(unicode.Zp, r) {
150+
if unicode.IsSpace(r) || unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || unicode.Is(unicode.Zl, r) || unicode.Is(unicode.Zp, r) {
151151
return false
152152
}
153153
}

‎internal/acp/translate_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,9 @@ func TestBrowserPermissionTitlesMirrorSafeToolArguments(t *testing.T) {
224224

225225
func TestBrowserOpenTitlesRejectDecodedUnicodePresentationControls(t *testing.T) {
226226
for _, rawURL := range []string{
227+
"https://safe.example%C2%A0evil.test/path",
228+
"https://safe.example%E2%80%83evil.test/path",
229+
"https://safe.example%E3%80%80evil.test/path",
227230
"https://safe.example%E2%80%AEevil.test/path",
228231
"https://safe.example%E2%81%A6evil.test/path",
229232
"https://safe.example%C2%85evil.test/path",
@@ -243,6 +246,9 @@ func TestBrowserOpenTitlesRejectDecodedUnicodePresentationControls(t *testing.T)
243246
if err != nil {
244247
t.Fatal(err)
245248
}
249+
if title := browserToolTitle("open", string(args)); title != "browser open" {
250+
t.Errorf("unsafe origin accepted before permission translation: %q", title)
251+
}
246252
updates := []ToolCallUpdate{
247253
toolCallStart(agent.ToolCall{ID: "start", Name: "browser_open", Arguments: string(args)}),
248254
permissionToolCall(agent.PermissionRequest{ToolCallID: "permission", ToolName: "browser_open", Args: map[string]any{"url": rawURL}}),

‎internal/specialist/resume_model_test.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,8 +229,9 @@ func TestRunResumePassesTheParentModelThroughToTheChild(t *testing.T) {
229229
Prompt: "keep going",
230230
Resume: "child_task",
231231
}, TaskRunOptions{
232-
ParentSessionID: parent.SessionID,
233-
ParentModel: "claude-opus-4.1",
232+
ParentSessionID: parent.SessionID,
233+
ParentModel: "claude-opus-4.1",
234+
ParentReasoningEffort: "high",
234235
}); err != nil {
235236
t.Fatalf("Run(resume): %v", err)
236237
}
@@ -246,6 +247,9 @@ func TestRunResumePassesTheParentModelThroughToTheChild(t *testing.T) {
246247
if model != "claude-opus-4.1" {
247248
t.Fatalf("the resumed child was launched with --model %q, want the parent's %q", model, "claude-opus-4.1")
248249
}
250+
if effort, ok := argValue(captured, "--reasoning-effort"); !ok || effort != "high" {
251+
t.Fatalf("the resumed child was launched with --reasoning-effort %q (present=%t), want the parent's", effort, ok)
252+
}
249253
}
250254

251255
// THE FRESH CALL SITE NEEDS THE SAME GUARD, FOR THE SAME REASON.

‎internal/tui/dictation.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,12 @@ type dictationController struct {
7777
regionStart int
7878
regionEnd int
7979
regionPrefix string
80+
// regionRendered is the exact text last inserted into [regionStart,
81+
// regionEnd). A later partial or cancel may remove that range only while it
82+
// still matches, so stale bounds never consume user replacement text.
83+
regionRendered string
8084
// regionAnchor snapshots the text BEFORE the live region, so the next
81-
// partial can detect external edits (typing, paste) and shift [start,end)
82-
// to stay aligned. Updated alongside regionStart on each render.
85+
// partial can detect edits before the tracked range.
8386
regionAnchor string
8487

8588
// waveBars is the recording waveform's recent bar heights (a scrolling ring):
@@ -327,6 +330,7 @@ func (d *dictationController) reset() {
327330
d.streamStop = nil
328331
d.regionActive = false
329332
d.regionPrefix = ""
333+
d.regionRendered = ""
330334
d.waveBars = nil
331335
d.waveTick = 0
332336
}

‎internal/tui/dictation_stream.go‎

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package tui
22

33
import (
44
"context"
5-
"strings"
65

76
tea "charm.land/bubbletea/v2"
87

@@ -92,52 +91,30 @@ func (m model) handleDictationPartial(msg sttPartialMsg) model {
9291

9392
func (m *model) applyStreamingText(text string) {
9493
state := m.currentComposerState()
94+
stateRunes := []rune(state.text)
9595
if !m.dictation.regionActive {
9696
m.dictation.regionActive = true
9797
m.dictation.regionStart = state.cursor
9898
m.dictation.regionEnd = state.cursor
9999
m.dictation.regionPrefix = ""
100-
// Anchor the prefix text BEFORE the live region. If the user types or
101-
// pastes outside the region, the next partial can detect the change in
102-
// this prefix and shift [start,end) to stay aligned.
103-
m.dictation.regionAnchor = state.text
100+
// Anchor the prefix text before the live region. A later partial removes
101+
// the tracked range only while both this prefix and the rendered region
102+
// still match.
103+
m.dictation.regionAnchor = string(stateRunes[:state.cursor])
104104
if needsLeadingSpace(state) {
105105
// Fold the separator into the region so a cancel removes it too.
106106
m.dictation.regionPrefix = " "
107107
}
108-
} else {
109-
// Compare the prefix before the live region. If it changed (the user
110-
// typed/pasted there) shift [start,end) by the length delta so the
111-
// next partial's slice targets the right span. If the user edited
112-
// INSIDE the region, drop the live region and re-anchor at the
113-
// current cursor — we can't tell the partial from the user's text
114-
// after that point.
115-
prefix := string([]rune(state.text)[:m.dictation.regionStart])
116-
anchor := m.dictation.regionAnchor
117-
switch {
118-
case prefix == anchor:
119-
// External edit at or after the region — no shift needed.
120-
case len(prefix) > len(anchor) && strings.HasPrefix(prefix, anchor):
121-
// External edit inserted text just before the region (i.e. at
122-
// position regionStart, pushing the region right).
123-
delta := len([]rune(prefix)) - len([]rune(anchor))
124-
m.dictation.regionStart += delta
125-
m.dictation.regionEnd += delta
126-
m.dictation.regionAnchor = prefix
127-
case len(anchor) > len(prefix) && strings.HasPrefix(anchor, prefix):
128-
// External delete just before the region.
129-
delta := len([]rune(anchor)) - len([]rune(prefix))
130-
m.dictation.regionStart -= delta
131-
m.dictation.regionEnd -= delta
132-
m.dictation.regionAnchor = prefix
133-
default:
134-
// User edited inside or across the region — can't safely
135-
// overwrite. Drop the live region and re-anchor at the cursor
136-
// so the next partial inserts as a fresh span.
137-
m.dictation.regionStart = state.cursor
138-
m.dictation.regionEnd = state.cursor
139-
m.dictation.regionPrefix = ""
140-
m.dictation.regionAnchor = string([]rune(state.text)[:state.cursor])
108+
} else if !m.dictation.liveRegionMatches(stateRunes) {
109+
// The composer changed before or inside the tracked range. Its old
110+
// bounds no longer prove which bytes belong to dictation, so preserve
111+
// everything and insert the next partial as a fresh live region.
112+
m.dictation.regionStart = state.cursor
113+
m.dictation.regionEnd = state.cursor
114+
m.dictation.regionPrefix = ""
115+
m.dictation.regionAnchor = string(stateRunes[:state.cursor])
116+
if needsLeadingSpace(state) {
117+
m.dictation.regionPrefix = " "
141118
}
142119
}
143120
// Replace [regionStart, regionEnd) with prefix + the new cumulative text.
@@ -153,14 +130,22 @@ func (m *model) applyStreamingText(text string) {
153130
}
154131
updated := insertComposerText(cleared, rendered)
155132
m.dictation.regionEnd = m.dictation.regionStart + len([]rune(rendered))
133+
m.dictation.regionRendered = rendered
156134
m.setComposerState(updated)
157135
}
158136

137+
func (d dictationController) liveRegionMatches(stateRunes []rune) bool {
138+
return d.regionStart >= 0 && d.regionEnd >= d.regionStart && d.regionEnd <= len(stateRunes) &&
139+
string(stateRunes[:d.regionStart]) == d.regionAnchor &&
140+
string(stateRunes[d.regionStart:d.regionEnd]) == d.regionRendered
141+
}
142+
159143
// commitDictationRegion keeps the streamed text in the composer and stops
160144
// tracking it as a live region (used on successful completion — the final
161145
// transcript equals the last partial already rendered).
162146
func (m model) commitDictationRegion() model {
163147
m.dictation.regionActive = false
148+
m.dictation.regionRendered = ""
164149
return m
165150
}
166151

@@ -169,8 +154,11 @@ func (m model) commitDictationRegion() model {
169154
func (m model) discardDictationRegion() model {
170155
if m.dictation.regionActive {
171156
state := m.currentComposerState()
172-
m.setComposerState(deleteComposerRange(state, m.dictation.regionStart, m.dictation.regionEnd))
157+
if m.dictation.liveRegionMatches([]rune(state.text)) {
158+
m.setComposerState(deleteComposerRange(state, m.dictation.regionStart, m.dictation.regionEnd))
159+
}
173160
m.dictation.regionActive = false
161+
m.dictation.regionRendered = ""
174162
}
175163
return m
176164
}

‎internal/tui/dictation_test.go‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package tui
33
import (
44
"context"
55
"errors"
6+
"fmt"
67
"strings"
78
"testing"
89

@@ -175,6 +176,145 @@ func TestDictationStreamingPartialReplacesRegion(t *testing.T) {
175176
}
176177
}
177178

179+
func TestDictationStreamingPartialWithCaretInsideExistingText(t *testing.T) {
180+
m := model{}
181+
m.setComposerState(composerState{text: "hello world", cursor: 5})
182+
m.dictation.phase = dictRecording
183+
m.dictation.streaming = true
184+
185+
m = m.handleDictationPartial(sttPartialMsg{text: "there"})
186+
if m.composer.text != "hello there world" {
187+
t.Fatalf("after first partial: %q", m.composer.text)
188+
}
189+
m = m.handleDictationPartial(sttPartialMsg{text: "there friend"})
190+
m = m.handleDictationPartial(sttPartialMsg{text: "there friend again"})
191+
if m.composer.text != "hello there friend again world" {
192+
t.Fatalf("after third partial: %q", m.composer.text)
193+
}
194+
}
195+
196+
func TestDictationStreamingPartialReanchorsRegionAfterComposerShrink(t *testing.T) {
197+
m := model{}
198+
m.setComposerState(composerState{text: "hello world", cursor: 11})
199+
m.dictation.phase = dictRecording
200+
m.dictation.streaming = true
201+
m = m.handleDictationPartial(sttPartialMsg{text: "there"})
202+
203+
// Simulate the user replacing the composer while a later partial is in
204+
// flight. The old live-region bounds now exceed the new composer length.
205+
m.setComposerState(composerState{text: "hi ", cursor: 3})
206+
m = m.handleDictationPartial(sttPartialMsg{text: "again"})
207+
if m.composer.text != "hi again" {
208+
t.Fatalf("partial after composer shrink: %q", m.composer.text)
209+
}
210+
}
211+
212+
func TestDictationStreamingPartialDoesNotClampStaleRegionIntoUserText(t *testing.T) {
213+
m := model{}
214+
m.setComposerState(composerState{text: "aOLDz", cursor: 1})
215+
m.dictation.phase = dictRecording
216+
m.dictation.streaming = true
217+
m = m.handleDictationPartial(sttPartialMsg{text: "first"})
218+
219+
// Replace the composer while another cumulative partial is in flight. The
220+
// old region starts after the same prefix but no longer contains dictation
221+
// text, so its clamped bounds must not consume the user's "b".
222+
m.setComposerState(composerState{text: "ab", cursor: 2})
223+
m = m.handleDictationPartial(sttPartialMsg{text: "next"})
224+
if m.composer.text != "ab next" {
225+
t.Fatalf("stale live region replaced user text: %q", m.composer.text)
226+
}
227+
}
228+
229+
func TestDictationStreamingCancelDoesNotClampStaleRegionIntoUserText(t *testing.T) {
230+
m := model{}
231+
m.setComposerState(composerState{text: "aOLDz", cursor: 1})
232+
m.dictation.phase = dictRecording
233+
m.dictation.streaming = true
234+
m = m.handleDictationPartial(sttPartialMsg{text: "first"})
235+
236+
m.setComposerState(composerState{text: "ab", cursor: 2})
237+
m = m.discardDictationRegion()
238+
if m.composer.text != "ab" {
239+
t.Fatalf("discarded stale live region from user text: %q", m.composer.text)
240+
}
241+
}
242+
243+
func TestDictationStreamingSameLengthRegionEditPreserved(t *testing.T) {
244+
for _, partial := range []bool{false, true} {
245+
t.Run(fmt.Sprintf("later partial=%v", partial), func(t *testing.T) {
246+
m := model{}
247+
m.setComposerState(composerState{text: "hello world", cursor: 11})
248+
m.applyStreamingText("there")
249+
// Keep the prefix and rune bounds unchanged, but replace precisely
250+
// the rendered region with user text.
251+
m.setComposerState(composerState{text: "hello world MINE!", cursor: 17})
252+
if partial {
253+
m.applyStreamingText("next")
254+
if !strings.Contains(m.composer.text, " MINE!") {
255+
t.Fatalf("partial deleted same-length user edit: %q", m.composer.text)
256+
}
257+
}
258+
m = m.discardDictationRegion()
259+
if m.composer.text != "hello world MINE!" {
260+
t.Fatalf("cancel deleted same-length user edit: %q", m.composer.text)
261+
}
262+
})
263+
}
264+
}
265+
266+
func TestDictationStreamingBackspaceAcrossRegionPreservesUserTextOnCancel(t *testing.T) {
267+
m := model{}
268+
m.setComposerState(composerState{text: "hello world", cursor: 11})
269+
m.dictation.phase = dictRecording
270+
m.dictation.streaming = true
271+
m = m.handleDictationPartial(sttPartialMsg{text: "there"})
272+
273+
// The user backspaces through the live transcript and into their original
274+
// text while newer cumulative partials are still in flight.
275+
m.setComposerState(composerState{text: "hello wor", cursor: 9})
276+
m = m.handleDictationPartial(sttPartialMsg{text: "there friend"})
277+
m = m.handleDictationPartial(sttPartialMsg{text: "there friend again"})
278+
if m.composer.text != "hello wor there friend again" {
279+
t.Fatalf("partials replaced user text after backspace: %q", m.composer.text)
280+
}
281+
282+
m, _ = m.cancelDictation()
283+
if m.composer.text != "hello wor" {
284+
t.Fatalf("cancel did not restore the user's edited text: %q", m.composer.text)
285+
}
286+
}
287+
288+
func TestDictationStreamingInvalidRegionBoundsReanchor(t *testing.T) {
289+
tests := []struct {
290+
name string
291+
start int
292+
end int
293+
anchor string
294+
want string
295+
}{
296+
{name: "start past end", start: 40, end: 60, anchor: "a much longer previous composer", want: "ab partial text"},
297+
{name: "negative start", start: -3, end: 1, anchor: "", want: "ab partial text"},
298+
{name: "end past end", start: 1, end: 40, anchor: "a", want: "ab partial text"},
299+
}
300+
301+
for _, tt := range tests {
302+
t.Run(tt.name, func(t *testing.T) {
303+
m := model{}
304+
m.setComposerState(composerState{text: "ab", cursor: 2})
305+
m.dictation.regionActive = true
306+
m.dictation.regionStart = tt.start
307+
m.dictation.regionEnd = tt.end
308+
m.dictation.regionAnchor = tt.anchor
309+
310+
m.applyStreamingText("partial text")
311+
if m.composer.text != tt.want {
312+
t.Fatalf("composer = %q, want %q", m.composer.text, tt.want)
313+
}
314+
})
315+
}
316+
}
317+
178318
func TestDictationCanceledStreamRaceDoesNotAutoSubmit(t *testing.T) {
179319
// Esc can race an already-buffered realtime event: cancelDictation discards
180320
// the live region and resets state synchronously, but the streaming

0 commit comments

Comments
 (0)