Merge pull request #3765 from jeremymcs/fix/tui-content-cursor-desync

fix(pi-tui): use contentCursorRow for render movement baseline

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);

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<void> {}
+
+  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",
+    );
+  });
+});