diff --git a/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VIRTUALIZATION.md b/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VIRTUALIZATION.md index e9ce863a8..55e46a901 100644 --- a/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VIRTUALIZATION.md +++ b/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VIRTUALIZATION.md @@ -30,18 +30,23 @@ inside the window changes height, the browser reflows the ones below it in the same layout pass, so there is no frame where the scroll has been corrected but the items have not moved yet. -**The virtualizer does not compensate for its own late measurements.** -`shouldAdjustScrollPositionOnItemSizeChange` is set to refuse, always. Its rule -is the right shape — this item's delta, only for an item above the viewport — -but it applies that delta to `scrollOffset`, the library's own copy of the -scroll position, refreshed only from scroll events. Every continuous writer here -assigns `scrollTop` directly and the matching scroll event lands a frame later, -so a measurement arriving in between is compensated from a position the viewport -has already left. Measured on session open: **nine corrections across two frames -walked the viewport from 7440 back to 3556**, and the follow loop wrote 7440 -again on the next frame. The interception this replaces was written for -react-virtuoso and removed on the assumption that TanStack asked the right -question. It does — from a stale base. +**The virtualizer does not use TanStack's own late-measurement adjustment.** +`shouldAdjustScrollPositionOnItemSizeChange` reads the real scroller position +and asks the viewport owner to apply the delta only when the whole item is above +the viewport. A partly visible row is left alone because its changed content is +inside what the reader is looking at. TanStack's adjustment is always refused: +it applies its delta to `scrollOffset`, the library's copy refreshed only from +scroll events. Every continuous writer here assigns `scrollTop` directly and +the matching scroll event lands a frame later, so that base can be stale. The +owner's displacement is applied before the new size enters the cache, while +the anchor remains responsible for restoring relationships across larger layout +transactions. + +The measurement decision is recorded as the switch-gated, coalesced +`virtualizer.itemResize` probe: item identity, estimated and measured sizes, +the above-viewport decision, and the real scroll geometry before and after the +owner's displacement. It deliberately omits flow-item contents, which made the +temporary investigation probe too large for a lasting diagnostic trail. **Measurement is forced before any position is read in the commit that changed the items.** The library skips its inline resize while the reader is scrolling, diff --git a/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx b/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx index ecc6ae87f..b22e35686 100644 --- a/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx +++ b/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx @@ -131,6 +131,7 @@ interface ModelRoundItemProps { turnEndedAt?: number; turnDurationMs?: number; turnTokenUsage?: TokenUsage; + expandedThinkingItemIds?: string[]; } function sortRoundAttempts(attempts: ModelRoundAttempt[]): ModelRoundAttempt[] { @@ -362,6 +363,7 @@ export const ModelRoundItem = React.memo( turnEndedAt, turnDurationMs, turnTokenUsage, + expandedThinkingItemIds = [], }) => { const { t } = useTranslation('flow-chat'); const { formatDate, formatNumber } = useI18n('flow-chat'); @@ -488,6 +490,7 @@ export const ModelRoundItem = React.memo( turnId={turnId} roundId={options.roundId} isLastItem={isLast && itemIdx === group.items.length - 1} + expandedThinkingItemIds={expandedThinkingItemIds} /> )); @@ -516,6 +519,7 @@ export const ModelRoundItem = React.memo( turnId={turnId} roundId={options.roundId} isLastItem={isLast} + expandedThinkingItemIds={expandedThinkingItemIds} /> ); } @@ -524,7 +528,7 @@ export const ModelRoundItem = React.memo( return null; } }) - ), [sessionId, turnId]); + ), [expandedThinkingItemIds, sessionId, turnId]); const handleCopyScope = useCallback(async (scope: TranscriptExportScope) => { setIsCopyMenuOpen(false); @@ -842,6 +846,7 @@ export const ModelRoundItem = React.memo( prev.round.historyRounds === next.round.historyRounds && prev.isLastRound === next.isLastRound && prev.isTurnComplete === next.isTurnComplete && + prev.expandedThinkingItemIds === next.expandedThinkingItemIds && prev.turnStartedAt === next.turnStartedAt && prev.turnEndedAt === next.turnEndedAt && prev.turnDurationMs === next.turnDurationMs && @@ -860,6 +865,7 @@ interface FlowItemRendererProps { turnId: string; roundId?: string; isLastItem?: boolean; + expandedThinkingItemIds?: string[]; } // Do not memoize: streaming content updates frequently. @@ -868,6 +874,7 @@ const FlowItemRenderer: React.FC = ({ turnId, roundId, isLastItem, + expandedThinkingItemIds = [], }) => { const { onToolConfirm, @@ -898,7 +905,11 @@ const FlowItemRenderer: React.FC = ({ case 'thinking': return ( - + ); case 'tool': { diff --git a/src/web-ui/src/flow_chat/components/modern/VirtualItemRenderer.tsx b/src/web-ui/src/flow_chat/components/modern/VirtualItemRenderer.tsx index e5c9e1dbc..cda9ad1c1 100644 --- a/src/web-ui/src/flow_chat/components/modern/VirtualItemRenderer.tsx +++ b/src/web-ui/src/flow_chat/components/modern/VirtualItemRenderer.tsx @@ -69,6 +69,7 @@ export const VirtualItemRenderer = React.memo( turnEndedAt={item.turnEndedAt} turnDurationMs={item.turnDurationMs} turnTokenUsage={item.turnTokenUsage} + expandedThinkingItemIds={item.layoutHints?.expandedThinkingItemIds ?? []} /> ); diff --git a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.layout.test.ts b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.layout.test.ts index aca78d3c4..774d97deb 100644 --- a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.layout.test.ts +++ b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.layout.test.ts @@ -71,6 +71,35 @@ describe('estimateVirtualMessageItemHeight', () => { expect(estimateVirtualMessageItemHeight(item)).toBeGreaterThan(1000); }); + it('uses the shared collapsed hint for completed thinking rounds', () => { + const item = { + type: 'model-round', + turnId: 'turn-1', + isLastRound: false, + isTurnComplete: true, + layoutHints: { expandedThinkingItemIds: [] }, + data: { + id: 'round-thinking', + status: 'completed', + isStreaming: false, + items: [{ + id: 'thinking-1', + type: 'thinking', + content: 'x'.repeat(13_016), + status: 'completed', + timestamp: 1, + }], + }, + } as VirtualItem; + + expect(estimateVirtualMessageItemHeight(item)).toBe(200); + + expect(estimateVirtualMessageItemHeight({ + ...item, + layoutHints: { expandedThinkingItemIds: ['thinking-1'] }, + })).toBeGreaterThan(1000); + }); + it('keeps compact user-only rows small enough for partial history tails', () => { const item = { type: 'user-message', diff --git a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx index f8dab99d1..1745be04a 100644 --- a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx +++ b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx @@ -488,6 +488,7 @@ const VirtualMessageListSession = forwardRef virtualItems diff --git a/src/web-ui/src/flow_chat/components/modern/flowChatViewportOwnership.test.ts b/src/web-ui/src/flow_chat/components/modern/flowChatViewportOwnership.test.ts index 7237a8961..e71ea494f 100644 --- a/src/web-ui/src/flow_chat/components/modern/flowChatViewportOwnership.test.ts +++ b/src/web-ui/src/flow_chat/components/modern/flowChatViewportOwnership.test.ts @@ -166,12 +166,13 @@ describe('canShiftViewport', () => { expect(canShiftViewport(heldBy('user-gesture', { holdForMs: 200 }), NOW)).toBe(true); }); - it('leaves the displacement to anyone holding a target', () => { - // All three re-assert a position of their own, so a shift underneath is - // either redundant or a fight. + it('leaves the displacement to navigation and follow targets', () => { + // Those writers re-assert a position of their own. Snap-back is different: + // a late history measurement invalidates its target coordinate, so the + // displacement must cancel the stale animation and be reconsidered. expect(canShiftViewport(heldBy('follow-output'), NOW)).toBe(false); expect(canShiftViewport(heldBy('one-shot-navigation', { holdForMs: 600 }), NOW)).toBe(false); - expect(canShiftViewport(heldBy('snap-back', { holdForMs: 1_200 }), NOW)).toBe(false); + expect(canShiftViewport(heldBy('snap-back', { holdForMs: 1_200 }), NOW)).toBe(true); }); it('shifts an unheld viewport, and one held only by a correction', () => { diff --git a/src/web-ui/src/flow_chat/components/modern/flowChatViewportOwnership.ts b/src/web-ui/src/flow_chat/components/modern/flowChatViewportOwnership.ts index 4ef3c8068..8c0e03ac1 100644 --- a/src/web-ui/src/flow_chat/components/modern/flowChatViewportOwnership.ts +++ b/src/web-ui/src/flow_chat/components/modern/flowChatViewportOwnership.ts @@ -197,12 +197,15 @@ export function claimViewport( * the reader belongs, so a displacement applied underneath them is either * redundant or a fight. * + * `snap-back` is deliberately not one of them either: a late measurement of + * history above the reader invalidates the snap target's coordinate. Letting + * that displacement through cancels the stale animation by changing the real + * scroll position; the next settle re-evaluates the target from fresh geometry. * `user-gesture` is deliberately not one of them, and that is the whole point * of this being a separate question. */ const OWNERS_THAT_HOLD_A_TARGET: ReadonlySet = new Set([ 'one-shot-navigation', - 'snap-back', 'follow-output', ]); diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.test.ts b/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.test.ts index daec9004d..fe5127de6 100644 --- a/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.test.ts +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.test.ts @@ -1,10 +1,21 @@ import { describe, expect, it } from 'vitest'; import { + isItemFullyAboveViewport, virtualWindowPaddingPx, visibleRowRange, type FlowChatVirtualRow, } from './useFlowChatVirtualizer'; +describe('isItemFullyAboveViewport', () => { + it('does not compensate a row that is partly visible', () => { + expect(isItemFullyAboveViewport(8280, 5450)).toBe(false); + }); + + it('compensates only a row whose end is above the viewport', () => { + expect(isItemFullyAboveViewport(4722, 5450)).toBe(true); + }); +}); + /** Header above the items, which every offset below is measured past. */ const CONTENT_START = 24; diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.ts b/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.ts index 3348e3098..cd74131d7 100644 --- a/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.ts +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.ts @@ -30,6 +30,8 @@ import { useVirtualizer } from '@tanstack/react-virtual'; import { roundViewportPx, traceViewport, + traceViewportRepeating, + isViewportDiagnosticsEnabled, } from '@/infrastructure/diagnostics/flowChatViewportDiagnostics'; import type { FlowChatViewportOwner } from './flowChatViewportOwnership'; @@ -47,7 +49,7 @@ const FLOW_CHAT_OVERSCAN_ITEMS = 6; const VIRTUALIZER_REAIM_WINDOW_MS = 5_000; /** - * The virtualizer does not compensate for its own late measurements. + * TanStack's default late-measurement adjustment is not safe for this list. * * Its rule is the right shape — adjust by *this item's* delta, and only for an * item above the viewport — but it applies that delta to `scrollOffset`, which @@ -59,11 +61,10 @@ const VIRTUALIZER_REAIM_WINDOW_MS = 5_000; * two frames walked the viewport from 7440 back to 3556 before the follow loop * wrote 7440 again. * - * The viewport anchor is the compensator instead. It restores a relationship - * rather than replaying a delta, so it has no base to go stale. + * For a row wholly above the reader, the real viewport owner applies the + * measured delta before TanStack updates its cache. Rows intersecting the + * viewport are left alone: their content is what the reader is looking at. */ -const neverAdjustScrollPositionOnItemResize = () => false; - export interface FlowChatVirtualRow { index: number; key: string; @@ -76,6 +77,19 @@ export interface FlowChatItemBounds { endPx: number; } +/** A resize can shift the reader only when the whole row is above it. */ +export function isItemFullyAboveViewport(itemEndPx: number, scrollTopPx: number): boolean { + return itemEndPx <= scrollTopPx; +} + +function resizeDeltaBand(deltaPx: number): string { + const magnitudePx = Math.abs(deltaPx); + if (magnitudePx < 1) return 'subpixel'; + if (magnitudePx < 16) return 'small'; + if (magnitudePx < 128) return 'medium'; + return 'large'; +} + /** * The measurement pass, which the published types keep to themselves. * @@ -125,6 +139,8 @@ export interface UseFlowChatVirtualizerOptions { behavior?: ScrollBehavior; holdForMs?: number; }) => boolean; + /** Shift the viewport before a measurement changes row heights. */ + shiftViewport?: (byPx: number) => boolean; } export interface FlowChatVirtualizer { @@ -264,6 +280,7 @@ export function useFlowChatVirtualizer({ estimateItemHeightPx, scrollPaddingStartPx, writeViewport, + shiftViewport = () => false, }: UseFlowChatVirtualizerOptions): FlowChatVirtualizer { const itemsRef = useRef(items); itemsRef.current = items; @@ -358,7 +375,65 @@ export function useFlowChatVirtualizer({ }); // An instance field rather than an option, so it is assigned here — before // any measurement callback can reach `resizeItem`. - virtualizer.shouldAdjustScrollPositionOnItemSizeChange = neverAdjustScrollPositionOnItemResize; + virtualizer.shouldAdjustScrollPositionOnItemSizeChange = (item, delta) => { + const scroller = scrollerRef.current; + if (!scroller) return false; + const beforeScrollTopPx = scroller.scrollTop; + const beforeScrollHeightPx = scroller.scrollHeight; + // A row that merely starts above the viewport may still be visible. Its + // resize changes content inside the reader rather than moving content that + // is wholly above it, so compensating its full delta would pull the reader + // by the size of a row they are looking at (history model rounds can be + // several thousand pixels). Only a row whose end is above the viewport can + // move the reader's existing content and needs a viewport shift. + const fullyAboveViewport = isItemFullyAboveViewport(item.end, beforeScrollTopPx); + const applied = fullyAboveViewport ? shiftViewport(delta) : false; + const virtualItem = itemsRef.current[item.index]; + if (isViewportDiagnosticsEnabled()) { + const diagnosticItem = virtualItem as { + type?: unknown; + turnId?: unknown; + } | undefined; + const itemKey = virtualItem === undefined ? null : getItemKeyRef.current(virtualItem); + traceViewportRepeating( + `itemResize|${itemKey ?? 'unknown'}|${fullyAboveViewport}|${applied}|${resizeDeltaBand(delta)}`, + { + location: 'virtualizer.itemResize', + message: 'an item changed size during virtualizer measurement', + travelPx: delta, + data: () => ({ + index: item.index, + itemKey, + itemType: typeof diagnosticItem?.type === 'string' ? diagnosticItem.type : null, + turnId: typeof diagnosticItem?.turnId === 'string' ? diagnosticItem.turnId : null, + estimatedSizePx: virtualItem === undefined + ? null + : roundViewportPx(estimateItemHeightRef.current(virtualItem)), + previousItemSizePx: roundViewportPx(item.size), + nextItemSizePx: roundViewportPx(item.size + delta), + itemStartPx: roundViewportPx(item.start), + itemEndPx: roundViewportPx(item.end), + deltaPx: roundViewportPx(delta), + fullyAboveViewport, + beforeScrollTopPx: roundViewportPx(beforeScrollTopPx), + beforeScrollHeightPx: roundViewportPx(beforeScrollHeightPx), + applied, + afterScrollTopPx: roundViewportPx(scroller.scrollTop), + afterScrollHeightPx: roundViewportPx(scroller.scrollHeight), + }), + }, + ); + } + /* + * TanStack's default adjustment is based on its cached scroll offset. This + * list writes the real scroller through the viewport register, so that copy + * can be one frame stale. Move the real viewport before the new size enters + * the cache; otherwise a shrinking range can make the browser clamp + * scrollTop to the physical tail before the reader's anchor gets a chance + * to restore it. + */ + return false; + }; const virtualRows = virtualizer.getVirtualItems(); const totalSizePx = virtualizer.getTotalSize(); diff --git a/src/web-ui/src/flow_chat/components/modern/virtualMessageListLayout.ts b/src/web-ui/src/flow_chat/components/modern/virtualMessageListLayout.ts index 6653b9af7..f43f3cec0 100644 --- a/src/web-ui/src/flow_chat/components/modern/virtualMessageListLayout.ts +++ b/src/web-ui/src/flow_chat/components/modern/virtualMessageListLayout.ts @@ -72,7 +72,8 @@ function getFlowItemTextLength(item: AnyFlowItem): number { return 0; } -function estimateFlowItemHeight(item: AnyFlowItem): number { +function estimateFlowItemHeight(item: AnyFlowItem, expandedThinkingItemIds: readonly string[]): number { + if (item.type === 'thinking' && !expandedThinkingItemIds.includes(item.id)) return 40; const textLength = getFlowItemTextLength(item); if (textLength > 0) { return Math.min( @@ -103,7 +104,10 @@ function estimateModelRoundHeight(item: Extract total + estimateFlowItemHeight(flowItem), + (total, flowItem) => total + estimateFlowItemHeight( + flowItem, + item.layoutHints?.expandedThinkingItemIds ?? [], + ), 0, ); return Math.min(3600, Math.max(LIVE_SESSION_DEFAULT_ITEM_HEIGHT_PX, MODEL_ROUND_BASE_HEIGHT_PX + contentHeight)); diff --git a/src/web-ui/src/flow_chat/store/modernFlowChatStore.test.ts b/src/web-ui/src/flow_chat/store/modernFlowChatStore.test.ts index f3deb996e..b981ba592 100644 --- a/src/web-ui/src/flow_chat/store/modernFlowChatStore.test.ts +++ b/src/web-ui/src/flow_chat/store/modernFlowChatStore.test.ts @@ -370,6 +370,55 @@ describe('sessionToVirtualItems explore grouping', () => { expect(items.map(item => item.type)).toEqual(['user-message', 'model-round']); }); + it('projects the thinking expansion state used by the renderer into layout hints', () => { + const thinkingItem = { + id: 'thinking-1', + type: 'thinking' as const, + content: 'Inspecting the implementation', + isStreaming: true, + timestamp: 1000, + status: 'streaming' as const, + }; + const session = makeSession({ + sessionId: 'thinking-layout-session', + dialogTurns: [{ + id: 'turn-1', + sessionId: 'thinking-layout-session', + userMessage: { + id: 'user-1', + content: 'Help', + timestamp: 900, + }, + modelRounds: [ + makeRound({ + id: 'earlier-thinking', + items: [{ ...thinkingItem, id: 'thinking-0', isStreaming: false, status: 'completed' }], + renderHints: { disableExploreGrouping: true }, + }), + makeRound({ + id: 'active-thinking', + index: 1, + items: [thinkingItem], + isStreaming: true, + isComplete: false, + status: 'streaming', + renderHints: { disableExploreGrouping: true }, + }), + ], + status: 'processing', + startTime: 900, + }], + }); + + const modelRounds = sessionToVirtualItems(session) + .filter((item): item is ModelRoundVirtualItem => item.type === 'model-round'); + + expect(modelRounds.map(item => item.layoutHints?.expandedThinkingItemIds)).toEqual([ + [], + ['thinking-1'], + ]); + }); + it('appends a completion notice for abnormal completed turns', () => { const session = makeSession({ dialogTurns: [{ diff --git a/src/web-ui/src/flow_chat/store/modernFlowChatStore.ts b/src/web-ui/src/flow_chat/store/modernFlowChatStore.ts index 13009ff98..621616775 100644 --- a/src/web-ui/src/flow_chat/store/modernFlowChatStore.ts +++ b/src/web-ui/src/flow_chat/store/modernFlowChatStore.ts @@ -76,6 +76,9 @@ export type VirtualItem = turnId: string; isLastRound: boolean; isTurnComplete: boolean; + layoutHints?: { + expandedThinkingItemIds: string[]; + }; turnStartedAt?: number; turnEndedAt?: number; turnDurationMs?: number; @@ -501,6 +504,12 @@ export function sessionToVirtualItems(session: Session | null): VirtualItem[] { turnId: turn.id, isLastRound: roundIndex === rounds.length - 1, isTurnComplete, + layoutHints: { + expandedThinkingItemIds: roundIndex === rounds.length - 1 + && round.items.at(-1)?.type === 'thinking' + ? [round.items.at(-1)!.id] + : [], + }, turnStartedAt: turn.startTime, turnEndedAt: turn.endTime, turnDurationMs: typeof turn.endTime === 'number' diff --git a/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.tsx b/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.tsx index f7f317505..3c39981db 100644 --- a/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.tsx @@ -29,12 +29,14 @@ interface ModelThinkingDisplayProps { thinkingItem: FlowThinkingItem; /** Whether this is the last item in the current round. */ isLastItem?: boolean; + forceExpanded?: boolean; displayContext?: 'default' | 'subagent-projection'; } export const ModelThinkingDisplay: React.FC = ({ thinkingItem, isLastItem = true, + forceExpanded = false, displayContext = 'default', }) => { const { t } = useTranslation('flow-chat'); @@ -50,10 +52,11 @@ export const ModelThinkingDisplay: React.FC = ({ const isActive = isStreaming || status === 'streaming'; const { displayText: displayContent, isRevealing } = useTypewriter(content, isActive); useReportTypewriterReveal(thinkingItem.id, isRevealing); - const shouldDefaultExpanded = + const shouldDefaultExpanded = forceExpanded || ( displayContext === 'subagent-projection' ? isActive || isLastItem - : isLastItem; + : isLastItem + ); const [isExpanded, setIsExpanded] = useState(shouldDefaultExpanded); const userToggledRef = useRef(false);