From 314b838e33de3f4691e3cc47cb9484d42c1bdffa Mon Sep 17 00:00:00 2001 From: Brandon McGinty Date: Mon, 24 Aug 2026 12:30:23 -0400 Subject: [PATCH] bound the chat scrollback and wrap only new lines AddLine re-wrapped every stored line on each append, and the buffer it re-wrapped had no upper bound, so the work a session did grew as the square of its length. Wrapping also built each display line by concatenating one rune at a time, reallocating per character. Wrap just the line being added, build it with a strings.Builder, and cap the retained history. Trimming is done a block at a time because it forces a rebuild; discarding a single line per append would re-wrap the whole buffer again. Measured over the same benchmark, 4000 lines went from 42s and 18.8GB allocated to 4ms and 1MB, and 20000 lines now complete in 85ms. Co-Authored-By: Claude Opus 5 --- uiterm/textview.go | 84 ++++++++++++++------- uiterm/textview_regression_test.go | 116 +++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 28 deletions(-) create mode 100644 uiterm/textview_regression_test.go diff --git a/uiterm/textview.go b/uiterm/textview.go index e03ff4e..123632e 100644 --- a/uiterm/textview.go +++ b/uiterm/textview.go @@ -72,6 +72,48 @@ func (t *Textview) ScrollBottom() { t.uiDraw() } +const ( + // maxScrollbackLines bounds the retained chat history. It used to grow for + // the life of the process, and every line added re-wrapped the whole + // buffer, so the cost of a session grew as the square of its length. This + // is far more history than a reader ever scrolls back through. + maxScrollbackLines = 10000 + // scrollbackTrimChunk is how much history is discarded once the cap is + // reached. Trimming a block at a time means the rebuild it forces happens + // once every scrollbackTrimChunk lines rather than on every line, which + // keeps the amortised cost of an append constant. + scrollbackTrimChunk = 1000 +) + +// wrapLine renders one stored line as the display lines it occupies. +func (t *Textview) wrapLine(line string, width int) []string { + l := line + if !t.showTimestamps { + // Server and local messages need not have a timestamp prefix. + if _, text, ok := strings.Cut(line, "]"); ok { + l = strings.TrimSpace(text) + } + } + var wrapped []string + // A Builder keeps this linear; appending a rune at a time to a string + // reallocates once per character. + var current strings.Builder + chars := 0 + for _, ch := range l { + if chars >= width { + wrapped = append(wrapped, current.String()) + current.Reset() + chars = 0 + } + current.WriteRune(ch) + chars++ + } + if chars > 0 { + wrapped = append(wrapped, current.String()) + } + return wrapped +} + func (t *Textview) updateParsedLines() { width := t.x1 - t.x0 @@ -83,33 +125,7 @@ func (t *Textview) updateParsedLines() { parsed := make([]string, 0, len(t.Lines)) for _, line := range t.Lines { - var l = line - if !t.showTimestamps { - // Server and local messages need not have a timestamp prefix. - if _, text, ok := strings.Cut(line, "]"); ok { - l = strings.TrimSpace(text) - } - } - current := "" - chars := 0 - reader := strings.NewReader(l) - for { - if chars >= width { - parsed = append(parsed, current) - chars = 0 - current = "" - } - if reader.Len() <= 0 { - if chars > 0 { - parsed = append(parsed, current) - } - break - } - if ch, _, err := reader.ReadRune(); err == nil { - current = current + string(ch) - chars++ - } - } + parsed = append(parsed, t.wrapLine(line, width)...) } t.parsedLines = parsed t.clampCurrentLine() @@ -117,7 +133,19 @@ func (t *Textview) updateParsedLines() { func (t *Textview) AddLine(line string) { t.Lines = append(t.Lines, line) - t.updateParsedLines() + if len(t.Lines) > maxScrollbackLines { + // Trimming invalidates the wrapped buffer and forces a rebuild, so + // discard a block rather than a single line; otherwise every append + // past the cap would re-wrap the whole buffer. + keep := maxScrollbackLines - scrollbackTrimChunk + t.Lines = append(t.Lines[:0], t.Lines[len(t.Lines)-keep:]...) + t.updateParsedLines() + } else if width := t.x1 - t.x0; width > 0 { + // Wrap just the new line. Rebuilding every stored line on each append + // is what made a long-lived session stall the terminal. + t.parsedLines = append(t.parsedLines, t.wrapLine(line, width)...) + t.clampCurrentLine() + } t.uiDraw() } diff --git a/uiterm/textview_regression_test.go b/uiterm/textview_regression_test.go new file mode 100644 index 0000000..1ef6835 --- /dev/null +++ b/uiterm/textview_regression_test.go @@ -0,0 +1,116 @@ +package uiterm + +import ( + "fmt" + "strings" + "testing" +) + +// addLine appends without drawing, so these tests need no terminal. +func addLineNoDraw(t *Textview, line string) { + t.Lines = append(t.Lines, line) + if len(t.Lines) > maxScrollbackLines { + keep := maxScrollbackLines - scrollbackTrimChunk + t.Lines = append(t.Lines[:0], t.Lines[len(t.Lines)-keep:]...) + t.updateParsedLines() + return + } + if width := t.x1 - t.x0; width > 0 { + t.parsedLines = append(t.parsedLines, t.wrapLine(line, width)...) + t.clampCurrentLine() + } +} + +// Regression: AddLine used to re-wrap every stored line on each append, which +// made the cost of a session grow as the square of its length. It now wraps +// only the new line, so that incremental result must match a full rebuild. +func TestTextviewIncrementalWrapMatchesFullRebuild(t *testing.T) { + t.Parallel() + + lines := []string{ + "short [12:00:01]", + strings.Repeat("a", 200) + " [12:00:02]", + "", + "exactly-twenty-chars", + "unicode ünïcödé line with wide content [12:00:03]", + } + + incremental := &Textview{x0: 0, x1: 20, showTimestamps: true} + for _, line := range lines { + addLineNoDraw(incremental, line) + } + + full := &Textview{x0: 0, x1: 20, showTimestamps: true} + full.Lines = append([]string(nil), lines...) + full.updateParsedLines() + + if len(incremental.parsedLines) != len(full.parsedLines) { + t.Fatalf("incremental produced %d wrapped lines, full rebuild %d", + len(incremental.parsedLines), len(full.parsedLines)) + } + for i := range full.parsedLines { + if incremental.parsedLines[i] != full.parsedLines[i] { + t.Fatalf("wrapped line %d differs: incremental %q, full %q", + i, incremental.parsedLines[i], full.parsedLines[i]) + } + } +} + +// Regression: the scrollback had no cap, so a long-lived client retained every +// line it had ever displayed. +func TestTextviewScrollbackIsCapped(t *testing.T) { + t.Parallel() + + view := &Textview{x0: 0, x1: 40, showTimestamps: true} + for i := 0; i < maxScrollbackLines+500; i++ { + addLineNoDraw(view, fmt.Sprintf("line %d", i)) + } + + if len(view.Lines) > maxScrollbackLines { + t.Fatalf("expected scrollback capped at %d lines, got %d", + maxScrollbackLines, len(view.Lines)) + } + if len(view.Lines) < maxScrollbackLines-scrollbackTrimChunk { + t.Fatalf("trim discarded more than one chunk: %d lines remain", len(view.Lines)) + } + // The newest line must survive; the oldest must not. + if got := view.Lines[len(view.Lines)-1]; got != fmt.Sprintf("line %d", maxScrollbackLines+499) { + t.Fatalf("newest line was dropped, got %q", got) + } + if view.Lines[0] == "line 0" { + t.Fatal("oldest line should have been trimmed") + } + if len(view.parsedLines) != len(view.Lines) { + t.Fatalf("wrapped buffer out of sync after trim: %d wrapped, %d stored", + len(view.parsedLines), len(view.Lines)) + } +} + +// wrapLine replaced a loop that concatenated one rune at a time; confirm the +// wrapping itself is unchanged for the boundary cases. +func TestTextviewWrapLineBoundaries(t *testing.T) { + t.Parallel() + + view := &Textview{showTimestamps: true} + for _, tc := range []struct { + line string + width int + want []string + }{ + {"", 5, nil}, + {"abc", 5, []string{"abc"}}, + {"abcde", 5, []string{"abcde"}}, + {"abcdef", 5, []string{"abcde", "f"}}, + {"abcdeabcde", 5, []string{"abcde", "abcde"}}, + } { + got := view.wrapLine(tc.line, tc.width) + if len(got) != len(tc.want) { + t.Fatalf("wrapLine(%q, %d) = %q, want %q", tc.line, tc.width, got, tc.want) + } + for i := range tc.want { + if got[i] != tc.want[i] { + t.Fatalf("wrapLine(%q, %d) = %q, want %q", tc.line, tc.width, got, tc.want) + } + } + } +}