LAB-1803: Trim styled blank scrollback cells

[cweill]

Motivation

LAB-1803's live heap profile reproduced the reported post-LAB-1750 shape. During verification on 2026-05-13, a fresh server heap profile for the installed d266004 build showed 676.69 MB in-use with vt.cloneLineInto retaining 411.63 MB / 60.83%; the issue artifact from 2026-05-13 showed the same post-LAB-1750 shape at 507.70 MB with cloneLineInto at 298.41 MB / 58.78%. A pre-LAB-1750 heap artifact from 2026-05-12 showed 300.18 MB total with cloneLineInto at 91.16 MB / 30.37%.

go tool pprof -peek cloneLineInto pointed at vt.(*scrollbackRing).push, not an amux snapshot cache, so the fix targets the vt scrollback clone input instead of adding amux-side memoization.

Summary

• Add a resize-preservation benchmark that writes full-width styled scrollback rows before shrinking and widening the emulator.
• Pin github.com/charmbracelet/x/vt to weill-labs/x commit fc372e8574ca, which trims trailing styled blank cells before cloning rows into scrollback.
• Keep cloneLineInto's API unchanged; the fork-side change is tracked in LAB-1803: Trim styled blanks in vt scrollback x#15.
• LAB-1794 is already fixed on fork main by 6adbf8184605, so this PR pins a later fork commit rather than cherry-picking the soft-wrap API restoration here.

Baseline numbers

Hardware: AMD EPYC-Milan Processor, Linux amd64.

Measurement	Before	After
BenchmarkVTEmulatorResizePreservationStyledScrollback bytes/op	~14.00 MB/op	~10.16 MB/op
BenchmarkVTEmulatorResizePreservationStyledScrollback allocs/op	1518-1519	1506-1507
Styled workload heap in-use	112.13 MB	48.10 MB
Styled workload vt.cloneLineInto in-use	67.62 MB / 60.30%	5.00 MB / 10.41%

Testing

• go test ./internal/mux -run '^$' -bench BenchmarkVTEmulatorResizePreservationStyledScrollback -benchmem -count=5
• go test ./internal/mux -run 'TestRenderWithCursorRoundTripPreserves(SoftWrapForResize|WrappedTrailingSpaces|BlankWrappedSpacesAtCursor|BlankWrappedRowsBeforeHardNewline|PhantomCursorForResize)|TestVTEmulatorResizeShrink' -count=100
• go test -race ./internal/mux -timeout 120s -count=10
• go test ./... -timeout 180s
• go test ./test -run '^TestSwapForward$' -timeout 120s -count=1 after one go test ./... -timeout 120s run hit that integration timing flake.
• In weill-labs/x/vt: go test -run 'TestScrollback/trims_trailing_styled_blanks' -count=100
• In weill-labs/x/vt: go test -run TestScrollback -count=1
• In weill-labs/x/vt: go test ./...
• go test -race ./... -timeout 120s -count=10 was attempted and currently fails outside the touched package set in timing-heavy tests/packages (internal/cli, internal/client, internal/dialutil, internal/remote, internal/server, and test). internal/mux passed the same race/count loop.

Review focus

• Whether the fork-side scrollback trim semantics are the right layer versus amux memoization; profiles showed retained bytes in vt scrollback ring storage.
• Confirming LAB-1750 preserve-output behavior stays intact; the preserve-output regression tests re-pass with -count=100.
• Whether to merge LAB-1803: Trim styled blanks in vt scrollback x#15 first and then update this PR to the merged fork commit.

Closes LAB-1803.

[github-actions]

Claude finished @cweill's task in 1m 52s —— View job

---

PR Review: LAB-1803 Trim styled blank scrollback cells

• Gather context
• Review go.mod/go.sum dependency pin
• Review new benchmark in emulator_test.go
• Check regression coverage for LAB-1750 preserve-output behavior
• Assess overall correctness

---

Dependency pin (go.mod / go.sum)

The version bump from 6adbf8184605 → fc372e8574ca looks correct. The go.mod hash (u1LOIABor9JqY54oZdktK3TCRrgzP6tzHrDYx1nd3wY=) being identical between the two commits is expected when only .go files changed in the fork — the module go.mod itself was untouched.

Benchmark (internal/mux/emulator_test.go:177)

The benchmark structure is correct:

• Payload allocated once outside the loop (avoids inflating alloc counts with setup).
• b.ReportAllocs() + b.SetBytes() are called before the loop, which is fine since resizePreservationStyledScrollbackPayload is trivial — no need for b.ResetTimer().
• NewVTEmulatorWithDrainAndScrollback starts a drain goroutine that exits promptly on Close(), so there is no goroutine leak between iterations.
• io.ErrClosedPipe guard on the Close() result is defensive but matches the existing pattern in TestVTEmulatorCloseIgnoresClosedResponsePipe.

