fix(runtime): place the declared caret on the right row in fullscreen (no trailing newline) (#198)

`buildCursorSuffix` computed `moveUp = visibleLineCount - clampedY`, assuming the
cursor rests on the blank row just past the content (row `visibleLineCount`).
That holds only when the frame ends with a newline. Fullscreen frames are written
WITHOUT a trailing newline (render.ts:962 `isFullscreen ? output : output + "\n"`,
and fullscreen is automatic whenever content fills the viewport), so the cursor
stays on the LAST visible row (`visibleLineCount - 1`). The suffix therefore moved
up one row too many: the declared caret landed a row too high, and the next
frame's `buildReturnToBottom` (which already measures from `previousLineCount - 1`)
then undershot the true bottom — erasing/rewriting the wrong rows and leaving stale
content. Reachable by any full-height TUI that declares a cursor (e.g. useCursor).

Found by differential fuzzing the incremental renderer (apply emitted bytes to a
terminal emulator seeded with the previous frame; result must equal a full repaint
of the next frame): 5,666 content mismatches in the no-trailing-newline + caret
regime, 0 once trailing newlines were forced — pinning the cause exactly.

Fix: thread `hasTrailingNewline` to `buildCursorSuffix` (and via `CursorOnlyInput`)
and move up from the real cursor row — `visibleLineCount - 1` when there's no
trailing newline. Defaults to true, so trailing-newline frames (the common
non-fullscreen path) are byte-for-byte unchanged. All log-update call sites pass
the frame's actual trailing-newline state.

TDD: cursor-helpers unit tests for the no-trailing-newline suffix math, plus
frame-writer regression tests that drive a fullscreen frame with a declared caret
through both the first-render and diff paths (red before the fix, green after).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Yunfei He
2026-06-15 04:28:17 +08:00
committed by GitHub
parent 6469b08c46
commit 8afc82fcd3
4 changed files with 151 additions and 11 deletions
@@ -120,4 +120,50 @@ describe("cursor-helpers", () => {
expect(result).not.toContain(hideCursorEscape);
expect(result).toContain(showCursorEscape);
});
// Fullscreen frames are written WITHOUT a trailing newline (render.ts:962
// `isFullscreen ? output : output + "\n"`), so the cursor rests on the LAST
// visible row (visibleLineCount - 1), not one row below the content like the
// trailing-newline case. buildCursorSuffix must move up from that real row, or
// it overshoots by one — misplacing a declared caret and then making the next
// frame's buildReturnToBottom undershoot the true bottom (stale-row corruption).
test("buildCursorSuffix - no trailing newline moves up from the last visible row", () => {
// hasTrailingNewline=false: cursor rests at row 2 (visibleLineCount-1), so
// moveUp = 2 - 1 = 1 (NOT 3 - 1 = 2 as the trailing-newline case computes).
const result = buildCursorSuffix(3, { x: 5, y: 1 }, undefined, false);
expect(result).toBe(ansiEscapes.cursorUp(1) + ansiEscapes.cursorTo(5) + showCursorEscape);
});
test("buildCursorSuffix - no trailing newline, single-line frame needs no cursorUp", () => {
// Single visible line, no trailing newline: cursor already on row 0, caret y=0,
// moveUp = 0 - 0 = 0 → no cursorUp (the trailing-newline case would emit up(1)).
const result = buildCursorSuffix(1, { x: 4, y: 0 }, undefined, false);
expect(result).toBe(ansiEscapes.cursorTo(4) + showCursorEscape);
});
test("buildCursorSuffix - trailing-newline default is unchanged (no regression)", () => {
// Omitting the flag (or passing true) keeps the original behavior exactly.
expect(buildCursorSuffix(3, { x: 5, y: 1 })).toBe(
buildCursorSuffix(3, { x: 5, y: 1 }, undefined, true),
);
expect(buildCursorSuffix(3, { x: 5, y: 1 })).toBe(
ansiEscapes.cursorUp(2) + ansiEscapes.cursorTo(5) + showCursorEscape,
);
});
test("buildCursorOnlySequence - no trailing newline threads the flag to the suffix", () => {
const result = buildCursorOnlySequence({
cursorWasShown: true,
previousLineCount: 3,
previousCursorPosition: { x: 0, y: 0 },
visibleLineCount: 3,
cursorPosition: { x: 3, y: 1 },
hasTrailingNewline: false,
});
const expected =
hideCursorEscape +
buildReturnToBottom(3, { x: 0, y: 0 }) +
buildCursorSuffix(3, { x: 3, y: 1 }, undefined, false);
expect(result).toBe(expected);
});
});
+27 -6
View File
@@ -21,14 +21,23 @@ export const cursorPositionChanged = (
/**
* Build escape sequence to move cursor from bottom of output to the target
* position and show it.
* Assumes cursor is at (col 0, line visibleLineCount) — i.e. just after the
* last output line.
*
* The starting row depends on the trailing newline. A frame written WITH a
* trailing newline leaves the cursor on the blank row just past the content
* (row `visibleLineCount`); a fullscreen frame is written WITHOUT a trailing
* newline (render.ts:962 `isFullscreen ? output : output + "\n"`), so the cursor
* stays on the LAST visible row (`visibleLineCount - 1`). `hasTrailingNewline`
* selects the correct basis — using `visibleLineCount` for a no-trailing-newline
* frame would move up one row too many, misplacing the declared caret and then
* desyncing the next frame's buildReturnToBottom (it would undershoot the true
* bottom, leaving stale rows). Defaults to true to preserve the trailing-newline
* callers byte-for-byte.
*
* The position is clamped to the visible region before emitting: under the
* persistent-declaration re-emit (the caret is re-asserted every commit until
* the declaration changes), a stale {x,y} left over from a larger frame —
* after a resize, overflow, or content shrink — must not produce an
* out-of-range move. `y` is clamped to `[0, visibleLineCount]` (so a y past the
* out-of-range move. `y` is clamped to `[0, cursorRow]` (so a y past the
* shrunk content lands on the last visible line, never below it) and `x` to
* `[0, width - 1]` when `width` is known (so a column past the terminal edge
* lands at the rightmost cell, not beyond it). This is D5 in the cursor design
@@ -39,18 +48,21 @@ export const buildCursorSuffix = (
visibleLineCount: number,
cursorPosition: CursorPosition | undefined,
width?: number,
hasTrailingNewline = true,
): string => {
if (!cursorPosition) {
return "";
}
const clampedY = Math.max(0, Math.min(cursorPosition.y, visibleLineCount));
// The row the cursor actually rests on after the frame is written (see above).
const cursorRow = hasTrailingNewline ? visibleLineCount : Math.max(0, visibleLineCount - 1);
const clampedY = Math.max(0, Math.min(cursorPosition.y, cursorRow));
const clampedX =
width !== undefined && width > 0
? Math.max(0, Math.min(cursorPosition.x, width - 1))
: Math.max(0, cursorPosition.x);
const moveUp = visibleLineCount - clampedY;
const moveUp = cursorRow - clampedY;
return (
(moveUp > 0 ? ansiEscapes.cursorUp(moveUp) : "") +
ansiEscapes.cursorTo(clampedX) +
@@ -92,6 +104,10 @@ export type CursorOnlyInput = {
visibleLineCount: number;
cursorPosition: CursorPosition | undefined;
width?: number;
// Whether the (unchanged) output ends with a newline; threaded to
// buildCursorSuffix so a fullscreen frame's caret lands on the right row.
// Defaults to true (trailing-newline) when omitted.
hasTrailingNewline?: boolean;
};
/**
@@ -102,7 +118,12 @@ export type CursorOnlyInput = {
export const buildCursorOnlySequence = (input: CursorOnlyInput): string => {
const hidePrefix = input.cursorWasShown ? hideCursorEscape : "";
const returnToBottom = buildReturnToBottom(input.previousLineCount, input.previousCursorPosition);
const cursorSuffix = buildCursorSuffix(input.visibleLineCount, input.cursorPosition, input.width);
const cursorSuffix = buildCursorSuffix(
input.visibleLineCount,
input.cursorPosition,
input.width,
input.hasTrailingNewline ?? true,
);
return hidePrefix + returnToBottom + cursorSuffix;
};
@@ -709,6 +709,51 @@ describe("incremental rendering - no trailing newline (fullscreen)", () => {
const lastCursorNextLine = thirdCall.lastIndexOf(ansiEscapes.cursorNextLine);
expect(lastCursorNextLine).toBe(-1);
});
// Regression: a fullscreen frame has NO trailing newline, so the cursor rests
// on the last visible row, not one below it. The declared-caret suffix must
// move up from that real row. Before the fix it used the trailing-newline
// basis and moved up one row too many — placing the caret a row too high and
// then making the next frame's return-to-bottom undershoot (stale-row
// corruption). Found by differential fuzzing the incremental renderer.
test("declared caret lands on the correct row on a fullscreen first render", () => {
const stdout = createStdout();
const render = logUpdate.create(stdout, {
showCursor: true,
incremental: true,
});
// 3 visible rows, no trailing newline; caret on row 1. Cursor rests on row 2
// (last visible), so moveUp must be 2 - 1 = 1 (NOT 3 - 1 = 2).
render.setCursorPosition({ x: 5, y: 1 });
render("L1\nL2\nL3");
const written = stdout.write.firstCall.args[0] as string;
expect(
written.endsWith(ansiEscapes.cursorUp(1) + ansiEscapes.cursorTo(5) + showCursorEscape),
).toBe(true);
});
test("declared caret lands on the correct row on a fullscreen diff update", () => {
const stdout = createStdout();
const render = logUpdate.create(stdout, {
showCursor: true,
incremental: true,
});
render.setCursorPosition({ x: 5, y: 1 });
render("L1\nL2\nL3");
// Diff path (a line changed). 3 visible rows, no trailing newline, caret on
// row 0 → moveUp must be 2 - 0 = 2 (the real last row, not 3).
render.setCursorPosition({ x: 2, y: 0 });
render("X1\nL2\nL3");
const second = stdout.write.secondCall.args[0] as string;
expect(
second.endsWith(ansiEscapes.cursorUp(2) + ansiEscapes.cursorTo(2) + showCursorEscape),
).toBe(true);
});
});
// ---------------------------------------------------------------------------
+33 -5
View File
@@ -120,7 +120,13 @@ const createStandard = (
const lines = str.split("\n");
const visibleCount = visibleLineCount(lines, str);
const cursorSuffix = buildCursorSuffix(visibleCount, activeCursor, streamWidth(stream));
const hasTrailingNewline = str.endsWith("\n");
const cursorSuffix = buildCursorSuffix(
visibleCount,
activeCursor,
streamWidth(stream),
hasTrailingNewline,
);
if (str === previousOutput && cursorChanged) {
stream.write(
@@ -131,6 +137,7 @@ const createStandard = (
visibleLineCount: visibleCount,
cursorPosition: activeCursor,
width: streamWidth(stream),
hasTrailingNewline,
}),
);
} else {
@@ -212,7 +219,12 @@ const createStandard = (
if (activeCursor) {
stream.write(
buildCursorSuffix(visibleLineCount(lines, str), activeCursor, streamWidth(stream)),
buildCursorSuffix(
visibleLineCount(lines, str),
activeCursor,
streamWidth(stream),
str.endsWith("\n"),
),
);
}
@@ -280,6 +292,7 @@ const createIncremental = (
visibleLineCount: visibleCount,
cursorPosition: activeCursor,
width: streamWidth(stream),
hasTrailingNewline: str.endsWith("\n"),
}),
);
previousCursorPosition = activeCursor ? { ...activeCursor } : undefined;
@@ -294,7 +307,12 @@ const createIncremental = (
);
if (str === "\n" || previousOutput.length === 0) {
const cursorSuffix = buildCursorSuffix(visibleCount, activeCursor, streamWidth(stream));
const cursorSuffix = buildCursorSuffix(
visibleCount,
activeCursor,
streamWidth(stream),
str.endsWith("\n"),
);
stream.write(
returnPrefix + ansiEscapes.eraseLines(previousLines.length) + str + cursorSuffix,
);
@@ -349,7 +367,12 @@ const createIncremental = (
);
}
const cursorSuffix = buildCursorSuffix(visibleCount, activeCursor, streamWidth(stream));
const cursorSuffix = buildCursorSuffix(
visibleCount,
activeCursor,
streamWidth(stream),
hasTrailingNewline,
);
buffer.push(cursorSuffix);
stream.write(buffer.join(""));
@@ -424,7 +447,12 @@ const createIncremental = (
if (activeCursor) {
stream.write(
buildCursorSuffix(visibleLineCount(lines, str), activeCursor, streamWidth(stream)),
buildCursorSuffix(
visibleLineCount(lines, str),
activeCursor,
streamWidth(stream),
str.endsWith("\n"),
),
);
}