Skip to content

fix(tui): align composer cursorLayout with wrap-ansi to kill multiline cursor drift#27489

Merged
OutThisLife merged 7 commits into
mainfrom
bb/tui-composer-cursor-drift-v2
May 17, 2026
Merged

fix(tui): align composer cursorLayout with wrap-ansi to kill multiline cursor drift#27489
OutThisLife merged 7 commits into
mainfrom
bb/tui-composer-cursor-drift-v2

Conversation

@OutThisLife

Copy link
Copy Markdown
Collaborator

What

Fixes the long-running "multi-cell blank gap between last typed character and cursor block" bug in hermes --tui composer. Reproduces reliably on narrow terminals (~50-60 cols), especially the Cursor IDE built-in terminal, but happens at any width once the composer wraps.

Closes the bug class that PRs #26717, #25860, #22197 all chased without resolving.

Root cause

The composer's cursorLayout (ui-tui/src/lib/inputMetrics.ts) used a hand-rolled word-wrap algorithm (visualLines) to decide where useDeclaredCursor should park the hardware cursor. But Ink's <Text wrap="wrap"> renders the same text via wrap-ansi. The two algorithms disagreed on many real inputs.

Concrete examples I verified by feeding both functions the same strings:

Input cols wrap-ansi renders old cursorLayout claimed
"branch investigate" 20 ["branch investigate"] end=(0,18) (1,0) phantom wrap
"hello world" 8 ["hello ", "world"] end=(1,5) (2,0) phantom row
"abcdefgh" 8 ["abcdefgh"] end=(0,8) (1,0) phantom wrap
"hello investigatex" 18 one line end=(0,18) (1,0)

useDeclaredCursor published cursorLayout's phantom position. Ink rendered the text at wrap-ansi's actual position. Ink then parked the hardware cursor at the published (rect.x + relativeX, rect.y + relativeY). The visible gap between the last rendered char and the cursor block is exactly that mismatch.