One minor observation: the 260-line payload fills a 24-row terminal, so 236 lines land in the 512-slot scrollback ring. The benchmark correctly exercises the fork's trim path: each row is 132 cells wide with a solid background applied to all cells including trailing blanks, which is exactly the shape that made cloneLineInto retain so much memory before the fix.

LAB-1750 regression coverage

The existing round-trip tests (TestRenderWithCursorRoundTripPreservesWrappedTrailingSpaces, PreservesBlankWrappedSpacesAtCursor, PreservesBlankWrappedRowsBeforeHardNewline, PreservesPhantomCursorForResize) cover the cases where trailing blank content carries semantic meaning for reflow. These test the screen buffer, not the scrollback ring, so they are unaffected by the scrollback trim.

The TestScrollbackCellAt test confirms styled non-blank cells survive the scrollback path with color intact, which remains correct after the fork change since only trailing blank cells are trimmed.

Semantic note (non-blocking)

Trimming trailing styled-blank cells from scrollback changes how background-colored rows (e.g., a full-width highlighted status bar) appear when scrolled back: cells beyond the last non-blank column revert to default background. This is the standard terminal behavior and matches what most terminal emulators do, but it is worth confirming the fork-side PR #15 documents this intent explicitly if it hasn't already.

---

LGTM

[github-actions]

Claude finished @cweill's task in 1m 48s —— View job

---

PR Review: LAB-1803 Trim styled blank scrollback cells

• Gather context
• Review go.mod/go.sum dependency pin
• Review new benchmark in emulator_test.go
• Check regression coverage for LAB-1750 preserve-output behavior
• Assess overall correctness

---

Dependency pin (go.mod / go.sum)

The version bump 6adbf8184605 → fc372e8574ca looks correct. The go.mod hash (u1LOIABor9JqY54oZdktK3TCRrgzP6tzHrDYx1nd3wY=) is identical between the two commits — expected when only .go files changed and the fork's go.mod itself was not touched. The h1 content hash changes as expected.

Benchmark (internal/mux/emulator_test.go:178)

Structure is sound:

• Payload allocated once outside the loop — setup cost doesn't skew alloc counts.
• b.ReportAllocs() + b.SetBytes() before the loop is correct; no b.ResetTimer() needed since no long setup precedes the loop under b context.
• Drain goroutine exits cleanly when Close() shuts the pipe — no goroutine leak between iterations.
• 260 lines into a 24-row terminal fills 236 scrollback slots in the 512-slot ring, and each row is full-width with a background color applied across all trailing blanks — exactly the shape that triggered the pre-fix retention.

Two minor observations (non-blocking):

1. buf.Grow(width * lines) underestimates capacity by ~15%. The actual bytes per line are ~156 (ANSI escape + label + spaces + reset + CRLF), but Grow is called with 132 × 260 = 34,320. At 156 bytes/line the total is ~40,568, so at least one reallocation happens during payload build. Since this runs once outside b.N, it doesn't affect the measurement — just a trivial extra alloc at setup.

2. !errors.Is(err, io.ErrClosedPipe) guard is redundant. vtEmulator.Close() (emulator.go:223-229) already converts io.ErrClosedPipe → nil before returning, so emu.Close() can never return that error. The guard is harmless but unnecessary.

LAB-1750 regression coverage

The existing round-trip tests (PreservesWrappedTrailingSpaces, PreservesBlankWrappedSpacesAtCursor, PreservesBlankWrappedRowsBeforeHardNewline, PreservesPhantomCursorForResize) exercise the screen buffer on resize — unaffected by the scrollback trim since trimming only applies to lines pushed into the scrollback ring. TestScrollbackCellAt confirms styled non-blank cells survive the scrollback path with color intact, which remains correct since only trailing blank cells are trimmed.

Semantic note (non-blocking)

Trimming trailing styled-blank cells from scrollback changes the visual appearance of background-colored full-width rows (e.g., highlighted status bars) when scrolled back: cells beyond the last non-blank column revert to default background. This is standard terminal behavior (matches xterm, kitty, etc.) and is the right trade-off given the memory savings, but it's worth confirming weill-labs/x PR #15 documents this intent explicitly.

---

LGTM

[greptile-apps]

Greptile Summary

Pins the charmbracelet/x/vt upstream fork to commit fc372e8574ca, which trims trailing styled blank cells before cloning rows into scrollback, reducing heap retention in cloneLineInto from ~60% to ~10% of total in-use bytes. A new benchmark (BenchmarkVTEmulatorResizePreservationStyledScrollback) is added to track the memory improvement across shrink–widen resize cycles on styled scrollback.

• go.mod / go.sum: replace directive for charmbracelet/x/vt bumped from 6adbf8184605 to fc372e8574ca; both the module and zip hashes are updated, and the /go.mod hash is unchanged, confirming only source files changed between the two commits.
• emulator_test.go: Adds a benchmark that writes 260 full-width SGR-styled rows into a 132×24 emulator with 512-line scrollback, then performs a 132→80→132 resize cycle per iteration, with proper b.ReportAllocs() and b.SetBytes() instrumentation and graceful io.ErrClosedPipe handling on close.

