From 561d73d3aa10f024fddfd3b354c847132ac42b06 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 7 Apr 2026 22:34:17 -0500 Subject: [PATCH 1/2] fix(pi-tui): use contentCursorRow for render movement baseline instead of cursorRow PR #3744 fixed autocomplete ghost lines by introducing a local contentCursorRow initialized from this.cursorRow, but this.cursorRow tracks the content end (last line), not where the cursor actually ended up after rendering. This caused computeLineDiff to compute wrong movement deltas, making content clear and jump on every keystroke. Fix: add an instance field contentCursorRow that stores finalCursorRow after content rendering but before positionHardwareCursor moves the cursor for IME. This correctly separates three cursor concepts: - cursorRow: logical content end (viewport calculation) - contentCursorRow: post-render cursor position (movement baseline) - hardwareCursorRow: actual terminal cursor (may differ due to IME) Closes #3764 --- packages/pi-tui/src/tui.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/pi-tui/src/tui.ts b/packages/pi-tui/src/tui.ts index 3408016a0..b0240aad3 100644 --- a/packages/pi-tui/src/tui.ts +++ b/packages/pi-tui/src/tui.ts @@ -239,6 +239,7 @@ export class TUI extends Container { public onDebug?: () => void; private renderRequested = false; private cursorRow = 0; // Logical cursor row (end of rendered content) + private contentCursorRow = 0; // Cursor row after content rendering, before IME repositioning private hardwareCursorRow = 0; // Actual terminal cursor row (may differ due to IME positioning) private inputBuffer = ""; // Buffer for parsing terminal responses private cellSizeQueryPending = false; @@ -498,6 +499,7 @@ export class TUI extends Container { this.previousWidth = -1; // -1 triggers widthChanged, forcing a full clear this.previousHeight = -1; // -1 triggers heightChanged, forcing a full clear this.cursorRow = 0; + this.contentCursorRow = 0; this.hardwareCursorRow = 0; this.maxLinesRendered = 0; this.previousViewportTop = 0; @@ -616,10 +618,9 @@ export class TUI extends Container { const height = this.terminal.rows; let viewportTop = Math.max(0, this.maxLinesRendered - height); let prevViewportTop = this.previousViewportTop; - let contentCursorRow = this.cursorRow; - let hardwareCursorRow = this.hardwareCursorRow; + let hardwareCursorRow = this.contentCursorRow; const computeLineDiff = (targetRow: number): number => { - const currentScreenRow = contentCursorRow - prevViewportTop; + const currentScreenRow = hardwareCursorRow - prevViewportTop; const targetScreenRow = targetRow - viewportTop; return targetScreenRow - currentScreenRow; }; @@ -664,6 +665,7 @@ export class TUI extends Container { buffer += "\x1b[?2026l"; // End synchronized output this.terminal.write(buffer); this.cursorRow = Math.max(0, newLines.length - 1); + this.contentCursorRow = this.cursorRow; this.hardwareCursorRow = this.cursorRow; // Reset max lines when clearing, otherwise track growth if (clear) { @@ -771,6 +773,7 @@ export class TUI extends Container { buffer += "\x1b[?2026l"; this.terminal.write(buffer); this.cursorRow = targetRow; + this.contentCursorRow = targetRow; this.hardwareCursorRow = targetRow; } this.positionHardwareCursor(cursorPos, newLines.length); @@ -806,7 +809,6 @@ export class TUI extends Container { buffer += "\r\n".repeat(scroll); prevViewportTop += scroll; viewportTop += scroll; - contentCursorRow = moveTargetRow; hardwareCursorRow = moveTargetRow; } @@ -889,8 +891,10 @@ export class TUI extends Container { // Track cursor position for next render // cursorRow tracks end of content (for viewport calculation) - // hardwareCursorRow tracks actual terminal cursor position (for movement) + // contentCursorRow tracks cursor after content rendering (before IME repositioning) + // hardwareCursorRow tracks actual terminal cursor position (may differ due to IME) this.cursorRow = Math.max(0, newLines.length - 1); + this.contentCursorRow = finalCursorRow; this.hardwareCursorRow = finalCursorRow; // Track terminal's working area (grows but doesn't shrink unless cleared) this.maxLinesRendered = Math.max(this.maxLinesRendered, newLines.length); From a28a56b3e4ec4088463b1e419867547b3bf3a48d Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 7 Apr 2026 22:39:00 -0500 Subject: [PATCH 2/2] test(pi-tui): add regression tests for contentCursorRow tracking Verify that contentCursorRow is correctly maintained across renders and that IME repositioning does not cause spurious cursor jumps during normal typing or content shrinking. Refs #3764 --- src/tests/tui-content-cursor-desync.test.ts | 151 ++++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 src/tests/tui-content-cursor-desync.test.ts diff --git a/src/tests/tui-content-cursor-desync.test.ts b/src/tests/tui-content-cursor-desync.test.ts new file mode 100644 index 000000000..1fa31148e --- /dev/null +++ b/src/tests/tui-content-cursor-desync.test.ts @@ -0,0 +1,151 @@ +/** + * Regression test for #3764: TUI input clears and jumps up after PR #3744. + * + * PR #3744 used this.cursorRow (content end) as the movement baseline in + * computeLineDiff, but it should be the post-render cursor position + * (finalCursorRow). This test verifies that after IME cursor repositioning, + * the next render computes correct movement deltas — no spurious jumps. + */ + +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { CURSOR_MARKER, TUI, type Component, type Terminal } from "@gsd/pi-tui"; + +class MockTTYTerminal implements Terminal { + public writtenData: string[] = []; + + readonly isTTY = true; + + start(_onInput: (data: string) => void, _onResize: () => void): void {} + stop(): void {} + async drainInput(_maxMs?: number, _idleMs?: number): Promise {} + + write(data: string): void { + this.writtenData.push(data); + } + + get columns(): number { + return 80; + } + + get rows(): number { + return 24; + } + + get kittyProtocolActive(): boolean { + return false; + } + + moveBy(_lines: number): void {} + hideCursor(): void {} + showCursor(): void {} + clearLine(): void {} + clearFromCursor(): void {} + clearScreen(): void {} + setTitle(_title: string): void {} +} + +class DynamicLinesComponent implements Component { + public lines: string[]; + + constructor(lines: string[]) { + this.lines = lines; + } + + render(_width: number): string[] { + return this.lines; + } + + invalidate(): void {} +} + +describe("TUI contentCursorRow tracking (#3764)", () => { + it("does not produce spurious cursor jumps when content changes after IME positioning", () => { + const terminal = new MockTTYTerminal(); + const tui = new TUI(terminal, false); + const component = new DynamicLinesComponent([ + "header", + `input: hello${CURSOR_MARKER}`, + "status line", + ]); + + tui.addChild(component); + (tui as any).doRender(); + + // After first render, hardwareCursorRow is at IME position (row 1), + // but contentCursorRow should be at finalCursorRow (row 2, end of content). + // Verify contentCursorRow is set correctly. + assert.strictEqual( + (tui as any).contentCursorRow, + 2, + "contentCursorRow should be at content end (row 2) after first render", + ); + assert.strictEqual( + (tui as any).hardwareCursorRow, + 1, + "hardwareCursorRow should be at IME cursor position (row 1) after positionHardwareCursor", + ); + + // Simulate typing — content changes on the same line + terminal.writtenData = []; + component.lines = [ + "header", + `input: hello world${CURSOR_MARKER}`, + "status line", + ]; + + (tui as any).doRender(); + + // The differential render should update line 1 (the changed input line). + // With the bug from PR #3744, computeLineDiff would use this.cursorRow (2) + // instead of contentCursorRow (2), which happened to be the same — but the + // critical test is that the buffer does NOT contain large cursor jumps. + assert.ok(terminal.writtenData.length >= 1, "typing should trigger a render"); + + const buffer = terminal.writtenData[0]; + // Should not contain \x1b[2A or \x1b[3A etc. (large upward jumps) + const largeUpJump = buffer.match(/\x1b\[([3-9]|\d{2,})A/); + assert.strictEqual( + largeUpJump, + null, + `should not produce large upward cursor jumps, got: ${JSON.stringify(buffer)}`, + ); + }); + + it("contentCursorRow persists correctly across renders with shrinking content", () => { + const terminal = new MockTTYTerminal(); + const tui = new TUI(terminal, false); + const component = new DynamicLinesComponent([ + "line 1", + `line 2${CURSOR_MARKER}`, + "line 3", + "line 4", + "line 5", + ]); + + tui.addChild(component); + (tui as any).doRender(); + + assert.strictEqual( + (tui as any).contentCursorRow, + 4, + "contentCursorRow should be 4 after rendering 5 lines", + ); + + // Shrink content + terminal.writtenData = []; + component.lines = [ + "line 1", + `line 2${CURSOR_MARKER}`, + "line 3", + ]; + + (tui as any).doRender(); + + assert.strictEqual( + (tui as any).contentCursorRow, + 2, + "contentCursorRow should update to 2 after shrinking to 3 lines", + ); + }); +});