fix(tui): bound live dictation regions

[PierrunoYT]

Summary

• anchor a new live-dictation region to the composer prefix before the caret instead of the full composer text
• clamp the tracked region before slicing and again after prefix-delta adjustments
• add the reported caret-in-the-middle regression with successive partial transcripts
• add coverage for an in-flight partial after the user shrinks/replaces the composer

The caret-middle regression panicked before the fix with slice bounds out of range [:-1] at dictation_stream.go:115.

Linked issue

Fixes #965

Checklist

• The linked issue already has the issue-approved label.
• go build ./..., go vet ./..., and go test ./... pass locally.
• Changed Go files are gofmt clean.
• Tests added and run under -race.
• No visual UI changes requiring screenshots.

Validation

• go build ./...
• go vet ./...
• go test ./...
• go test -race ./internal/tui -count=1
• go run ./cmd/zero-release build
• go run ./cmd/zero-release smoke
• make lint-static — 0 issues
• make vulncheck — no vulnerabilities
• git diff HEAD --check

make fmt-check remains blocked on the current base by existing formatting findings under internal/perfbench/testdata/; this PR does not modify those fixtures.

Summary by CodeRabbit

• Bug Fixes

  • Improved streaming dictation when inserting text within existing composer content.
  • Preserved correct replacement behavior for successive partial dictation results.
  • Prevented errors and corrupted text when composer content is shortened during active dictation.
  • Preserved user-edited text when backspacing through live dictation content and canceling.
  • Correctly handled invalid dictation regions so partial results render in the right location.

• Tests

  • Added coverage for caret-position insertion, composer changes, backspacing, cancellation, and invalid region bounds during streaming.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f59f9a7d-4913-481a-a98f-28d82665af18

📥 Commits

Reviewing files that changed from the base of the PR and between 9e4e744 and 329dd12.

📒 Files selected for processing (3)

• internal/tui/dictation.go
• internal/tui/dictation_stream.go
• internal/tui/dictation_test.go

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

Walkthrough

Live dictation now tracks the exact rendered region, validates it against the composer, and reanchors after mismatches. Commit and discard clear or remove text only when the tracked region still matches. Tests cover caret movement, edits, cancellation, and invalid bounds.

Changes

Live dictation region tracking

Layer / File(s)	Summary
Region tracking and reanchoring internal/tui/dictation.go, internal/tui/dictation_stream.go	Streaming updates record rendered text, validate region bounds and prefixes, reanchor mismatched regions, and protect user edits during commit or discard. Reset clears the tracked region state.
Streaming regression coverage internal/tui/dictation_test.go	Tests cover internal-caret insertion, composer shrinkage, stale bounds, backspacing through a live region, cancellation, and invalid bounds without panics.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 329dd

The change bounds live dictation regions and adds coverage for caret and composer edits; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: anandh8x, vasanthdev2004

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: correcting live dictation region bounds in the TUI.
Linked Issues check	✅ Passed	The changes satisfy issue #965. They prevent invalid live-region bounds, preserve user edits after composer changes, and add regression tests for caret-middle streaming, stale regions, invalid bounds,…
Out of Scope Changes check	✅ Passed	All production and test changes directly support the live dictation bounds fix in issue #965. No unrelated code changes are identified.

Full details: Linked Issues check

Explanation

The changes satisfy issue #965. They prevent invalid live-region bounds, preserve user edits after composer changes, and add regression tests for caret-middle streaming, stale regions, invalid bounds, and successive partial updates.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[Vasanthdev2004]

Approving. The real fix here is the anchor change, and it is load-bearing: reverting regionAnchor from string(stateRunes[:state.cursor]) back to state.text fails TestDictationStreamingPartialWithCaretInsideExistingText. Anchoring to the whole composer meant prefix could never equal anchor once the caret sat mid-text, which is what drove the arithmetic negative.

One thing to fix before this is done, and it is about coverage rather than correctness.

Neither clamp is pinned by any test. I removed the first, then the second, then both, and every DictationStreaming test stayed green each time, including TestDictationStreamingPartialClampsRegionAfterComposerShrink, which is named for exactly the case the clamps exist to handle.