Why previous PRs missed it: they focused on fast-echo display-cursor synchronization (#26717), in-band vs native cursor rendering (#25860), and arrow-key navigation (#22197). All correct fixes for their respective bugs, none of them touched the wrap-algorithm mismatch underneath.

Fix

cursorLayout is now sourced from wrap-ansi

visualLinesFromWrappedOutput calls wrapAnsi(value, cols, { hard: true, trim: false }) (the exact same call Ink makes) and walks the emitted string char-by-char to map back to original-string offsets. Every soft-wrap \n wrap-ansi inserts becomes a VisualLine boundary; hard \ns in the input are consumed normally.

This guarantees the cursor position and the rendered text agree by construction — same input, same algorithm, same output.

Drop the column >= w overflow rule

The old code had:

if (column >= w) {
  lineIndex++
  column = 0
}

…which pushed exact-fill text onto a phantom next row. wrap-ansi does NOT do this (an 8-char string in an 8-col viewport stays on one line). Removing it eliminates the phantom-row drift. The hardware cursor naturally auto-wraps if the terminal's CUP target is past the row's right edge — there's no need to lie about the logical position.

canFastBackspaceShape checks both representations

After the rewrite, exact-fill reports as (line: 0, column: cols) instead of the previous (line: 1, column: 0). The physical state is identical (terminal auto-wraps either way), but the boolean for "are we at a wrap boundary" needs to check both:

if (layout.column === 0 || layout.column >= columns) {
  return false   // can't fast-backspace across a wrap
}

Tests

Updated ui-tui/src/__tests__/textInputWrap.test.ts

Three tests were pinning the buggy behavior (asserting cursorLayout returned the phantom-row position). They're updated to assert wrap-ansi parity — the actual invariant. Plus a new typing-prefix test that walks a long string char-by-char and asserts agreement with wrap-ansi at every prefix.

New ui-tui/src/__tests__/cursorDriftRegression.test.ts

4 tests pinning the fix directly:

  1. Walks the user-reported bug message char-by-char at 7 widths (40, 50, 55, 60, 65, 70, 80) and asserts agreement with wrap-ansi at every prefix.
  2. Pins exact-fill behavior (no phantom wrap) at widths 8, 12, 18, 24.
  3. Pins the specific "branch investigate" case from the user's screenshot.
  4. Pins word-wrap behavior ("hello world" at cols=8).

Verification

  • 791/791 vitest tests pass (including 4 new regression tests).
  • 84/84 tui-gateway pytest tests pass via scripts/run_tests.sh tests/tui_gateway/.
  • npm run type-check introduces zero new errors.
  • npm run build clean.
  • npm run lint introduces zero new errors.

Live PTY reproduction

I wrote a pyte + ptyprocess harness that spawns real hermes --tui in a PTY, types the user's bug-report message char-by-char at human-realistic speed (40ms/char), and captures the resulting screen + cursor position.

At cols=55 typing the message ending in "right":

  • Before fix: hardware cursor parked several cells past "right" with blank cells between.
  • After fix: cursor lands at exactly the cell immediately after the last t in "right", row 29.

Same result at cols=50 and cols=60.

Risk

Low. The hot path is a pure-function rewrite that delegates to a library Ink already depends on. The behavior change is in the direction of "match what's actually on the screen" — no caller can rely on the phantom-row reports (they were always visually wrong). The test suite update converts 3 wrong-behavior assertions into right-behavior assertions; everything else passes unchanged.

Branch: bb/tui-composer-cursor-drift-v2

…e cursor drift

The composer's `cursorLayout` (in `ui-tui/src/lib/inputMetrics.ts`) used a
hand-rolled word-wrap algorithm to decide where `useDeclaredCursor`
should park the hardware cursor. But Ink's `<Text wrap="wrap">` renders
the same text via `wrap-ansi`. The two algorithms disagreed on common
real-world inputs — `"branch investigate"` at cols=20, `"hello world"`
at cols=8, exact-fill strings like `"abcdefgh"` at cols=8 — so the
hardware cursor parked several cells past where Ink actually rendered
the last character. Users saw a multi-cell blank gap between their
last-typed letter and the cursor block, especially on narrow terminals
(the Cursor IDE built-in terminal was the worst offender).

Three previous PRs (#26717, #25860, #22197) chased fast-echo
displayCursor/cursorDeclaration drift and in-band-vs-native cursor
heuristics. None of them touched the underlying wrap-algorithm
mismatch, which is why the bug kept resurfacing.

Fix: source cursorLayout's line breaks from wrap-ansi directly. Walk
its emitted string char-by-char, tracking original-string offsets, push
a VisualLine at each '\n'. Also drop the buggy `column >= w` overflow
rule in cursorLayout — that's what pushed exact-fill text onto a
phantom next row.

canFastBackspaceShape now detects the wrap boundary in BOTH coordinate
conventions (column === 0 OR column >= columns), since exact-fill now
reports as (0, columns) instead of the previous (1, 0). The physical
state is identical — the terminal auto-wraps at column N either way —
but the layout function reports the position more honestly.

Tests:
- ui-tui/src/__tests__/textInputWrap.test.ts: 3 tests that pinned the
  BUGGY behavior were updated to assert wrap-ansi parity (the real
  invariant). Added a typing-prefix invariant: cursorLayout must agree
  with wrap-ansi at every character of a long input.
- ui-tui/src/__tests__/cursorDriftRegression.test.ts: new file. Walks
  the user-reported bug message char-by-char at 7 widths and asserts
  agreement with wrap-ansi at every prefix.

Verification:
- 791/791 vitest tests pass.
- 84/84 tui-gateway pytest tests pass via scripts/run_tests.sh.
- PTY repro (typing into a real `hermes --tui` PTY at cols=50/55/60):
  cursor lands exactly 1 cell past the last typed char in every case
  the bug previously drifted.
@github-actions

github-actions Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: bb/tui-composer-cursor-drift-v2 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8705 on HEAD, 8705 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4587 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a long-standing TUI composer cursor-drift bug by replacing the hand-rolled word-wrap algorithm used by cursorLayout with one that mirrors Ink's <Text wrap="wrap"> (which delegates to wrap-ansi). Since the cursor declaration and the rendered text now derive line breaks from the same algorithm, the hardware cursor stays aligned with the last rendered character. The column >= w "overflow to next line" rule is dropped, and canFastBackspaceShape is updated to recognize both representations of a wrap boundary.

Changes:

  • Rewrite visualLines/cursorLayout in ui-tui/src/lib/inputMetrics.ts to source visual line boundaries from wrap-ansi and remove the phantom-row overflow rule.
  • Update canFastBackspaceShape in ui-tui/src/components/textInput.tsx to also reject fast-backspace at exact-fill (column >= columns) in addition to column === 0.
  • Update textInputWrap.test.ts to assert wrap-ansi parity and add a typing-prefix test; add a new cursorDriftRegression.test.ts covering exact-fill, word-wrap, and the user-reported message at multiple widths.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
ui-tui/src/lib/inputMetrics.ts Replaces hand-rolled visualLines with a wrap-ansi-driven walker; drops the column>=w cursor overflow rule.
ui-tui/src/components/textInput.tsx canFastBackspaceShape rejects both column === 0 and column >= columns wrap boundaries.
ui-tui/src/tests/textInputWrap.test.ts Updates 3 assertions to pin wrap-ansi parity; adds typing-prefix parity test.
ui-tui/src/tests/cursorDriftRegression.test.ts New regression tests pinning agreement with wrap-ansi across widths and edge cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ui-tui/src/lib/inputMetrics.ts Outdated
Comment thread ui-tui/src/lib/inputMetrics.ts Outdated
Comment thread ui-tui/src/lib/inputMetrics.ts Outdated
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) javascript Pull requests that update javascript code labels May 17, 2026
Three small follow-ups from the Copilot review on #27489:

1. Declare `wrap-ansi` as a direct dependency of `ui-tui`. It was a
   phantom dep that resolved via npm hoisting from `@hermes/ink`'s
   transitive graph — fine on hoisted installs, but breaks under pnpm
   or `npm install --no-install-strategy=hoisted` style isolated
   installs. Now listed as `"wrap-ansi": "^9.0.0"` matching the
   @hermes/ink version. Lockfile regenerated.

2. Implement the defensive resync the comment promised. Previously the
   comment claimed the loop would "fall back to advancing by one to
   stay in lockstep" on wrap-ansi desync, but the code unconditionally
   advanced `originalIdx` with no actual check — so any future
   wrap-ansi option change or styled-input caller could silently slide
   `originalIdx` past the end of `value` and emit garbage line ranges.
   Now actually compares `value[originalIdx] === ch`, re-syncs via
   `indexOf` on mismatch, and bails out (returning whatever was built
   so far) if the desync is unrecoverable. Production paths still hit
   the equality fast-path on every char.

3. Drop the `visualLines` wrapper. It was a one-line indirection over
   `visualLinesFromWrappedOutput`. Renamed the implementation to
   `visualLines` and removed the wrapper — same name, no extra layer.

No behavior change beyond the defensive realign; all 791 vitest tests
still pass.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • ui-tui/package-lock.json: Language not supported

Comment thread ui-tui/src/lib/inputMetrics.ts Outdated
…n runtime parity)

Copilot caught an important runtime parity gap on PR #27489: the fix
imported the npm `wrap-ansi` package directly, but Ink's `<Text
wrap="wrap">` uses a runtime-selecting shim
(`ui-tui/packages/hermes-ink/src/ink/wrapAnsi.ts`) that prefers
`Bun.wrapAnsi` when running under Bun and falls back to the npm package
elsewhere. So under Bun, Ink would render via `Bun.wrapAnsi` while
`cursorLayout` would compute breaks via the npm package — any
disagreement reintroduces the exact cursor-drift symptom the PR is
meant to eliminate.

Fix:

- Export `wrapAnsi` from `@hermes/ink` (`packages/hermes-ink/src/entry-exports.ts`
  and `packages/hermes-ink/index.d.ts`) so the shim is the public surface.
- Switch `ui-tui/src/lib/inputMetrics.ts` from `import wrapAnsi from
  'wrap-ansi'` to `import { wrapAnsi } from '@hermes/ink'`. Both
  renderer (Ink) and cursor layout now traverse the same shim, so
  they share the runtime-selected implementation by construction.
- Same swap in `textInputWrap.test.ts` and `cursorDriftRegression.test.ts`
  — tests now assert parity through the shim, which means under Bun
  they actually exercise Bun's implementation instead of asserting a
  tautology against the npm package.
- Drop the direct `"wrap-ansi": "^9.0.0"` from `ui-tui/package.json`.
  `@hermes/ink` (which IS a declared dep) pulls wrap-ansi in
  transitively — that's not a phantom dep because the import path
  goes through `@hermes/ink`'s public exports, not through a
  hoisting accident.

Verified: 791/791 vitest tests pass. `@hermes/ink` rebuilt
(`dist/entry-exports.js` includes `wrapAnsi` export). TUI bundle
rebuilt clean.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • ui-tui/package-lock.json: Language not supported

Comment thread ui-tui/src/lib/inputMetrics.ts Outdated
Comment thread ui-tui/src/lib/inputMetrics.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 8 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • ui-tui/package-lock.json: Language not supported

@OutThisLife OutThisLife merged commit 08a66b2 into main May 17, 2026
12 of 13 checks passed
@OutThisLife OutThisLife deleted the bb/tui-composer-cursor-drift-v2 branch May 17, 2026 18:39
aashizpoudel pushed a commit to aashizpoudel/hermes-agent that referenced this pull request May 21, 2026
…n runtime parity)

Copilot caught an important runtime parity gap on PR NousResearch#27489: the fix
imported the npm `wrap-ansi` package directly, but Ink's `<Text
wrap="wrap">` uses a runtime-selecting shim
(`ui-tui/packages/hermes-ink/src/ink/wrapAnsi.ts`) that prefers
`Bun.wrapAnsi` when running under Bun and falls back to the npm package
elsewhere. So under Bun, Ink would render via `Bun.wrapAnsi` while
`cursorLayout` would compute breaks via the npm package — any
disagreement reintroduces the exact cursor-drift symptom the PR is
meant to eliminate.

Fix:

- Export `wrapAnsi` from `@hermes/ink` (`packages/hermes-ink/src/entry-exports.ts`
  and `packages/hermes-ink/index.d.ts`) so the shim is the public surface.
- Switch `ui-tui/src/lib/inputMetrics.ts` from `import wrapAnsi from
  'wrap-ansi'` to `import { wrapAnsi } from '@hermes/ink'`. Both
  renderer (Ink) and cursor layout now traverse the same shim, so
  they share the runtime-selected implementation by construction.
- Same swap in `textInputWrap.test.ts` and `cursorDriftRegression.test.ts`
  — tests now assert parity through the shim, which means under Bun
  they actually exercise Bun's implementation instead of asserting a
  tautology against the npm package.
- Drop the direct `"wrap-ansi": "^9.0.0"` from `ui-tui/package.json`.
  `@hermes/ink` (which IS a declared dep) pulls wrap-ansi in
  transitively — that's not a phantom dep because the import path
  goes through `@hermes/ink`'s public exports, not through a
  hoisting accident.

Verified: 791/791 vitest tests pass. `@hermes/ink` rebuilt
(`dist/entry-exports.js` includes `wrapAnsi` export). TUI bundle
rebuilt clean.

(cherry picked from commit 8c78f53)
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
Three small follow-ups from the Copilot review on NousResearch#27489:

1. Declare `wrap-ansi` as a direct dependency of `ui-tui`. It was a
   phantom dep that resolved via npm hoisting from `@hermes/ink`'s
   transitive graph — fine on hoisted installs, but breaks under pnpm
   or `npm install --no-install-strategy=hoisted` style isolated
   installs. Now listed as `"wrap-ansi": "^9.0.0"` matching the
   @hermes/ink version. Lockfile regenerated.

2. Implement the defensive resync the comment promised. Previously the
   comment claimed the loop would "fall back to advancing by one to
   stay in lockstep" on wrap-ansi desync, but the code unconditionally
   advanced `originalIdx` with no actual check — so any future
   wrap-ansi option change or styled-input caller could silently slide
   `originalIdx` past the end of `value` and emit garbage line ranges.
   Now actually compares `value[originalIdx] === ch`, re-syncs via
   `indexOf` on mismatch, and bails out (returning whatever was built
   so far) if the desync is unrecoverable. Production paths still hit
   the equality fast-path on every char.

3. Drop the `visualLines` wrapper. It was a one-line indirection over
   `visualLinesFromWrappedOutput`. Renamed the implementation to
   `visualLines` and removed the wrapper — same name, no extra layer.

No behavior change beyond the defensive realign; all 791 vitest tests
still pass.
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…n runtime parity)

Copilot caught an important runtime parity gap on PR NousResearch#27489: the fix
imported the npm `wrap-ansi` package directly, but Ink's `<Text
wrap="wrap">` uses a runtime-selecting shim
(`ui-tui/packages/hermes-ink/src/ink/wrapAnsi.ts`) that prefers
`Bun.wrapAnsi` when running under Bun and falls back to the npm package
elsewhere. So under Bun, Ink would render via `Bun.wrapAnsi` while
`cursorLayout` would compute breaks via the npm package — any
disagreement reintroduces the exact cursor-drift symptom the PR is
meant to eliminate.

Fix:

- Export `wrapAnsi` from `@hermes/ink` (`packages/hermes-ink/src/entry-exports.ts`
  and `packages/hermes-ink/index.d.ts`) so the shim is the public surface.
- Switch `ui-tui/src/lib/inputMetrics.ts` from `import wrapAnsi from
  'wrap-ansi'` to `import { wrapAnsi } from '@hermes/ink'`. Both
  renderer (Ink) and cursor layout now traverse the same shim, so
  they share the runtime-selected implementation by construction.
- Same swap in `textInputWrap.test.ts` and `cursorDriftRegression.test.ts`
  — tests now assert parity through the shim, which means under Bun
  they actually exercise Bun's implementation instead of asserting a
  tautology against the npm package.
- Drop the direct `"wrap-ansi": "^9.0.0"` from `ui-tui/package.json`.
  `@hermes/ink` (which IS a declared dep) pulls wrap-ansi in
  transitively — that's not a phantom dep because the import path
  goes through `@hermes/ink`'s public exports, not through a
  hoisting accident.

Verified: 791/791 vitest tests pass. `@hermes/ink` rebuilt
(`dist/entry-exports.js` includes `wrapAnsi` export). TUI bundle
rebuilt clean.
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…er-cursor-drift-v2

fix(tui): align composer cursorLayout with wrap-ansi to kill multiline cursor drift
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) javascript Pull requests that update javascript code P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants