From 4e4d7c186a6d77048a47da2ea6e52f3ac1fafdbd Mon Sep 17 00:00:00 2001 From: kami Date: Mon, 22 Jun 2026 10:09:56 +0000 Subject: [PATCH] fix(tui-go): make the write/edit output-panel number meaningful MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The collapsed tool row showed "tool output (N chars)" where N was len() of the raw unified diff — i.e. the diff blob's *byte* length (headers, @@, +/-/context all counted), labelled "chars". That number corresponded to nothing the operator cares about, and len() is bytes not characters. - Summarise a write/edit by the diff's row count instead — "· diff (7 rows) — ^x to view" — matching exactly what the ^x modal reports. Non-diff output (the defensive fallback) now counts runes, not bytes, so "N chars" is truthful. Retire itoaLen (the byte-count-as-"len" footgun). - Harden the (+a −b) line counts in diffSummary: guard the file header with the trailing space ("+++ "/"--- "), so a changed line whose content starts with "++"/"--" is counted instead of mistaken for a header. Apply the same guard in parseUnifiedDiff so such a line is rendered (and counted) rather than dropped. tool_summary_test.go covers the diff-row summary, the rune (not byte) count, and the ++/-- content-line edge case. Co-Authored-By: Claude Opus 4.8 --- apps/tui-go/internal/app/diff.go | 5 +- apps/tui-go/internal/app/server.go | 4 +- apps/tui-go/internal/app/tool_summary_test.go | 47 +++++++++++++++++++ apps/tui-go/internal/app/view.go | 14 +++++- 4 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 apps/tui-go/internal/app/tool_summary_test.go diff --git a/apps/tui-go/internal/app/diff.go b/apps/tui-go/internal/app/diff.go index f8f5f707..bf77ec35 100644 --- a/apps/tui-go/internal/app/diff.go +++ b/apps/tui-go/internal/app/diff.go @@ -53,7 +53,10 @@ func parseUnifiedDiff(diff string) []diffRow { } for _, ln := range strings.Split(strings.TrimRight(diff, "\n"), "\n") { switch { - case strings.HasPrefix(ln, "+++"), strings.HasPrefix(ln, "---"), + // File headers carry a trailing space ("+++ b/x"); requiring it means a real + // changed line whose content starts with "++"/"--" isn't mistaken for a header + // and silently dropped from the rendered diff (and its row count). + case strings.HasPrefix(ln, "+++ "), strings.HasPrefix(ln, "--- "), strings.HasPrefix(ln, "diff "), strings.HasPrefix(ln, "index "): continue case strings.HasPrefix(ln, "@@"): diff --git a/apps/tui-go/internal/app/server.go b/apps/tui-go/internal/app/server.go index 6b5c0dc2..7be87790 100644 --- a/apps/tui-go/internal/app/server.go +++ b/apps/tui-go/internal/app/server.go @@ -425,9 +425,9 @@ func diffSummary(diff string) (path string, added, removed int) { if p != "" && p != "/dev/null" { path = p } - case strings.HasPrefix(ln, "+") && !strings.HasPrefix(ln, "+++"): + case strings.HasPrefix(ln, "+") && !strings.HasPrefix(ln, "+++ "): added++ - case strings.HasPrefix(ln, "-") && !strings.HasPrefix(ln, "---"): + case strings.HasPrefix(ln, "-") && !strings.HasPrefix(ln, "--- "): removed++ } } diff --git a/apps/tui-go/internal/app/tool_summary_test.go b/apps/tui-go/internal/app/tool_summary_test.go new file mode 100644 index 00000000..737afa9b --- /dev/null +++ b/apps/tui-go/internal/app/tool_summary_test.go @@ -0,0 +1,47 @@ +package app + +import ( + "strings" + "testing" +) + +// The collapsed tool row for a write/edit must summarise the diff by its row count (what +// ^x shows), not the raw diff byte length that used to read as a meaningless "N chars". +func TestToolRowSummary_DiffReportsRows(t *testing.T) { + diff := "--- a/f\n+++ b/f\n@@ -1,2 +1,3 @@\n context\n-old line\n+new line\n+added line\n" + got := toolRowSummary(diff) + wantRows := previewRowCount(diff) + if !strings.Contains(got, "diff (") || !strings.Contains(got, itoa(wantRows)+" rows") { + t.Fatalf("diff summary = %q, want a diff row count of %d", got, wantRows) + } + if strings.Contains(got, "tool output") { + t.Errorf("a diff should not be labelled 'tool output': %q", got) + } +} + +// Non-diff output reports a true character (rune) count, not a byte count. +func TestToolRowSummary_NonDiffCountsRunes(t *testing.T) { + content := "café — déjà" // 11 runes, but 14 bytes (é, —, à are multi-byte) + got := toolRowSummary(content) + if !strings.Contains(got, "(11 chars)") { + t.Fatalf("non-diff summary = %q, want 11 chars (runes, not bytes)", got) + } +} + +// A changed line whose content starts with "++"/"--" must still be counted (and rendered): +// requiring the trailing space on the file-header guard prevents mistaking it for a header. +func TestDiffSummary_CountsDoublePrefixContentLines(t *testing.T) { + // The added line's content is "++flag"; in the diff that's "+" + "++flag" = "+++flag". + diff := "--- a/f\n+++ b/f\n@@ -0,0 +1,2 @@\n+++flag\n+normal\n" + _, added, removed := diffSummary(diff) + if added != 2 { + t.Errorf("added = %d, want 2 (the '+++flag' content line must count)", added) + } + if removed != 0 { + t.Errorf("removed = %d, want 0", removed) + } + // The renderer must keep the line too (2 add rows), not drop it as a header. + if n := previewRowCount(diff); n != 2 { + t.Errorf("rendered rows = %d, want 2", n) + } +} diff --git a/apps/tui-go/internal/app/view.go b/apps/tui-go/internal/app/view.go index b42abb72..2b212ccc 100644 --- a/apps/tui-go/internal/app/view.go +++ b/apps/tui-go/internal/app/view.go @@ -589,7 +589,7 @@ func (m Model) buildTranscriptRows(w int) ([]string, []int) { rows = append(rows, t.span(s, t.P.Faint)) } case "tool": - rows = append(rows, t.span("· tool output ("+itoaLen(e.Content)+" chars) — ^x to view", t.P.Dim)) + rows = append(rows, t.span(toolRowSummary(e.Content), t.P.Dim)) case "narration_llm": diamond := lipgloss.NewStyle().Foreground(t.P.Accent).Background(t.P.Bg).Bold(true).Render("◆") lines := wrap(e.Content, w-2) @@ -856,7 +856,17 @@ func itoa(n int) string { return string(b[i:]) } -func itoaLen(s string) string { return itoa(len(s)) } +// toolRowSummary collapses a tool-output transcript entry into a one-line pointer. A +// write/edit entry holds a unified diff, so summarise it by what ^x actually shows — the +// diff's row count — instead of the raw diff byte length (which read as a meaningless +// "N chars": it counted +/-/@@/header bytes, not anything the operator cares about, and +// len() is bytes not characters). Non-diff output falls back to a true character count. +func toolRowSummary(content string) string { + if isUnifiedDiff(content) { + return "· diff (" + itoa(previewRowCount(content)) + " rows) — ^x to view" + } + return "· tool output (" + itoa(len([]rune(content))) + " chars) — ^x to view" +} // metricsSuffix formats the faint latency+token annotation for a ROUTER turn. func metricsSuffix(mtr *TurnMetrics) string {