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 <noreply@anthropic.com>
2026-08-24 12:30:23 -04:00
parent 0c8eec1bf6
commit 314b838e33
2 changed files with 172 additions and 28 deletions
@@ -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()
 }

@@ -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)
+			}
+		}
+	}
+}