From 68391930d0aa2bd580d8ba9f0e65a8624a15171d Mon Sep 17 00:00:00 2001 From: Gabriel Lesperance Date: Wed, 22 Jul 2026 11:31:22 -0400 Subject: [PATCH] perf(tui): bound scroll rendering and preserve anchors --- .../src/ink/components/ScrollBox.tsx | 102 ++- ui-tui/packages/hermes-ink/src/ink/dom.ts | 5 + .../hermes-ink/src/ink/render-border.test.ts | 200 ++++++ .../hermes-ink/src/ink/render-border.ts | 293 +++++--- .../src/ink/render-node-to-output.ts | 487 +++++++++++--- ui-tui/scripts/bench-history-scroll.tsx | 483 ++++++++++++++ ui-tui/src/__tests__/messages.test.ts | 16 +- ui-tui/src/__tests__/scroll.test.ts | 35 +- .../__tests__/scrollBoxRendererBounds.test.ts | 627 ++++++++++++++++++ .../useVirtualHistoryHeights.test.ts | 20 + .../virtualHistoryOffsetCache.test.ts | 376 ++++++++++- ui-tui/src/app/scroll.ts | 5 +- ui-tui/src/app/useMainApp.ts | 61 +- ui-tui/src/hooks/useVirtualHistory.ts | 241 +++++-- ui-tui/src/lib/messages.ts | 9 + ui-tui/src/types/hermes-ink.d.ts | 1 + 16 files changed, 2641 insertions(+), 320 deletions(-) create mode 100644 ui-tui/packages/hermes-ink/src/ink/render-border.test.ts create mode 100644 ui-tui/scripts/bench-history-scroll.tsx create mode 100644 ui-tui/src/__tests__/scrollBoxRendererBounds.test.ts diff --git a/ui-tui/packages/hermes-ink/src/ink/components/ScrollBox.tsx b/ui-tui/packages/hermes-ink/src/ink/components/ScrollBox.tsx index 4f2604be0e..456ea5fceb 100644 --- a/ui-tui/packages/hermes-ink/src/ink/components/ScrollBox.tsx +++ b/ui-tui/packages/hermes-ink/src/ink/components/ScrollBox.tsx @@ -10,9 +10,31 @@ import { markCommitStart } from '../reconciler.js' import type { Styles } from '../styles.js' import Box from './Box.js' + +const MAX_SCROLL_GEOMETRY = 1_000_000_000 + +const validUnsignedGeometry = (value: number): boolean => + Number.isFinite(value) && value >= 0 && value <= MAX_SCROLL_GEOMETRY + +const validClampMaximum = (value: number): boolean => value === Number.POSITIVE_INFINITY || validUnsignedGeometry(value) + +const validSignedGeometry = (value: number): boolean => Number.isFinite(value) && Math.abs(value) <= MAX_SCROLL_GEOMETRY + +const safeUnsignedGeometry = (value: number | undefined): number => + value !== undefined && validUnsignedGeometry(value) ? value : 0 + +const safeSignedGeometry = (value: number | undefined): number => + value !== undefined && validSignedGeometry(value) ? value : 0 + export type ScrollBoxHandle = { scrollTo: (y: number) => void scrollBy: (dy: number) => void + /** + * Offset the committed viewport after content above it changes height. + * Unlike scrollTo, this preserves pending input, sticky state, anchor seeks, + * and the manual-scroll timestamp. + */ + adjustScrollTop: (dy: number) => void /** * Scroll so `el`'s top is at the viewport top (plus `offset`). Unlike * scrollTo which bakes a number that's stale by the time the throttled @@ -49,9 +71,9 @@ export type ScrollBoxHandle = { isSticky: () => boolean /** * Subscribe to scroll viewport changes. Fires for imperative scroll changes - * (scrollTo/scrollBy/scrollToBottom) and for renderer-computed scroll bounds - * changes such as content growth or terminal resize. Callers use this to - * keep virtualized ranges aligned with the currently visible viewport. + * (scrollTo/scrollBy/adjustScrollTop/scrollToBottom) and for renderer-computed + * scroll bounds changes such as content growth or terminal resize. Callers + * use this to keep virtualized ranges aligned with the visible viewport. */ subscribe: (listener: () => void) => () => void /** @@ -85,7 +107,7 @@ export type ScrollBoxProps = Except): React.ReactNode { const domRef = useRef(null) - // scrollTo/scrollBy bypass React: they mutate scrollTop on the DOM node, + // Imperative position changes bypass React: they mutate scrollTop on the DOM node, // mark it dirty, and call the root's throttled scheduleRender directly. // The Ink renderer reads scrollTop from the node — no React state needed, // no reconciler overhead per wheel event. The microtask defer coalesces @@ -127,10 +149,29 @@ function ScrollBox({ children, ref, stickyScroll, ...style }: PropsWithChildren< useImperativeHandle( ref, (): ScrollBoxHandle => ({ + adjustScrollTop(dy: number) { + const el = domRef.current + + if (!el || !validSignedGeometry(dy)) { + return + } + + const current = safeUnsignedGeometry(el.scrollTop) + const next = Math.max(0, current + Math.floor(dy)) + const compensation = safeSignedGeometry(el.scrollTopCompensation) + (next - current) + + if (next === current || !validUnsignedGeometry(next) || !validSignedGeometry(compensation)) { + return + } + + el.scrollTop = next + el.scrollTopCompensation = compensation + scrollMutated(el) + }, scrollTo(y: number) { const el = domRef.current - if (!el) { + if (!el || !validSignedGeometry(y)) { return } @@ -139,6 +180,7 @@ function ScrollBox({ children, ref, stickyScroll, ...style }: PropsWithChildren< el.stickyScroll = false manualScrollAtRef.current = Date.now() el.pendingScrollDelta = undefined + el.scrollTopCompensation = undefined el.scrollAnchor = undefined el.scrollTop = Math.max(0, Math.floor(y)) scrollMutated(el) @@ -146,13 +188,14 @@ function ScrollBox({ children, ref, stickyScroll, ...style }: PropsWithChildren< scrollToElement(el: DOMElement, offset = 0) { const box = domRef.current - if (!box) { + if (!box || !validSignedGeometry(offset)) { return } box.stickyScroll = false manualScrollAtRef.current = Date.now() box.pendingScrollDelta = undefined + box.scrollTopCompensation = undefined box.scrollAnchor = { el, offset @@ -162,14 +205,20 @@ function ScrollBox({ children, ref, stickyScroll, ...style }: PropsWithChildren< scrollBy(dy: number) { const el = domRef.current - if (!el) { + if (!el || !validSignedGeometry(dy)) { + return + } + + const pending = safeSignedGeometry(el.pendingScrollDelta) + Math.floor(dy) + + if (!validSignedGeometry(pending)) { return } el.stickyScroll = false manualScrollAtRef.current = Date.now() el.scrollAnchor = undefined - el.pendingScrollDelta = (el.pendingScrollDelta ?? 0) + Math.floor(dy) + el.pendingScrollDelta = pending scrollMutated(el) }, scrollToBottom() { @@ -180,30 +229,35 @@ function ScrollBox({ children, ref, stickyScroll, ...style }: PropsWithChildren< } el.pendingScrollDelta = undefined + el.scrollTopCompensation = undefined el.stickyScroll = true markDirty(el) notify() forceRender(n => n + 1) }, getScrollTop() { - return domRef.current?.scrollTop ?? 0 + return safeUnsignedGeometry(domRef.current?.scrollTop) }, getPendingDelta() { // Accumulated-but-not-yet-drained delta. useVirtualScroll needs // this to mount the union [committed, committed+pending] range — // otherwise intermediate drain frames find no children (blank). - return domRef.current?.pendingScrollDelta ?? 0 + return safeSignedGeometry(domRef.current?.pendingScrollDelta) }, getScrollHeight() { - return domRef.current?.scrollHeight ?? 0 + return safeUnsignedGeometry(domRef.current?.scrollHeight) }, getFreshScrollHeight() { const content = domRef.current?.childNodes[0] as DOMElement | undefined - return content?.yogaNode?.getComputedHeight() ?? domRef.current?.scrollHeight ?? 0 + const height = content?.yogaNode?.getComputedHeight() + + return validUnsignedGeometry(height ?? Number.NaN) + ? height! + : safeUnsignedGeometry(domRef.current?.scrollHeight) }, getViewportHeight() { - return domRef.current?.scrollViewportHeight ?? 0 + return safeUnsignedGeometry(domRef.current?.scrollViewportHeight) }, getViewportTop() { return domRef.current?.scrollViewportTop ?? 0 @@ -232,6 +286,26 @@ function ScrollBox({ children, ref, stickyScroll, ...style }: PropsWithChildren< return } + if (min === undefined && max === undefined) { + el.scrollClampMin = undefined + el.scrollClampMax = undefined + + return + } + + if ( + min === undefined || + max === undefined || + !validUnsignedGeometry(min) || + !validClampMaximum(max) || + min > max + ) { + el.scrollClampMin = undefined + el.scrollClampMax = undefined + + return + } + el.scrollClampMin = min el.scrollClampMax = max } @@ -260,7 +334,7 @@ function ScrollBox({ children, ref, stickyScroll, ...style }: PropsWithChildren< domRef.current = el if (el) { - el.scrollTop ??= 0 + el.scrollTop = safeUnsignedGeometry(el.scrollTop) el.notifyScrollChange = notify } }} diff --git a/ui-tui/packages/hermes-ink/src/ink/dom.ts b/ui-tui/packages/hermes-ink/src/ink/dom.ts index 69fc74fff4..834532abc6 100644 --- a/ui-tui/packages/hermes-ink/src/ink/dom.ts +++ b/ui-tui/packages/hermes-ink/src/ink/dom.ts @@ -53,6 +53,11 @@ export type DOMElement = { // intermediate frames instead of one big jump. Direction reversal // naturally cancels (pure accumulator, no target tracking). pendingScrollDelta?: number + // One-render record of additive scrollTop changes made to preserve the + // visual anchor after content above the viewport changes height. The + // renderer subtracts this when evaluating positional bottom-follow and + // defers pending input for that paint, then clears the record. + scrollTopCompensation?: number // Render-time clamp bounds for virtual scroll. useVirtualScroll writes // the currently-mounted children's coverage span; render-node-to-output // clamps scrollTop to stay within it. Prevents blank screen when diff --git a/ui-tui/packages/hermes-ink/src/ink/render-border.test.ts b/ui-tui/packages/hermes-ink/src/ink/render-border.test.ts new file mode 100644 index 0000000000..a6373907d3 --- /dev/null +++ b/ui-tui/packages/hermes-ink/src/ink/render-border.test.ts @@ -0,0 +1,200 @@ +import { describe, expect, it } from 'vitest' + +import type { DOMNode } from './dom.js' +import Output from './output.js' +import renderBorder from './render-border.js' +import { cellAt, CellWidth, CharPool, createScreen, HyperlinkPool, type Screen, StylePool } from './screen.js' + +const WIDTH = 12 +const HEIGHT = 6 + +function createOutput() { + const stylePool = new StylePool() + const screen = createScreen(WIDTH, HEIGHT, stylePool, new CharPool(), new HyperlinkPool()) + + return { output: new Output({ height: HEIGHT, screen, stylePool, width: WIDTH }), stylePool } +} + +function borderNode(style: Record = {}, width = 8, height = 4): DOMNode { + return { + style: { borderStyle: 'single', ...style }, + yogaNode: { + getComputedHeight: () => height, + getComputedWidth: () => width + } + } as unknown as DOMNode +} + +function snapshot(screen: Screen, styles: StylePool) { + return Array.from({ length: screen.height }, (_, y) => + Array.from({ length: screen.width }, (_, x) => { + const cell = cellAt(screen, x, y)! + + return [cell.char, cell.width, styles.get(cell.styleId).map(code => code.code)] as const + }) + ) +} + +function paint( + node: DOMNode, + visible: readonly [number, number, number, number] = [0, WIDTH, 0, HEIGHT], + decorate?: (output: Output, phase: 'before' | 'after') => void +) { + const { output, stylePool } = createOutput() + + decorate?.(output, 'before') + renderBorder(1, 1, node, output, ...visible) + decorate?.(output, 'after') + + return { cells: snapshot(output.get(), stylePool), stylePool } +} + +function expectClippedParity( + full: ReturnType['cells'], + clipped: ReturnType['cells'], + [x1, x2, y1, y2]: readonly [number, number, number, number] +) { + for (let y = 0; y < HEIGHT; y++) { + for (let x = 0; x < WIDTH; x++) { + if (x >= x1 && x < x2 && y >= y1 && y < y2) { + expect(clipped[y]![x]).toEqual(full[y]![x]) + } else { + expect(clipped[y]![x]![0]).toBe(' ') + } + } + } +} + +function expectWideTextClippedParity( + full: ReturnType['cells'], + clipped: ReturnType['cells'], + visible: readonly [number, number, number, number] +) { + const [x1, x2, y1, y2] = visible + + for (let y = 0; y < HEIGHT; y++) { + for (let x = 0; x < WIDTH; x++) { + const fullCell = full[y]![x]! + const isVisible = x >= x1 && x < x2 && y >= y1 && y < y2 + const clipsWideHead = isVisible && fullCell[1] === CellWidth.Wide && x + 1 >= x2 + const clipsWideTail = isVisible && fullCell[1] === CellWidth.SpacerTail && x - 1 < x1 + + if (clipsWideHead || clipsWideTail) { + expect(clipped[y]![x]).toEqual([' ', CellWidth.Narrow, []]) + } else if (isVisible) { + expect(clipped[y]![x]).toEqual(fullCell) + } else { + expect(clipped[y]![x]![0]).toBe(' ') + } + } + } +} + +describe('renderBorder viewport parity', () => { + it.each([ + ['all edges', {}], + ['disabled edges', { borderLeft: false, borderTop: false }] + ])('matches the unclipped border inside a clipped viewport with %s', (_name, style) => { + const visible = [3, 8, 1, 4] as const + const full = paint(borderNode(style)).cells + const clipped = paint(borderNode(style), visible).cells + + expectClippedParity(full, clipped, visible) + }) + + it.each([ + ['left edge at wide title', [3, 9, 1, 2]], + ['left edge bisects wide title', [4, 9, 1, 2]], + ['left edge after wide grapheme', [5, 9, 1, 2]], + ['right edge bisects wide title', [1, 4, 1, 2]], + ['right edge after wide grapheme', [1, 5, 1, 2]], + ['right edge after narrow suffix', [1, 6, 1, 2]] + ] as const)('preserves ANSI wide-title cell coordinates when the %s', (_name, visible) => { + const node = borderNode({ + borderText: { + align: 'center', + content: '\u001B[31m界A\u001B[39m', + position: 'top' + } + }) + + const full = paint(node).cells + const clipped = paint(node, visible).cells + + expectWideTextClippedParity(full, clipped, visible) + expect(full[1]!.some(([char]) => char === '界')).toBe(true) + expect(full[1]!.some(([char, , codes]) => char === '界' && codes.includes('\u001B[31m'))).toBe(true) + expect(full[1]![6]![2]).not.toContain('\u001B[31m') + + if (visible[0] === 4) { + expect(clipped[1]![4]).toEqual([' ', CellWidth.Narrow, []]) + expect(clipped[1]![5]![0]).toBe('A') + } + }) + + it.each([ + ['left edge inside title', [4, 9, 1, 2]], + ['right edge inside title', [1, 5, 1, 2]] + ] as const)('preserves narrow ANSI-title parity when the %s', (_name, visible) => { + const node = borderNode({ + borderText: { + align: 'center', + content: '\u001B[32mABC\u001B[39m', + position: 'top' + } + }) + + const full = paint(node).cells + const clipped = paint(node, visible).cells + + expectClippedParity(full, clipped, visible) + }) + + it('intersects nested output clips without changing surviving border cells', () => { + const node = borderNode() + const full = paint(node).cells + const { output, stylePool } = createOutput() + + output.clip({ x1: 0, x2: 10, y1: 0, y2: 5 }) + output.clip({ x1: 1, x2: 7, y1: 1, y2: 4 }) + renderBorder(1, 1, node, output) + output.unclip() + output.unclip() + + const nested = snapshot(output.get(), stylePool) + + expectClippedParity(full, nested, [1, 7, 1, 4]) + }) + + it('renders borders after opaque fills and preserves the fill interior', () => { + const decorated = paint(borderNode(), [0, WIDTH, 0, HEIGHT], (output, phase) => { + if (phase === 'before') { + const line = '\u001B[44m \u001B[49m' + + output.write(1, 1, [line, line, line, line].join('\n')) + } + }) + + expect(decorated.cells[1]![1]![0]).toBe('┌') + expect(decorated.cells[4]![8]![0]).toBe('┘') + expect(decorated.cells[2]![2]![2]).toContain('\u001B[44m') + }) + + it('keeps clipped border parity when an absolute-style overlay paints afterward', () => { + const overlay = (output: Output, phase: 'before' | 'after') => { + if (phase === 'after') { + output.clip({ x1: 4, x2: 6, y1: 1, y2: 2 }) + output.write(4, 1, 'OV') + output.unclip() + } + } + + const visible = [3, 7, 1, 4] as const + const full = paint(borderNode(), [0, WIDTH, 0, HEIGHT], overlay).cells + const clipped = paint(borderNode(), visible, overlay).cells + + expectClippedParity(full, clipped, visible) + expect(clipped[1]![4]![0]).toBe('O') + expect(clipped[1]![5]![0]).toBe('V') + }) +}) diff --git a/ui-tui/packages/hermes-ink/src/ink/render-border.ts b/ui-tui/packages/hermes-ink/src/ink/render-border.ts index a4fff7cb50..526729422d 100644 --- a/ui-tui/packages/hermes-ink/src/ink/render-border.ts +++ b/ui-tui/packages/hermes-ink/src/ink/render-border.ts @@ -1,6 +1,8 @@ import chalk from 'chalk' import cliBoxes, { type Boxes, type BoxStyle } from 'cli-boxes' +import sliceAnsi from '../utils/sliceAnsi.js' + import { applyColor } from './colorize.js' import type { DOMNode } from './dom.js' import type Output from './output.js' @@ -30,41 +32,6 @@ export const CUSTOM_BORDER_STYLES = { export type BorderStyle = keyof Boxes | keyof typeof CUSTOM_BORDER_STYLES | BoxStyle -function embedTextInBorder( - borderLine: string, - text: string, - align: 'start' | 'end' | 'center', - offset: number = 0, - borderChar: string -): [before: string, text: string, after: string] { - const textLength = stringWidth(text) - const borderLength = borderLine.length - - if (textLength >= borderLength - 2) { - return ['', text.substring(0, borderLength), ''] - } - - let position: number - - if (align === 'center') { - position = Math.floor((borderLength - textLength) / 2) - } else if (align === 'start') { - position = offset + 1 // +1 to account for corner character - } else { - // align === 'end' - position = borderLength - textLength - offset - 1 // -1 for corner character - } - - // Ensure position is valid - position = Math.max(1, Math.min(position, borderLength - textLength - 1)) - - const before = borderLine.substring(0, 1) + borderChar.repeat(position - 1) - - const after = borderChar.repeat(borderLength - position - textLength - 1) + borderLine.substring(borderLength - 1) - - return [before, text, after] -} - function styleBorderLine(line: string, color: Color | undefined, dim: boolean | undefined): string { let styled = applyColor(line, color) @@ -75,7 +42,138 @@ function styleBorderLine(line: string, color: Color | undefined, dim: boolean | return styled } -const renderBorder = (x: number, y: number, node: DOMNode, output: Output): void => { +function sliceAnsiToWidth(text: string, start: number, end: number): { leadingColumns: number; text: string } { + // sliceAnsi omits a wide grapheme bisected by `start`; retain its partial + // cell as blank space so later graphemes keep their source coordinates. + const leadingColumns = Math.max(0, stringWidth(sliceAnsi(text, 0, start)) - start) + let sliced = sliceAnsi(text, start, end) + + if (stringWidth(sliced) > end - start - leadingColumns) { + sliced = sliceAnsi(text, start, end - 1) + } + + return { leadingColumns, text: sliced } +} + +function borderRun( + start: number, + end: number, + borderLength: number, + borderChar: string, + startCorner: string, + endCorner: string +): string { + if (start >= end) { + return '' + } + + const includeStartCorner = start === 0 && startCorner.length > 0 + const includeEndCorner = end === borderLength && endCorner.length > 0 + const repeated = Math.max(0, end - start - (includeStartCorner ? 1 : 0) - (includeEndCorner ? 1 : 0)) + + return (includeStartCorner ? startCorner : '') + borderChar.repeat(repeated) + (includeEndCorner ? endCorner : '') +} + +function renderHorizontalBorder( + x: number, + y: number, + width: number, + output: Output, + visibleX1: number, + visibleX2: number, + borderChar: string, + startCorner: string, + endCorner: string, + color: Color | undefined, + dim: boolean | undefined, + borderText: BorderTextOptions | undefined +): void { + const borderLength = + Math.max(0, width - (startCorner ? 1 : 0) - (endCorner ? 1 : 0)) + (startCorner ? 1 : 0) + (endCorner ? 1 : 0) + + const clippedX1 = Math.max(0, Math.floor(visibleX1)) + const clippedX2 = Math.min(output.width, Math.ceil(visibleX2)) + const sliceStart = Math.max(0, Math.ceil(clippedX1 - x)) + const sliceEnd = Math.min(borderLength, Math.ceil(clippedX2 - x)) + + if ( + !Number.isSafeInteger(width) || + width < 0 || + !Number.isSafeInteger(sliceStart) || + !Number.isSafeInteger(sliceEnd) || + sliceStart >= sliceEnd + ) { + return + } + + const writeBorderRun = (start: number, end: number) => { + const text = borderRun(start, end, borderLength, borderChar, startCorner, endCorner) + + if (text) { + output.write(x + start, y, styleBorderLine(text, color, dim)) + } + } + + if (!borderText) { + writeBorderRun(sliceStart, sliceEnd) + + return + } + + const textLength = stringWidth(borderText.content) + + if (textLength >= borderLength - 2) { + const { leadingColumns, text } = sliceAnsiToWidth(borderText.content, sliceStart, sliceEnd) + + if (text) { + output.write(x + sliceStart + leadingColumns, y, text) + } + + return + } + + let position: number + + if (borderText.align === 'center') { + position = Math.floor((borderLength - textLength) / 2) + } else if (borderText.align === 'start') { + position = (borderText.offset ?? 0) + 1 + } else { + position = borderLength - textLength - (borderText.offset ?? 0) - 1 + } + + position = Math.max(1, Math.min(position, borderLength - textLength - 1)) + + writeBorderRun(sliceStart, Math.min(sliceEnd, position)) + + const visibleTextStart = Math.max(sliceStart, position) + const visibleTextEnd = Math.min(sliceEnd, position + textLength) + + if (visibleTextStart < visibleTextEnd) { + const { leadingColumns, text } = sliceAnsiToWidth( + borderText.content, + visibleTextStart - position, + visibleTextEnd - position + ) + + if (text) { + output.write(x + visibleTextStart + leadingColumns, y, text) + } + } + + writeBorderRun(Math.max(sliceStart, position + textLength), sliceEnd) +} + +const renderBorder = ( + x: number, + y: number, + node: DOMNode, + output: Output, + visibleX1 = 0, + visibleX2 = output.width, + visibleY1 = 0, + visibleY2 = output.height +): void => { if (node.style.borderStyle) { const width = Math.floor(node.yogaNode!.getComputedWidth()) const height = Math.floor(node.yogaNode!.getComputedHeight()) @@ -107,98 +205,75 @@ const renderBorder = (x: number, y: number, node: DOMNode, output: Output): void const showLeftBorder = node.style.borderLeft !== false const showRightBorder = node.style.borderRight !== false - const contentWidth = Math.max(0, width - (showLeftBorder ? 1 : 0) - (showRightBorder ? 1 : 0)) + const verticalTop = Math.floor(y + (showTopBorder ? 1 : 0)) + const verticalBottom = Math.ceil(y + height - (showBottomBorder ? 1 : 0)) + const clippedVerticalTop = Math.max(0, Math.floor(visibleY1), verticalTop) + const clippedVerticalBottom = Math.min(output.height, Math.ceil(visibleY2), verticalBottom) + const clippedVerticalHeight = Math.max(0, clippedVerticalBottom - clippedVerticalTop) - const topBorderLine = showTopBorder - ? (showLeftBorder ? box.topLeft : '') + box.top.repeat(contentWidth) + (showRightBorder ? box.topRight : '') - : '' - - // Handle text in top border - let topBorder: string | undefined - - if (showTopBorder && node.style.borderText?.position === 'top') { - const [before, text, after] = embedTextInBorder( - topBorderLine, - node.style.borderText.content, - node.style.borderText.align, - node.style.borderText.offset, - box.top - ) - - topBorder = - styleBorderLine(before, topBorderColor, dimTopBorderColor) + - text + - styleBorderLine(after, topBorderColor, dimTopBorderColor) - } else if (showTopBorder) { - topBorder = styleBorderLine(topBorderLine, topBorderColor, dimTopBorderColor) - } - - let verticalBorderHeight = height - - if (showTopBorder) { - verticalBorderHeight -= 1 - } - - if (showBottomBorder) { - verticalBorderHeight -= 1 - } - - verticalBorderHeight = Math.max(0, verticalBorderHeight) - - let leftBorder = (applyColor(box.left, leftBorderColor) + '\n').repeat(verticalBorderHeight) + let leftBorder = (applyColor(box.left, leftBorderColor) + '\n').repeat(clippedVerticalHeight) if (dimLeftBorderColor) { leftBorder = chalk.dim(leftBorder) } - let rightBorder = (applyColor(box.right, rightBorderColor) + '\n').repeat(verticalBorderHeight) + let rightBorder = (applyColor(box.right, rightBorderColor) + '\n').repeat(clippedVerticalHeight) if (dimRightBorderColor) { rightBorder = chalk.dim(rightBorder) } - const bottomBorderLine = showBottomBorder - ? (showLeftBorder ? box.bottomLeft : '') + - box.bottom.repeat(contentWidth) + - (showRightBorder ? box.bottomRight : '') - : '' - - // Handle text in bottom border - let bottomBorder: string | undefined - - if (showBottomBorder && node.style.borderText?.position === 'bottom') { - const [before, text, after] = embedTextInBorder( - bottomBorderLine, - node.style.borderText.content, - node.style.borderText.align, - node.style.borderText.offset, - box.bottom + if (showTopBorder && y >= visibleY1 && y < visibleY2 && y >= 0 && y < output.height) { + renderHorizontalBorder( + x, + y, + width, + output, + visibleX1, + visibleX2, + box.top, + showLeftBorder ? box.topLeft : '', + showRightBorder ? box.topRight : '', + topBorderColor, + dimTopBorderColor, + node.style.borderText?.position === 'top' ? node.style.borderText : undefined ) - - bottomBorder = - styleBorderLine(before, bottomBorderColor, dimBottomBorderColor) + - text + - styleBorderLine(after, bottomBorderColor, dimBottomBorderColor) - } else if (showBottomBorder) { - bottomBorder = styleBorderLine(bottomBorderLine, bottomBorderColor, dimBottomBorderColor) } - const offsetY = showTopBorder ? 1 : 0 - - if (topBorder) { - output.write(x, y, topBorder) + if (showLeftBorder && clippedVerticalHeight > 0 && x >= visibleX1 && x < visibleX2 && x >= 0 && x < output.width) { + output.write(x, clippedVerticalTop, leftBorder) } - if (showLeftBorder) { - output.write(x, y + offsetY, leftBorder) + const rightX = x + width - 1 + + if ( + showRightBorder && + clippedVerticalHeight > 0 && + rightX >= visibleX1 && + rightX < visibleX2 && + rightX >= 0 && + rightX < output.width + ) { + output.write(rightX, clippedVerticalTop, rightBorder) } - if (showRightBorder) { - output.write(x + width - 1, y + offsetY, rightBorder) - } + const bottomY = y + height - 1 - if (bottomBorder) { - output.write(x, y + height - 1, bottomBorder) + if (showBottomBorder && bottomY >= visibleY1 && bottomY < visibleY2 && bottomY >= 0 && bottomY < output.height) { + renderHorizontalBorder( + x, + bottomY, + width, + output, + visibleX1, + visibleX2, + box.bottom, + showLeftBorder ? box.bottomLeft : '', + showRightBorder ? box.bottomRight : '', + bottomBorderColor, + dimBottomBorderColor, + node.style.borderText?.position === 'bottom' ? node.style.borderText : undefined + ) } } } diff --git a/ui-tui/packages/hermes-ink/src/ink/render-node-to-output.ts b/ui-tui/packages/hermes-ink/src/ink/render-node-to-output.ts index fdd21c143f..d1f6325fe9 100644 --- a/ui-tui/packages/hermes-ink/src/ink/render-node-to-output.ts +++ b/ui-tui/packages/hermes-ink/src/ink/render-node-to-output.ts @@ -15,6 +15,33 @@ import { isXtermJs } from './terminal.js' import { widestLine } from './widest-line.js' import wrapText from './wrap-text.js' +const MAX_SCROLL_GEOMETRY = 1_000_000_000 +const MAX_YOGA_DIMENSION = 100_000_000 + +const validUnsignedGeometry = (value: number): boolean => + Number.isFinite(value) && value >= 0 && value <= MAX_SCROLL_GEOMETRY + +const validClampMaximum = (value: number): boolean => value === Number.POSITIVE_INFINITY || validUnsignedGeometry(value) + +const validSignedGeometry = (value: number): boolean => Number.isFinite(value) && Math.abs(value) <= MAX_SCROLL_GEOMETRY + +const safeUnsignedGeometry = (value: number | undefined, fallback = 0): number => + value !== undefined && validUnsignedGeometry(value) ? value : fallback + +const safeSignedGeometry = (value: number | undefined, fallback = 0): number => + value !== undefined && validSignedGeometry(value) ? value : fallback + +const validYogaDimension = (value: number): boolean => + Number.isFinite(value) && value >= 0 && value <= MAX_YOGA_DIMENSION + +const validYogaRect = (x: number, y: number, width: number, height: number): boolean => + validSignedGeometry(x) && + validSignedGeometry(y) && + validYogaDimension(width) && + validYogaDimension(height) && + validSignedGeometry(x + width) && + validSignedGeometry(y + height) + // Matches detectXtermJsWheel() in ScrollKeybindingHandler.tsx — the curve // and drain must agree on terminal detection. TERM_PROGRAM check is the sync // fallback; isXtermJs() is the authoritative XTVERSION-probe result. @@ -370,12 +397,30 @@ function wrapWithSoftWrap( // and use it as offset for the rest of the nodes // Only first node is taken into account, because other text nodes can't have margin or padding, // so their coordinates will be relative to the first node anyway -function applyPaddingToText(node: DOMElement, text: string, softWrap?: boolean[]): string { +function applyPaddingToText( + node: DOMElement, + text: string, + softWrap: boolean[] | undefined, + maxOffsetX: number, + maxOffsetY: number +): string { const yogaNode = node.childNodes[0]?.yogaNode if (yogaNode) { const offsetX = yogaNode.getComputedLeft() const offsetY = yogaNode.getComputedTop() + + if ( + !Number.isSafeInteger(offsetX) || + offsetX < 0 || + offsetX > maxOffsetX || + !Number.isSafeInteger(offsetY) || + offsetY < 0 || + offsetY > maxOffsetY + ) { + return '' + } + text = '\n'.repeat(offsetY) + indentString(text, offsetX) if (softWrap && offsetY > 0) { @@ -397,7 +442,11 @@ function renderNodeToOutput( offsetY = 0, prevScreen, skipSelfBlit = false, - inheritedBackgroundColor + inheritedBackgroundColor, + visibleX1 = 0, + visibleX2 = output.width, + visibleY1 = 0, + visibleY2 = output.height }: { offsetX?: number offsetY?: number @@ -409,6 +458,10 @@ function renderNodeToOutput( // opaque descendants' narrower rects are safe to blit. skipSelfBlit?: boolean inheritedBackgroundColor?: Color + visibleX1?: number + visibleX2?: number + visibleY1?: number + visibleY2?: number } ): void { const { yogaNode } = node @@ -456,6 +509,21 @@ function renderNodeToOutput( y = 0 } + // Yoga values are an untrusted renderer boundary. Invalid or implausible + // dimensions must never reach culling, string construction, or recursive + // rendering: NaN makes every comparison false, while huge finite heights + // can turn an opaque box or border into a catastrophic allocation. + if (!validYogaRect(x, y, width, height)) { + dropSubtreeCache(node) + + return + } + + const activeVisibleX1 = Math.max(0, Math.floor(visibleX1)) + const activeVisibleX2 = Math.min(output.width, Math.ceil(visibleX2)) + const activeVisibleY1 = Math.max(0, Math.floor(visibleY1)) + const activeVisibleY2 = Math.min(output.height, Math.ceil(visibleY2)) + // Check if we can skip this subtree (clean node with unchanged layout). // Blit cells from previous screen instead of re-rendering. const cached = nodeCache.get(node) @@ -504,15 +572,26 @@ function renderNodeToOutput( } if (cached && (node.dirty || positionChanged)) { - output.clear( - { - x: Math.floor(cached.x), - y: Math.floor(cached.y), - width: Math.floor(cached.width), - height: Math.floor(cached.height) - }, - node.style.position === 'absolute' - ) + if (validYogaRect(cached.x, cached.y, cached.width, cached.height)) { + const clearX1 = Math.max(activeVisibleX1, Math.floor(cached.x)) + const clearX2 = Math.min(activeVisibleX2, Math.ceil(cached.x + cached.width)) + const clearY1 = Math.max(activeVisibleY1, Math.floor(cached.y)) + const clearY2 = Math.min(activeVisibleY2, Math.ceil(cached.y + cached.height)) + + if (clearX1 < clearX2 && clearY1 < clearY2) { + output.clear( + { + x: clearX1, + y: clearY1, + width: clearX2 - clearX1, + height: clearY2 - clearY1 + }, + node.style.position === 'absolute' + ) + } + } else { + dropSubtreeCache(node) + } } // Read before deleting — hasRemovedChild disables prevScreen blitting @@ -633,7 +712,7 @@ function renderNodeToOutput( .join('') } - text = applyPaddingToText(node, text, softWrap) + text = applyPaddingToText(node, text, softWrap, output.width, output.height) output.write(x, y, text, softWrap) } @@ -670,13 +749,15 @@ function renderNodeToOutput( const isScrollY = overflowY === 'scroll' const needsClip = clipHorizontally || clipVertically + let x1: number | undefined + let x2: number | undefined let y1: number | undefined let y2: number | undefined if (needsClip) { - const x1 = clipHorizontally ? x + yogaNode.getComputedBorder(LayoutEdge.Left) : undefined + x1 = clipHorizontally ? x + yogaNode.getComputedBorder(LayoutEdge.Left) : undefined - const x2 = clipHorizontally + x2 = clipHorizontally ? x + yogaNode.getComputedWidth() - yogaNode.getComputedBorder(LayoutEdge.Right) : undefined @@ -689,6 +770,11 @@ function renderNodeToOutput( output.clip({ x1, x2, y1, y2 }) } + const childVisibleX1 = Math.max(activeVisibleX1, Math.floor(x1 ?? activeVisibleX1)) + const childVisibleX2 = Math.min(activeVisibleX2, Math.ceil(x2 ?? activeVisibleX2)) + const childVisibleY1 = Math.max(activeVisibleY1, Math.floor(y1 ?? activeVisibleY1)) + const childVisibleY2 = Math.min(activeVisibleY2, Math.ceil(y2 ?? activeVisibleY2)) + if (isScrollY) { // Scroll containers follow the ScrollBox component structure: // a single content-wrapper child with flexShrink:0 (doesn't shrink @@ -698,9 +784,8 @@ function renderNodeToOutput( // culled against the visible window. const padTop = yogaNode.getComputedPadding(LayoutEdge.Top) - const innerHeight = Math.max( - 0, - (y2 ?? y + height) - (y1 ?? y) - padTop - yogaNode.getComputedPadding(LayoutEdge.Bottom) + const innerHeight = safeUnsignedGeometry( + Math.max(0, (y2 ?? y + height) - (y1 ?? y) - padTop - yogaNode.getComputedPadding(LayoutEdge.Bottom)) ) const content = node.childNodes.find(c => (c as DOMElement).yogaNode) as DOMElement | undefined @@ -710,31 +795,32 @@ function renderNodeToOutput( // after terminal resizes Yoga can leave tall descendants overflowing // that wrapper. Use the deepest direct child bottom so sticky-bottom // math can still reach the real final rendered row. - let scrollHeight = Math.ceil(contentYoga?.getComputedHeight() ?? 0) + let scrollHeight = safeUnsignedGeometry(Math.ceil(contentYoga?.getComputedHeight() ?? 0)) if (content) { for (const child of content.childNodes) { const childYoga = (child as DOMElement).yogaNode if (childYoga) { - scrollHeight = Math.max( - scrollHeight, - Math.ceil(childYoga.getComputedTop() + childYoga.getComputedHeight()) - ) + const childBottom = Math.ceil(childYoga.getComputedTop() + childYoga.getComputedHeight()) + + if (validUnsignedGeometry(childBottom)) { + scrollHeight = Math.max(scrollHeight, childBottom) + } } } } // Capture previous scroll bounds BEFORE overwriting — the at-bottom // follow check compares against last frame's max. - const prevScrollHeight = node.scrollHeight ?? scrollHeight - const prevInnerHeight = node.scrollViewportHeight ?? innerHeight + const prevScrollHeight = safeUnsignedGeometry(node.scrollHeight, scrollHeight) + const prevInnerHeight = safeUnsignedGeometry(node.scrollViewportHeight, innerHeight) node.scrollHeight = scrollHeight node.scrollViewportHeight = innerHeight // Absolute screen-buffer row where the scrollable area (inside // padding) begins. Exposed via ScrollBoxHandle.getViewportTop() so // drag-to-scroll can detect when the drag leaves the scroll viewport. - node.scrollViewportTop = (y1 ?? y) + padTop + node.scrollViewportTop = safeUnsignedGeometry((y1 ?? y) + padTop) const maxScroll = Math.max(0, scrollHeight - innerHeight) @@ -751,9 +837,16 @@ function renderNodeToOutput( // plumbing; shipping instant first. stickyScroll overrides. if (node.scrollAnchor) { const anchorTop = node.scrollAnchor.el.yogaNode?.getComputedTop() + const anchorOffset = node.scrollAnchor.offset + const anchorTarget = (anchorTop ?? Number.NaN) + anchorOffset - if (anchorTop != null) { - node.scrollTop = anchorTop + node.scrollAnchor.offset + if ( + anchorTop != null && + validUnsignedGeometry(anchorTop) && + validSignedGeometry(anchorOffset) && + validUnsignedGeometry(anchorTarget) + ) { + node.scrollTop = anchorTarget node.pendingScrollDelta = undefined } @@ -771,8 +864,17 @@ function renderNodeToOutput( // Capture scrollTop before follow so ink.tsx can translate any // active text selection by the same delta (native terminal behavior: // view keeps scrolling, highlight walks up with the text). - const scrollTopBeforeFollow = node.scrollTop ?? 0 + const scrollTopBeforeFollow = safeUnsignedGeometry(node.scrollTop) const stickyBeforeFollow = node.stickyScroll + const scrollTopCompensation = safeSignedGeometry(node.scrollTopCompensation) + + // Compensation is additive and one-shot. Positional bottom-follow + // must judge where the viewport was before the adjustment; otherwise + // a near-tail manual viewport can cross prevMaxScroll solely because + // an above-row grew and be mistaken for an intentional bottom pin. + const scrollTopBeforeCompensation = safeUnsignedGeometry(scrollTopBeforeFollow - scrollTopCompensation) + + node.scrollTopCompensation = undefined const sticky = node.stickyScroll ?? Boolean(node.attributes['stickyScroll']) @@ -783,7 +885,11 @@ function renderNodeToOutput( // because the user was at bottom. const grew = scrollHeight >= prevScrollHeight - const atBottom = sticky || (grew && scrollTopBeforeFollow >= prevMaxScroll) + if (node.pendingScrollDelta !== undefined && !validSignedGeometry(node.pendingScrollDelta)) { + node.pendingScrollDelta = undefined + } + + const atBottom = sticky || (grew && scrollTopBeforeCompensation >= prevMaxScroll) if (atBottom && (node.pendingScrollDelta ?? 0) >= 0) { node.scrollTop = maxScroll @@ -799,7 +905,7 @@ function renderNodeToOutput( // undefined (never set by user action) leave it alone — setting it // would make the sticky flag sticky-by-default and lock out // direct scrollTop writes (e.g. the alt-screen-perf test). - if (node.stickyScroll === false && scrollTopBeforeFollow >= prevMaxScroll) { + if (node.stickyScroll === false && scrollTopBeforeCompensation >= prevMaxScroll) { node.stickyScroll = true } } @@ -822,13 +928,33 @@ function renderNodeToOutput( // (pendingScrollDelta is only set by wheel events, >>50ms after // startup) the probe has resolved — same timing guarantee the // wheel-accel curve relies on. - let cur = node.scrollTop ?? 0 - const pending = node.pendingScrollDelta + let cur = safeUnsignedGeometry(node.scrollTop) + let pending = node.pendingScrollDelta + + if (pending !== undefined && !validSignedGeometry(pending)) { + node.pendingScrollDelta = undefined + pending = undefined + } + const cMin = node.scrollClampMin const cMax = node.scrollClampMax - const haveClamp = cMin !== undefined && cMax !== undefined - if (pending !== undefined && pending !== 0) { + const haveClamp = + cMin !== undefined && + cMax !== undefined && + validUnsignedGeometry(cMin) && + validClampMaximum(cMax) && + cMin <= cMax + + if (!haveClamp && (cMin !== undefined || cMax !== undefined)) { + node.scrollClampMin = undefined + node.scrollClampMax = undefined + } + + // Preserve pending user intent for the compensation paint. Draining + // resumes on the next frame; this keeps the anchor adjustment from + // being conflated with a user move at the old bottom boundary. + if (scrollTopCompensation === 0 && pending !== undefined && pending !== 0) { // Drain continues even past the clamp — the render-clamp below // holds the VISUAL at the mounted edge regardless. Hard-stopping // here caused stop-start jutter: drain hits edge → pause → React @@ -844,14 +970,18 @@ function renderNodeToOutput( const pastClamp = haveClamp && ((pending < 0 && cur < cMin) || (pending > 0 && cur > cMax)) const eff = pastClamp ? Math.min(4, innerHeight >> 3) : innerHeight - cur += isXtermJsHost() ? drainAdaptive(node, pending, eff) : drainProportional(node, pending, eff) - } else if (pending === 0) { + + const drained = + cur + (isXtermJsHost() ? drainAdaptive(node, pending, eff) : drainProportional(node, pending, eff)) + + cur = safeUnsignedGeometry(drained, cur) + } else if (scrollTopCompensation === 0 && pending === 0) { // Opposite scrollBy calls cancelled to zero — clear so we don't // schedule an infinite loop of no-op drain frames. node.pendingScrollDelta = undefined } - let scrollTop = Math.max(0, Math.min(cur, maxScroll)) + let scrollTop = safeUnsignedGeometry(Math.max(0, Math.min(cur, maxScroll))) // Virtual-scroll clamp: if scrollTop raced past the currently-mounted // range (burst PageUp before React re-renders), render at the EDGE of @@ -948,7 +1078,42 @@ function renderNodeToOutput( const prevHeight = contentCached?.height ?? scrollHeight const heightDelta = scrollHeight - prevHeight - const safeForFastPath = !hint || heightDelta === 0 || (hint.delta > 0 && heightDelta === hint.delta) + const heightSafeForFastPath = !hint || heightDelta === 0 || (hint.delta > 0 && heightDelta === hint.delta) + const outputWidth = Number.isSafeInteger(output.width) && output.width > 0 ? output.width : 0 + const outputHeight = Number.isSafeInteger(output.height) && output.height > 0 ? output.height : 0 + + const fastPathBounds = (() => { + if ( + !hint || + !Number.isSafeInteger(hint.top) || + !Number.isSafeInteger(hint.bottom) || + !Number.isSafeInteger(hint.delta) || + hint.delta === 0 || + hint.top > hint.bottom || + !Number.isFinite(x) || + !Number.isFinite(width) || + width < 0 || + !Number.isFinite(childVisibleX1) || + !Number.isFinite(childVisibleX2) || + !Number.isFinite(childVisibleY1) || + !Number.isFinite(childVisibleY2) + ) { + return null + } + + const x1 = Math.max(0, Math.floor(x), Math.floor(childVisibleX1)) + const x2 = Math.min(outputWidth, Math.ceil(x + width), Math.ceil(childVisibleX2)) + const top = Math.max(0, hint.top, Math.floor(childVisibleY1)) + const bottom = Math.min(outputHeight, hint.bottom + 1, Math.ceil(childVisibleY2)) + + if (x1 >= x2 || top >= bottom || Math.abs(hint.delta) > bottom - top) { + return null + } + + return { bottom, top, width: x2 - x1, x: x1 } + })() + + const safeForFastPath = heightSafeForFastPath && (!hint || fastPathBounds !== null) // Diagnostics (opt-in via scrollFastPathStats reader). Only // counts when a hint was captured — cases where nothing scrolled @@ -960,9 +1125,12 @@ function renderNodeToOutput( scrollFastPathStats.lastPrevHeight = prevHeight scrollFastPathStats.lastHeightDelta = heightDelta - if (!safeForFastPath) { + if (!heightSafeForFastPath) { scrollFastPathStats.declined.heightDeltaMismatch++ scrollFastPathStats.lastDeclineReason = `heightDelta=${heightDelta} hintDelta=${hint.delta}` + } else if (!fastPathBounds) { + scrollFastPathStats.declined.other++ + scrollFastPathStats.lastDeclineReason = 'invalidOrEmptyRepairBounds' } else if (!prevScreen) { scrollFastPathStats.declined.noPrevScreen++ scrollFastPathStats.lastDeclineReason = 'noPrevScreen' @@ -979,26 +1147,19 @@ function renderNodeToOutput( scrollHint = null } - if (hint && prevScreen && safeForFastPath) { - const { top, bottom, delta } = hint - const w = Math.floor(width) - output.blit(prevScreen, Math.floor(x), top, w, bottom - top + 1) - output.shift(top, bottom, delta) + if (hint && prevScreen && safeForFastPath && fastPathBounds) { + const { delta } = hint + const { bottom, top, width: repairWidth, x: repairX } = fastPathBounds + const bottomInclusive = bottom - 1 + + // Keep the terminal hint aligned with the same bounded rows used + // to construct next.screen. Valid in-bounds hints are unchanged. + scrollHint = { bottom: bottomInclusive, delta, top } + output.blit(prevScreen, repairX, top, repairWidth, bottom - top) + output.shift(top, bottomInclusive, delta) // Edge rows: new content entering the viewport. - const edgeTop = delta > 0 ? bottom - delta + 1 : top - const edgeBottom = delta > 0 ? bottom : top - delta - 1 - output.clear({ - x: Math.floor(x), - y: edgeTop, - width: w, - height: edgeBottom - edgeTop + 1 - }) - output.clip({ - x1: undefined, - x2: undefined, - y1: edgeTop, - y2: edgeBottom + 1 - }) + const edgeTop = Math.max(top, delta > 0 ? bottom - delta : top) + const edgeBottom = Math.min(bottom, delta > 0 ? bottom : top - delta) // Snapshot dirty children before the first pass — the first // pass clears dirty flags, and edge-spanning children would be @@ -1007,20 +1168,33 @@ function renderNodeToOutput( ? new Set(content.childNodes.filter(c => (c as DOMElement).dirty)) : null - renderScrolledChildren( - content, - output, - contentX, - contentY, - hasRemovedChild, - undefined, - // Cull to edge in child-local coords (inverse of contentY offset). - edgeTop - contentY, - edgeBottom + 1 - contentY, - boxBackgroundColor, - true - ) - output.unclip() + if (edgeTop < edgeBottom) { + output.clear({ + x: repairX, + y: edgeTop, + width: repairWidth, + height: edgeBottom - edgeTop + }) + output.clip({ x1: repairX, x2: repairX + repairWidth, y1: edgeTop, y2: edgeBottom }) + renderScrolledChildren( + content, + output, + contentX, + contentY, + hasRemovedChild, + undefined, + // Cull to edge in child-local coords (inverse of contentY offset). + edgeTop - contentY, + edgeBottom - contentY, + boxBackgroundColor, + true, + repairX, + repairX + repairWidth, + edgeTop, + edgeBottom + ) + output.unclip() + } // Second pass: re-render children in stable rows whose screen // position doesn't match where the shift put their old pixels. @@ -1040,8 +1214,7 @@ function renderNodeToOutput( // path preserved. if (dirtyChildren) { const edgeTopLocal = edgeTop - contentY - const edgeBottomLocal = edgeBottom + 1 - contentY - const spaces = ' '.repeat(w) + const edgeBottomLocal = edgeBottom - contentY // Track cumulative height change of children iterated so far. // A clean child's yogaTop is unchanged iff this is zero (no // sibling above it grew/shrank/mounted). When zero, the skip @@ -1075,8 +1248,17 @@ function renderNodeToOutput( continue } + const childLeft = cy.getComputedLeft() const childTop = cy.getComputedTop() + const childW = cy.getComputedWidth() const childH = cy.getComputedHeight() + + if (!validYogaRect(childLeft, childTop, childW, childH)) { + dropSubtreeCache(childElem) + + continue + } + const childBottom = childTop + childH if (isDirty) { @@ -1094,7 +1276,23 @@ function renderNodeToOutput( continue } - const screenY = Math.floor(contentY + childTop) + const childScreenX = contentX + childLeft + const childScreenRight = childScreenX + childW + const childScreenY = contentY + childTop + const childScreenBottom = contentY + childBottom + + if ( + !validSignedGeometry(childScreenX) || + !validSignedGeometry(childScreenRight) || + !validSignedGeometry(childScreenY) || + !validSignedGeometry(childScreenBottom) + ) { + dropSubtreeCache(childElem) + + continue + } + + const screenY = Math.floor(childScreenY) // Clean children reaching here have cumHeightShift ≠ 0 OR // no cache. Re-check precisely: cached.y − delta is where @@ -1113,28 +1311,39 @@ function renderNodeToOutput( // Wipe this child's region with spaces to overwrite stale // blitted content — output.clear() only expands damage and // cannot zero cells that the blit already wrote. - const screenBottom = Math.min( - Math.floor(contentY + childBottom), + const repairChildX = Math.max(repairX, Math.floor(childScreenX)) + const repairChildRight = Math.min(repairX + repairWidth, Math.ceil(childScreenRight)) + const repairChildTop = Math.max(top, screenY) + + const repairChildBottom = Math.min( + bottom, + Math.floor(childScreenBottom), Math.floor((y1 ?? y) + padTop + innerHeight) ) - if (screenY < screenBottom) { - const fill = Array(screenBottom - screenY) + if (repairChildX < repairChildRight && repairChildTop < repairChildBottom) { + const spaces = ' '.repeat(repairChildRight - repairChildX) + + const fill = Array(repairChildBottom - repairChildTop) .fill(spaces) .join('\n') - output.write(Math.floor(x), screenY, fill) + output.write(repairChildX, repairChildTop, fill) output.clip({ - x1: undefined, - x2: undefined, - y1: screenY, - y2: screenBottom + x1: repairChildX, + x2: repairChildRight, + y1: repairChildTop, + y2: repairChildBottom }) renderNodeToOutput(childElem, output, { offsetX: contentX, offsetY: contentY, prevScreen: undefined, - inheritedBackgroundColor: boxBackgroundColor + inheritedBackgroundColor: boxBackgroundColor, + visibleX1: repairChildX, + visibleX2: repairChildRight, + visibleY1: repairChildTop, + visibleY2: repairChildBottom }) output.unclip() } @@ -1148,34 +1357,36 @@ function renderNodeToOutput( // pixels sit at (rect.y - delta) — neither edge render nor the // overlay's own re-render covers them. Wipe and re-render // ScrollBox content so the diff writes correct cells. - const spaces = absoluteRectsPrev.length ? ' '.repeat(w) : '' - for (const r of absoluteRectsPrev) { - if (r.y >= bottom + 1 || r.y + r.height <= top) { + if (!validYogaRect(r.x, r.y, r.width, r.height)) { continue } + const repairOverlayX = Math.max(repairX, Math.floor(r.x)) + const repairOverlayRight = Math.min(repairX + repairWidth, Math.ceil(r.x + r.width)) const shiftedTop = Math.max(top, Math.floor(r.y) - delta) - const shiftedBottom = Math.min(bottom + 1, Math.floor(r.y + r.height) - delta) + const shiftedBottom = Math.min(bottom, Math.floor(r.y + r.height) - delta) // Skip if entirely within edge rows (already rendered). - if (shiftedTop >= edgeTop && shiftedBottom <= edgeBottom + 1) { + if (edgeTop < edgeBottom && shiftedTop >= edgeTop && shiftedBottom <= edgeBottom) { continue } - if (shiftedTop >= shiftedBottom) { + if (repairOverlayX >= repairOverlayRight || shiftedTop >= shiftedBottom) { continue } + const spaces = ' '.repeat(repairOverlayRight - repairOverlayX) + const fill = Array(shiftedBottom - shiftedTop) .fill(spaces) .join('\n') - output.write(Math.floor(x), shiftedTop, fill) + output.write(repairOverlayX, shiftedTop, fill) output.clip({ - x1: undefined, - x2: undefined, + x1: repairOverlayX, + x2: repairOverlayRight, y1: shiftedTop, y2: shiftedBottom }) @@ -1189,7 +1400,11 @@ function renderNodeToOutput( shiftedTop - contentY, shiftedBottom - contentY, boxBackgroundColor, - true + true, + repairOverlayX, + repairOverlayRight, + shiftedTop, + shiftedBottom ) output.unclip() } @@ -1233,7 +1448,12 @@ function renderNodeToOutput( scrolled || positionChanged ? undefined : prevScreen, scrollTop, scrollTop + innerHeight, - boxBackgroundColor + boxBackgroundColor, + false, + childVisibleX1, + childVisibleX2, + childVisibleY1, + childVisibleY2 ) } @@ -1264,14 +1484,23 @@ function renderNodeToOutput( const innerHeight = Math.floor(height) - borderTop - borderBottom if (innerWidth > 0 && innerHeight > 0) { - const spaces = ' '.repeat(innerWidth) + const fillX1 = Math.max(0, Math.floor(x + borderLeft)) + const fillX2 = Math.min(output.width, Math.ceil(x + width - borderRight)) + const fillY1 = Math.max(childVisibleY1, Math.floor(y + borderTop)) + const fillY2 = Math.min(childVisibleY2, Math.ceil(y + height - borderBottom)) + const fillWidth = Math.max(0, fillX2 - fillX1) + const fillHeight = Math.max(0, fillY2 - fillY1) + + const spaces = ' '.repeat(fillWidth) const fillLine = ownBackgroundColor ? applyTextStyles(spaces, { backgroundColor: ownBackgroundColor }) : spaces - const fill = Array(innerHeight).fill(fillLine).join('\n') - output.write(x + borderLeft, y + borderTop, fill) + if (fillWidth > 0 && fillHeight > 0) { + const fill = Array(fillHeight).fill(fillLine).join('\n') + output.write(fillX1, fillY1, fill) + } } } @@ -1289,7 +1518,11 @@ function renderNodeToOutput( // valid composite, but children CAN reposition (ScrollBox remeasure // on re-render → /permissions body blanked on Down arrow, #25436). ownBackgroundColor || node.style.opaque ? undefined : prevScreen, - boxBackgroundColor + boxBackgroundColor, + childVisibleX1, + childVisibleX2, + childVisibleY1, + childVisibleY2 ) } @@ -1300,9 +1533,21 @@ function renderNodeToOutput( // Render border AFTER children to ensure it's not overwritten by child // clearing operations. When a child shrinks, it clears its old area, // which may overlap with where the parent's border now is. - renderBorder(x, y, node, output) + renderBorder(x, y, node, output, activeVisibleX1, activeVisibleX2, activeVisibleY1, activeVisibleY2) } else if (node.nodeName === 'ink-root') { - renderChildren(node, output, x, y, hasRemovedChild, prevScreen, inheritedBackgroundColor) + renderChildren( + node, + output, + x, + y, + hasRemovedChild, + prevScreen, + inheritedBackgroundColor, + activeVisibleX1, + activeVisibleX2, + activeVisibleY1, + activeVisibleY2 + ) } // Cache layout bounds for dirty tracking @@ -1352,7 +1597,11 @@ function renderChildren( offsetY: number, hasRemovedChild: boolean, prevScreen: Screen | undefined, - inheritedBackgroundColor: Color | undefined + inheritedBackgroundColor: Color | undefined, + visibleX1: number, + visibleX2: number, + visibleY1: number, + visibleY2: number ): void { let seenDirtyChild = false let seenDirtyClipped = false @@ -1370,7 +1619,11 @@ function renderChildren( // the opaque/bg reads don't happen per-child per-frame. skipSelfBlit: seenDirtyClipped && isAbsolute && !childElem.style.opaque && childElem.style.backgroundColor === undefined, - inheritedBackgroundColor + inheritedBackgroundColor, + visibleX1, + visibleX2, + visibleY1, + visibleY2 }) if (wasDirty && !seenDirtyChild) { @@ -1499,7 +1752,11 @@ function renderScrolledChildren( // When true (DECSTBM fast path), culled children keep their cache — // the blit+shift put stable rows in next.screen so stale cache is // never read. Avoids walking O(total_children * subtree_depth) per frame. - preserveCulledCache = false + preserveCulledCache = false, + visibleX1 = 0, + visibleX2 = output.width, + visibleY1 = 0, + visibleY2 = output.height ): void { let seenDirtyChild = false // Track cumulative height shift of dirty children iterated so far. When @@ -1527,6 +1784,14 @@ function renderScrolledChildren( top = cy.getComputedTop() height = cy.getComputedHeight() + const bottom = top + height + + if (!validSignedGeometry(top) || !validYogaDimension(height) || !validSignedGeometry(bottom) || bottom < top) { + dropSubtreeCache(childElem) + + continue + } + if (childElem.dirty) { cumHeightShift += height - (cached ? cached.height : 0) } @@ -1542,6 +1807,12 @@ function renderScrolledChildren( const bottom = top + height + if (!validSignedGeometry(top) || !validYogaDimension(height) || !validSignedGeometry(bottom) || bottom < top) { + dropSubtreeCache(childElem) + + continue + } + if (bottom <= scrollTopY || top >= scrollBottomY) { // Culled — outside visible window. Drop stale cache entries from // the subtree so when this child re-enters it doesn't fire clears @@ -1560,7 +1831,11 @@ function renderScrolledChildren( offsetX, offsetY, prevScreen: hasRemovedChild || seenDirtyChild ? undefined : prevScreen, - inheritedBackgroundColor + inheritedBackgroundColor, + visibleX1, + visibleX2, + visibleY1: Math.max(visibleY1, offsetY + scrollTopY), + visibleY2: Math.min(visibleY2, offsetY + scrollBottomY) }) if (wasDirty) { diff --git a/ui-tui/scripts/bench-history-scroll.tsx b/ui-tui/scripts/bench-history-scroll.tsx new file mode 100644 index 0000000000..7f7a73234f --- /dev/null +++ b/ui-tui/scripts/bench-history-scroll.tsx @@ -0,0 +1,483 @@ +// Deterministic virtual-history benchmark. The file intentionally uses only +// APIs present before the performance candidate so the exact same script can +// be copied/run on base and candidate checkouts. +// +// Run from ui-tui: +// npx tsx scripts/bench-history-scroll.tsx +// npx tsx scripts/bench-history-scroll.tsx --warmups=2 --samples=5 --items=100,1000,10000 +// +// In addition to the virtual-history workloads, every run mounts one +// oversized bordered/fill box at each RENDERER_EXTENT inside the fixed +// viewport. Keeping that tree to a few Yoga nodes isolates renderer clipping +// from node-construction cost and makes the workload revision-comparable. + +import { PassThrough } from 'stream' + +import { Box, renderSync, ScrollBox, type ScrollBoxHandle, Text } from '@hermes/ink' +import React, { useLayoutEffect, useRef } from 'react' + +import { useVirtualHistory } from '../src/hooks/useVirtualHistory.js' + +const DEFAULT_WORKLOADS = [100, 1_000, 10_000] +const RENDERER_EXTENTS = [100, 1_000, 10_000] +const DEFAULT_WARMUPS = 1 +const DEFAULT_SAMPLES = 5 +const COLUMNS = 100 +const ROWS = 30 +const MAX_MOUNTED = 120 + +interface BenchItem { + height: number + key: string + text: string +} + +interface Exposed { + scroll: ScrollBoxHandle | null + virtual: ReturnType +} + +interface Sample { + anchorError: number + heapDeltaBytes: number | null + invalidOffsets: number + measuredHeightReconciliationMs: number + mountMs: number + mountedRowsMax: number + nonMonotoneOffsets: number + rerenderMs: number + scrollMs: number + terminalBytes: number + terminalWrites: number +} + +interface WorkloadResult { + distributions: { + anchorError: ReturnType + heapDeltaBytes: ReturnType + measuredHeightReconciliationMs: ReturnType + mountMs: ReturnType + mountedRowsMax: ReturnType + rerenderMs: ReturnType + scrollMs: ReturnType + terminalBytes: ReturnType + terminalWrites: ReturnType + } + invalidOffsets: number + itemCount: number + nonMonotoneOffsets: number + samples: Sample[] +} + +interface OversizedRendererSample { + freshMountRenderMs: number + terminalBytes: number + terminalWrites: number +} + +interface OversizedRendererResult { + distributions: { + freshMountRenderMs: ReturnType + terminalBytes: ReturnType + terminalWrites: ReturnType + } + extent: number + samples: OversizedRendererSample[] +} + +class CountingStream extends PassThrough { + columns = COLUMNS + rows = ROWS + isTTY = false + bytes = 0 + writes = 0 + + override _write(chunk: Buffer | string, encoding: BufferEncoding, callback: (error?: Error | null) => void) { + this.bytes += Buffer.byteLength(chunk) + this.writes++ + callback() + } +} + +const immediate = () => new Promise(resolve => setImmediate(resolve)) + +async function settle(frames = 4) { + for (let frame = 0; frame < frames; frame++) { + await immediate() + } +} + +async function waitUntil(predicate: () => boolean, attempts = 40) { + for (let attempt = 0; attempt < attempts; attempt++) { + if (predicate()) { + return true + } + + await immediate() + } + + return predicate() +} + +function makeItems(count: number): BenchItem[] { + return Array.from({ length: count }, (_, index) => ({ + height: 1 + ((index * 17) % 4), + key: `row-${index}`, + text: `row ${index} ${'history '.repeat(2 + (index % 5))}` + })) +} + +function Harness({ expose, items }: { expose: React.MutableRefObject; items: readonly BenchItem[] }) { + const scrollRef = useRef(null) + + const virtual = useVirtualHistory(scrollRef, items, COLUMNS, { + coldStartCount: 30, + estimateHeight: index => items[index]?.height ?? 1, + maxMounted: MAX_MOUNTED, + overscan: 20 + }) + + useLayoutEffect(() => { + expose.current = { scroll: scrollRef.current, virtual } + }) + + return ( + + + {virtual.topSpacer > 0 ? : null} + {items.slice(virtual.start, virtual.end).map(item => ( + + {item.text} + + ))} + {virtual.bottomSpacer > 0 ? : null} + + + ) +} + +function OversizedRendererHarness({ extent }: { extent: number }) { + return ( + + + deterministic oversized height workload + + + deterministic oversized width workload + + + ) +} + +function inspectOffsets(offsets: ArrayLike, count: number) { + let invalidOffsets = 0 + let nonMonotoneOffsets = 0 + + for (let index = 0; index <= count; index++) { + const value = offsets[index] + + if (!Number.isFinite(value)) { + invalidOffsets++ + } + + if (index > 0 && value! < offsets[index - 1]!) { + nonMonotoneOffsets++ + } + } + + return { invalidOffsets, nonMonotoneOffsets } +} + +async function runSample(itemCount: number): Promise { + const stdout = new CountingStream() + const stderr = new CountingStream() + const stdin = new PassThrough() + const expose = { current: null as Exposed | null } + let items = makeItems(itemCount) + const heapBefore = process.memoryUsage?.().heapUsed ?? null + const mountStart = performance.now() + + const instance = renderSync(, { + patchConsole: false, + stderr: stderr as unknown as NodeJS.WriteStream, + stdin: stdin as unknown as NodeJS.ReadStream, + stdout: stdout as unknown as NodeJS.WriteStream + }) + + await waitUntil(() => expose.current?.scroll !== null) + await settle() + const mountMs = performance.now() - mountStart + let mountedRowsMax = expose.current!.virtual.end - expose.current!.virtual.start + + const rerenderItems = items.map((item, index) => + index === items.length - 1 ? { ...item, text: `${item.text} rerender` } : item + ) + + const rerenderStart = performance.now() + + instance.rerender() + await settle() + const rerenderMs = performance.now() - rerenderStart + items = rerenderItems + mountedRowsMax = Math.max(mountedRowsMax, expose.current!.virtual.end - expose.current!.virtual.start) + + const scroll = expose.current!.scroll! + const total = expose.current!.virtual.offsets[itemCount] ?? 0 + const scrollStart = performance.now() + + scroll.scrollTo(Math.max(0, Math.floor(total * 0.55))) + await settle(8) + const scrollMs = performance.now() - scrollStart + mountedRowsMax = Math.max(mountedRowsMax, expose.current!.virtual.end - expose.current!.virtual.start) + + const beforeOffsets = expose.current!.virtual.offsets + const beforeTop = scroll.getScrollTop() + let measuredIndex = expose.current!.virtual.start + + while (measuredIndex + 1 < expose.current!.virtual.end && (beforeOffsets[measuredIndex + 1] ?? 0) > beforeTop) { + measuredIndex++ + } + + if ((beforeOffsets[measuredIndex + 1] ?? Number.POSITIVE_INFINITY) > beforeTop) { + measuredIndex = Math.max(expose.current!.virtual.start, measuredIndex - 1) + } + + const heightDelta = 3 + const oldTotal = beforeOffsets[itemCount] ?? 0 + + const measuredItems = items.map((item, index) => + index === measuredIndex ? { ...item, height: item.height + heightDelta } : item + ) + + const reconcileStart = performance.now() + + instance.rerender() + await waitUntil(() => (expose.current!.virtual.offsets[itemCount] ?? 0) === oldTotal + heightDelta) + await settle(2) + const measuredHeightReconciliationMs = performance.now() - reconcileStart + const measuredWasAbove = (beforeOffsets[measuredIndex + 1] ?? 0) <= beforeTop + const expectedTop = beforeTop + (measuredWasAbove ? heightDelta : 0) + const anchorError = Math.abs(scroll.getScrollTop() - expectedTop) + mountedRowsMax = Math.max(mountedRowsMax, expose.current!.virtual.end - expose.current!.virtual.start) + + const offsetHealth = inspectOffsets(expose.current!.virtual.offsets, itemCount) + const heapAfter = process.memoryUsage?.().heapUsed ?? null + const heapDeltaBytes = heapBefore === null || heapAfter === null ? null : heapAfter - heapBefore + const terminalBytes = stdout.bytes + const terminalWrites = stdout.writes + + instance.unmount() + instance.cleanup() + stdin.destroy() + stdout.destroy() + stderr.destroy() + + return { + anchorError, + heapDeltaBytes, + ...offsetHealth, + measuredHeightReconciliationMs, + mountMs, + mountedRowsMax, + rerenderMs, + scrollMs, + terminalBytes, + terminalWrites + } +} + +async function runOversizedRendererSample(extent: number): Promise { + const stdout = new CountingStream() + const stderr = new CountingStream() + const stdin = new PassThrough() + const mountStart = performance.now() + let instance: ReturnType | undefined + + try { + instance = renderSync(, { + patchConsole: false, + stderr: stderr as unknown as NodeJS.WriteStream, + stdin: stdin as unknown as NodeJS.ReadStream, + stdout: stdout as unknown as NodeJS.WriteStream + }) + + const rendered = await waitUntil(() => stdout.writes > 0) + + if (!rendered) { + throw new Error(`oversized renderer extent ${extent} did not produce a terminal frame`) + } + + return { + freshMountRenderMs: performance.now() - mountStart, + terminalBytes: stdout.bytes, + terminalWrites: stdout.writes + } + } finally { + instance?.unmount() + instance?.cleanup() + + stdin.destroy() + stdout.destroy() + stderr.destroy() + } +} + +function distribution(values: number[]) { + const sorted = [...values].sort((a, b) => a - b) + + const percentile = (p: number) => + sorted[Math.min(sorted.length - 1, Math.max(0, Math.ceil(sorted.length * p) - 1))] ?? 0 + + return { + max: sorted.at(-1) ?? 0, + mean: sorted.reduce((sum, value) => sum + value, 0) / Math.max(1, sorted.length), + min: sorted[0] ?? 0, + p50: percentile(0.5), + p95: percentile(0.95), + p99: percentile(0.99) + } +} + +function numericArg(name: string, fallback: number) { + const raw = process.argv + .slice(2) + .find(arg => arg.startsWith(`--${name}=`)) + ?.split('=', 2)[1] + + const parsed = Number(raw) + + return Number.isSafeInteger(parsed) && parsed >= 0 ? parsed : fallback +} + +function workloadsArg() { + const raw = process.argv + .slice(2) + .find(arg => arg.startsWith('--items=')) + ?.split('=', 2)[1] + + if (!raw) { + return DEFAULT_WORKLOADS + } + + const parsed = raw.split(',').map(Number) + + if (parsed.some(value => !Number.isSafeInteger(value) || value <= 0)) { + throw new Error(`invalid --items workload list: ${raw}`) + } + + return parsed +} + +async function main() { + const workloads = workloadsArg() + const warmups = numericArg('warmups', DEFAULT_WARMUPS) + const samplesPerWorkload = numericArg('samples', DEFAULT_SAMPLES) + const results: WorkloadResult[] = [] + const oversizedRendererResults: OversizedRendererResult[] = [] + + for (const itemCount of workloads) { + for (let warmup = 0; warmup < warmups; warmup++) { + await runSample(itemCount) + } + + const samples: Sample[] = [] + + for (let sample = 0; sample < samplesPerWorkload; sample++) { + samples.push(await runSample(itemCount)) + } + + results.push({ + itemCount, + distributions: { + anchorError: distribution(samples.map(sample => sample.anchorError)), + heapDeltaBytes: distribution(samples.flatMap(sample => sample.heapDeltaBytes ?? [])), + measuredHeightReconciliationMs: distribution(samples.map(sample => sample.measuredHeightReconciliationMs)), + mountMs: distribution(samples.map(sample => sample.mountMs)), + mountedRowsMax: distribution(samples.map(sample => sample.mountedRowsMax)), + rerenderMs: distribution(samples.map(sample => sample.rerenderMs)), + scrollMs: distribution(samples.map(sample => sample.scrollMs)), + terminalBytes: distribution(samples.map(sample => sample.terminalBytes)), + terminalWrites: distribution(samples.map(sample => sample.terminalWrites)) + }, + invalidOffsets: samples.reduce((sum, sample) => sum + sample.invalidOffsets, 0), + nonMonotoneOffsets: samples.reduce((sum, sample) => sum + sample.nonMonotoneOffsets, 0), + samples + }) + } + + for (const extent of RENDERER_EXTENTS) { + for (let warmup = 0; warmup < warmups; warmup++) { + await runOversizedRendererSample(extent) + } + + const samples: OversizedRendererSample[] = [] + + for (let sample = 0; sample < samplesPerWorkload; sample++) { + samples.push(await runOversizedRendererSample(extent)) + } + + oversizedRendererResults.push({ + distributions: { + freshMountRenderMs: distribution(samples.map(sample => sample.freshMountRenderMs)), + terminalBytes: distribution(samples.map(sample => sample.terminalBytes)), + terminalWrites: distribution(samples.map(sample => sample.terminalWrites)) + }, + extent, + samples + }) + } + + const scaling = results.slice(1).map((result, index) => { + const previous = results[index]! + + const ratio = (metric: keyof (typeof result)['distributions']) => + result.distributions[metric].p50 / Math.max(Number.EPSILON, previous.distributions[metric].p50) + + return { + fromItems: previous.itemCount, + itemFactor: result.itemCount / previous.itemCount, + measuredHeightReconciliationP50Factor: ratio('measuredHeightReconciliationMs'), + mountP50Factor: ratio('mountMs'), + rerenderP50Factor: ratio('rerenderMs'), + scrollP50Factor: ratio('scrollMs'), + terminalBytesP50Factor: ratio('terminalBytes'), + toItems: result.itemCount + } + }) + + const oversizedRendererScaling = oversizedRendererResults.slice(1).map((result, index) => { + const previous = oversizedRendererResults[index]! + + const ratio = (metric: keyof (typeof result)['distributions']) => + result.distributions[metric].p50 / Math.max(Number.EPSILON, previous.distributions[metric].p50) + + return { + extentFactor: result.extent / previous.extent, + freshMountRenderP50Factor: ratio('freshMountRenderMs'), + fromExtent: previous.extent, + terminalBytesP50Factor: ratio('terminalBytes'), + terminalWritesP50Factor: ratio('terminalWrites'), + toExtent: result.extent + } + }) + + process.stdout.write( + `${JSON.stringify( + { + config: { columns: COLUMNS, maxMounted: MAX_MOUNTED, rows: ROWS, samples: samplesPerWorkload, warmups }, + oversizedRenderer: { + extents: RENDERER_EXTENTS, + results: oversizedRendererResults, + scaling: oversizedRendererScaling + }, + results, + scaling, + workloads + }, + null, + 2 + )}\n` + ) +} + +await main() diff --git a/ui-tui/src/__tests__/messages.test.ts b/ui-tui/src/__tests__/messages.test.ts index d5baa1b311..e572bd5b8c 100644 --- a/ui-tui/src/__tests__/messages.test.ts +++ b/ui-tui/src/__tests__/messages.test.ts @@ -5,8 +5,9 @@ import React from 'react' import { describe, expect, it } from 'vitest' import { MessageLine } from '../components/messageLine.js' +import { MAX_HISTORY } from '../config/limits.js' import { toTranscriptMessages } from '../domain/messages.js' -import { upsert } from '../lib/messages.js' +import { capTranscriptHistory, upsert } from '../lib/messages.js' import { stripAnsi } from '../lib/text.js' import { DEFAULT_THEME } from '../theme.js' @@ -148,3 +149,16 @@ describe('upsert', () => { expect(prev).toHaveLength(1) }) }) + +describe('capTranscriptHistory', () => { + it('keeps the intro and the newest bounded display rows', () => { + const intro = { kind: 'intro' as const, role: 'system' as const, text: '' } + const rows = Array.from({ length: 1_005 }, (_, index) => ({ role: 'user' as const, text: `m${index}` })) + const capped = capTranscriptHistory([intro, ...rows]) + + expect(capped).toHaveLength(MAX_HISTORY) + expect(capped[0]).toBe(intro) + expect(capped[1]?.text).toBe(`m${rows.length - (MAX_HISTORY - 1)}`) + expect(capped.at(-1)?.text).toBe('m1004') + }) +}) diff --git a/ui-tui/src/__tests__/scroll.test.ts b/ui-tui/src/__tests__/scroll.test.ts index b9bbdb5fea..89f2356b92 100644 --- a/ui-tui/src/__tests__/scroll.test.ts +++ b/ui-tui/src/__tests__/scroll.test.ts @@ -13,12 +13,13 @@ function makeScroll(overrides: Partial> = {}) { getViewportHeight: vi.fn(() => 20), getViewportTop: vi.fn(() => 0), scrollBy: vi.fn(), + scrollTo: vi.fn(), ...overrides } } describe('scrollWithSelectionBy', () => { - it('clamps to the actual remaining scroll distance before calling scrollBy', () => { + it('commits the clamped target directly instead of queueing a scroll delta', () => { const s = makeScroll({ getScrollHeight: vi.fn(() => 30), getScrollTop: vi.fn(() => 9), @@ -34,7 +35,8 @@ describe('scrollWithSelectionBy', () => { scrollWithSelectionBy(10, { scrollRef: { current: s as never }, selection }) - expect(s.scrollBy).toHaveBeenCalledWith(1) + expect(s.scrollTo).toHaveBeenCalledWith(10) + expect(s.scrollBy).not.toHaveBeenCalled() }) it('uses fresh scroll height when cached height would swallow a down-scroll at a fake bottom', () => { @@ -54,7 +56,8 @@ describe('scrollWithSelectionBy', () => { scrollWithSelectionBy(10, { scrollRef: { current: s as never }, selection }) - expect(s.scrollBy).toHaveBeenCalledWith(4) + expect(s.scrollTo).toHaveBeenCalledWith(14) + expect(s.scrollBy).not.toHaveBeenCalled() }) it('uses fresh height when pending down-scroll reaches the cached fake bottom', () => { @@ -75,7 +78,8 @@ describe('scrollWithSelectionBy', () => { scrollWithSelectionBy(10, { scrollRef: { current: s as never }, selection }) - expect(s.scrollBy).toHaveBeenCalledWith(6) + expect(s.scrollTo).toHaveBeenCalledWith(18) + expect(s.scrollBy).not.toHaveBeenCalled() }) it('does nothing at the edge instead of queueing dead pending deltas', () => { @@ -94,6 +98,29 @@ describe('scrollWithSelectionBy', () => { scrollWithSelectionBy(10, { scrollRef: { current: s as never }, selection }) + expect(s.scrollTo).not.toHaveBeenCalled() + expect(s.scrollBy).not.toHaveBeenCalled() + }) + + it('preserves selection capture and shifting on the direct path', () => { + const s = makeScroll({ + getScrollTop: vi.fn(() => 10), + getViewportHeight: vi.fn(() => 20), + getViewportTop: vi.fn(() => 5) + }) + + const selection = { + captureScrolledRows: vi.fn(), + getState: vi.fn(() => ({ anchor: { row: 10 }, focus: { row: 12 } })), + shiftAnchor: vi.fn(), + shiftSelection: vi.fn() + } + + scrollWithSelectionBy(3, { scrollRef: { current: s as never }, selection }) + + expect(selection.captureScrolledRows).toHaveBeenCalledWith(5, 7, 'above') + expect(selection.shiftSelection).toHaveBeenCalledWith(-3, 5, 24) + expect(s.scrollTo).toHaveBeenCalledWith(13) expect(s.scrollBy).not.toHaveBeenCalled() }) }) diff --git a/ui-tui/src/__tests__/scrollBoxRendererBounds.test.ts b/ui-tui/src/__tests__/scrollBoxRendererBounds.test.ts new file mode 100644 index 0000000000..25ed017e48 --- /dev/null +++ b/ui-tui/src/__tests__/scrollBoxRendererBounds.test.ts @@ -0,0 +1,627 @@ +import { PassThrough } from 'stream' + +import { Box, renderSync, ScrollBox, type ScrollBoxHandle, Text } from '@hermes/ink' +import React, { useLayoutEffect, useRef } from 'react' +import { describe, expect, it, vi } from 'vitest' + +import SourceBox from '../../packages/hermes-ink/src/ink/components/Box.js' +import SourceScrollBox from '../../packages/hermes-ink/src/ink/components/ScrollBox.js' +import SourceText from '../../packages/hermes-ink/src/ink/components/Text.js' +import type { DOMElement } from '../../packages/hermes-ink/src/ink/dom.js' +import Output from '../../packages/hermes-ink/src/ink/output.js' +import { scrollFastPathStats as sourceScrollFastPathStats } from '../../packages/hermes-ink/src/ink/render-node-to-output.js' +import { renderSync as renderSourceSync } from '../../packages/hermes-ink/src/ink/root.js' +import { useVirtualHistory } from '../hooks/useVirtualHistory.js' + +interface Item { + height: number + heightAfterResize?: number + key: string + text?: string +} + +interface Exposed { + scroll: ScrollBoxHandle | null + virtualHistory: ReturnType +} + +const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)) + +const makeStreams = () => { + const stdout = new PassThrough() + const stdin = new PassThrough() + const stderr = new PassThrough() + + Object.assign(stdout, { columns: 80, isTTY: false, rows: 20 }) + Object.assign(stdin, { isTTY: false }) + Object.assign(stderr, { isTTY: false }) + stdout.on('data', () => {}) + + return { stderr, stdin, stdout } +} + +const itemHeightForColumns = (item: Item | undefined, columns: number) => + columns >= 80 ? (item?.heightAfterResize ?? item?.height ?? 1) : (item?.height ?? 1) + +function Harness({ + columns = 80, + expose, + height = 10, + generation = 0, + initialHeights, + items, + maxMounted = 16 +}: { + columns?: number + expose: React.MutableRefObject + height?: number + generation?: number + initialHeights?: ReadonlyMap + items: readonly Item[] + maxMounted?: number +}) { + const scrollRef = useRef(null) + + const virtualHistory = useVirtualHistory(scrollRef, items, columns, { + coldStartCount: 16, + estimateHeight: index => itemHeightForColumns(items[index], columns), + generation, + initialHeights, + maxMounted, + overscan: 2 + }) + + useLayoutEffect(() => { + expose.current = { scroll: scrollRef.current, virtualHistory } + }) + + return React.createElement( + ScrollBox, + { flexDirection: 'column', height, ref: scrollRef, stickyScroll: true }, + React.createElement( + Box, + { flexDirection: 'column', width: '100%' }, + virtualHistory.topSpacer > 0 ? React.createElement(Box, { height: virtualHistory.topSpacer }) : null, + ...items.slice(virtualHistory.start, virtualHistory.end).map(item => + React.createElement( + Box, + { + height: itemHeightForColumns(item, columns), + key: item.key, + ref: virtualHistory.measureRef(item.key) + }, + React.createElement(Text, null, item.text ?? item.key) + ) + ), + virtualHistory.bottomSpacer > 0 ? React.createElement(Box, { height: virtualHistory.bottomSpacer }) : null + ) + ) +} + +function CorruptGeometryHarness({ expose, tick }: { expose: React.MutableRefObject; tick: number }) { + const nodes = useRef([]) + + useLayoutEffect(() => { + expose.current = nodes.current + }) + + return React.createElement( + ScrollBox, + { flexDirection: 'column', height: 8 }, + ...['nan-top', 'positive-infinity', 'negative-infinity', 'billion-rows', 'clipped-huge-fill'].map((label, index) => + React.createElement( + Box, + { + backgroundColor: 'blue', + borderStyle: label === 'clipped-huge-fill' ? 'single' : undefined, + height: 1, + key: label, + opaque: true, + ref: node => { + if (node) { + nodes.current[index] = node + } + }, + width: '100%' + }, + React.createElement(Text, null, `${label}-${tick}`) + ) + ), + React.createElement( + Box, + { + height: 1, + ref: node => { + if (node) { + nodes.current[5] = node + } + } + }, + React.createElement(Text, null, React.createElement(Text, null, `nested-corrupt-${tick}`)) + ), + React.createElement(Text, null, `adjacent-valid-${tick}`), + React.createElement(Box, { height: 20 }, React.createElement(Text, null, `tail-${tick}`)) + ) +} + +interface FastPathRepairExpose { + adjacent: DOMElement | null + dirtyChild: DOMElement | null + overlay: DOMElement | null + scroll: ScrollBoxHandle | null + scrollBox: DOMElement | null +} + +function FastPathRepairHarness({ + expose, + tick, + dirtyTick = tick, + includeOverlay = true +}: { + dirtyTick?: number + expose: React.MutableRefObject + includeOverlay?: boolean + tick: number +}) { + return React.createElement( + SourceBox, + { flexDirection: 'column', height: 12, width: 40 }, + React.createElement( + SourceScrollBox, + { + flexDirection: 'column', + height: 8, + ref: scroll => { + if (expose.current) { + expose.current.scroll = scroll + } + }, + width: 40 + }, + React.createElement(SourceBox, { height: 2 }, React.createElement(SourceText, null, 'head-row')), + React.createElement( + SourceBox, + { + height: 2, + ref: dirtyChild => { + if (expose.current) { + expose.current.dirtyChild = dirtyChild + expose.current.scrollBox = dirtyChild?.parentNode?.parentNode ?? null + } + } + }, + React.createElement(SourceText, null, `dirty-row-${dirtyTick}`) + ), + React.createElement(SourceBox, { height: 20 }, React.createElement(SourceText, null, 'tail-row')) + ), + includeOverlay + ? React.createElement( + SourceBox, + { + height: 2, + left: 0, + position: 'absolute', + ref: overlay => { + if (expose.current) { + expose.current.overlay = overlay + } + }, + top: 2, + width: 40 + }, + React.createElement(SourceText, null, 'overlay-row') + ) + : null, + React.createElement( + SourceBox, + { + height: 1, + ref: adjacent => { + if (expose.current) { + expose.current.adjacent = adjacent + } + } + }, + React.createElement(SourceText, null, `adjacent-fast-path-${tick}`) + ) + ) +} + +function guardFastPathRepairAllocations(maxWidth: number, maxHeight: number) { + const originalArrayFill = Array.prototype.fill + const originalBlit = Output.prototype.blit + const originalClear = Output.prototype.clear + const originalRepeat = String.prototype.repeat + const originalWrite = Output.prototype.write + + const observed = { + largestArrayRows: 0, + largestBlitHeight: 0, + largestBlitWidth: 0, + largestClearHeight: 0, + largestClearWidth: 0, + largestRepeat: 0, + largestWrite: 0, + repairWhitespaceWrites: 0 + } + + vi.spyOn(String.prototype, 'repeat').mockImplementation(function (count: number) { + observed.largestRepeat = Math.max(observed.largestRepeat, count) + + if (!Number.isSafeInteger(count) || count < 0 || count > maxWidth) { + throw new Error(`unbounded fast-path repeat: ${count}`) + } + + return originalRepeat.call(this, count) + }) + vi.spyOn(Array.prototype, 'fill').mockImplementation(function ( + this: unknown[], + value: unknown, + start?: number, + end?: number + ) { + observed.largestArrayRows = Math.max(observed.largestArrayRows, this.length) + + if (this.length > maxHeight) { + throw new Error(`unbounded fast-path row array: ${this.length}`) + } + + return Reflect.apply(originalArrayFill, this, [value, start, end]) + } as typeof Array.prototype.fill) + vi.spyOn(Output.prototype, 'blit').mockImplementation(function (...args: Parameters) { + observed.largestBlitWidth = Math.max(observed.largestBlitWidth, args[3]) + observed.largestBlitHeight = Math.max(observed.largestBlitHeight, args[4]) + + return originalBlit.apply(this, args) + }) + vi.spyOn(Output.prototype, 'clear').mockImplementation(function (...args: Parameters) { + observed.largestClearWidth = Math.max(observed.largestClearWidth, args[0].width) + observed.largestClearHeight = Math.max(observed.largestClearHeight, args[0].height) + + return originalClear.apply(this, args) + }) + vi.spyOn(Output.prototype, 'write').mockImplementation(function (...args: Parameters) { + observed.largestWrite = Math.max(observed.largestWrite, args[2].length) + + if (args[2].length > 0 && /^[ \n]+$/.test(args[2])) { + observed.repairWhitespaceWrites++ + } + + if (args[2].length > maxWidth * maxHeight + maxHeight) { + throw new Error(`unbounded fast-path Output.write input: ${args[2].length}`) + } + + return originalWrite.apply(this, args) + }) + + return observed +} + +describe('ScrollBox renderer bounds', () => { + it('rejects invalid imperative geometry without poisoning scroll state', async () => { + const items = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `item-${index}` })) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + + const instance = renderSync(React.createElement(Harness, { expose, items }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + const scroll = expose.current!.scroll! + + scroll.scrollTo(4) + scroll.scrollTo(Number.NaN) + scroll.scrollBy(Number.POSITIVE_INFINITY) + scroll.adjustScrollTop(Number.NEGATIVE_INFINITY) + scroll.setClampBounds(Number.NaN, Number.POSITIVE_INFINITY) + await delay(20) + + expect(scroll.getScrollTop()).toBe(4) + expect(scroll.getPendingDelta()).toBe(0) + expect(Number.isFinite(scroll.getScrollHeight())).toBe(true) + } finally { + instance.unmount() + instance.cleanup() + } + }) + + it('fails closed on corrupt ScrollBox child geometry and keeps adjacent rows renderable', async () => { + const expose = { current: [] as DOMElement[] } + const streams = makeStreams() + const originalRepeat = String.prototype.repeat + const originalWrite = Output.prototype.write + let largestWriteInput = 0 + let largestWrite = 0 + let output = '' + + vi.spyOn(String.prototype, 'repeat').mockImplementation(function (count: number) { + if (!Number.isSafeInteger(count) || count < 0 || count > 10_000) { + throw new Error(`unbounded string repeat: ${count}`) + } + + return originalRepeat.call(this, count) + }) + vi.spyOn(Output.prototype, 'write').mockImplementation(function (...args: Parameters) { + largestWriteInput = Math.max(largestWriteInput, args[2].length) + + if (args[2].length > 10_000) { + throw new Error(`unbounded Output.write input: ${args[2].length}`) + } + + return originalWrite.apply(this, args) + }) + + streams.stdout.removeAllListeners('data') + streams.stdout.on('data', chunk => { + largestWrite = Math.max(largestWrite, chunk.length) + output += chunk.toString() + }) + + const instance = renderSync(React.createElement(CorruptGeometryHarness, { expose, tick: 0 }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + + const [nanTop, positiveInfinity, negativeInfinity, billionRows, clippedHugeFill, nestedTextWrapper] = + expose.current + + expect(nanTop?.yogaNode).toBeDefined() + expect(positiveInfinity?.yogaNode).toBeDefined() + expect(negativeInfinity?.yogaNode).toBeDefined() + expect(billionRows?.yogaNode).toBeDefined() + expect(clippedHugeFill?.yogaNode).toBeDefined() + expect(nestedTextWrapper?.yogaNode).toBeDefined() + + const nestedText = nestedTextWrapper!.childNodes.find(child => child.nodeName === 'ink-text') as + | DOMElement + | undefined + + const nestedTextChild = nestedText?.childNodes[0] + + expect(nestedTextChild).toBeDefined() + + vi.spyOn(nanTop!.yogaNode!, 'getComputedTop').mockReturnValue(Number.NaN) + vi.spyOn(positiveInfinity!.yogaNode!, 'getComputedHeight').mockReturnValue(Number.POSITIVE_INFINITY) + vi.spyOn(negativeInfinity!.yogaNode!, 'getComputedHeight').mockReturnValue(Number.NEGATIVE_INFINITY) + vi.spyOn(billionRows!.yogaNode!, 'getComputedHeight').mockReturnValue(1_000_000_000) + vi.spyOn(clippedHugeFill!.yogaNode!, 'getComputedHeight').mockReturnValue(100_000_000) + vi.spyOn(clippedHugeFill!.yogaNode!, 'getComputedWidth').mockReturnValue(100_000_000) + + let nestedOffsetX = 0 + let nestedOffsetY = 0 + + nestedTextChild!.yogaNode = { + getComputedLeft: () => nestedOffsetX, + getComputedTop: () => nestedOffsetY + } as DOMElement['yogaNode'] + + output = '' + largestWrite = 0 + largestWriteInput = 0 + + const corruptNestedOffsets = [ + [Number.NaN, 0], + [Number.POSITIVE_INFINITY, 0], + [Number.NEGATIVE_INFINITY, 0], + [-1, 0], + [0.5, 0], + [100_000_000, 0], + [0, Number.NaN], + [0, Number.POSITIVE_INFINITY], + [0, Number.NEGATIVE_INFINITY], + [0, -1], + [0, 0.5], + [0, 100_000_000] + ] as const + + for (const [index, [offsetX, offsetY]] of corruptNestedOffsets.entries()) { + nestedOffsetX = offsetX + nestedOffsetY = offsetY + + expect(() => { + instance.rerender(React.createElement(CorruptGeometryHarness, { expose, tick: index + 1 })) + }).not.toThrow() + await delay(5) + } + + await delay(40) + + expect(output).toContain(`adjacent-valid-${corruptNestedOffsets.length}`) + expect(largestWriteInput).toBeLessThan(10_000) + expect(largestWrite).toBeLessThan(10_000) + expect(output.length).toBeLessThan(50_000) + } finally { + vi.restoreAllMocks() + instance.unmount() + instance.cleanup() + } + }) + + it('clips corrupt dirty-child DECSTBM repairs before allocating or recursing', async () => { + const expose = { + current: { + adjacent: null, + dirtyChild: null, + overlay: null, + scroll: null, + scrollBox: null + } as FastPathRepairExpose + } + + const streams = makeStreams() + let output = '' + + const instance = renderSourceSync( + React.createElement(FastPathRepairHarness, { expose, includeOverlay: false, tick: 0 }), + { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + } + ) + + try { + await delay(20) + const dirtyChild = expose.current!.dirtyChild! + + expect(dirtyChild, streams.stderr.read()?.toString()).not.toBeNull() + expect(dirtyChild.yogaNode).toBeDefined() + vi.spyOn(dirtyChild.yogaNode!, 'getComputedTop').mockReturnValue(-99_999_995) + vi.spyOn(dirtyChild.yogaNode!, 'getComputedWidth').mockReturnValue(100_000_000) + vi.spyOn(dirtyChild.yogaNode!, 'getComputedHeight').mockReturnValue(100_000_000) + + instance.rerender(React.createElement(FastPathRepairHarness, { expose, includeOverlay: false, tick: 1 })) + await delay(20) + + streams.stdout.removeAllListeners('data') + streams.stdout.on('data', chunk => { + output += chunk.toString() + }) + + const observed = guardFastPathRepairAllocations(80, 20) + const fastPathsBefore = sourceScrollFastPathStats.taken + const capturedBefore = sourceScrollFastPathStats.captured + + expect(() => expose.current!.scroll!.scrollTo(1)).not.toThrow() + expect(() => + instance.rerender(React.createElement(FastPathRepairHarness, { expose, includeOverlay: false, tick: 2 })) + ).not.toThrow() + await delay(40) + + expect(sourceScrollFastPathStats.captured, JSON.stringify(sourceScrollFastPathStats)).toBeGreaterThan( + capturedBefore + ) + expect(sourceScrollFastPathStats.taken, JSON.stringify(sourceScrollFastPathStats)).toBeGreaterThan( + fastPathsBefore + ) + expect(observed.repairWhitespaceWrites).toBeGreaterThan(0) + expect(observed.largestRepeat).toBeGreaterThan(0) + expect(observed.largestArrayRows).toBeGreaterThan(0) + + output = '' + expect(() => + instance.rerender( + React.createElement(FastPathRepairHarness, { + dirtyTick: 2, + expose, + includeOverlay: false, + tick: 3 + }) + ) + ).not.toThrow() + await delay(40) + + expect(observed.largestRepeat).toBeLessThanOrEqual(80) + expect(observed.largestArrayRows).toBeLessThanOrEqual(20) + expect(observed.largestBlitWidth).toBeLessThanOrEqual(80) + expect(observed.largestBlitHeight).toBeLessThanOrEqual(20) + expect(observed.largestClearWidth).toBeLessThanOrEqual(80) + expect(observed.largestClearHeight).toBeLessThanOrEqual(20) + expect(observed.largestWrite).toBeLessThanOrEqual(1_620) + expect(expose.current!.scroll!.getScrollTop()).toBe(1) + expect(expose.current!.scroll!.getViewportHeight()).toBeGreaterThan(0) + expect(output).toContain('adjacent-fast-path-3') + expect(streams.stderr.read()?.toString() ?? '').toBe('') + } finally { + vi.restoreAllMocks() + instance.unmount() + instance.cleanup() + } + }) + + it('clips corrupt absolute-overlay DECSTBM repairs before allocating or recursing', async () => { + const expose = { + current: { + adjacent: null, + dirtyChild: null, + overlay: null, + scroll: null, + scrollBox: null + } as FastPathRepairExpose + } + + const streams = makeStreams() + let output = '' + + const instance = renderSourceSync(React.createElement(FastPathRepairHarness, { expose, tick: 0 }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + const overlay = expose.current!.overlay! + const scrollBox = expose.current!.scrollBox! + + expect(overlay, streams.stderr.read()?.toString()).not.toBeNull() + expect(overlay.yogaNode).toBeDefined() + expect(scrollBox.yogaNode).toBeDefined() + vi.spyOn(scrollBox.yogaNode!, 'getComputedWidth').mockReturnValue(100_000_000) + vi.spyOn(overlay.yogaNode!, 'getComputedWidth').mockReturnValue(100_000_000) + vi.spyOn(overlay.yogaNode!, 'getComputedHeight').mockReturnValue(100_000_000) + + instance.rerender(React.createElement(FastPathRepairHarness, { expose, tick: 1 })) + await delay(20) + instance.rerender(React.createElement(FastPathRepairHarness, { expose, tick: 1 })) + await delay(20) + + streams.stdout.removeAllListeners('data') + streams.stdout.on('data', chunk => { + output += chunk.toString() + }) + + const observed = guardFastPathRepairAllocations(80, 20) + const fastPathsBefore = sourceScrollFastPathStats.taken + const capturedBefore = sourceScrollFastPathStats.captured + + expect(() => expose.current!.scroll!.scrollTo(1)).not.toThrow() + expect(expose.current!.scroll!.getScrollTop()).toBe(1) + await delay(40) + + expect(sourceScrollFastPathStats.captured, JSON.stringify(sourceScrollFastPathStats)).toBeGreaterThan( + capturedBefore + ) + expect(sourceScrollFastPathStats.taken, JSON.stringify(sourceScrollFastPathStats)).toBeGreaterThan( + fastPathsBefore + ) + expect(observed.repairWhitespaceWrites).toBeGreaterThan(0) + expect(observed.largestRepeat).toBeGreaterThan(0) + expect(observed.largestArrayRows).toBeGreaterThan(0) + + output = '' + expect(() => + instance.rerender(React.createElement(FastPathRepairHarness, { dirtyTick: 1, expose, tick: 2 })) + ).not.toThrow() + await delay(40) + + expect(observed.largestRepeat).toBeLessThanOrEqual(80) + expect(observed.largestArrayRows).toBeLessThanOrEqual(20) + expect(observed.largestBlitWidth).toBeLessThanOrEqual(80) + expect(observed.largestBlitHeight).toBeLessThanOrEqual(20) + expect(observed.largestClearWidth).toBeLessThanOrEqual(80) + expect(observed.largestClearHeight).toBeLessThanOrEqual(20) + expect(observed.largestWrite).toBeLessThanOrEqual(1_620) + expect(output.length).toBeLessThan(2_000) + expect(expose.current!.scroll!.getViewportHeight()).toBeGreaterThan(0) + expect(output).toContain('adjacent-fast-path-2') + expect(streams.stderr.read()?.toString() ?? '').toBe('') + } finally { + vi.restoreAllMocks() + instance.unmount() + instance.cleanup() + } + }) +}) diff --git a/ui-tui/src/__tests__/useVirtualHistoryHeights.test.ts b/ui-tui/src/__tests__/useVirtualHistoryHeights.test.ts index ae5658f83e..c1aa868f87 100644 --- a/ui-tui/src/__tests__/useVirtualHistoryHeights.test.ts +++ b/ui-tui/src/__tests__/useVirtualHistoryHeights.test.ts @@ -36,4 +36,24 @@ describe('ensureVirtualItemHeight', () => { expect(ensureVirtualItemHeight(heights, 'd', 0, 0, estimateHeight)).toBe(1) expect(heights.get('d')).toBe(1) }) + + it.each([Number.NaN, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY, -4, 1_000_000_000])( + 'quarantines invalid cached height %s and reseeds it', + cached => { + const heights = new Map([['bad', cached]]) + + expect(ensureVirtualItemHeight(heights, 'bad', 0, 4, () => 7)).toBe(7) + expect(heights.get('bad')).toBe(7) + } + ) + + it.each([Number.NaN, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY, -4, 1_000_000_000])( + 'falls back when the estimator returns invalid height %s', + estimate => { + const heights = new Map() + + expect(ensureVirtualItemHeight(heights, 'bad', 0, 4, () => estimate)).toBe(4) + expect(heights.get('bad')).toBe(4) + } + ) }) diff --git a/ui-tui/src/__tests__/virtualHistoryOffsetCache.test.ts b/ui-tui/src/__tests__/virtualHistoryOffsetCache.test.ts index 010d12c9ca..e5bed9d966 100644 --- a/ui-tui/src/__tests__/virtualHistoryOffsetCache.test.ts +++ b/ui-tui/src/__tests__/virtualHistoryOffsetCache.test.ts @@ -2,14 +2,16 @@ import { PassThrough } from 'stream' import { Box, renderSync, ScrollBox, type ScrollBoxHandle, Text } from '@hermes/ink' import React, { useLayoutEffect, useRef } from 'react' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' -import { useVirtualHistory, virtualHistorySnapshotKey } from '../hooks/useVirtualHistory.js' +import { MAX_HISTORY } from '../config/limits.js' +import { pruneVirtualHeightCache, useVirtualHistory, virtualHistorySnapshotKey } from '../hooks/useVirtualHistory.js' interface Item { height: number heightAfterResize?: number key: string + text?: string } interface Exposed { @@ -61,12 +63,16 @@ function Harness({ columns = 80, expose, height = 10, + generation = 0, + initialHeights, items, maxMounted = 16 }: { columns?: number expose: React.MutableRefObject height?: number + generation?: number + initialHeights?: ReadonlyMap items: readonly Item[] maxMounted?: number }) { @@ -75,6 +81,8 @@ function Harness({ const virtualHistory = useVirtualHistory(scrollRef, items, columns, { coldStartCount: 16, estimateHeight: index => itemHeightForColumns(items[index], columns), + generation, + initialHeights, maxMounted, overscan: 2 }) @@ -98,7 +106,7 @@ function Harness({ key: item.key, ref: virtualHistory.measureRef(item.key) }, - React.createElement(Text, null, item.key) + React.createElement(Text, null, item.text ?? item.key) ) ), virtualHistory.bottomSpacer > 0 ? React.createElement(Box, { height: virtualHistory.bottomSpacer }) : null @@ -107,6 +115,17 @@ function Harness({ } describe('useVirtualHistory offset cache reuse', () => { + it('prunes stable-session external height caches to active history keys', () => { + const cache = new Map([ + ['outgoing', 9], + ['active', 3] + ]) + + pruneVirtualHeightCache(cache, [{ key: 'active' }]) + + expect([...cache]).toEqual([['active', 3]]) + }) + it('includes viewport height in the external-store snapshot key', () => { const base = { getPendingDelta: () => 0, @@ -247,9 +266,358 @@ describe('useVirtualHistory offset cache reuse', () => { } }) + it('adjusts the committed viewport without consuming pending scroll intent', async () => { + const items = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `item-${index}` })) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + + const instance = renderSync(React.createElement(Harness, { expose, items }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + const scroll = expose.current!.scroll! + + scroll.scrollTo(3) + scroll.scrollBy(2) + scroll.adjustScrollTop(4) + + expect(scroll.getScrollTop()).toBe(7) + expect(scroll.getPendingDelta()).toBe(2) + expect(scroll.isSticky()).toBe(false) + } finally { + instance.unmount() + instance.cleanup() + } + }) + + it('keeps the tail clamp open while a manual non-sticky tail grows', async () => { + const before = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `item-${index}` })) + const after = before.map((item, index) => (index === before.length - 1 ? { ...item, height: 8 } : item)) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + const initialHeights = new Map(before.map(item => [item.key, item.height])) + + const instance = renderSync(React.createElement(Harness, { expose, initialHeights, items: before }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + const scroll = expose.current!.scroll! + const setClampBounds = vi.spyOn(scroll, 'setClampBounds') + + scroll.scrollTo(28) + await delay(20) + instance.rerender(React.createElement(Harness, { expose, initialHeights, items: after })) + await delay(60) + + expect(scroll.isSticky()).toBe(false) + expect(setClampBounds.mock.calls.some(([, max]) => max === Number.POSITIVE_INFINITY)).toBe(true) + + scroll.scrollTo(36) + await delay(20) + expect(scroll.getScrollTop()).toBe(36) + } finally { + instance.unmount() + instance.cleanup() + } + }) + + it('quarantines invalid measured heights before cache and compensation', async () => { + const items = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `item-${index}` })) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + const initialHeights = new Map(items.map(item => [item.key, item.height])) + + const instance = renderSync(React.createElement(Harness, { expose, initialHeights, items }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + const scroll = expose.current!.scroll! + + scroll.scrollTo(5) + await delay(20) + const adjustScrollTop = vi.spyOn(scroll, 'adjustScrollTop') + const ref = expose.current!.virtualHistory.measureRef('item-1') + + for (const height of [Number.NaN, Number.POSITIVE_INFINITY, -1, 1_000_000_000]) { + ref({ yogaNode: { getComputedHeight: () => height } }) + ref(null) + } + + expect(adjustScrollTop).not.toHaveBeenCalled() + expect(expose.current!.virtualHistory.offsets[items.length]).toBe(40) + expect(Number.isFinite(scroll.getScrollTop())).toBe(true) + } finally { + instance.unmount() + instance.cleanup() + } + }) + + it('preserves the visual anchor when a measured row above the viewport changes height', async () => { + const before = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `item-${index}` })) + const after = before.map((item, index) => (index === 0 ? { ...item, height: 5 } : item)) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + const initialHeights = new Map(before.map(item => [item.key, item.height])) + + const instance = renderSync(React.createElement(Harness, { expose, initialHeights, items: before }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + expose.current!.scroll!.scrollTo(3) + await delay(20) + + instance.rerender(React.createElement(Harness, { expose, initialHeights, items: after })) + await delay(40) + + expect(expose.current!.scroll!.getScrollTop()).toBe(6) + } finally { + instance.unmount() + instance.cleanup() + } + }) + + it('keeps a compensated near-tail viewport manual', async () => { + const before = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `item-${index}` })) + const after = before.map((item, index) => (index === 13 ? { ...item, height: 5 } : item)) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + const initialHeights = new Map(before.map(item => [item.key, item.height])) + + const instance = renderSync(React.createElement(Harness, { expose, initialHeights, items: before }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + const scroll = expose.current!.scroll! + + scroll.scrollTo(29) + await delay(20) + instance.rerender(React.createElement(Harness, { expose, initialHeights, items: after })) + + expect(scroll.getScrollTop()).toBe(32) + expect(scroll.isSticky()).toBe(false) + } finally { + instance.unmount() + instance.cleanup() + } + }) + + it('ignores stale unmount measurement from the previous width layout', async () => { + const items = Array.from({ length: 20 }, (_, index) => ({ + height: 4, + heightAfterResize: index === 0 ? 5 : 2, + key: `item-${index}` + })) + + const expose = { current: null as Exposed | null } + const streams = makeStreams() + const initialHeights = new Map(items.map(item => [item.key, item.height])) + + const instance = renderSync(React.createElement(Harness, { columns: 40, expose, initialHeights, items }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + const scroll = expose.current!.scroll! + + scroll.scrollTo(0) + await delay(20) + scroll.scrollTo(5) + const adjustScrollTop = vi.spyOn(scroll, 'adjustScrollTop') + + instance.rerender(React.createElement(Harness, { columns: 80, expose, initialHeights, items })) + await delay(40) + + expect(adjustScrollTop).not.toHaveBeenCalled() + expect(scroll.getScrollTop()).toBe(5) + expect(scroll.isSticky()).toBe(false) + expect(expose.current!.virtualHistory.start).toBeGreaterThan(0) + expect(expose.current!.virtualHistory.offsets[1]).toBe(2) + expect(expose.current!.virtualHistory.offsets[items.length]).toBe(40) + } finally { + instance.unmount() + instance.cleanup() + } + }) + + it('does not let outgoing transcript refs compensate a new layout generation', async () => { + const outgoing = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `old-${index}` })) + const incoming = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `new-${index}` })) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + const initialHeights = new Map(outgoing.map(item => [item.key, item.height])) + + const instance = renderSync(React.createElement(Harness, { expose, initialHeights, items: outgoing }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + const scroll = expose.current!.scroll! + + scroll.scrollTo(5) + await delay(20) + const adjustScrollTop = vi.spyOn(scroll, 'adjustScrollTop') + + const replacementCache = new Map([ + ...outgoing.map(item => [item.key, item.key === 'old-1' ? 1 : item.height] as const), + ...incoming.map(item => [item.key, item.height] as const) + ]) + + instance.rerender( + React.createElement(Harness, { + expose, + generation: 1, + initialHeights: replacementCache, + items: incoming + }) + ) + await delay(40) + + expect(adjustScrollTop).not.toHaveBeenCalled() + expect(scroll.getScrollTop()).toBe(5) + expect(expose.current!.virtualHistory.offsets[incoming.length]).toBe(40) + } finally { + instance.unmount() + instance.cleanup() + } + }) + + it('corrects and compensates a same-layout row measured at unmount', async () => { + const items = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `item-${index}` })) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + const initialHeights = new Map(items.map(item => [item.key, item.height])) + + const instance = renderSync(React.createElement(Harness, { expose, initialHeights, items }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + const scroll = expose.current!.scroll! + + scroll.scrollTo(0) + await delay(20) + scroll.scrollTo(5) + const adjustScrollTop = vi.spyOn(scroll, 'adjustScrollTop') + const staleHeights = new Map(initialHeights) + + staleHeights.set(items[0]!.key, 1) + instance.rerender(React.createElement(Harness, { expose, initialHeights: staleHeights, items })) + await delay(40) + + expect(adjustScrollTop).toHaveBeenCalledOnce() + expect(adjustScrollTop).toHaveBeenCalledWith(1) + expect(scroll.getScrollTop()).toBe(6) + expect(scroll.isSticky()).toBe(false) + expect(expose.current!.virtualHistory.start).toBeGreaterThan(0) + expect(expose.current!.virtualHistory.offsets[1]).toBe(2) + } finally { + instance.unmount() + instance.cleanup() + } + }) + + it('does not compensate for measured height changes in or below the viewport', async () => { + const before = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `item-${index}` })) + const visibleChanged = before.map((item, index) => (index === 1 ? { ...item, height: 5 } : item)) + const belowChanged = visibleChanged.map((item, index) => (index === 8 ? { ...item, height: 5 } : item)) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + const initialHeights = new Map(before.map(item => [item.key, item.height])) + + const instance = renderSync(React.createElement(Harness, { expose, initialHeights, items: before }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + expose.current!.scroll!.scrollTo(3) + await delay(20) + + instance.rerender(React.createElement(Harness, { expose, initialHeights, items: visibleChanged })) + await delay(40) + expect(expose.current!.scroll!.getScrollTop()).toBe(3) + + instance.rerender(React.createElement(Harness, { expose, initialHeights, items: belowChanged })) + await delay(40) + expect(expose.current!.scroll!.getScrollTop()).toBe(3) + } finally { + instance.unmount() + instance.cleanup() + } + }) + + it('does not compensate measured heights while sticky at the live tail', async () => { + const before = Array.from({ length: 20 }, (_, index) => ({ height: 2, key: `item-${index}` })) + const after = before.map((item, index) => (index === 14 ? { ...item, height: 5 } : item)) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + const initialHeights = new Map(before.map(item => [item.key, item.height])) + + const instance = renderSync(React.createElement(Harness, { expose, initialHeights, items: before }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + const adjustScrollTop = vi.spyOn(expose.current!.scroll!, 'adjustScrollTop') + + instance.rerender(React.createElement(Harness, { expose, initialHeights, items: after })) + await delay(40) + + expect(adjustScrollTop).not.toHaveBeenCalled() + expect(expose.current!.scroll!.isSticky()).toBe(true) + } finally { + instance.unmount() + instance.cleanup() + } + }) + it('ignores stale reused offset-array entries after the item count shrinks', async () => { const beforeShrink = Array.from({ length: 1400 }, (_, index) => ({ height: 1, key: `old${index}` })) - const afterShrink = Array.from({ length: 800 }, (_, index) => ({ height: 7, key: `new${index}` })) + const afterShrink = Array.from({ length: MAX_HISTORY }, (_, index) => ({ height: 7, key: `new${index}` })) const expose = { current: null as Exposed | null } const streams = makeStreams() diff --git a/ui-tui/src/app/scroll.ts b/ui-tui/src/app/scroll.ts index e3a53734a3..6e284dde25 100644 --- a/ui-tui/src/app/scroll.ts +++ b/ui-tui/src/app/scroll.ts @@ -67,5 +67,8 @@ export function scrollWithSelectionBy(delta: number, { scrollRef, selection }: S shift(-actual, top, bottom) } - s.scrollBy(actual) + // The target is already accepted and clamped here. Commit it directly so + // wheel/page input does not enter ScrollBox's multi-frame pending-delta + // drain and produce visible stair steps at virtual row boundaries. + s.scrollTo(cur + actual) } diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index 18277fd663..283ebe5a11 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -12,7 +12,7 @@ import { useStore } from '@nanostores/react' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { DASHBOARD_TUI_MODE, STARTUP_RESUME_ID } from '../config/env.js' -import { MAX_HISTORY, WHEEL_SCROLL_STEP } from '../config/limits.js' +import { WHEEL_SCROLL_STEP } from '../config/limits.js' import { RESIZE_COALESCE_MS } from '../config/timing.js' import { hasLeadGap, prevRenderedMsg } from '../domain/blockLayout.js' import { SECTION_NAMES, sectionMode } from '../domain/details.js' @@ -28,9 +28,9 @@ import type { TerminalResizeResponse } from '../gatewayTypes.js' import { useGitBranch } from '../hooks/useGitBranch.js' -import { useVirtualHistory } from '../hooks/useVirtualHistory.js' +import { pruneVirtualHeightCache, useVirtualHistory } from '../hooks/useVirtualHistory.js' import { composerPromptWidth } from '../lib/inputMetrics.js' -import { appendTranscriptMessage } from '../lib/messages.js' +import { appendTranscriptMessage, capTranscriptHistory } from '../lib/messages.js' import { DEFAULT_VOICE_RECORD_KEY, isMac, type ParsedVoiceRecordKey } from '../lib/platform.js' import { createResizeCoalescer } from '../lib/resizeCoalescer.js' import { asRpcResult, rpcErrorMessage } from '../lib/rpc.js' @@ -63,14 +63,6 @@ const BRACKET_PASTE_ON = '\x1b[?2004h' const BRACKET_PASTE_OFF = '\x1b[?2004l' const MAX_HEIGHT_CACHE_BUCKETS = 12 -const capHistory = (items: Msg[]): Msg[] => { - if (items.length <= MAX_HISTORY) { - return items - } - - return items[0]?.kind === 'intro' ? [items[0]!, ...items.slice(-(MAX_HISTORY - 1))] : items.slice(-MAX_HISTORY) -} - const statusColorOf = (status: string, t: { error: string; muted: string; ok: string; warn: string }) => { if (status === 'ready') { return t.ok @@ -183,7 +175,17 @@ export function useMainApp(gw: GatewayClient) { } }, [stdout]) - const [historyItems, setHistoryItems] = useState(() => [{ kind: 'intro', role: 'system', text: '' }]) + const [historyItems, setHistoryItemsState] = useState(() => [{ kind: 'intro', role: 'system', text: '' }]) + const [historyGeneration, setHistoryGeneration] = useState(0) + + const setHistoryItems = useCallback>(value => { + if (typeof value !== 'function') { + setHistoryGeneration(generation => generation + 1) + } + + setHistoryItemsState(previous => capTranscriptHistory(typeof value === 'function' ? value(previous) : value)) + }, []) + const [lastUserMsg, setLastUserMsg] = useState('') const [stickyPrompt, setStickyPrompt] = useState('') const [catalog, setCatalog] = useState(null) @@ -355,20 +357,20 @@ export function useMainApp(gw: GatewayClient) { const userPromptWidth = composerPromptWidth(ui.theme.brand.prompt) const heightCacheKey = `${ui.sid ?? 'draft'}:${cols}:${userPromptWidth}:${ui.compact ? '1' : '0'}:${detailsLayoutKey}` - const heightCache = useMemo(() => { - let cache = heightCachesRef.current.get(heightCacheKey) + // Build a render-local snapshot. Registering/pruning the shared cache is a + // post-commit transition below, so an abandoned concurrent render cannot + // delete heights still owned by the committed transcript generation. + const activeHeightCache = useMemo(() => new Map(heightCachesRef.current.get(heightCacheKey)), [heightCacheKey]) - if (!cache) { - cache = new Map() - heightCachesRef.current.set(heightCacheKey, cache) + useEffect(() => { + pruneVirtualHeightCache(activeHeightCache, virtualRows) + heightCachesRef.current.delete(heightCacheKey) + heightCachesRef.current.set(heightCacheKey, activeHeightCache) - if (heightCachesRef.current.size > MAX_HEIGHT_CACHE_BUCKETS) { - heightCachesRef.current.delete(heightCachesRef.current.keys().next().value!) - } + while (heightCachesRef.current.size > MAX_HEIGHT_CACHE_BUCKETS) { + heightCachesRef.current.delete(heightCachesRef.current.keys().next().value!) } - - return cache - }, [heightCacheKey]) + }, [activeHeightCache, heightCacheKey, historyGeneration, virtualRows]) // Index of the first user-role message — separator-rendering in // appLayout.tsx skips this row, so the height estimator must skip it @@ -414,16 +416,17 @@ export function useMainApp(gw: GatewayClient) { const h = heights.get(row.key) if (h) { - heightCache.set(row.key, h) + activeHeightCache.set(row.key, h) } } }, - [heightCache, virtualRows] + [activeHeightCache, virtualRows] ) const virtualHistory = useVirtualHistory(scrollRef, virtualRows, cols, { estimateHeight: estimateRowHeight, - initialHeights: heightCache, + generation: historyGeneration, + initialHeights: activeHeightCache, liveTailActive: turnLiveTailActive, onHeightsChange: syncHeightCache }) @@ -434,8 +437,8 @@ export function useMainApp(gw: GatewayClient) { ) const appendMessage = useCallback( - (msg: Msg) => setHistoryItems(prev => capHistory(appendTranscriptMessage(prev, msg))), - [] + (msg: Msg) => setHistoryItems(prev => appendTranscriptMessage(prev, msg)), + [setHistoryItems] ) const sys = useCallback((text: string) => appendMessage({ role: 'system', text }), [appendMessage]) @@ -801,6 +804,7 @@ export function useMainApp(gw: GatewayClient) { session.newSession, session.resetSession, session.resumeById, + setHistoryItems, setVoiceEnabled, setVoiceProcessing, setVoiceRecording, @@ -912,6 +916,7 @@ export function useMainApp(gw: GatewayClient) { selection, send, session, + setHistoryItems, sys ] ) diff --git a/ui-tui/src/hooks/useVirtualHistory.ts b/ui-tui/src/hooks/useVirtualHistory.ts index 592d20e9a0..c9b760a00e 100644 --- a/ui-tui/src/hooks/useVirtualHistory.ts +++ b/ui-tui/src/hooks/useVirtualHistory.ts @@ -48,17 +48,39 @@ const FREEZE_RENDERS = 2 // from 25 → 12: each new item adds ~100 fibers / Yoga nodes, and a // 25-item commit was the dominant contributor to the 100ms+ p99 frames. const SLIDE_STEP = 12 +const MAX_VIRTUAL_ITEM_HEIGHT = 100_000 +const MAX_VIRTUAL_GEOMETRY = 1_000_000_000 const NOOP = () => {} +const validVirtualItemHeight = (value: number): boolean => + Number.isFinite(value) && value > 0 && value <= MAX_VIRTUAL_ITEM_HEIGHT + +const safeUnsignedGeometry = (value: number, fallback = 0): number => + Number.isFinite(value) && value >= 0 && value <= MAX_VIRTUAL_GEOMETRY ? value : fallback + +const safeSignedGeometry = (value: number, fallback = 0): number => + Number.isFinite(value) && Math.abs(value) <= MAX_VIRTUAL_GEOMETRY ? value : fallback + +export const pruneVirtualHeightCache = (cache: Map, items: readonly { key: string }[]): void => { + const active = new Set(items.map(item => item.key)) + + for (const key of cache.keys()) { + if (!active.has(key) || !validVirtualItemHeight(cache.get(key)!)) { + cache.delete(key) + } + } +} + export const virtualHistorySnapshotKey = (s?: ScrollBoxHandle | null): string => { if (!s) { return 'none' } - const target = s.getScrollTop() + s.getPendingDelta() + const target = safeUnsignedGeometry(safeUnsignedGeometry(s.getScrollTop()) + safeSignedGeometry(s.getPendingDelta())) + const bin = Math.floor(target / QUANTUM) - const viewportHeight = Math.max(0, s.getViewportHeight()) + const viewportHeight = safeUnsignedGeometry(s.getViewportHeight()) return `${s.isSticky() ? ~bin : bin}:${viewportHeight}` } @@ -97,11 +119,17 @@ export const ensureVirtualItemHeight = ( ) => { const cached = heights.get(key) - if (cached !== undefined) { - return Math.max(1, Math.floor(cached)) + if (cached !== undefined && validVirtualItemHeight(cached)) { + return Math.floor(cached) } - const seeded = Math.max(1, Math.floor(estimateHeight?.(index, key) ?? estimate)) + if (cached !== undefined) { + heights.delete(key) + } + + const fallback = validVirtualItemHeight(estimate) ? Math.floor(estimate) : estimate === 0 ? 1 : ESTIMATE + const candidate = estimateHeight?.(index, key) ?? fallback + const seeded = validVirtualItemHeight(candidate) ? Math.floor(candidate) : candidate === 0 ? 1 : fallback heights.set(key, seeded) return seeded @@ -114,6 +142,7 @@ export function useVirtualHistory( { estimate = ESTIMATE, estimateHeight, + generation = 0, initialHeights, liveTailActive = false, onHeightsChange, @@ -126,15 +155,16 @@ export function useVirtualHistory( const heights = useRef(new Map(initialHeights)) const initialHeightsRef = useRef(initialHeights) const refs = useRef(new Map void>()) + const measuredBottoms = useRef(new Map()) + const unmountViewport = useRef<{ sticky: boolean; top: number } | null>(null) const onHeightsChangeRef = useRef(onHeightsChange) // Bump whenever heightCache mutates so offsets rebuild on next read. // Ref (not state) — checked during render phase, zero extra commits. const offsetVersion = useRef(0) - // Cached offsets: reused Float64Array keyed on (itemCount, version) so we - // only rebuild when something actually changed. Previous approach allocated - // a fresh Array(n+1) every render — at n=10k that's ~80KB/render of GC - // pressure during streaming. + // Cached offsets: reused Float64Array keyed on (itemCount, version). Clean + // renders reuse it, but a point-height invalidation still rebuilds the full + // prefix array; this is allocation reuse, not incremental prefix indexing. const offsetsCache = useRef<{ arr: Float64Array; n: number; version: number }>({ arr: new Float64Array(0), n: -1, @@ -158,9 +188,25 @@ export function useVirtualHistory( const skipMeasurement = useRef(false) const prevRange = useRef(null) const freezeRenders = useRef(0) + const generationRef = useRef(generation) onHeightsChangeRef.current = onHeightsChange + if (generationRef.current !== generation) { + generationRef.current = generation + nodes.current.clear() + refs.current.clear() + measuredBottoms.current.clear() + unmountViewport.current = null + heights.current = new Map(initialHeights) + initialHeightsRef.current = initialHeights + prevRange.current = null + freezeRenders.current = 0 + skipMeasurement.current = false + lastScrollTopRef.current = 0 + offsetVersion.current++ + } + if (initialHeightsRef.current !== initialHeights) { initialHeightsRef.current = initialHeights heights.current = new Map(initialHeights) @@ -173,7 +219,13 @@ export function useVirtualHistory( prevColumns.current = columns for (const [k, h] of heights.current) { - heights.current.set(k, Math.max(1, Math.round(h * ratio))) + const scaled = Math.round(h * ratio) + + if (validVirtualItemHeight(scaled)) { + heights.current.set(k, scaled) + } else { + heights.current.delete(k) + } } offsetVersion.current++ @@ -209,6 +261,7 @@ export function useVirtualHistory( heights.current.delete(k) nodes.current.delete(k) refs.current.delete(k) + measuredBottoms.current.delete(k) dirty = true } } @@ -238,10 +291,10 @@ export function useVirtualHistory( const offsets = offsetsCache.current.arr const total = offsets[n] ?? 0 - const top = Math.max(0, scrollRef.current?.getScrollTop() ?? 0) - const pendingDelta = scrollRef.current?.getPendingDelta() ?? 0 - const target = Math.max(0, top + pendingDelta) - const vp = Math.max(0, scrollRef.current?.getViewportHeight() ?? 0) + const top = safeUnsignedGeometry(scrollRef.current?.getScrollTop() ?? 0) + const pendingDelta = safeSignedGeometry(scrollRef.current?.getPendingDelta() ?? 0) + const target = safeUnsignedGeometry(top + pendingDelta) + const vp = safeUnsignedGeometry(scrollRef.current?.getViewportHeight() ?? 0) const sticky = scrollRef.current?.isSticky() ?? true const recentManual = Date.now() - (scrollRef.current?.getLastManualScrollAt() ?? 0) < 1200 @@ -417,44 +470,90 @@ export function useVirtualHistory( } } - const measureRef = useCallback((key: string) => { - let fn = refs.current.get(key) + const measureRef = useCallback( + (key: string) => { + let fn = refs.current.get(key) - if (!fn) { - fn = (el: unknown) => { - if (el) { - nodes.current.set(key, el) + if (!fn) { + const refGeneration = generationRef.current - return + fn = (el: unknown) => { + if (refGeneration !== generationRef.current) { + return + } + + if (el) { + nodes.current.set(key, el) + + return + } + + // A width-change render has already scaled the cache, but outgoing + // refs still point at Yoga from the previous layout. The render-phase + // skip flag remains set through mutation refs, so ignore that stale + // measurement and let the post-resize layout pass own correction. + if (skipMeasurement.current) { + nodes.current.delete(key) + measuredBottoms.current.delete(key) + + return + } + + // Measure-at-unmount: the yogaNode is still valid here (reconciler + // calls ref(null) before removeChild → freeRecursive), so we grab + // the final height before WASM release. Without this, items + // scrolled out during fast pan keep a stale estimate in heightCache + // and offset math drifts until the next mount/remount cycle. + const existing = nodes.current.get(key) as MeasuredNode | undefined + const h = Math.ceil(existing?.yogaNode?.getComputedHeight?.() ?? 0) + const previousHeight = heights.current.get(key) + + if (validVirtualItemHeight(h) && previousHeight !== h) { + const s = scrollRef.current + const measuredBottom = measuredBottoms.current.get(key) + + // All null refs in this commit share the viewport boundary captured + // before the first adjustment. Otherwise an earlier compensation + // can make an intersecting sibling look wholly above later in the + // same unmount batch. + const viewport = (unmountViewport.current ??= { + sticky: s?.isSticky() ?? true, + top: safeUnsignedGeometry(s?.getScrollTop() ?? 0) + }) + + if ( + s && + previousHeight !== undefined && + measuredBottom !== undefined && + measuredBottom <= viewport.top && + !viewport.sticky + ) { + s.adjustScrollTop(h - previousHeight) + } + + heights.current.set(key, h) + offsetVersion.current++ + onHeightsChangeRef.current?.(heights.current) + } + + nodes.current.delete(key) + measuredBottoms.current.delete(key) } - // Measure-at-unmount: the yogaNode is still valid here (reconciler - // calls ref(null) before removeChild → freeRecursive), so we grab - // the final height before WASM release. Without this, items - // scrolled out during fast pan keep a stale estimate in heightCache - // and offset math drifts until the next mount/remount cycle. - const existing = nodes.current.get(key) as MeasuredNode | undefined - const h = Math.ceil(existing?.yogaNode?.getComputedHeight?.() ?? 0) - - if (h > 0 && heights.current.get(key) !== h) { - heights.current.set(key, h) - offsetVersion.current++ - onHeightsChangeRef.current?.(heights.current) - } - - nodes.current.delete(key) + refs.current.set(key, fn) } - refs.current.set(key, fn) - } - - return fn - }, []) + return fn + }, + [scrollRef] + ) useLayoutEffect(() => { + unmountViewport.current = null const s = scrollRef.current let dirty = false let heightDirty = false + let anchorDelta = 0 // Give the renderer the mounted-row coverage for passive scroll clamping. // Clamp MUST use the EFFECTIVE (deferred) range, not the immediate one. @@ -466,20 +565,40 @@ export function useVirtualHistory( if (s && shouldSetVirtualClamp({ itemCount: n, liveTailActive, sticky, viewportHeight: vp })) { const effTopSpacer = offsets[effStart] ?? 0 const effBottom = offsets[effEnd] ?? total - // At effEnd=n there's no bottomSpacer — use Infinity so render-node- - // to-output's own Math.min(cur, maxScroll) governs. Using offsets[n] - // here would bake in heightCache (one render behind Yoga), and during - // streaming the tail item's cached height lags its real height — - // sticky-break would then clamp below the real max and push - // streaming text off-viewport. const clampMin = effStart === 0 ? 0 : effTopSpacer - const clampMax = effEnd === n ? Infinity : Math.max(effTopSpacer, effBottom - vp) + // Preserve the intentional open tail: when the mounted range reaches + // the final row, Yoga may already know about growth that the measured + // height cache has not reconciled yet. A finite estimated clamp would + // trap a manual, non-sticky viewport above that newly grown tail. + const clampMax = effEnd === n ? Number.POSITIVE_INFINITY : Math.max(effTopSpacer, effBottom - vp) - s.setClampBounds(clampMin, clampMax) + if ( + safeUnsignedGeometry(clampMin, -1) >= 0 && + (clampMax === Number.POSITIVE_INFINITY || safeUnsignedGeometry(clampMax, -1) >= clampMin) + ) { + s.setClampBounds(clampMin, clampMax) + } else { + s.setClampBounds(undefined, undefined) + } } else { s?.setClampBounds(undefined, undefined) } + // Stable metadata for measure-at-unmount. Ref(null) runs before the next + // commit's layout effects, so an outgoing row sees the bottom recorded by + // the last committed mounted range. This stays O(mounted), not O(history). + for (let i = effStart; i < effEnd; i++) { + const k = items[i]?.key + + if (k) { + const bottom = offsets[i + 1] ?? 0 + + if (safeUnsignedGeometry(bottom, -1) >= 0) { + measuredBottoms.current.set(k, bottom) + } + } + } + if (skipMeasurement.current) { skipMeasurement.current = false bumpMeasuredHeightVersion(n => n + 1) @@ -492,8 +611,16 @@ export function useVirtualHistory( } const h = Math.ceil((nodes.current.get(k) as MeasuredNode | undefined)?.yogaNode?.getComputedHeight?.() ?? 0) + const previousHeight = heights.current.get(k) + + if (validVirtualItemHeight(h) && previousHeight !== h) { + // Keep the same content at the same screen row when estimates above + // the committed viewport converge. Rows intersecting or below the + // viewport intentionally retain the current scrollTop. + if (previousHeight !== undefined && (offsets[i + 1] ?? 0) <= top) { + anchorDelta += h - previousHeight + } - if (h > 0 && heights.current.get(k) !== h) { heights.current.set(k, h) dirty = true heightDirty = true @@ -501,11 +628,18 @@ export function useVirtualHistory( } } + // Sticky/live-tail positioning is owned by ScrollBox's bottom-follow + // logic. Manual viewports need an additive adjustment that leaves any + // pending input intact; scrollTo would clear that intent and other state. + if (s && anchorDelta !== 0 && !s.isSticky()) { + s.adjustScrollTop(anchorDelta) + } + if (s) { const next = { sticky: s.isSticky(), - top: Math.max(0, s.getScrollTop() + s.getPendingDelta()), - vp: Math.max(0, s.getViewportHeight()) + top: safeUnsignedGeometry(s.getScrollTop() + safeSignedGeometry(s.getPendingDelta())), + vp: safeUnsignedGeometry(s.getViewportHeight()) } if ( @@ -526,7 +660,7 @@ export function useVirtualHistory( if (heightDirty) { bumpMeasuredHeightVersion(n => n + 1) } - }, [effEnd, effStart, items, liveTailActive, measuredHeightVersion, n, offsets, scrollRef, sticky, total, vp]) + }, [effEnd, effStart, items, liveTailActive, measuredHeightVersion, n, offsets, scrollRef, sticky, top, total, vp]) return { bottomSpacer: Math.max(0, total - (offsets[effEnd] ?? total)), @@ -546,6 +680,7 @@ interface VirtualHistoryOptions { coldStartCount?: number estimate?: number estimateHeight?: (index: number, key: string) => number + generation?: number | string initialHeights?: ReadonlyMap liveTailActive?: boolean maxMounted?: number diff --git a/ui-tui/src/lib/messages.ts b/ui-tui/src/lib/messages.ts index b8e89421e5..19a2877aca 100644 --- a/ui-tui/src/lib/messages.ts +++ b/ui-tui/src/lib/messages.ts @@ -1,8 +1,17 @@ +import { MAX_HISTORY } from '../config/limits.js' import type { Msg, Role } from '../types.js' import { appendToolShelfMessage } from './liveProgress.js' export const appendTranscriptMessage = (prev: Msg[], msg: Msg): Msg[] => appendToolShelfMessage(prev, msg) +export const capTranscriptHistory = (items: Msg[]): Msg[] => { + if (items.length <= MAX_HISTORY) { + return items + } + + return items[0]?.kind === 'intro' ? [items[0], ...items.slice(-(MAX_HISTORY - 1))] : items.slice(-MAX_HISTORY) +} + export const upsert = (prev: Msg[], role: Role, text: string): Msg[] => prev.at(-1)?.role === role ? [...prev.slice(0, -1), { role, text }] : [...prev, { role, text }] diff --git a/ui-tui/src/types/hermes-ink.d.ts b/ui-tui/src/types/hermes-ink.d.ts index 7f7a53d976..4069609cb3 100644 --- a/ui-tui/src/types/hermes-ink.d.ts +++ b/ui-tui/src/types/hermes-ink.d.ts @@ -77,6 +77,7 @@ declare module '@hermes/ink' { } export type ScrollBoxHandle = { + readonly adjustScrollTop: (dy: number) => void readonly scrollTo: (y: number) => void readonly scrollBy: (dy: number) => void readonly scrollToElement: (el: unknown, offset?: number) => void