Confidence Score: 5/5

Safe to merge — the change is a targeted fork-pin bump with a new benchmark; no production logic is modified in this repo.

The diff is confined to a dependency version bump and an additive test file. The fork's go.mod hash is unchanged between the two commits, confirming only source files differ. The benchmark correctly instruments allocs and bytes, handles the drain-pipe close path, and the Grow hint is arithmetically accurate. Existing preserve-output regression tests re-pass at -count=100, and the heap profiles show the intended reduction.

No files require special attention.

Important Files Changed

Filename	Overview
go.mod	Fork pin updated from 6adbf8184605 to fc372e8574ca; replace directive is syntactically correct and consistent with go.sum.
go.sum	Old vt hashes removed, new zip and go.mod hashes added; /go.mod hash is unchanged between the two commits, as expected when only source files differ.
internal/mux/emulator_test.go	Adds benchmark and payload helper for styled-scrollback resize preservation; Grow hint, SetBytes, and ErrClosedPipe handling are all correct.

Sequence Diagram

sequenceDiagram
    participant B as Benchmark loop
    participant E as VTEmulator (132×24, 512 scrollback)
    participant R as scrollbackRing
    participant vt as weill-labs/x/vt fc372e8574ca

    B->>E: Write(payload: 260 styled rows)
    loop each row beyond viewport (236 rows)
        E->>vt: push row into scrollback
        vt->>vt: trim trailing styled blank cells
        vt->>R: cloneLineInto (trimmed row)
    end
    B->>E: Resize(80, 24)
    E->>R: reflow scrollback
    B->>E: Resize(132, 24)
    E->>R: reflow scrollback
    B->>E: Close()

Loading

_{Reviews (2): Last reviewed commit: "LAB-1803: Account for ANSI bytes in benc..." | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

[github-actions]

Claude finished @cweill's task in 3m 12s —— View job

---

PR Review: LAB-1803 Trim styled blank scrollback cells

• Read changed files
• Review dependency pin (go.mod / go.sum)
• Review benchmark structure
• Check Greptile buf.Grow suggestion
• Verify LAB-1750 regression coverage
• Assess overall correctness

---

Dependency pin (go.mod / go.sum)

The bump from 6adbf8184605 → fc372e8574ca is correct. The timestamp in the pseudo-version advances (20260513001905 → 20260513175146), confirming a later fork commit. The unchanged go.mod hash (u1LOIABor9JqY54oZdktK3TCRrgzP6tzHrDYx1nd3wY=) is expected when only .go files changed and the fork's go.mod itself was untouched. The h1 content hash changes as expected.

Benchmark (internal/mux/emulator_test.go:178)

Structure is sound:

• Payload allocated once outside the loop — setup cost does not skew alloc counts.
• b.ReportAllocs() + b.SetBytes() before the loop is correct.
• 260 lines into a 24-row terminal fills ~236 scrollback slots in the 512-slot ring; each row is full-width with background color applied to all trailing blanks — exactly the shape that triggered pre-fix retention.
• Drain goroutine exits cleanly when Close() shuts the pipe; no goroutine leak between iterations.

Greptile buf.Grow suggestion (non-issue — stale)

The Greptile inline suggestion was filed against commit 7f38d34, before commit e0852ab ("Account for ANSI bytes in benchmark payload") addressed the undercount. The current code at internal/mux/emulator_test.go:205 is already correct:

rowBytes := len(sgrOpen) + max(width, len("row-000 ")) + len(sgrReset) + len("\r\n")

len("\x1b[48;2;24;28;36m") = 16, max(132, 8) = 132, len("\x1b[0m") = 4, len("\r\n") = 2 → rowBytes = 154. Actual bytes/line: 16 (sgrOpen) + 8 (label) + 124 (spaces) + 4 (sgrReset) + 2 (CRLF) = 154. The Grow(154×260+8 = 40,048) is exact — no reallocation during payload construction.

errors.Is(err, io.ErrClosedPipe) guard

The guard at line 190 is redundant: vtEmulator.Close() (emulator.go:224-228) already converts io.ErrClosedPipe → nil before returning, so emu.Close() can never return that error. However it's harmless and the commit message ("Use lint-safe close check") explains why it's there — errcheck or a similar linter required the error to be inspected. The guard keeps the linter quiet without masking a real error.

LAB-1750 regression coverage

The scrollback trim applies only to lines pushed into the scrollback ring, not to the screen buffer. The preserve-output round-trip tests (PreservesWrappedTrailingSpaces, PreservesBlankWrappedSpacesAtCursor, PreservesBlankWrappedRowsBeforeHardNewline, PreservesPhantomCursorForResize) exercise screen-buffer reflow on resize and are unaffected. TestScrollbackCellAt confirms styled non-blank cells survive the scrollback path with color intact — correct, since only trailing blank cells are trimmed.

---

LGTM