From c3a57dc132055a6428be1d221b9732853d869427 Mon Sep 17 00:00:00 2001 From: wsp Date: Wed, 29 Jul 2026 16:12:43 +0800 Subject: [PATCH] fix(web): retain FlowChat anchors through delayed Virtuoso compensation - Replace the fixed element-anchor TTL with active and retained lifecycle phases. - Keep delayed Virtuoso compensation under viewport coordinator ownership. - Synchronize stream-end reservation changes with semantic anchor restoration. - Standardize tool-card collapse measurement on visible card roots. - Add FlowChat log analysis tooling, documentation, and regression tests. --- scripts/diagnostics/analyze-flowchat-log.mjs | 213 ++++++++++++++++++ .../modern/ExploreGroupRenderer.tsx | 5 - .../modern/FLOWCHAT_SCROLL_STABILITY.md | 22 +- .../FlowChatViewportCoordinator.test.ts | 83 +++++++ .../modern/FlowChatViewportCoordinator.ts | 104 +++++---- .../components/modern/VirtualMessageList.tsx | 11 +- .../tool-cards/ModelThinkingDisplay.tsx | 11 +- src/web-ui/src/flow_chat/tool-cards/README.md | 7 +- .../useToolCardHeightContract.test.tsx | 53 ++++- .../tool-cards/useToolCardHeightContract.ts | 6 +- 10 files changed, 440 insertions(+), 75 deletions(-) create mode 100644 scripts/diagnostics/analyze-flowchat-log.mjs diff --git a/scripts/diagnostics/analyze-flowchat-log.mjs b/scripts/diagnostics/analyze-flowchat-log.mjs new file mode 100644 index 0000000000..5cc894753c --- /dev/null +++ b/scripts/diagnostics/analyze-flowchat-log.mjs @@ -0,0 +1,213 @@ +import { createReadStream } from 'node:fs'; +import { createInterface } from 'node:readline'; + +function printUsage() { + console.log(`Usage: + node scripts/diagnostics/analyze-flowchat-log.mjs [options] + +Options: + --top Maximum rows per summary table (default: 20) + --min-delta Minimum positive reservation jump to show (default: 100) + --around Show a compact event window around a sequence + --radius Sequence radius for --around (default: 8) + --help Show this help`); +} + +function parseNumberOption(args, index, optionName) { + const rawValue = args[index + 1]; + const value = Number(rawValue); + if (!rawValue || !Number.isFinite(value)) { + throw new Error(`${optionName} requires a finite number`); + } + return value; +} + +function parseArgs(argv) { + if (argv.includes('--help')) { + printUsage(); + process.exit(0); + } + + const logPath = argv[0]; + if (!logPath || logPath.startsWith('--')) { + printUsage(); + throw new Error('A FlowChat JSONL log path is required'); + } + + const options = { + logPath, + top: 20, + minDelta: 100, + around: null, + radius: 8, + }; + + for (let index = 1; index < argv.length; index += 1) { + const arg = argv[index]; + if (arg === '--top') { + options.top = Math.max(1, Math.floor(parseNumberOption(argv, index, arg))); + index += 1; + } else if (arg === '--min-delta') { + options.minDelta = Math.max(0, parseNumberOption(argv, index, arg)); + index += 1; + } else if (arg === '--around') { + options.around = Math.floor(parseNumberOption(argv, index, arg)); + index += 1; + } else if (arg === '--radius') { + options.radius = Math.max(0, Math.floor(parseNumberOption(argv, index, arg))); + index += 1; + } else { + throw new Error(`Unknown option: ${arg}`); + } + } + + return options; +} + +function finiteNumber(value) { + const number = Number(value); + return Number.isFinite(number) ? number : 0; +} + +function round(value) { + return Math.round(finiteNumber(value) * 100) / 100; +} + +function reservationTotal(reservation) { + return finiteNumber(reservation?.collapse?.px) + finiteNumber(reservation?.pin?.px); +} + +function compactData(data) { + if (!data) return ''; + const serialized = JSON.stringify(data); + return serialized.length <= 240 ? serialized : `${serialized.slice(0, 237)}...`; +} + +async function analyze(options) { + const eventCounts = new Map(); + const reservationJumps = []; + const collapseIntents = []; + const sequenceWindow = []; + let lineCount = 0; + let eventCount = 0; + let parseErrorCount = 0; + + const input = createReadStream(options.logPath, { encoding: 'utf8' }); + const lines = createInterface({ input, crlfDelay: Infinity }); + + for await (const line of lines) { + lineCount += 1; + if (!line.trim()) continue; + + let event; + try { + event = JSON.parse(line); + } catch { + parseErrorCount += 1; + continue; + } + eventCount += 1; + + const countKey = `${event.location ?? ''}\u0000${event.message ?? ''}`; + const existingCount = eventCounts.get(countKey); + if (existingCount) { + existingCount.count += 1; + } else { + eventCounts.set(countKey, { + count: 1, + location: event.location ?? '', + message: event.message ?? '', + }); + } + + if ( + event.location === 'VirtualMessageList.updateBottomReservationState' && + event.data?.before && + event.data?.after + ) { + const before = reservationTotal(event.data.before); + const after = reservationTotal(event.data.after); + const delta = after - before; + if (delta >= options.minDelta) { + reservationJumps.push({ + sequence: event.sequence, + deltaPx: round(delta), + beforePx: round(before), + afterPx: round(after), + collapsePx: round(event.data.after.collapse?.px), + pinPx: round(event.data.after.pin?.px), + coordinatorMode: event.data.coordinatorMode ?? '', + following: event.data.isFollowingOutput === true, + streaming: event.data.isStreamingOutput === true, + }); + } + } + + if ( + event.location === 'VirtualMessageList.handleToolCardCollapseIntent' && + event.message === 'Tool card collapse reservation calculated' + ) { + const current = finiteNumber(event.data?.currentTotalCompensationPx); + const provisional = finiteNumber(event.data?.provisionalTotalCompensationPx); + collapseIntents.push({ + sequence: event.sequence, + tool: event.data?.nextIntent?.toolName ?? '', + cardHeightPx: round(event.data?.estimatedShrink), + distancePx: round(event.data?.effectiveDistanceFromBottom), + addedPx: round(provisional - current), + totalPx: round(provisional), + coordinatorMode: event.data?.coordinatorMode ?? '', + }); + } + + if ( + options.around !== null && + finiteNumber(event.sequence) >= options.around - options.radius && + finiteNumber(event.sequence) <= options.around + options.radius + ) { + sequenceWindow.push({ + sequence: event.sequence, + location: event.location ?? '', + message: event.message ?? '', + data: compactData(event.data), + }); + } + } + + console.log(`FlowChat log: ${options.logPath}`); + console.log(`Lines: ${lineCount}, events: ${eventCount}, parse errors: ${parseErrorCount}`); + + console.log('\nMost frequent events'); + console.table( + [...eventCounts.values()] + .sort((left, right) => right.count - left.count) + .slice(0, options.top), + ); + + console.log(`\nLargest reservation increases (>= ${options.minDelta}px)`); + console.table( + reservationJumps + .sort((left, right) => right.deltaPx - left.deltaPx) + .slice(0, options.top), + ); + + console.log('\nLargest collapse-intent estimates'); + console.table( + collapseIntents + .sort((left, right) => right.addedPx - left.addedPx) + .slice(0, options.top), + ); + + if (options.around !== null) { + console.log(`\nEvents around sequence ${options.around} (+/- ${options.radius})`); + console.table(sequenceWindow); + } +} + +try { + const options = parseArgs(process.argv.slice(2)); + await analyze(options); +} catch (error) { + console.error(error instanceof Error ? error.message : String(error)); + process.exitCode = 1; +} diff --git a/src/web-ui/src/flow_chat/components/modern/ExploreGroupRenderer.tsx b/src/web-ui/src/flow_chat/components/modern/ExploreGroupRenderer.tsx index 71d688c882..de3dcc10ae 100644 --- a/src/web-ui/src/flow_chat/components/modern/ExploreGroupRenderer.tsx +++ b/src/web-ui/src/flow_chat/components/modern/ExploreGroupRenderer.tsx @@ -76,11 +76,6 @@ export const ExploreGroupRenderer: React.FC = React.m } = useToolCardHeightContract({ toolId: groupId, toolName: 'explore-group', - getCardHeight: () => ( - containerRef.current?.scrollHeight - ?? containerRef.current?.getBoundingClientRect().height - ?? null - ), }); const hasExplicitState = exploreGroupStates?.has(groupId) ?? false; diff --git a/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_SCROLL_STABILITY.md b/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_SCROLL_STABILITY.md index adb4dfb3e7..3ef6c0a64d 100644 --- a/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_SCROLL_STABILITY.md +++ b/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_SCROLL_STABILITY.md @@ -161,7 +161,18 @@ restores it after the list remeasures. While an element anchor is active, the coordinator also owns virtualizer compensation corrections, so independent scroll writers cannot fight the pinned header. -There is no persistent scroll-position lock or scroll-listener lock. For an unsignaled +Collapse anchors have an active phase while layout is changing and a retained +phase after the collapse intent settles. Retained anchors do not expire on a +wall-clock timer: Virtuoso can publish a delayed size compensation after the +collapse animation and intent have finished. They keep owning virtualizer +compensation until user navigation, tail/pin ownership transfer, session reset, +or DOM disconnection. The retained phase stops the continuous animation-frame +guard; observer and scroll paths still restore the anchor on demand. Active +preservation blocks automatic tail takeover, while retained preservation allows +the tail controller to take ownership when its normal distance and intent rules +say that following should resume. + +There is no persistent raw `scrollTop` lock or scroll-listener lock. For an unsignaled shrink with no semantic element anchor, `restoreScrollPositionOnce()` performs one clamped `scrollTop` fallback using the pre-change position. It is a bounded last resort, not a second controller: subsequent layout changes are handled by @@ -281,7 +292,8 @@ unless animation is explicitly disabled. During those transitions, the DOM may report intermediate sizes for multiple frames. The collapse intent carries a hard TTL (`expiresAtMs`, currently 1000 ms), but -its settlement is autonomous rather than scroll-driven. Automatic collapses are +that TTL only bounds collapse measurement and reservation settlement; it does +not expire the semantic element anchor. Automatic collapses are finalized after `FLOWCHAT_COLLAPSE_DURATION_MS` plus a short settle-frame window; manual or otherwise unsignaled intents use the TTL timer. The scroll handler keeps only a throttled-background timer fallback for browsers that delay timers. While the intent is alive, the @@ -362,8 +374,10 @@ Current producer: - `ExploreGroupRenderer.tsx` Most tool cards now emit these events through `useToolCardHeightContract`. -Components that need more accurate collapse estimation can pass a custom -`getCardHeight` function to the helper. +The helper measures the visible `cardRootRef` and retains recent visible +measurements so state-driven collapses still report the pre-collapse height. +Never substitute an inner scroll container's `scrollHeight`; hidden overflow is +not layout height removed from the FlowChat list. If a future collapsible component shows the same "header drops" or "flash on collapse" symptom, it should likely emit `flowchat:tool-card-collapse-intent` before collapsing. diff --git a/src/web-ui/src/flow_chat/components/modern/FlowChatViewportCoordinator.test.ts b/src/web-ui/src/flow_chat/components/modern/FlowChatViewportCoordinator.test.ts index be928a4727..edf43fd441 100644 --- a/src/web-ui/src/flow_chat/components/modern/FlowChatViewportCoordinator.test.ts +++ b/src/web-ui/src/flow_chat/components/modern/FlowChatViewportCoordinator.test.ts @@ -97,6 +97,68 @@ describe('FlowChatViewportCoordinator', () => { expect(coordinator.getMode()).toBe('following-tail'); }); + it('retains a settled card anchor without a wall-clock expiry', () => { + const scroller = document.createElement('div'); + scroller.dataset.virtuosoScroller = 'true'; + const card = document.createElement('div'); + scroller.append(card); + document.body.append(scroller); + setScrollerGeometry(scroller, 900); + setRect(scroller, 0); + setRect(card, 120); + + const now = vi.spyOn(performance, 'now').mockReturnValue(1_000); + const coordinator = new FlowChatViewportCoordinator(); + expect(coordinator.preserveElement(card)).toBe(true); + expect(coordinator.settleElementPreservation('test-settled')).toBe(true); + + now.mockReturnValue(60_000); + expect(coordinator.ownsElementAnchor()).toBe(true); + expect(coordinator.getMode()).toBe('preserving-element'); + + setRect(card, 80); + expect(coordinator.restoreElementAnchor(scroller, 'test-delayed-layout')).toBe(true); + expect(scroller.scrollTop).toBe(860); + }); + + it('allows automatic tail follow to take ownership from a retained anchor', () => { + const scroller = document.createElement('div'); + scroller.dataset.virtuosoScroller = 'true'; + const card = document.createElement('div'); + scroller.append(card); + document.body.append(scroller); + setScrollerGeometry(scroller, 900); + setRect(scroller, 0); + setRect(card, 120); + + const coordinator = new FlowChatViewportCoordinator(); + coordinator.preserveElement(card); + coordinator.settleElementPreservation('test-settled'); + + expect(coordinator.followTail()).toBe(true); + expect(coordinator.getMode()).toBe('following-tail'); + expect(coordinator.ownsElementAnchor()).toBe(false); + }); + + it('releases a retained anchor when its DOM element disconnects', () => { + const scroller = document.createElement('div'); + scroller.dataset.virtuosoScroller = 'true'; + const card = document.createElement('div'); + scroller.append(card); + document.body.append(scroller); + setScrollerGeometry(scroller, 900); + setRect(scroller, 0); + setRect(card, 120); + + const coordinator = new FlowChatViewportCoordinator(); + coordinator.preserveElement(card); + coordinator.settleElementPreservation('test-settled'); + card.remove(); + + expect(coordinator.ownsElementAnchor()).toBe(false); + expect(coordinator.getMode()).toBe('idle'); + }); + it('keeps a pinned item anchored until follow mode takes ownership', () => { const scroller = document.createElement('div'); scroller.dataset.virtuosoScroller = 'true'; @@ -289,4 +351,25 @@ describe('FlowChatViewportCoordinator', () => { expect(scroller.scrollTop).toBe(800); coordinator.release('test-cleanup'); }); + + it('stops the animation-frame guard after element preservation settles', () => { + vi.spyOn(window, 'requestAnimationFrame').mockImplementation(() => 17); + const cancelFrame = vi.spyOn(window, 'cancelAnimationFrame').mockImplementation(() => {}); + + const scroller = document.createElement('div'); + scroller.dataset.virtuosoScroller = 'true'; + const card = document.createElement('div'); + scroller.append(card); + document.body.append(scroller); + setScrollerGeometry(scroller, 900); + setRect(scroller, 0); + setRect(card, 120); + + const coordinator = new FlowChatViewportCoordinator(); + coordinator.preserveElement(card); + coordinator.settleElementPreservation('test-settled'); + + expect(cancelFrame).toHaveBeenCalledWith(17); + expect(coordinator.ownsElementAnchor()).toBe(true); + }); }); diff --git a/src/web-ui/src/flow_chat/components/modern/FlowChatViewportCoordinator.ts b/src/web-ui/src/flow_chat/components/modern/FlowChatViewportCoordinator.ts index 55395befeb..d19b153fc0 100644 --- a/src/web-ui/src/flow_chat/components/modern/FlowChatViewportCoordinator.ts +++ b/src/web-ui/src/flow_chat/components/modern/FlowChatViewportCoordinator.ts @@ -18,10 +18,9 @@ type ElementAnchor = { element: HTMLElement; scroller: HTMLElement; offsetFromScrollerTop: number; - expiresAtMs: number | null; + preservationPhase: 'active' | 'retained' | null; }; -const ELEMENT_ANCHOR_TTL_MS = 1000; const ELEMENT_ANCHOR_EPSILON_PX = 0.5; const ELEMENT_ANCHOR_RANGE_GUARD_PX = 1; @@ -37,10 +36,6 @@ export function canHandoffPinnedItemToTail(options: { ); } -function nowMs(): number { - return typeof performance === 'undefined' ? Date.now() : performance.now(); -} - /** Owns anchor priority independently from the virtualizer implementation. */ export class FlowChatViewportCoordinator { private mode: FlowChatViewportAnchorMode = 'idle'; @@ -53,12 +48,12 @@ export class FlowChatViewportCoordinator { } getMode(): FlowChatViewportAnchorMode { - this.expireElementAnchor(); + this.validateElementAnchor('get-mode'); return this.mode; } ownsElementAnchor(): boolean { - this.expireElementAnchor(); + this.validateElementAnchor('owns-element-anchor'); return Boolean( this.elementAnchor && (this.mode === 'pinned-item' || this.mode === 'preserving-element'), @@ -81,18 +76,26 @@ export class FlowChatViewportCoordinator { } pinElement(element: HTMLElement | null | undefined): boolean { - return this.captureElement(element, 'pinned-item', null); + return this.captureElement(element, 'pinned-item'); } followTail(options?: { force?: boolean }): boolean { - this.expireElementAnchor(); - if (this.mode === 'preserving-element' && !options?.force) { + this.validateElementAnchor('follow-tail'); + const hasActiveElementPreservation = ( + this.mode === 'preserving-element' && + this.elementAnchor?.preservationPhase === 'active' + ); + if (hasActiveElementPreservation && !options?.force) { if (flowChatDiagnostics.isEnabled()) { flowChatDiagnostics.trace({ hypothesis: 'B', location: 'FlowChatViewportCoordinator.followTail', - message: 'Tail follow rejected while preserving an element', - data: () => ({ mode: this.mode, force: options?.force === true }), + message: 'Tail follow rejected during active element preservation', + data: () => ({ + mode: this.mode, + preservationPhase: this.elementAnchor?.preservationPhase ?? null, + force: options?.force === true, + }), }); } return false; @@ -114,7 +117,7 @@ export class FlowChatViewportCoordinator { } preserveElement(element: HTMLElement | null | undefined): boolean { - this.expireElementAnchor(); + this.validateElementAnchor('preserve-element'); if (!element || this.mode === 'following-tail' || this.mode === 'pinned-item') { if (flowChatDiagnostics.isEnabled()) { flowChatDiagnostics.trace({ @@ -130,14 +133,33 @@ export class FlowChatViewportCoordinator { return this.captureElement( element, 'preserving-element', - nowMs() + ELEMENT_ANCHOR_TTL_MS, ); } + settleElementPreservation(source = 'unspecified'): boolean { + this.validateElementAnchor('settle-element-preservation'); + const anchor = this.elementAnchor; + if (!anchor || this.mode !== 'preserving-element') { + return false; + } + + const previousPhase = anchor.preservationPhase; + anchor.preservationPhase = 'retained'; + this.stopAnchorGuard(); + if (flowChatDiagnostics.isEnabled()) { + flowChatDiagnostics.trace({ + hypothesis: 'E', + location: 'FlowChatViewportCoordinator.settleElementPreservation', + message: 'Element preservation retained after layout settlement', + data: () => ({ previousPhase, source }), + }); + } + return true; + } + private captureElement( element: HTMLElement | null | undefined, mode: 'pinned-item' | 'preserving-element', - expiresAtMs: number | null, ): boolean { if (!element) { return false; @@ -162,7 +184,7 @@ export class FlowChatViewportCoordinator { element, scroller, offsetFromScrollerTop: elementRect.top - scrollerRect.top, - expiresAtMs, + preservationPhase: mode === 'preserving-element' ? 'active' : null, }; this.mode = mode; this.startAnchorGuard(); @@ -173,6 +195,7 @@ export class FlowChatViewportCoordinator { message: 'Semantic element anchor captured', data: () => ({ mode, + preservationPhase: this.elementAnchor?.preservationPhase ?? null, elementConnected: element.isConnected, offsetFromScrollerTop: this.elementAnchor?.offsetFromScrollerTop ?? null, scrollTop: scroller.scrollTop, @@ -185,22 +208,11 @@ export class FlowChatViewportCoordinator { } restoreElementAnchor(scroller: HTMLElement, source = 'external'): boolean { - this.expireElementAnchor(); + this.validateElementAnchor(`restore:${source}`); const anchor = this.elementAnchor; if (!anchor || (this.mode !== 'preserving-element' && this.mode !== 'pinned-item')) { return false; } - if (!anchor.element.isConnected) { - if (flowChatDiagnostics.isEnabled()) { - flowChatDiagnostics.trace({ - hypothesis: 'B', - location: 'FlowChatViewportCoordinator.restoreElementAnchor', - message: 'Semantic anchor restore skipped for disconnected element', - data: () => ({ mode: this.mode, source }), - }); - } - return false; - } const readCorrection = () => { const elementRect = anchor.element.getBoundingClientRect(); @@ -301,6 +313,7 @@ export class FlowChatViewportCoordinator { release(reason = 'unspecified'): void { const previousMode = this.mode; const hadElementAnchor = Boolean(this.elementAnchor); + const previousPreservationPhase = this.elementAnchor?.preservationPhase ?? null; this.stopAnchorGuard(); this.elementAnchor = null; this.mode = 'idle'; @@ -309,27 +322,27 @@ export class FlowChatViewportCoordinator { hypothesis: 'B', location: 'FlowChatViewportCoordinator.release', message: 'Viewport coordinator released semantic ownership', - data: () => ({ previousMode, hadElementAnchor, reason }), + data: () => ({ previousMode, previousPreservationPhase, hadElementAnchor, reason }), }); } } - private expireElementAnchor(): void { - if ( - this.elementAnchor?.expiresAtMs !== null && - this.elementAnchor?.expiresAtMs !== undefined && - this.elementAnchor.expiresAtMs < nowMs() - ) { - this.elementAnchor = null; - this.stopAnchorGuard(); - if (this.mode === 'preserving-element') { - this.mode = 'idle'; - } + private validateElementAnchor(source: string): void { + const anchor = this.elementAnchor; + if (anchor && (!anchor.element.isConnected || !anchor.scroller.isConnected)) { + this.release(`element-anchor-disconnected:${source}`); } } private startAnchorGuard(): void { - if (this.anchorGuardFrame !== null || typeof requestAnimationFrame === 'undefined') { + if ( + this.anchorGuardFrame !== null || + typeof requestAnimationFrame === 'undefined' || + ( + this.mode === 'preserving-element' && + this.elementAnchor?.preservationPhase === 'retained' + ) + ) { return; } this.anchorGuardFrame = requestAnimationFrame(this.runAnchorGuardFrame); @@ -346,12 +359,15 @@ export class FlowChatViewportCoordinator { private runAnchorGuardFrame = (): void => { this.anchorGuardFrame = null; - this.expireElementAnchor(); + this.validateElementAnchor('anchor-guard'); const anchor = this.elementAnchor; if ( !anchor || (this.mode !== 'pinned-item' && this.mode !== 'preserving-element') || - !anchor.scroller.isConnected + ( + this.mode === 'preserving-element' && + anchor.preservationPhase === 'retained' + ) ) { return; } 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 ad780b0488..a6caec5dec 100644 --- a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx +++ b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx @@ -2475,6 +2475,11 @@ const VirtualMessageListSession = forwardRef = ({ }) => { const { t } = useTranslation('flow-chat'); const { content, isStreaming, status } = thinkingItem; - const wrapperRef = useRef(null); const contentRef = useRef(null); const shouldFollowTailRef = useRef(true); const tailFollowPauseVersionRef = useRef(0); @@ -48,15 +47,9 @@ export const ModelThinkingDisplay: React.FC = ({ const [isExpanded, setIsExpanded] = useState(shouldDefaultExpanded); const userToggledRef = useRef(false); - const { applyExpandedState } = useToolCardHeightContract({ + const { cardRootRef, applyExpandedState } = useToolCardHeightContract({ toolId: thinkingItem.id, toolName: 'thinking', - getAnchorElement: () => wrapperRef.current, - getCardHeight: () => { - const contentScrollHeight = contentRef.current?.scrollHeight ?? null; - const wrapperHeight = wrapperRef.current?.getBoundingClientRect().height ?? null; - return contentScrollHeight ?? wrapperHeight; - }, }); useLayoutEffect(() => { @@ -245,7 +238,7 @@ export const ModelThinkingDisplay: React.FC = ({ return (