They are not decorative. With both removed, driving the region out of bounds panics in the same class as the bug this PR fixes:

region past end after shrink   PANIC: slice bounds out of range [:40] with capacity 32
negative start                 PANIC: slice bounds out of range [:-3]

So the clamps guard reachable states, and nothing stops a later refactor deleting them with CI green and reintroducing the panic. The shrink test presumably keeps the region in range through the prefix-delta path and never reaches the clamp. Setting the region out of bounds directly is enough to pin it:

m.setComposerState(composerState{text: "ab", cursor: 2})
m.dictation.regionActive = true
m.dictation.regionStart = 40
m.dictation.regionEnd = 60
m.dictation.regionAnchor = "a much longer previous composer"
m.applyStreamingText("partial text")   // panics without the clamps

A negative-start case covers the second clamp the same way.

One concern of mine that turned out to be nothing, in case it saves you the thought: the first activation slices stateRunes[:state.cursor] before regionActive is true, so it is outside the clamped branch. I checked whether currentComposerState can return a cursor past the end on the !composerActive path, where it uses m.input.Position() raw with no normalizeComposerState. It cannot; bubbles/textinput bounds the cursor on both SetValue and SetCursor, so that slice is safe.

Approving because the code is correct as written and the missing coverage does not make it wrong, but I would rather the clamps were pinned before this lands.

go test ./internal/tui passes in full.

[Vasanthdev2004]

Retracting my approval. I approved this earlier today and I was wrong: the first clamp introduces a data-loss regression that main does not have. I found it by going back over the half I had flagged as untested, which is exactly where it was hiding.

The user's own text is destroyed

Type hello world, dictate, then backspace past the live region while partials keep arriving, then cancel:

                head (dc80cecd)                      main (eeea3308)
p1     "hello world there"  [11,17)          "hello world there"  [11,17)
edits  "hello wor" cursor=9                  "hello wor" cursor=9
p2     "hello wor there friend"  [7,20)      "hello worthere friend"  [9,21)
p3     "hello there friend againiend" [5,24) "hello worthere friend again" [9,27)
cancel "helloiend"                           "hello wor"

On main the user's hello wor survives and cancel restores it exactly. On this head their wor is eaten, a stray iend is left behind, and cancel leaves helloiend. The region desyncs progressively, [11,17) to [7,20) to [5,24), so each partial deletes a span the dictation never wrote.

Why the first clamp causes it

The pre-clamp at dictation_stream.go:96-101 runs before line 122 computes prefix := string(stateRunes[:m.dictation.regionStart]). Once regionStart has been clamped down to the shrunken composer length, prefix is no longer "the text before the live region", it is a truncation of the new composer, and a truncation is almost always a prefix of the old anchor. So the delete-before-the-region branch at 134-139 fires for an edit that was not before the region, and subtracts a delta that has no meaning. The second clamp then stops the panic but leaves the region pointing at unrelated text.

The fix is smaller than the current diff

Removing only lines 96-101 and keeping the regionAnchor change restores correct behaviour and leaves your own tests green:

p3     "hello worthere friend again"  [9,27)
cancel "hello wor"
ok  github.com/Gitlawb/zero/internal/tui

The anchor change is the actual fix for #965, and it stands on its own: reverting just it fails TestDictationStreamingPartialWithCaretInsideExistingText with the reported slice bounds out of range [:-1]. The clamps were added as defence and the first one is doing harm instead.

If you want to keep a clamp for the genuinely out-of-range case, it needs to sit after the anchor comparison rather than before it, so the prefix is still compared against the region that actually existed. And it wants a test: I noted in my earlier review that removing either clamp left every dictation test passing, which is what let this through. That observation was right and I did not follow it far enough before approving.

Sorry for the churn on this one.

[anandh8x]

The mid-caret anchor change fixes the reported crash, but the pre-comparison clamp can corrupt user text. If the user backspaces across a live dictation region while later partials arrive, clamping before comparing the anchor makes the shrink look like an edit before the region; subsequent partials then replace normal composer text, and cancel does not restore it. Please move/remove that first clamp so anchor comparison uses the original region position, retain bounds protection only at the slice/delete boundary, and add a regression covering the shrink-plus-cancel path.

[anandh8x]

Correction to my review: the problematic value is regionStart. Clamping regionStart before the anchor comparison causes the destructive misclassification described above.

[jatmn]

I found issues that need to be addressed before this is ready.

Merge readiness

• [P1] Rebase onto current main before merge
  internal/tui/dictation_stream.go:93
  This branch's merge base is 27b319ca88a3180bed5183f0c599e9307f3ece12, while live main is 1b5db1765672820caac1684b168c9898b5ba3593. GitHub currently reports it mergeable, but the repository requires a fresh base before review. Please rebase and re-run the affected validation on the resolved head.

Findings

• [P1] Do not clamp a stale live region into user text
  internal/tui/dictation_stream.go:109
  The new prefix anchor only proves that the text before regionStart is unchanged; it does not prove that [regionStart, regionEnd) is still the rendered transcript. For example, after rendering a partial at cursor 1 in aOLDz, replacing the composer with ab leaves regionStart == 1 and the prefix a intact but makes regionEnd stale. The final clamp turns that old range into [1,2), so the next partial deletes the user's b. The same sequence preserves b on the merge base. Re-anchor or otherwise preserve the replacement before applying the delayed partial, and add regression coverage, so it never overwrites user text.

[Vasanthdev2004]

Approving. The regression I retracted over is gone, and I drove it rather than reading the diff.

Replayed the exact sequence through the real key path, eight tea.KeyBackspace presses through applyComposerKey rather than a stand-in, then cancel:

                HEAD 329dd123                            MAIN
p1      "hello world there"          [11,17)     "hello world there"          [11,17)
edits   "hello wor"            cur=9 [11,17)     "hello wor"            cur=9 [11,17)
p2      "hello worthere friend"       [9,21)     "hello worthere friend"       [9,21)
p3      "hello worthere friend again" [9,27)     "hello worthere friend again" [9,27)
cancel  "hello wor"                              "hello wor"

Byte-identical at every step, bounds included. The values I reported (p3 "hello there friend againiend", cancel "helloiend") do not reproduce. The mechanism is gone by construction rather than patched around it: 058ba80 deleted the pre-clamp, and 329dd12 deleted the post-clamp along with the whole prefix-versus-anchor shift switch. There is no clamp( left in dictation_stream.go.

What replaced it is better than what I asked for. Both delete sites are now gated on liveRegionMatches, which requires the span being removed to be byte-identical to what dictation actually rendered. That is an invariant main does not have, and it means this subsystem structurally cannot delete text it did not write. Driving #965 itself shows the difference: main goes [5,11) to [-1,12) to a slice bounds out of range [:-1] panic and cancels to "d there world", while head cancels to "hello world".

One thing worth knowing rather than fixing. dictation_stream.go:137, the string(stateRunes[d.regionStart:d.regionEnd]) == d.regionRendered clause, is not pinned by any test: replacing it with true leaves the dictation suite green. It is load-bearing though, and it is the sole cause of the one behaviour difference from main. When a user edits inside the live region and the bounds stay valid, head abandons the region and re-anchors, leaving the edited copy behind:

   user overtypes the dictated span (" there" -> " MINE!")
   HEAD  cancel "hello world MINE!"
   MAIN  cancel "hello world"          <- the user's " MINE!" silently deleted

So it diverges in the safe direction: head keeps bytes the user typed and leaves visible duplicate transcript, main deletes them. You cannot distinguish "fixed a typo" from "typed replacement text" at that point, so I read this as a deliberate trade rather than a defect, and blocking on it would be asking for main's data loss back.

Two things to pick up whenever, neither gating: a test pinning line 137 specifically (a same-length in-region edit with valid bounds, since every existing test short-circuits on the bounds arm first), and re-running needsLeadingSpace in the re-anchor branch so the separator space stops being orphaned. The second is present on main too.

Sorry again for the churn. The retraction was right and the fix is right.

[jatmn]

lgtm