Skip to content

fix(terminal): resolve absolute cursor moves against the TUI viewport - #654

Open
RonenMars wants to merge 1 commit into
mainfrom
fix/viewport-relative-cup
Open

fix(terminal): resolve absolute cursor moves against the TUI viewport#654
RonenMars wants to merge 1 commit into
mainfrom
fix/viewport-relative-cup

Conversation

@RonenMars

@RonenMars RonenMars commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Fixes the shipped transcript corruption described in #652.

The bug

VirtualTerminal resolved CSI H / CSI f against its whole append-only scrollback grid, but a TUI paints absolute cursor moves against a fixed-height screen, and the streamer spawns every PTY at 120x40 (tb-streamer src/pty-manager.ts:42-43, src/codex-pty-runner.ts:36-37).
Once the grid grew past 40 rows, CSI 40;1H addressed grid row 39 — deep inside scrollback — instead of the bottom of the screen, so every footer repaint landed mid-transcript and overwrote conversation content.
A spinner ticks continuously through a turn, so it repeated for the whole turn.

This is immediate on resume rather than gradual: terminal_replay carries up to 200 already-rendered rows which feedHistory feeds to the emulator before the first live frame arrives, so the grid is ~200 rows deep by the time the first raw CSI lands at absolute rows 0-39.

The change

Absolute row addressing is now relative to a viewport origin derived from grid.length, and clamped to the screen.
The origin is derived and never stored — a stored origin would have to be maintained correctly through every grid mutation, and each one becomes a way to desynchronise it.
The clamp is what keeps viewportTop() safe to read before ensureRow(): the target is at most grid.length - 1 whenever the grid is at least a screen tall, so a cursor move can never extend the grid and invalidate the origin it just read.

CSI A and CSI B are clamped to the viewport rather than the grid, for the same reason.
CSI A previously clamped at grid row 0, so a cursor-up from the footer region walked into scrollback and the next write overwrote transcript.
CSI B previously grew the grid from a cursor move, which shifts the derived origin mid-frame so a later absolute move in the same frame lands somewhere else.

CSI S is deliberately left alone, with a comment pointing at its follow-up.
It destroys the oldest scrollback rows rather than appending blanks at the viewport bottom, but that is not a cursor defect — the splice's implicit shift compensates exactly — and no capture yet shows Claude's TUI emitting SU at all.

Tests

Five new cases under VirtualTerminal – viewport-relative cursor positioning.
The fixture is built the way production builds it: 200 escape-free rows joined with \n, which is what feedHistory receives, then one live frame.

Each was verified against the specific code it protects rather than trusting a green run.
Two reproduce the original bug and fail on pre-fix code; two are guards that fail only when their clamp is removed; one is a no-throw contract test for the HTTP fallback path, which serves a byte-level tail slice of the PTY ring buffer and can begin mid-escape.
The comments mark which is which, so a guard is not later misread as a regression test.

Two assertions that look obvious do not work, and the tests avoid both.
Asserting that the footer "lands near the bottom" against getLines() fails after the fix, because lib/terminalChrome.ts:34 matches a correctly-placed footer exactly and filters it out, while the mangled hybrid the bug produces survives the ^ anchor.
Asserting that an out-of-range CSI 200;1H leaves getRawLines().length unchanged passes even with the clamp removed, because unclamped growth is blank rows and getRawLines() drops those — so that test now runs past the eviction cliff, where the grid crosses MAX_ROWS and the trim starts evicting real transcript.

Verification

npx jest --ci --runInBand --testPathPattern "useTerminalStream|TerminalView|terminal" — 11 suites, 157 tests, all passing.
npx tsc --noEmit --pretty false and npx eslint on both changed files are clean.

Verification item 4 from the plan — validating the VIEWPORT_ROWS = 40 constant itself against a real captured PTY log — is satisfied.
A capture of Claude Code v2.1.228, spawned by the streamer at its real 120x40 geometry, addresses exactly three absolute rows across every form of the sequence: row 1 (1483 occurrences), row 37 (1456) and row 40 (1456), for a maximum addressed row of exactly 40.
The constant is therefore measured against real TUI output rather than resting only on the two streamer constants and on tb-streamer/src/pty-host/protocol.ts:38-41.
A synthetic fixture was deliberately not substituted for this, because a log generated from the same 40-row assumption cannot test that assumption.

The same capture contains zero CSI S sequences, against a positive control returning 1 on a file known to contain one, which is the evidence for leaving scroll-up out of scope.
CSI A is likewise never emitted, so its clamp above is correct but defensive rather than reachable, unlike CSI B which the capture shows in heavy use.

Both figures are quoted here rather than only cited, because the capture lives at ~/.threadbase/captures/ outside any repository and would otherwise leave these as assertions if it were removed.
Two caveats travel with them, and they apply to different parts of the evidence.
The per-row frequencies are inflated, because the capture is assembled from overlapping full-ring-buffer dumps, so they should not be read as traffic.
The maximum addressed row and the distinct set of rows addressed are exact and are what the constant actually rests on: replaying the same bytes cannot introduce a row that was never addressed, nor raise a maximum.
Separately, this is a single turn with no tool use, and a tool-heavy capture is the only artifact that could change any of it.

`VirtualTerminal` resolved `CSI H`/`f` against the whole append-only scrollback grid, but a TUI paints absolute cursor moves against a fixed 40-row screen — the streamer spawns every PTY at 120x40.
Once the grid passed 40 rows every footer repaint landed mid-transcript and overwrote conversation content, and on resume it is immediate rather than gradual, because `terminal_replay` feeds up to 200 rendered rows before the first live frame arrives.
Absolute row addressing is now relative to a derived viewport origin and clamped to the screen, and `CSI A`/`B` are clamped to the viewport rather than the grid so that no cursor move can grow it.

Refs #652
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant