Merge pull request #27489 from NousResearch/bb/tui-composer-cursor-drift-v2

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

ui-tui/packages/hermes-ink/index.d.ts

@@ -34,5 +34,6 @@ export { default as measureElement } from './src/ink/measure-element.ts'
 export { createRoot, forceRedraw, default as render, renderSync } from './src/ink/root.ts'
 export type { Instance, RenderOptions, Root } from './src/ink/root.ts'
 export { stringWidth } from './src/ink/stringWidth.ts'
+export { wrapAnsi } from './src/ink/wrapAnsi.ts'
 export { default as TextInput, UncontrolledTextInput } from 'ink-text-input'
 export type { Props as TextInputProps } from 'ink-text-input'

ui-tui/packages/hermes-ink/src/entry-exports.ts

@@ -26,5 +26,6 @@ export { default as measureElement } from './ink/measure-element.js'
 export { scrollFastPathStats, type ScrollFastPathStats } from './ink/render-node-to-output.js'
 export { createRoot, forceRedraw, default as render, renderSync } from './ink/root.js'
 export { stringWidth } from './ink/stringWidth.js'
+export { wrapAnsi } from './ink/wrapAnsi.js'
 export { isXtermJs } from './ink/terminal.js'
 export { default as TextInput, UncontrolledTextInput } from 'ink-text-input'

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

@@ -0,0 +1,114 @@
+/**
+ * Pinned regression for the multi-line composer cursor-drift bug.
+ *
+ * Symptom: in `hermes --tui`, typing into the composer until the input
+ * wraps across multiple visual rows would leave several blank cells
+ * between the last typed character and the (hardware) cursor block.
+ * Worse on narrow terminals (the Cursor IDE built-in terminal in
+ * particular).
+ *
+ * Root cause: the composer's `cursorLayout` (used by `useDeclaredCursor`
+ * to place the hardware cursor) ran a hand-rolled word-wrap algorithm,
+ * while Ink's `<Text wrap="wrap">` renders via `wrap-ansi`. The two
+ * disagreed on many real inputs — wrap-ansi would keep "branch
+ * investigate" on one row while cursorLayout claimed it had wrapped,
+ * etc. — so the declared cursor position drifted from where the text
+ * was actually rendered. The fix sources cursorLayout's line breaks
+ * directly from wrap-ansi, guaranteeing agreement.
+ *
+ * This test pins the contract: for every char that would be typed into
+ * the composer, the cursor position reported by cursorLayout MUST equal
+ * the end-of-text position that wrap-ansi would render. Any future
+ * regression that lets the two diverge re-introduces the drift.
+ */
+import { wrapAnsi } from '@hermes/ink'
+import { describe, expect, it } from 'vitest'
+
+import { cursorLayout, inputVisualHeight } from '../lib/inputMetrics.js'
+
+function wrapAnsiEnd(text: string, cols: number): { line: number; column: number } {
+  const wrapped = wrapAnsi(text, cols, { hard: true, trim: false })
+  const lines = wrapped.split('\n')
+  const last = lines[lines.length - 1] ?? ''
+
+  return { line: lines.length - 1, column: last.length }
+}
+
+const USER_REPORT_MESSAGE =
+  // Paraphrase of the user's actual bug report, included verbatim so the
+  // test is grounded in a realistic typing pattern (long single line,
+  // mixed-length words, punctuation, no hard newlines).
+  'im in cursor terminal using hermes --tui and as i type multiline my caret at the end will often ' +
+  'go.. randomly.. like multiple spaces away lol and idk why. theres no rhyme/reason really but ' +
+  'there should literally never be a non-user added space at the end of my composer input right? ' +
+  'i dont think it happens on new sessions but only existing ones. there have been a few prs to ' +
+  'try to fix this and all not working. ok it just happened, to me, nowso attaching screenshot ' +
+  'and you can see its multiline, new session. on a new bb/<xxx> branch investigate'
+
+describe('cursor-drift regression — composer cursorLayout matches Ink rendering', () => {
+  it('agrees with wrap-ansi at every typing-prefix of the user-reported message', () => {
+    // Walks the message char-by-char (mirroring what the TUI sees when a
+    // user types). At every prefix, cursorLayout must place the cursor
+    // exactly where wrap-ansi would render the end of the text.
+    //
+    // Pre-fix: this failed on most narrow widths because the hand-rolled
+    // wrap algorithm broke at slightly different points than wrap-ansi.
+    for (const cols of [40, 50, 55, 60, 65, 70, 80]) {
+      let acc = ''
+
+      for (const ch of USER_REPORT_MESSAGE) {
+        acc += ch
+        const layout = cursorLayout(acc, acc.length, cols)
+        const expected = wrapAnsiEnd(acc, cols)
+
+        expect(
+          layout,
+          `mismatch at cols=${cols}, len=${acc.length}, last-char=${JSON.stringify(ch)}, ` +
+            `tail=${JSON.stringify(acc.slice(-30))}`
+        ).toEqual(expected)
+      }
+    }
+  })
+
+  it('keeps cursor on the same row when text exactly fills the terminal width', () => {
+    // wrap-ansi does NOT push exact-fill text onto a phantom next line.
+    // The previous algorithm did — that's what produced the visible
+    // "cursor parked one row below the last char" symptom on narrow
+    // terminals at certain message lengths.
+    for (const cols of [8, 12, 18, 24]) {
+      const text = 'a'.repeat(cols)
+      const layout = cursorLayout(text, text.length, cols)
+      const inkLines = wrapAnsi(text, cols, { hard: true, trim: false }).split('\n')
+
+      expect(layout.line).toBe(0)
+      expect(layout.column).toBe(cols)
+      expect(inkLines).toHaveLength(1)
+      expect(inputVisualHeight(text, cols)).toBe(1)
+    }
+  })
+
+  it('does not stuff a trailing whitespace word onto a phantom line', () => {
+    // "branch investigate" at cols=20 fits on one row in wrap-ansi. The
+    // bug claimed otherwise, parking the cursor at (line=1, col=?) and
+    // leaving the user's "branch investigate" rendered alone on row 0
+    // with the cursor block several cells past it.
+    const text = 'branch investigate'
+    const cols = 20
+
+    expect(cursorLayout(text, text.length, cols)).toEqual({ column: text.length, line: 0 })
+    expect(cursorLayout(text, text.length, cols)).toEqual(wrapAnsiEnd(text, cols))
+  })
+
+  it('agrees with wrap-ansi for word-wrap that pushes a word onto the next line', () => {
+    // "hello world" at cols=8 wraps to ["hello ", "world"] in wrap-ansi.
+    // The cursor at end-of-text must land at line=1, col=5 — where Ink
+    // actually renders the last 'd'. The previous algorithm reported
+    // (line=2, col=0) here (phantom extra wrap), which parked the
+    // cursor on a row Ink never painted.
+    const text = 'hello world'
+    const cols = 8
+
+    expect(cursorLayout(text, text.length, cols)).toEqual({ column: 5, line: 1 })
+    expect(cursorLayout(text, text.length, cols)).toEqual(wrapAnsiEnd(text, cols))
+  })
+})

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

