diff --git a/ui-tui/src/__tests__/virtualHistoryOffsetCache.test.ts b/ui-tui/src/__tests__/virtualHistoryOffsetCache.test.ts index b4a5e7cd62..5a3e8cd097 100644 --- a/ui-tui/src/__tests__/virtualHistoryOffsetCache.test.ts +++ b/ui-tui/src/__tests__/virtualHistoryOffsetCache.test.ts @@ -1,6 +1,7 @@ -import { Box, renderSync, ScrollBox, Text, type ScrollBoxHandle } from '@hermes/ink' -import React, { useLayoutEffect, useRef } from 'react' 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 { useVirtualHistory } from '../hooks/useVirtualHistory.js' @@ -50,6 +51,7 @@ const viewportIsMounted = (items: readonly Item[], virtualHistory: ReturnType; items: readonly Item[] }) { const scrollRef = useRef(null) + const virtualHistory = useVirtualHistory(scrollRef, items, 80, { coldStartCount: 16, estimateHeight: index => items[index]?.height ?? 1, @@ -83,11 +85,45 @@ function Harness({ expose, items }: { expose: React.MutableRefObject { + it('recomputes offsets after a mounted row height changes', async () => { + const tall = [ + { height: 6, key: 'a' }, + { height: 6, key: 'b' }, + { height: 6, key: 'c' } + ] + + const short = tall.map(item => ({ ...item, height: 2 })) + const expose = { current: null as Exposed | null } + const streams = makeStreams() + + const instance = renderSync(React.createElement(Harness, { expose, items: tall }), { + patchConsole: false, + stderr: streams.stderr as NodeJS.WriteStream, + stdin: streams.stdin as NodeJS.ReadStream, + stdout: streams.stdout as NodeJS.WriteStream + }) + + try { + await delay(20) + expect(expose.current!.virtualHistory.offsets[tall.length]).toBe(18) + + instance.rerender(React.createElement(Harness, { expose, items: short })) + await delay(40) + + expect(expose.current!.virtualHistory.offsets[short.length]).toBe(6) + expect(expose.current!.virtualHistory.bottomSpacer).toBe(0) + } 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 expose = { current: null as Exposed | null } const streams = makeStreams() + const instance = renderSync(React.createElement(Harness, { expose, items: beforeShrink }), { patchConsole: false, stderr: streams.stderr as NodeJS.WriteStream, diff --git a/ui-tui/src/hooks/useVirtualHistory.ts b/ui-tui/src/hooks/useVirtualHistory.ts index dbd3a2f666..ef96ae1078 100644 --- a/ui-tui/src/hooks/useVirtualHistory.ts +++ b/ui-tui/src/hooks/useVirtualHistory.ts @@ -130,6 +130,9 @@ export function useVirtualHistory( }) const [hasScrollRef, setHasScrollRef] = useState(false) + // Height cache writes happen in layout effects; bump once so offsets and + // clamp bounds rebuild without waiting for the next scroll/input event. + const [measuredHeightVersion, bumpMeasuredHeightVersion] = useState(0) const metrics = useRef({ sticky: true, top: 0, vp: 0 }) const lastScrollTopRef = useRef(0) @@ -434,6 +437,7 @@ export function useVirtualHistory( useLayoutEffect(() => { const s = scrollRef.current let dirty = false + let heightDirty = false // Give the renderer the mounted-row coverage for passive scroll clamping. // Clamp MUST use the EFFECTIVE (deferred) range, not the immediate one. @@ -474,6 +478,7 @@ export function useVirtualHistory( if (h > 0 && heights.current.get(k) !== h) { heights.current.set(k, h) dirty = true + heightDirty = true } } } @@ -499,7 +504,11 @@ export function useVirtualHistory( offsetVersion.current++ onHeightsChangeRef.current?.(heights.current) } - }) + + if (heightDirty) { + bumpMeasuredHeightVersion(n => n + 1) + } + }, [effEnd, effStart, items, liveTailActive, measuredHeightVersion, n, offsets, scrollRef, sticky, total, vp]) return { bottomSpacer: Math.max(0, total - (offsets[effEnd] ?? total)),