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/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) + } + } +} 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/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. 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