@@ -1,8 +1,20 @@
+import { wrapAnsi } from '@hermes/ink'
 import { describe, expect, it } from 'vitest'
 
 import { offsetFromPosition } from '../components/textInput.js'
 import { composerPromptWidth, cursorLayout, inputVisualHeight, stableComposerColumns } from '../lib/inputMetrics.js'
 
+// Helper: compute the "end of text" position that wrap-ansi would render
+// the input to. This is what Ink's <Text wrap="wrap"> uses, so cursorLayout
+// MUST agree. Disagreement is the cursor-drift bug.
+function wrapAnsiEndPosition(text: string, cols: number): { line: number; column: number } {
+  const wrapped = wrapAnsi(text, cols, { hard: true, trim: false })
+  const lines = wrapped.split('\n')
+  const last = lines[lines.length - 1] ?? ''
+
+  return { line: lines.length - 1, column: last.length }
+}
+
 describe('cursorLayout — word-wrap parity with wrap-ansi', () => {
   it('places cursor mid-line at its column', () => {
     expect(cursorLayout('hello world', 6, 40)).toEqual({ column: 6, line: 0 })
@@ -12,19 +24,36 @@ describe('cursorLayout — word-wrap parity with wrap-ansi', () => {
     expect(cursorLayout('hi', 2, 10)).toEqual({ column: 2, line: 0 })
   })
 
-  it('wraps to next line when cursor lands exactly at the right edge', () => {
-    // 8 chars on an 8-col line: text fills the row exactly; the cursor's
-    // inverted-space cell overflows to col 0 of the next row.
-    expect(cursorLayout('abcdefgh', 8, 8)).toEqual({ column: 0, line: 1 })
+  it('does not push exact-fill text onto a phantom next line', () => {
+    // Regression: the previous hand-rolled wrap algorithm forced the cursor
+    // onto (line+1, 0) when the text exactly filled the row. wrap-ansi keeps
+    // it on the same row (no soft-wrap), so the cursor must too — otherwise
+    // useDeclaredCursor parks the hardware cursor below the last char and
+    // the user sees several blank cells between text and cursor block
+    // (#cursor-drift-multiline).
+    expect(cursorLayout('abcdefgh', 8, 8)).toEqual({ column: 8, line: 0 })
+    expect(cursorLayout('abcdefgh', 8, 8)).toEqual(wrapAnsiEndPosition('abcdefgh', 8))
+  })
+
+  it('keeps short words on the current line when they fit (no phantom wrap)', () => {
+    // wrap-ansi: "hello wo" at cols=8 stays as one line "hello wo".
+    // The old cursorLayout incorrectly pushed to (1,0) because column=8 hit
+    // the column>=width check, but that disagreed with what Ink actually
+    // rendered.
+    expect(cursorLayout('hello wo', 8, 8)).toEqual({ column: 8, line: 0 })
+    expect(cursorLayout('hello wo', 8, 8)).toEqual(wrapAnsiEndPosition('hello wo', 8))
   })
 
   it('moves words across wrap boundaries instead of splitting them', () => {
-    // With wordWrap:true, "hello wor" at cols=8 is "hello \nwor" rather
-    // than "hello wo\nr".
-    expect(cursorLayout('hello wo', 8, 8)).toEqual({ column: 0, line: 1 })
+    // "hello wor" at cols=8: wrap-ansi breaks at the space, "hello \nwor".
     expect(cursorLayout('hello wor', 9, 8)).toEqual({ column: 3, line: 1 })
     expect(cursorLayout('hello worl', 10, 8)).toEqual({ column: 4, line: 1 })
     expect(cursorLayout('hello world', 11, 8)).toEqual({ column: 5, line: 1 })
+
+    // Each must match what wrap-ansi would actually render.
+    expect(cursorLayout('hello wor', 9, 8)).toEqual(wrapAnsiEndPosition('hello wor', 8))
+    expect(cursorLayout('hello worl', 10, 8)).toEqual(wrapAnsiEndPosition('hello worl', 8))
+    expect(cursorLayout('hello world', 11, 8)).toEqual(wrapAnsiEndPosition('hello world', 8))
   })
 
   it('wraps the next word instead of splitting it at the right edge', () => {
@@ -42,12 +71,33 @@ describe('cursorLayout — word-wrap parity with wrap-ansi', () => {
   it('does not wrap when cursor is before the right edge', () => {
     expect(cursorLayout('abcdefg', 7, 8)).toEqual({ column: 7, line: 0 })
   })
+
+  it('matches wrap-ansi end-position for typing-style incremental input', () => {
+    // Pins the actual fix: type a long message char-by-char at a narrow
+    // width and assert the cursor follows wrap-ansi every step of the way.
+    // Before the fix, ~5 boundary positions per pass disagreed and Ink
+    // parked the cursor several cells past the last rendered character.
+    const MSG = 'on a new bb branch investigate and fix the cursor drift bug here'
+
+    for (const cols of [10, 14, 20, 30, 50, 80]) {
+      let acc = ''
+
+      for (const ch of MSG) {
+        acc += ch
+        expect(cursorLayout(acc, acc.length, cols)).toEqual(wrapAnsiEndPosition(acc, cols))
+      }
+    }
+  })
 })
 
 describe('input metrics helpers', () => {
-  it('computes visual height from the wrapped cursor line', () => {
-    expect(inputVisualHeight('abcdefgh', 8)).toBe(2)
+  it('computes visual height matching wrap-ansi line count', () => {
+    // Exact-fill text stays on one line in wrap-ansi (no phantom wrap), so
+    // visual height is 1. The previous implementation reported 2 here.
+    expect(inputVisualHeight('abcdefgh', 8)).toBe(1)
     expect(inputVisualHeight('one\ntwo', 40)).toBe(2)
+    // Multi-line wrap case sanity
+    expect(inputVisualHeight('hello world', 8)).toBe(2)
   })
 
   it('counts the prompt gap as its own cell', () => {

ui-tui/src/components/textInput.tsx

@@ -272,10 +272,22 @@ export function canFastBackspaceShape(current: string, cursor: number, columns?:
   }
 
   // If we know the wrap width, reject at the soft-wrap boundary: the
-  // caret's visual column is 0, so "\b \b" can't represent the physical
-  // move back to the previous visual line.
-  if (columns !== undefined && cursorLayout(current, cursor, columns).column === 0) {
-    return false
+  // caret's physical column would be at (or past) the terminal's right
+  // edge, so the terminal has already auto-wrapped to the next row.
+  // "\b \b" can't represent the physical move back across that wrap.
+  //
+  // We check `column === 0` for the "wrap-ansi broke onto a new line"
+  // case AND `column >= columns` for the "exact-fill, terminal auto-wraps"
+  // case. Both manifest as the same physical state (cursor parked at
+  // col 0 of the next row) but cursorLayout reports them differently
+  // because it now mirrors wrap-ansi's break points exactly (see the
+  // cursor-drift-multiline fix in lib/inputMetrics.ts).
+  if (columns !== undefined) {
+    const layout = cursorLayout(current, cursor, columns)
+
+    if (layout.column === 0 || layout.column >= columns) {
+      return false
+    }
   }
 
   const removed = current.slice(prevPos(current, cursor), cursor)

ui-tui/src/lib/inputMetrics.ts

@@ -1,4 +1,4 @@
-import { stringWidth } from '@hermes/ink'
+import { stringWidth, wrapAnsi } from '@hermes/ink'
 
 import type { Role } from '../types.js'
 
@@ -12,8 +12,6 @@ interface VisualLine {
   start: number
 }
 
-const isWhitespace = (value: string) => /\s/.test(value)
-
 const graphemes = (value: string) =>
   [...seg().segment(value)].map(({ segment, index }) => ({
     end: index + segment.length,
@@ -22,76 +20,81 @@ const graphemes = (value: string) =>
     width: Math.max(1, stringWidth(segment))
   }))
 
+// Build VisualLines from wrap-ansi's output by mapping each emitted character
+// back to its original offset in `value`. wrap-ansi only INSERTS '\n' at wrap
+// boundaries — it never drops, reorders, or substitutes existing characters —
+// so a parallel walk uniquely identifies each line's source range.
+//
+// This used to be a hand-rolled word-wrap whose break points disagreed with
+// wrap-ansi in subtle but visible ways: exact-fill rows pushed the cursor to
+// a phantom next line, mid-word breaks landed one grapheme off, etc. The
+// composer's TextInput renders text via Ink's <Text wrap="wrap">, which
+// delegates to wrap-ansi — so any drift between the two algorithms parks the
+// hardware cursor several cells away from the last rendered character.
+// Sourcing both from wrap-ansi guarantees agreement.
 function visualLines(value: string, cols: number): VisualLine[] {
+  if (!value.length) {
+    return [{ start: 0, end: 0 }]
+  }
+
   const width = Math.max(1, cols)
+  const wrapped = wrapAnsi(value, width, { hard: true, trim: false })
   const lines: VisualLine[] = []
-  let sourceLineStart = 0
 
-  for (const sourceLine of value.split('\n')) {
-    const parts = graphemes(sourceLine)
+  let originalIdx = 0
+  let lineStart = 0
 
-    if (!parts.length) {
-      lines.push({ start: sourceLineStart, end: sourceLineStart })
-      sourceLineStart += 1
+  for (let i = 0; i < wrapped.length; i += 1) {
+    const ch = wrapped[i]!
+
+    if (ch === '\n') {
+      // wrap-ansi inserts '\n' to mark a soft-wrap boundary OR copies a
+      // literal '\n' from the input. Either way the next char in `wrapped`
+      // begins a new visual line. If the source character is a hard '\n',
+      // consume it (it doesn't appear in either line). Otherwise the '\n'
+      // is purely a wrap marker and originalIdx stays put.
+      lines.push({ start: lineStart, end: originalIdx })
+      const isHardNewline = originalIdx < value.length && value[originalIdx] === '\n'
+
+      if (isHardNewline) {
+        originalIdx += 1
+      }
+
+      lineStart = originalIdx
       continue
     }
 
-    let lineStartPart = 0
-    let lineStartOffset = sourceLineStart
-    let column = 0
-    let breakPart: null | number = null
-    let i = 0
-
-    while (i < parts.length) {
-      const part = parts[i]!
-      const partStart = sourceLineStart + part.index
-
-      if (column + part.width > width && i > lineStartPart) {
-        if (breakPart !== null && breakPart > lineStartPart) {
-          const breakOffset = sourceLineStart + parts[breakPart - 1]!.end
-          lines.push({ start: lineStartOffset, end: breakOffset })
-          lineStartPart = breakPart
-          lineStartOffset = breakOffset
-        } else {
-          lines.push({ start: lineStartOffset, end: partStart })
-          lineStartPart = i
-          lineStartOffset = partStart
-        }
-
-        column = 0
-        breakPart = null
-        i = lineStartPart
-        continue
-      }
-
-      column += part.width
-
-      if (isWhitespace(part.segment)) {
-        breakPart = i + 1
-      }
-
-      i += 1
-
-      if (column >= width && i < parts.length) {
-        const next = parts[i]!
-        const nextStartsWord = !isWhitespace(next.segment)
-
-        if (breakPart !== null && breakPart > lineStartPart && nextStartsWord) {
-          const breakOffset = sourceLineStart + parts[breakPart - 1]!.end
-          lines.push({ start: lineStartOffset, end: breakOffset })
-          lineStartPart = breakPart
-          lineStartOffset = breakOffset
-          column = 0
-          breakPart = null
-          i = lineStartPart
-        }
-      }
+    // Defensive sync check. wrap-ansi (with `hard: true, trim: false`, no
+    // styled input) is documented to only insert '\n' at break points and
+    // never substitute, drop, or reorder source characters — so under those
+    // options `wrapped[i]` should always equal `value[originalIdx]`. But
+    // future option changes, library upgrades, or callers that start passing
+    // styled input (ANSI escapes) could violate that invariant silently. If
+    // they do, we'd slide `originalIdx` past the end of `value` and emit
+    // garbage line ranges with no diagnostic. Realign by scanning forward
+    // for the matching character; bail out (return whatever we have) if the
+    // sync is unrecoverable rather than producing wrong-but-plausible output.
+    if (originalIdx >= value.length) {
+      break
     }
 
-    lines.push({ start: lineStartOffset, end: sourceLineStart + sourceLine.length })
-    sourceLineStart += sourceLine.length + 1
+    if (value[originalIdx] !== ch) {
+      const reSync = value.indexOf(ch, originalIdx)
+
+      if (reSync === -1) {
+        break
+      }
+
+      originalIdx = reSync
+    }
+
+    originalIdx += 1
   }
 
+  lines.push({ start: lineStart, end: originalIdx })
+
+  // wrap-ansi collapses an empty input into [""] which we already handled
+  // above; preserve the invariant that lines is never empty for any input.
   return lines.length ? lines : [{ start: 0, end: 0 }]
 }
 
@@ -108,6 +111,12 @@ function widthBetween(value: string, start: number, end: number) {
 /**
  * Mirrors the word-wrap behavior used by the composer TextInput.
  * Returns the zero-based visual line and column of the cursor cell.
+ *
+ * IMPORTANT: this MUST stay in lock-step with how Ink's `<Text wrap="wrap">`
+ * lays the value out (which uses `wrap-ansi`). Any divergence parks the
+ * hardware cursor several cells off the last rendered character — see the
+ * "cursor drift past blank cells" bug. `visualLines` is sourced directly
+ * from wrap-ansi to enforce that invariant.
  */
 export function cursorLayout(value: string, cursor: number, cols: number) {
   const pos = Math.max(0, Math.min(cursor, value.length))
@@ -124,14 +133,14 @@ export function cursorLayout(value: string, cursor: number, cols: number) {
   }
 
   const line = lines[lineIndex]!
-  let column = widthBetween(value, line.start, Math.min(pos, line.end))
-
-  // trailing cursor-cell overflows to the next row at the wrap column
-  if (column >= w) {
-    lineIndex++
-    column = 0
-  }
+  const column = widthBetween(value, line.start, Math.min(pos, line.end))
 
+  // NOTE: the previous implementation forced an extra line break when
+  // `column >= w` (the "trailing cursor-cell overflows" rule). With
+  // `visualLines` sourcing breaks from wrap-ansi, the line wrapping
+  // above already matches what Ink will actually render. Pushing the
+  // cursor onto a phantom next line here would re-introduce the same
+  // drift we're fixing, so we don't.
   return { column, line: lineIndex }
 }