From 5a0ff54a0d03caa65c5590adcb3b51555b2986ab Mon Sep 17 00:00:00 2001 From: Dennison Date: Sun, 6 Sep 2026 15:34:41 -0400 Subject: [PATCH 1/3] test(red): TASK-1409 failing test for ANSI-aware viewport clamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behavioral test added: viewport.View() must not drop visible content when a line contains real ANSI escape sequences (glamour output under a real color profile). The current clamp in model.go slices by rune count, which counts escape bytes toward the width budget and cuts mid-escape, dropping trailing visible text. Test runner output (expected: all failing): === RUN TestTUI1409_ANSIStyledLineNotTruncatedByRuneCount ansi_clamp_test.go:36: visible content within the width-40 budget was dropped by the clamp; view: "\n\n\n\n • Created \x1b[48;2;40;40;40m calc.go \x1b[0" --- FAIL: TestTUI1409_ANSIStyledLineNotTruncatedByRuneCount (0.00s) FAIL This test will pass after the implementation in the next commit. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5 --- .../components/viewport/ansi_clamp_test.go | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 cmd/harnesscli/tui/components/viewport/ansi_clamp_test.go diff --git a/cmd/harnesscli/tui/components/viewport/ansi_clamp_test.go b/cmd/harnesscli/tui/components/viewport/ansi_clamp_test.go new file mode 100644 index 00000000..de982405 --- /dev/null +++ b/cmd/harnesscli/tui/components/viewport/ansi_clamp_test.go @@ -0,0 +1,44 @@ +package viewport_test + +import ( + "strings" + "testing" + + "github.com/charmbracelet/lipgloss" + "go-agent-harness/cmd/harnesscli/tui/components/viewport" +) + +// TestTUI1409_ANSIStyledLineNotTruncatedByRuneCount reproduces issue #1409: +// a line containing a real ANSI escape sequence (as glamour emits under a +// real color profile) has more runes than visible cells. The viewport's +// horizontal clamp must cut by visible width, not rune count, or trailing +// visible content is lost even though the escape bytes push it past the +// rune-count budget. +func TestTUI1409_ANSIStyledLineNotTruncatedByRuneCount(t *testing.T) { + // Styled segment "\x1b[48;2;40;40;40m calc.go \x1b[0m" renders as + // " calc.go " (9 visible cells) but contributes many more runes than + // that via the escape codes. The full line has 42 visible cells and + // 62 runes, matching the diagnosis in issue #1409. + line := " • Created \x1b[48;2;40;40;40m calc.go \x1b[0m with an Add function" + + if got := lipgloss.Width(line); got != 42 { + t.Fatalf("test fixture invariant broken: want visible width 42, got %d", got) + } + if got := len([]rune(line)); got != 62 { + t.Fatalf("test fixture invariant broken: want rune count 62, got %d", got) + } + + vp := viewport.New(40, 5) + vp.AppendLine(line) + view := vp.View() + + if !strings.Contains(view, "Add") { + t.Errorf("visible content within the width-40 budget was dropped by the clamp; view:\n%q", view) + } + + for i, l := range strings.Split(view, "\n") { + if w := lipgloss.Width(l); w > 40 { + t.Errorf("line %d exceeds viewport width: got %d cells, want <= 40 (%q)", i, w, l) + } + } +} From 0dfa54aafbcbdc4353c080c58f598cf2661ae445 Mon Sep 17 00:00:00 2001 From: Dennison Date: Sun, 6 Sep 2026 15:36:34 -0400 Subject: [PATCH 2/3] fix: TASK-1409 truncate viewport lines by ANSI-aware visible width Implementation for the test added in 5a0ff54a. The viewport horizontal clamp in model.go sliced each visible line by rune count. Under a real color profile, glamour emits ANSI escape sequences whose bytes count as runes but occupy zero terminal cells, so the rune-count budget was exhausted early and the slice cut mid- escape, dropping trailing visible text (issue #1409). Under the ascii test profile there are no escapes, so rune count equals cell width and the bug was invisible to the existing suite. Fix: replace the rune slice with github.com/charmbracelet/x/ansi's Truncate(line, m.width, ""), which is ANSI-aware, keeps escape sequences intact, and is a no-op when the line already fits. github.com/charmbracelet/x/ansi v0.11.6 was already an indirect dependency; `go mod tidy` after adding the import moved it (and two other already-directly-imported packages, github.com/creack/pty and github.com/coder/acp-go-sdk, previously mislabeled) to the direct require block. No dependency versions changed (go.sum is unchanged). Test runner output (expected: all passing): === RUN TestTUI1409_ANSIStyledLineNotTruncatedByRuneCount --- PASS: TestTUI1409_ANSIStyledLineNotTruncatedByRuneCount (0.00s) PASS ok go-agent-harness/cmd/harnesscli/tui/components/viewport (cached) Full cmd/harnesscli/tui/... suite passes under -race; go vet clean; go build ./cmd/... ./internal/... clean (unrelated pre-existing benchmarks/terminal_bench/reference_solutions fixtures already fail `go build ./...` on origin/main and are out of scope). Behavioral tests covered: viewport clamp no longer drops content past an ANSI escape sequence. Files changed: cmd/harnesscli/tui/components/viewport/model.go, go.mod Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5 --- cmd/harnesscli/tui/components/viewport/model.go | 11 +++++++---- go.mod | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/cmd/harnesscli/tui/components/viewport/model.go b/cmd/harnesscli/tui/components/viewport/model.go index c3d16dc7..11cf24bb 100644 --- a/cmd/harnesscli/tui/components/viewport/model.go +++ b/cmd/harnesscli/tui/components/viewport/model.go @@ -5,6 +5,7 @@ import ( "strings" tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/x/ansi" ) // Model is the scrollable viewport for conversation content. @@ -319,10 +320,12 @@ func (m Model) View() string { } for _, line := range visible { - runes := []rune(line) - if len(runes) > m.width { - line = string(runes[:m.width]) - } + // Clamp by visible cell width, not rune count: lines can contain + // ANSI escape sequences (glamour output under a real color + // profile) whose bytes count as runes but occupy no cells. + // ansi.Truncate is ANSI-aware and a no-op when the line already + // fits within m.width. + line = ansi.Truncate(line, m.width, "") sb.WriteString(line) sb.WriteString("\n") } diff --git a/go.mod b/go.mod index 60434017..b652b279 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( modernc.org/sqlite v1.33.1 ) -require github.com/creack/pty v1.1.24 // indirect +require github.com/creack/pty v1.1.24 require ( github.com/Microsoft/go-winio v0.4.21 // indirect @@ -37,7 +37,7 @@ require ( github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/charmbracelet/colorprofile v0.4.1 // indirect - github.com/charmbracelet/x/ansi v0.11.6 // indirect + github.com/charmbracelet/x/ansi v0.11.6 github.com/charmbracelet/x/cellbuf v0.0.15 // indirect github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91 // indirect github.com/charmbracelet/x/exp/slice v0.0.0-20250327172914-2fdc97757edf // indirect @@ -45,7 +45,7 @@ require ( github.com/clipperhouse/displaywidth v0.9.0 // indirect github.com/clipperhouse/stringish v0.1.1 // indirect github.com/clipperhouse/uax29/v2 v2.5.0 // indirect - github.com/coder/acp-go-sdk v0.13.5 // indirect + github.com/coder/acp-go-sdk v0.13.5 github.com/containerd/errdefs v1.0.0 // indirect github.com/containerd/errdefs/pkg v0.3.0 // indirect github.com/containerd/log v0.1.0 // indirect From 5e06132063c13111be53b0c192733e4ca4c3b337 Mon Sep 17 00:00:00 2001 From: Dennison Date: Sun, 6 Sep 2026 15:44:36 -0400 Subject: [PATCH 3/3] test(regression): TASK-1409 regression coverage for viewport ANSI clamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression test added that would fail if the fix in 0dfa54aa is reverted: TestTUI1409_ModelViewPreservesANSIStyledTranscriptLine constructs a full tui.Model (New + tea.WindowSizeMsg), injects the same ANSI-styled transcript line used by the viewport-level test into the model's embedded viewport, and asserts the fully assembled Model.View() frame (header/separators/viewport/input/status bar) — not just the isolated viewport package — preserves visible content and respects the line width. Verified by hand-reverting the model.go clamp and confirming this test fails the same way ("Add" dropped) before restoring the fix. messagebubble/glamour could not be made to emit real ANSI escapes in this test process: even forcing lipgloss.SetColorProfile(termenv.TrueColor) left RenderMarkdown output escape-free (verified experimentally), because messagebubble's own stdoutIsTerminal probe is a syscall check on os.Stdout's fd, independent of lipgloss's global profile, and always resolves to the escape-free "notty" glamour style outside a real terminal. That probe lives in messagebubble, outside this fix's scope (cmd/harnesscli/tui/components/viewport, go.mod/go.sum, cmd/harnesscli/tui), so the regression test injects a realistic pre-rendered ANSI-styled line directly into m.vp rather than routing it through glamour. Also adds the docs/logs/engineering-log.md entry (symptom, cause, fix). Full test suite output: ok go-agent-harness/cmd/harnesscli/tui 46.592s ok go-agent-harness/cmd/harnesscli/tui/components/configpanel 0.658s ok go-agent-harness/cmd/harnesscli/tui/components/contextgrid 0.877s ok go-agent-harness/cmd/harnesscli/tui/components/costdisplay 0.532s ok go-agent-harness/cmd/harnesscli/tui/components/diffview 0.768s ok go-agent-harness/cmd/harnesscli/tui/components/helpdialog 2.057s ok go-agent-harness/cmd/harnesscli/tui/components/inputarea 1.924s ok go-agent-harness/cmd/harnesscli/tui/components/interruptui 2.261s ok go-agent-harness/cmd/harnesscli/tui/components/layout 1.811s ok go-agent-harness/cmd/harnesscli/tui/components/messagebubble 1.179s ok go-agent-harness/cmd/harnesscli/tui/components/modelswitcher 2.400s ok go-agent-harness/cmd/harnesscli/tui/components/permissionspanel 2.156s ok go-agent-harness/cmd/harnesscli/tui/components/profilepicker 1.702s ok go-agent-harness/cmd/harnesscli/tui/components/sessionpicker 1.373s ok go-agent-harness/cmd/harnesscli/tui/components/slashcomplete 1.256s ok go-agent-harness/cmd/harnesscli/tui/components/spinner 1.041s ok go-agent-harness/cmd/harnesscli/tui/components/statspanel 1.947s ok go-agent-harness/cmd/harnesscli/tui/components/statusbar 1.933s ok go-agent-harness/cmd/harnesscli/tui/components/streamrenderer 1.937s ok go-agent-harness/cmd/harnesscli/tui/components/taskspanel 1.939s ok go-agent-harness/cmd/harnesscli/tui/components/themepicker 1.889s ok go-agent-harness/cmd/harnesscli/tui/components/thinkingbar 1.858s ok go-agent-harness/cmd/harnesscli/tui/components/tooluse 1.922s ok go-agent-harness/cmd/harnesscli/tui/components/transcriptexport 1.897s ok go-agent-harness/cmd/harnesscli/tui/components/undopicker 1.760s ok go-agent-harness/cmd/harnesscli/tui/components/viewport 1.765s ok go-agent-harness/cmd/harnesscli/tui/plugin 1.830s ok go-agent-harness/cmd/harnesscli/tui/testhelpers 2.903s Full cmd/harnesscli/tui/... suite also verified clean under -race. Regression scenarios covered: - Naive rune-count clamp reintroduced in viewport.Model.View() (direct, package-level). - Naive rune-count clamp reintroduced anywhere in the assembled Model.View() render path (integration-level, via the embedded m.vp). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5 --- .../tui/ansi_clamp_regression_test.go | 53 ++++++++++++++++++ docs/logs/engineering-log.md | 56 +++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 cmd/harnesscli/tui/ansi_clamp_regression_test.go diff --git a/cmd/harnesscli/tui/ansi_clamp_regression_test.go b/cmd/harnesscli/tui/ansi_clamp_regression_test.go new file mode 100644 index 00000000..3c3ef101 --- /dev/null +++ b/cmd/harnesscli/tui/ansi_clamp_regression_test.go @@ -0,0 +1,53 @@ +package tui + +import ( + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" +) + +// TestTUI1409_ModelViewPreservesANSIStyledTranscriptLine is a regression +// guard for issue #1409 at the assembled tui.Model integration point, not +// just the standalone viewport package. +// +// Background: glamour (via messagebubble.RenderMarkdown) only emits real +// ANSI escape sequences when os.Stdout is a real terminal — messagebubble's +// stdoutIsTerminal probe falls back to the escape-free "notty" style +// otherwise, which is also why forcing lipgloss's global color profile +// (lipgloss.SetColorProfile(termenv.TrueColor)) does not make glamour emit +// escapes in this test process (verified experimentally: RenderMarkdown +// still produced zero ESC bytes for a code-span line with that profile set). +// That probe lives in the messagebubble package, which is out of scope for +// this fix, so this test injects a realistic ANSI-styled transcript line +// directly into the model's viewport (m.vp) — the same line shape +// messagebubble/glamour produce for a markdown list item with a styled code +// span under a real terminal — and exercises it through the full, +// assembled Model.View() (header/separators/viewport/input/status bar), +// not just viewport.Model.View() in isolation. +func TestTUI1409_ModelViewPreservesANSIStyledTranscriptLine(t *testing.T) { + cfg := DefaultTUIConfig() + m := New(cfg) + m2, _ := m.Update(tea.WindowSizeMsg{Width: 40, Height: 24}) + model := m2.(Model) + + // Same shape as the proven issue #1409 example: a glamour list item with + // a truecolor-background code span. Visible width 42, rune count 62 — + // the naive rune-count clamp this issue fixed would slice mid-escape at + // [:40] runes and drop "with an Add function" entirely. + styledLine := " • Created \x1b[48;2;40;40;40m calc.go \x1b[0m with an Add function" + model.vp.AppendLine(styledLine) + + view := model.View() + + if !strings.Contains(view, "Add") { + t.Errorf("assembled Model.View() dropped visible transcript content past an ANSI escape; view:\n%q", view) + } + + for i, l := range strings.Split(view, "\n") { + if w := lipgloss.Width(l); w > model.width { + t.Errorf("line %d of assembled Model.View() exceeds width: got %d cells, want <= %d (%q)", i, w, model.width, l) + } + } +} diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index d8279ad4..53b92bdb 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -6256,3 +6256,59 @@ Skipped creating separate issues for Op/EventMsg protocol (already covered by SS execute on this darwin worktree and is verified by `GOOS=linux go vet ./internal/harness/tools/` for syntax/type correctness only — it needs a real Linux CI run to prove behavior. + +# 2026-09-06 (Issue #1409 viewport horizontal clamp truncated ANSI-styled transcript lines) + +- Symptom: streamed markdown assistant output in the TUI transcript viewport + sometimes lost trailing visible text — a bullet item like "Created + `calc.go` with an Add function" could render with "with an Add function" + (or more) silently missing — but every unit test for the viewport passed. +- Cause: `viewport.Model.View()` + (`cmd/harnesscli/tui/components/viewport/model.go`) clamped each visible + line to the viewport width by rune count: `runes := []rune(line); if + len(runes) > m.width { line = string(runes[:m.width]) }`. Under a real + color profile, glamour (via `messagebubble.RenderMarkdown`) emits ANSI + escape sequences — e.g. a truecolor code-span background is ~20 extra + bytes — and each escape byte counts as a rune even though it occupies zero + terminal cells. A line with 42 visible cells but 62 runes hit the + rune-count budget at rune 40, which fell inside the escape sequence itself, + so the slice cut mid-escape and dropped everything after it. Under the + `ascii`/`notty` glamour style used when `os.Stdout` is not a real terminal + (the case for every test process, and for `messagebubble`'s own + `stdoutIsTerminal` probe), there are no escape bytes, so rune count equals + cell width and the clamp never mis-cut — which is why the existing test + suite never caught this. +- Fix: replaced the rune slice with + `github.com/charmbracelet/x/ansi`'s `Truncate(line, m.width, "")`, which is + ANSI-aware (it will not cut inside an escape sequence) and is a no-op when + the line already fits within `m.width`. `github.com/charmbracelet/x/ansi` + was already an indirect dependency (transitively required by the existing + charm ecosystem deps); `go mod tidy` after adding the import promoted it — + along with two other packages already imported directly elsewhere in the + tree but previously mislabeled indirect (`github.com/creack/pty`, + `github.com/coder/acp-go-sdk`) — to the direct `require` block in `go.mod`. + No dependency versions changed (`go.sum` is byte-for-byte unchanged). +- Verification: a new `viewport`-package test + (`TestTUI1409_ANSIStyledLineNotTruncatedByRuneCount`) feeds the viewport a + line with a real ANSI escape sequence (42 visible cells, 62 runes) at + width 40 and asserts the trailing marker word survives and no rendered + line exceeds width via `lipgloss.Width`; it fails against the pre-fix + clamp (marker dropped) and passes after. A second, model-level regression + test in `cmd/harnesscli/tui` + (`TestTUI1409_ModelViewPreservesANSIStyledTranscriptLine`) drives the same + fixture through the fully assembled `tui.Model` (constructed via `New` + + a `tea.WindowSizeMsg`, injected via the model's embedded `m.vp`) and + asserts the same invariant over the complete `Model.View()` frame + (header/separators/viewport/input/status bar), not just the isolated + viewport package — confirmed to fail the same way if the fix in + `model.go` is reverted. Glamour could not be made to emit real ANSI + escapes in this test process even with `lipgloss.SetColorProfile + (termenv.TrueColor)` forced (verified experimentally): `messagebubble`'s + own `stdoutIsTerminal` probe is a syscall-level check on `os.Stdout`'s fd, + independent of lipgloss's global color profile, so it always resolves to + the escape-free `notty` style outside a real terminal. That probe lives in + `messagebubble`, outside this fix's file scope, so the model-level test + injects a realistic pre-rendered ANSI-styled line directly rather than + routing it through glamour. `go test ./cmd/harnesscli/tui/... -race`, + `go vet ./cmd/harnesscli/tui/...`, and `go build ./cmd/... ./internal/...` + are all clean.