From e41bf9401b24428440ca7c4c5eb7d53c0fa696df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20Andr=C3=A9?= Date: Fri, 16 Oct 2026 06:15:04 +0200 Subject: [PATCH] perf(chat): persist transcript scroll when it settles; drop dead render notifications (#977) Scrolling no longer persists the transcript position on every scroll event: MessageSection records it once Virtua reports the scroll ended (40 -> 1 store writes, 1112 -> 160 rect reads over 1.5 s of wheel), and follow-mode changes (hold escapes immediately; a follow resume once the reader is at the bottom) are persisted at once because a pinned stream never settles. No reached position is lost: unmount captures are anchored on rows still laid out (shared with #972), and app shutdown, navigation and pagehide flush unsettled scrolls through messageStoreBus.flushScrollSnapshots(). An A/B trace showed markdown/text render notifications are needed to pin a following reader in the paint frame, while the reasoning-stream and bash RAF notifications moved nothing and the memory-accounting call was redundant; those are removed. Tests cover unsettled scrolls persisted on switch, unmount and flush with anchor and return-gap checks, settle-path persistence without a flush, a resumed follow, and the bus flush API. Gatekeeper rounds 1-6 resolved to zero findings. --- dev-docs/SESSION_HISTORY_NAVIGATION.md | 4 +- packages/ui/src/components/message-block.tsx | 43 ++---------- .../ui/src/components/message-section.tsx | 46 ++++++++++-- .../components/tool-call/renderers/bash.tsx | 51 ++------------ .../ui/src/components/virtual-follow-list.tsx | 13 ++-- .../lib/hooks/use-app-session-capture.test.ts | 4 ++ .../src/lib/hooks/use-app-session-capture.ts | 2 + packages/ui/src/stores/message-v2/bus.test.ts | 20 ++++++ packages/ui/src/stores/message-v2/bus.ts | 19 +++++ .../tests/browser/history-navigation.test.ts | 3 + .../ui/tests/browser/message-scroll.test.ts | 70 ++++++++++++++++++- 11 files changed, 180 insertions(+), 95 deletions(-) diff --git a/dev-docs/SESSION_HISTORY_NAVIGATION.md b/dev-docs/SESSION_HISTORY_NAVIGATION.md index da4b8406e..dc2f805e1 100644 --- a/dev-docs/SESSION_HISTORY_NAVIGATION.md +++ b/dev-docs/SESSION_HISTORY_NAVIGATION.md @@ -157,8 +157,8 @@ that default to their median, so a 40px fallback carried by an instance that rem before estimating lasts only until the first remount whose retained rows are all measured. A new instance reads offset 0 until the next scroll event, so a non-zero offset is seeded by one scroll event (the -remount has already recreated every row). The long-reply hold publishes its escaped -position itself, since it scrolls nothing. Gestures and explicit navigation supersede +remount has already recreated every row). The long-reply hold scrolls nothing; the +section persists its escaped position as a follow-mode change. Gestures and explicit navigation supersede restoration, alignment and bottom pins through one function; deactivation, session reset and unmount stop every list-owned positioning operation (including a restoration in flight) through another, and deactivation also releases Virtua's diff --git a/packages/ui/src/components/message-block.tsx b/packages/ui/src/components/message-block.tsx index 4da52f301..972fe4b5b 100644 --- a/packages/ui/src/components/message-block.tsx +++ b/packages/ui/src/components/message-block.tsx @@ -561,11 +561,6 @@ export default function MessageBlock(props: MessageBlockProps) { }))) const pendingFormToolTargets = () => props.pendingFormToolTargets?.() ?? localPendingFormToolTargets() const technicalCleanupPartKeys = () => props.technicalCleanupPartKeys?.() ?? new Set() - const handleContentRendered = () => { - props.onContentRendered?.() - accountSessionTranscript(props.instanceId, props.sessionId) - } - onCleanup(() => accountSessionTranscript(props.instanceId, props.sessionId)) createEffect(() => { @@ -979,7 +974,7 @@ export default function MessageBlock(props: MessageBlockProps) { (item() as ContentDisplayItem).startPartId, hovered, )} - onContentRendered={handleContentRendered} + onContentRendered={props.onContentRendered} /> @@ -997,7 +992,7 @@ export default function MessageBlock(props: MessageBlockProps) { store={props.store} messageId={(item() as ToolDisplayItem).messageId} partId={(item() as ToolDisplayItem).partId} - onContentRendered={handleContentRendered} + onContentRendered={props.onContentRendered} /> @@ -1025,7 +1020,7 @@ export default function MessageBlock(props: MessageBlockProps) { ? (expanded) => props.setTechnicalGroupExpanded?.((item() as ExplorationDisplayItem).technicalGroup!.id, expanded) : undefined} technicalCleanupPartKeys={technicalCleanupPartKeys} - onContentRendered={handleContentRendered} + onContentRendered={props.onContentRendered} /> @@ -1050,7 +1045,7 @@ export default function MessageBlock(props: MessageBlockProps) { instanceId={props.instanceId} sessionId={props.sessionId} messageId={props.messageId} - onContentRendered={handleContentRendered} + onContentRendered={props.onContentRendered} /> @@ -1067,7 +1062,7 @@ export default function MessageBlock(props: MessageBlockProps) { instanceId={props.instanceId} sessionId={props.sessionId} messageId={(item() as CompactionDisplayItem).messageId} - onContentRendered={handleContentRendered} + onContentRendered={props.onContentRendered} /> @@ -1082,7 +1077,6 @@ export default function MessageBlock(props: MessageBlockProps) { status={resolvedBlock().status} showAgentMeta={(item() as ReasoningDisplayItem).showAgentMeta} defaultExpanded={(item() as ReasoningDisplayItem).defaultExpanded} - onContentRendered={handleContentRendered} activePartId={activeSearchMatch()?.partId} showHeader={technicalGroupStartsHere((item() as ReasoningDisplayItem).technicalGroup)} expanded={(item() as ReasoningDisplayItem).technicalGroup @@ -1606,7 +1600,6 @@ function ReasoningGroupCard(props: { status: MessageRecord["status"] showAgentMeta?: boolean defaultExpanded?: boolean - onContentRendered?: () => void activePartId?: string showHeader?: boolean expanded?: Accessor @@ -1666,7 +1659,6 @@ function ReasoningGroupCard(props: { status={props.status} showAgentMeta={props.showAgentMeta} defaultExpanded={summaryParts().length > 1 || props.defaultExpanded} - onContentRendered={props.onContentRendered} forceExpanded={props.activePartId === item.partId} technicalCleanupSelected={() => props.technicalCleanupPartKeys().has(technicalPartKey(item.messageId, item.partId))} /> @@ -1738,7 +1730,6 @@ interface ReasoningCardProps { status: MessageRecord["status"] showAgentMeta?: boolean defaultExpanded?: boolean - onContentRendered?: () => void forceExpanded?: boolean technicalCleanupSelected: Accessor } @@ -1747,11 +1738,9 @@ function ReasoningStreamOutput(props: { text: Accessor scrollTopSnapshot: Accessor setScrollTopSnapshot: (next: number) => void - onContentRendered?: () => void ariaLabel: string }) { let preRef: HTMLPreElement | undefined - let pendingRenderNotificationFrame: number | null = null const followScroll = createFollowScroll({ getScrollTopSnapshot: props.scrollTopSnapshot, @@ -1759,31 +1748,14 @@ function ReasoningStreamOutput(props: { sentinelClassName: "reasoning-scroll-sentinel", }) - const notifyContentRendered = () => { - if (!props.onContentRendered || typeof requestAnimationFrame !== "function") return - if (pendingRenderNotificationFrame !== null) { - cancelAnimationFrame(pendingRenderNotificationFrame) - } - pendingRenderNotificationFrame = requestAnimationFrame(() => { - pendingRenderNotificationFrame = null - props.onContentRendered?.() - }) - } - + // The transcript's ResizeObserver handles any height change: a frame-delayed + // notification always arrived after it. createEffect(() => { const nextText = props.text() if (preRef && preRef.textContent !== nextText) { preRef.textContent = nextText } followScroll.restoreAfterRender() - notifyContentRendered() - }) - - onCleanup(() => { - if (pendingRenderNotificationFrame !== null) { - cancelAnimationFrame(pendingRenderNotificationFrame) - pendingRenderNotificationFrame = null - } }) return ( @@ -1976,7 +1948,6 @@ function ReasoningCard(props: ReasoningCardProps) { text={reasoningBody} scrollTopSnapshot={scrollTopSnapshot} setScrollTopSnapshot={setScrollTopSnapshot} - onContentRendered={props.onContentRendered} ariaLabel={t("messageBlock.reasoning.detailsAriaLabel")} /> diff --git a/packages/ui/src/components/message-section.tsx b/packages/ui/src/components/message-section.tsx index 829fcf6f8..b5cfb554e 100644 --- a/packages/ui/src/components/message-section.tsx +++ b/packages/ui/src/components/message-section.tsx @@ -555,8 +555,8 @@ export default function MessageSection(props: MessageSectionProps) { const canCapture = canCaptureScrollSnapshot({ requireActive: options?.requireActive }) if (allowCapture && canCapture) { const captured = options?.snapshot ?? listApi()?.captureScrollSnapshot() - // Only the bottom or an anchored row identifies the reader. A capture with - // neither (rows already torn down at unmount) keeps the last good position. + // Only the bottom or an anchored row identifies the reader; otherwise keep + // the last good position. const snapshot = captured && (captured.atBottom || captured.anchorKey !== undefined) ? overlayWindowOnSnapshot(captured) : undefined if (snapshot) { setLastGoodScrollSnapshot(sessionId, snapshot) @@ -583,8 +583,44 @@ export default function MessageSection(props: MessageSectionProps) { } + // A scroll persists once Virtua reports it settled, not on every event. An + // unsettled position is still recorded on unmount by the cleanup below (an + // inactive SessionView unmounts this section before hiding it) and by a + // state capture's flush. + let scrollUnsettled = false + function persistScroll() { + scrollUnsettled = false + persistMessageScrollSnapshot() + } + function persistSettledScroll() { + if (scrollUnsettled) persistScroll() + } + onCleanup(messageStoreBus.onScrollSnapshotFlush(persistSettledScroll)) + // A pinned stream keeps scrolling, so it never settles: persist a follow-mode + // change rather than wait for the stream to pause. A gesture resumes follow + // before its scroll lands, so a resume persists once the reader is at the + // bottom; earlier, the snapshot would restore as escaped. + let followResumePending = false + function persistFollowResume() { + const snapshot = listApi()?.captureScrollSnapshot() + if (!snapshot?.atBottom) return + followResumePending = false + scrollUnsettled = false + persistMessageScrollSnapshot({ snapshot }) + } + createEffect(on(() => listState()?.autoScroll(), (following, previous) => { + if (following === undefined || previous === undefined) return + followResumePending = following + if (following) persistFollowResume() + else persistScroll() + }, { defer: true })) + // Persist scroll position when switching sessions. This effect's cleanup runs // when `props.sessionId` changes, before the next session is rendered. + // Production sections are keyed per session: only the loading placeholder + // changes session, and it never persists (loading keeps its navigation + // unrestored). A retained section whose session changes after an unsettled + // scroll would keep its last settled position. createEffect(() => { const sessionId = props.sessionId onCleanup(() => { @@ -1250,10 +1286,12 @@ export default function MessageSection(props: MessageSectionProps) { const candidates = Array.from(itemWrapper.querySelectorAll(`.message-item-base[data-message-id="${key}"][data-message-role="assistant"][data-assistant-text-block="true"]`)) return candidates[candidates.length - 1] ?? null }} - onScroll={(snapshot) => { + onScroll={() => { clearQuoteSelection() - persistMessageScrollSnapshot({ snapshot }) + scrollUnsettled = true + if (followResumePending) persistFollowResume() }} + onScrollEnd={persistSettledScroll} onUserReachedTop={() => { void pageWindow("older", (api) => api.scrollToBottom({ immediate: true })) }} onScrollIntent={(direction) => { setRevealActiveToken(value => value + 1) diff --git a/packages/ui/src/components/tool-call/renderers/bash.tsx b/packages/ui/src/components/tool-call/renderers/bash.tsx index 1d2a815a6..85aec62cd 100644 --- a/packages/ui/src/components/tool-call/renderers/bash.tsx +++ b/packages/ui/src/components/tool-call/renderers/bash.tsx @@ -7,39 +7,23 @@ import { createStableAnsiStreamUpdater } from "../ansi-render" import { ansiToHtml, hasAnsi } from "../../../lib/ansi" import { getBashToolSearchText } from "../search-text" +// Output changes need no transcript notification: its ResizeObserver handles +// any height change, and a frame-delayed notification always arrived after it. function RunningBashOutput(props: { content: Accessor scrollHelpers?: ToolScrollHelpers - onContentRendered?: () => void }) { let preRef: HTMLPreElement | undefined - let pendingRenderNotificationFrame: number | null = null const updater = createStableAnsiStreamUpdater() - const notifyContentRendered = () => { - if (!props.onContentRendered || typeof requestAnimationFrame !== "function") return - if (pendingRenderNotificationFrame !== null) { - cancelAnimationFrame(pendingRenderNotificationFrame) - } - pendingRenderNotificationFrame = requestAnimationFrame(() => { - pendingRenderNotificationFrame = null - props.onContentRendered?.() - }) - } - createEffect(() => { const element = preRef if (!element) return updater.update(element, props.content()) props.scrollHelpers?.restoreAfterRender() - notifyContentRendered() }) onCleanup(() => { - if (pendingRenderNotificationFrame !== null) { - cancelAnimationFrame(pendingRenderNotificationFrame) - pendingRenderNotificationFrame = null - } preRef = undefined updater.reset() }) @@ -59,32 +43,10 @@ function RunningBashOutput(props: { function FinalAnsiOutput(props: { html: string scrollHelpers?: ToolScrollHelpers - onContentRendered?: () => void }) { - let pendingRenderNotificationFrame: number | null = null - - const notifyContentRendered = () => { - if (!props.onContentRendered || typeof requestAnimationFrame !== "function") return - if (pendingRenderNotificationFrame !== null) { - cancelAnimationFrame(pendingRenderNotificationFrame) - } - pendingRenderNotificationFrame = requestAnimationFrame(() => { - pendingRenderNotificationFrame = null - props.onContentRendered?.() - }) - } - createEffect(() => { props.html props.scrollHelpers?.restoreAfterRender() - notifyContentRendered() - }) - - onCleanup(() => { - if (pendingRenderNotificationFrame !== null) { - cancelAnimationFrame(pendingRenderNotificationFrame) - pendingRenderNotificationFrame = null - } }) return ( @@ -113,7 +75,6 @@ function BashToolBody(props: { toolState: Accessor renderMarkdown: (options: { content: string }) => ReturnType scrollHelpers?: ToolScrollHelpers - onContentRendered?: () => void }) { const state = createMemo(() => props.toolState()) @@ -165,12 +126,12 @@ function BashToolBody(props: { fallback={ {(html) => ( - + )} } > - + ) @@ -206,7 +167,7 @@ export const bashRenderer: ToolRenderer = { if (!hasCopyText) return undefined return { language: "bash", getCopyText: () => getBashCopyText(state), hasCopyText: true, wrapToggle: true, suppressInnerHeader: true } }, - renderBody({ toolState, renderMarkdown, scrollHelpers, onContentRendered }) { - return + renderBody({ toolState, renderMarkdown, scrollHelpers }) { + return }, } diff --git a/packages/ui/src/components/virtual-follow-list.tsx b/packages/ui/src/components/virtual-follow-list.tsx index c2c37ef04..12e3eb381 100644 --- a/packages/ui/src/components/virtual-follow-list.tsx +++ b/packages/ui/src/components/virtual-follow-list.tsx @@ -123,7 +123,9 @@ export interface VirtualFollowListProps { scrollToBottomAriaLabel?: Accessor onScrollElementChange?: (element: HTMLDivElement | undefined) => void onShellElementChange?: (element: HTMLDivElement | undefined) => void - onScroll?: (snapshot?: VirtualFollowScrollSnapshot) => void + onScroll?: () => void + /** Virtua's settled-scroll notification: no scroll for 150 ms and no active touch. */ + onScrollEnd?: () => void onJumpTop?: () => void onJumpBottom?: () => void onUserReachedTop?: () => void @@ -602,7 +604,7 @@ export default function VirtualFollowList(props: VirtualFollowListProps) { function handleScroll() { readerSettlement.notify() updateScrollStateFromDom() - props.onScroll?.(captureScrollSnapshot()) + props.onScroll?.() const handle = virtuaHandle() const element = scrollElement() @@ -665,10 +667,8 @@ export default function VirtualFollowList(props: VirtualFollowListProps) { dispatchFollowEvent({ type: "set-follow", enabled: false }, "hold") scrollController.recordProgrammaticOffset(virtuaHandle()?.scrollOffset ?? element.scrollTop) - // The hold changes the mode without a scroll, and growth below an escaped - // reader scrolls nothing either: publish the held position now, as the - // follow pin that preceded it did. - props.onScroll?.(captureScrollSnapshot()) + // The hold changes the mode without a scroll; consumers observing + // state.autoScroll persist the held position (as MessageSection does). return true } @@ -1375,6 +1375,7 @@ export default function VirtualFollowList(props: VirtualFollowListProps) { ...authority.probes.filter(index => index < virtualItems().length), ])]} onScroll={handleScroll} + onScrollEnd={props.onScrollEnd} > {(item, index) => { const key = props.getKey(item, index()) diff --git a/packages/ui/src/lib/hooks/use-app-session-capture.test.ts b/packages/ui/src/lib/hooks/use-app-session-capture.test.ts index 7d4e94882..3b3a938d2 100644 --- a/packages/ui/src/lib/hooks/use-app-session-capture.test.ts +++ b/packages/ui/src/lib/hooks/use-app-session-capture.test.ts @@ -25,6 +25,10 @@ it("resumes capture only after native shutdown cancellation", () => { assert.match(capture, /if \(nativeShutdownGeneration !== payload\.generation\) return[\s\S]*nativeShutdownGeneration = null[\s\S]*schedule\(\)/) }) +it("records unsettled scroll positions in the flushed payload", () => { + assert.match(capture, /messageStoreBus\.flushScrollSnapshots\(\)[\s\S]*?if \(timer\) clearTimeout\(timer\)[\s\S]*?const current = mergedState\(\)/) +}) + it("waits for server storage writes during native shutdown", () => { assert.match(capture, /nativeShutdown \? \[storage\.flushWrites\(\)\] : \[\]/) }) diff --git a/packages/ui/src/lib/hooks/use-app-session-capture.ts b/packages/ui/src/lib/hooks/use-app-session-capture.ts index 04c4f0112..79def88bd 100644 --- a/packages/ui/src/lib/hooks/use-app-session-capture.ts +++ b/packages/ui/src/lib/hooks/use-app-session-capture.ts @@ -171,6 +171,8 @@ export function useAppSessionCapture() { const flush = async (nativeShutdownGenerationRequest?: number) => { const nativeShutdown = nativeShutdownGenerationRequest !== undefined if (nativeShutdown) nativeShutdownGeneration = nativeShutdownGenerationRequest + // Record a scroll that has not settled yet; it may reschedule the capture cleared below. + messageStoreBus.flushScrollSnapshots() if (timer) clearTimeout(timer) timer = null if (enabled()) { diff --git a/packages/ui/src/stores/message-v2/bus.test.ts b/packages/ui/src/stores/message-v2/bus.test.ts index c01003676..0e86bc959 100644 --- a/packages/ui/src/stores/message-v2/bus.test.ts +++ b/packages/ui/src/stores/message-v2/bus.test.ts @@ -85,6 +85,26 @@ describe("message store scroll snapshots", () => { assert.equal(messagesLoaded().has(instanceId), false) }) + it("flushes registered scroll persisters until they unsubscribe, isolating failures", () => { + const calls: string[] = [] + const stopFailing = messageStoreBus.onScrollSnapshotFlush(() => { + calls.push("failing") + throw new Error("flush failure") + }) + const stop = messageStoreBus.onScrollSnapshotFlush(() => calls.push("persist")) + try { + messageStoreBus.flushScrollSnapshots() + assert.deepEqual(calls, ["failing", "persist"]) + stopFailing() + stop() + messageStoreBus.flushScrollSnapshots() + assert.deepEqual(calls, ["failing", "persist"]) + } finally { + stopFailing() + stop() + } + }) + it("replaces a stored position instead of merging a previous anchor into it", () => { const instanceId = "scroll-replace-anchor" const store = messageStoreBus.getOrCreate(instanceId) diff --git a/packages/ui/src/stores/message-v2/bus.ts b/packages/ui/src/stores/message-v2/bus.ts index 07c283627..569adaf14 100644 --- a/packages/ui/src/stores/message-v2/bus.ts +++ b/packages/ui/src/stores/message-v2/bus.ts @@ -24,6 +24,7 @@ class MessageStoreBus { private scrollSnapshotHandlers = new Set< (instanceId: string, sessionId: string, scope: string, snapshot: ScrollSnapshot) => void >() + private scrollSnapshotFlushHandlers = new Set<() => void>() private scrollSnapshotSeeds = new Map() registerInstance(instanceId: string, store?: InstanceMessageStore): InstanceMessageStore { @@ -109,6 +110,24 @@ class MessageStoreBus { } } + /** Views persisting scroll positions lazily register here so a state capture can record them first. */ + onScrollSnapshotFlush(handler: () => void): () => void { + this.scrollSnapshotFlushHandlers.add(handler) + return () => { + this.scrollSnapshotFlushHandlers.delete(handler) + } + } + + flushScrollSnapshots(): void { + for (const handler of this.scrollSnapshotFlushHandlers) { + try { + handler() + } catch (error) { + log.error("Failed to flush scroll snapshot", error) + } + } + } + seedScrollSnapshots(instanceId: string, seeds: MessageScrollSnapshotSeed[]): void { if (seeds.length === 0) return const store = this.stores.get(instanceId) diff --git a/packages/ui/tests/browser/history-navigation.test.ts b/packages/ui/tests/browser/history-navigation.test.ts index 3b7baa8ca..73c4f7af7 100644 --- a/packages/ui/tests/browser/history-navigation.test.ts +++ b/packages/ui/tests/browser/history-navigation.test.ts @@ -358,6 +358,9 @@ test("cancelling a committed timeline jump stores the reader's actual position", await f.page.waitForFunction(() => Boolean((window as any).cancelledPosition)) await f.page.locator(".history-navigation-status").waitFor({ state: "hidden" }) await f.page.evaluate(`new Promise(resolve => { let count = 30; const next = () => --count ? requestAnimationFrame(next) : resolve(); requestAnimationFrame(next) })`) + // The list keeps moving after the cancel; that movement is recorded when the + // scroll settles (Virtua's scroll end, 150-300 ms after the last event). + await f.page.waitForFunction(`(() => { const state = ${readStoredPosition}; return state.offset !== null && Math.abs(state.offset - state.scroll.anchorOffset) < 4 })()`, undefined, { timeout: 3000 }) const states = { cancelled: await f.page.evaluate(() => (window as any).cancelledPosition), settled: await f.page.evaluate(readStoredPosition) } for (const [when, state] of Object.entries(states)) { const actual = state.scroll.anchorKey !== undefined diff --git a/packages/ui/tests/browser/message-scroll.test.ts b/packages/ui/tests/browser/message-scroll.test.ts index 9b9fabdf4..e29a0b399 100644 --- a/packages/ui/tests/browser/message-scroll.test.ts +++ b/packages/ui/tests/browser/message-scroll.test.ts @@ -95,9 +95,15 @@ async function bottom(page: Page, scope = "") { await frames(page) } +// The store records a scroll once it settles (a pinned stream keeps scrolling); +// flush to read the position a state capture would persist now. async function readerState(page: Page, scope = "") { const gap = await page.locator(`${scope} .message-stream`).evaluate(el => el.scrollHeight - el.clientHeight - el.scrollTop) - const followModeType = await page.evaluate(() => (window as any).fixture.snapshot().scroll.followModeType) + const followModeType = await page.evaluate(async () => { + const { messageStoreBus } = await import("/src/stores/message-v2/bus.ts" as any) + messageStoreBus.flushScrollSnapshots() + return (window as any).fixture.snapshot().scroll.followModeType + }) return { gap, followModeType, latestButton: await latestButton(page, scope).count() } } @@ -323,6 +329,10 @@ for (const [gesture, zoom] of [["wheel 600", 1], ["wheel 5000", 1], ["wheel 600" else await page.mouse.wheel(0, gesture === "wheel 600" ? 600 : 5000) await pump await frames(page, 10) + // The pinned stream never settles: resuming follow is persisted without a + // flush, once at the bottom, so it restores as following. + const resumed = await page.evaluate(() => (window as any).fixture.snapshot().scroll) + assert.deepEqual({ followModeType: resumed.followModeType, atBottom: resumed.atBottom }, { followModeType: "following", atBottom: true }, JSON.stringify(resumed)) const state = await readerState(page) assert.deepEqual({ followModeType: state.followModeType, latestButton: state.latestButton }, { followModeType: "following", latestButton: 0 }, JSON.stringify(state)) assert.ok(state.gap <= 2, `The resumed reader must stay at the streaming tail: ${state.gap}px`) @@ -460,6 +470,62 @@ for (const reader of ["manual-32", "following"] as const) for (const transition }, { name: transition === "switch" ? "switch" : "session" }) }) +// Scrolling persists once settled, without a flush; a session switch or unmount +// (both unmount the section) or a state-capture flush before then records the +// reached position, anchored on the row then at the top of the viewport. +for (const transition of ["switch", "unmount", "flush"] as const) test(`an unsettled scroll is persisted on ${transition}`, async () => { + const scope = transition === "switch" ? '[data-session-id="A"]' : "" + // Live reader position: top-row anchor (as selectTopViewportAnchor picks it) and bottom gap. + const live = (scope: string) => { + const stream = document.querySelector(`${scope} .message-stream`)!, top = stream.getBoundingClientRect().top + const rows = Array.from(stream.querySelectorAll("[data-virtual-follow-key]"), row => { + const rect = row.getBoundingClientRect() + return { key: row.dataset.virtualFollowKey!, top: rect.top - top, bottom: rect.bottom - top } + }).filter(row => row.bottom > 0 && row.top < stream.clientHeight) + const anchor = rows.filter(row => row.top <= 0).sort((a, b) => b.top - a.top)[0] ?? rows.sort((a, b) => a.top - b.top)[0] + return { scrollTop: stream.scrollTop, gap: stream.scrollHeight - stream.clientHeight - stream.scrollTop, anchorKey: anchor?.key, anchorOffset: anchor?.top } + } + const assertAnchored = (persisted: any, reached: ReturnType) => { + assert.ok(Math.abs(persisted.scrollTop - reached.scrollTop) <= 1, JSON.stringify({ persisted, reached })) + assert.equal(persisted.anchorKey, reached.anchorKey, JSON.stringify({ persisted, reached })) + assert.ok(Math.abs(persisted.anchorOffset - reached.anchorOffset!) <= 2, JSON.stringify({ persisted, reached })) + } + await open(async page => { + await page.evaluate(scope => document.querySelector(`${scope} .message-stream`)!.addEventListener("scroll", () => { (window as any).lastScrollAt = Date.now() }, { passive: true }), scope) + await page.locator(`${scope} .message-stream`).hover() + await page.mouse.wheel(0, -900) + // Settles without a flush: wait for a write after the last scroll event, + // not a frame count. + await page.waitForFunction(`window.fixture.snapshot().scroll.updatedAt > window.lastScrollAt + && Math.abs(window.fixture.snapshot().scroll.scrollTop - document.querySelector(${JSON.stringify(`${scope} .message-stream`)}).scrollTop) <= 1`, undefined, { timeout: 3000 }) + const settled = await page.evaluate(`({ persisted: window.fixture.snapshot().scroll, reached: (${live})(${JSON.stringify(scope)}) })`) as any + assertAnchored(settled.persisted, settled.reached) + // Scroll again and transition in the scroll event's task, before the + // scroll-end timer can fire, whatever the load. + const result = await page.evaluate(`(async () => { + const fixture = window.fixture + const { messageStoreBus } = await import("/src/stores/message-v2/bus.ts") + const stream = document.querySelector(${JSON.stringify(`${scope} .message-stream`)}) + await new Promise(resolve => { stream.addEventListener("scroll", resolve, { once: true }); stream.scrollTop -= 600 }) + const reached = (${live})(${JSON.stringify(scope)}) + const pending = fixture.snapshot().scroll.scrollTop + if (${JSON.stringify(transition)} === "switch") fixture.switchTo("B") + else if (${JSON.stringify(transition)} === "unmount") fixture.switchAway() + else messageStoreBus.flushScrollSnapshots() + return { reached, pending, persisted: fixture.snapshot().scroll } + })()`) as any + assert.ok(Math.abs(result.pending - result.reached.scrollTop) > 100, `The scroll had already settled: ${JSON.stringify(result)}`) + assertAnchored(result.persisted, result.reached) + assert.equal(result.persisted.followModeType, "escaped") + if (transition === "flush") return + await frames(page, 30) + await page.evaluate(transition => transition === "switch" ? (window as any).fixture.switchTo("A") : (window as any).fixture.return(), transition) + await frames(page, 90) + const restored = await page.evaluate(`(${live})(${JSON.stringify(scope)})`) as any + assert.ok(Math.abs(restored.gap - result.reached.gap) <= 2, `Returned ${restored.gap}px above the bottom, left at ${result.reached.gap}px`) + }, { name: transition === "switch" ? "switch" : "session" }) +}) + for (const streamingSession of ["none", "A", "B"]) test(`retained A-B-A switch restores the reader (streaming=${streamingSession})`, async () => { await open(async page => { await page.locator('[data-session-id="A"] .message-stream').hover() @@ -621,7 +687,7 @@ async function streamUntilHold(page: Page, scope: string) { held = await stream.getAttribute("data-follow-mode") === "escaped" } assert.ok(held, "A long streamed reply must engage the hold") - // The hold scrolls nothing, so it must publish the held position itself. + // The hold scrolls nothing; the section persists it as a follow-mode change. assert.equal(await page.evaluate(() => (window as any).fixture.snapshot().scroll.followModeType), "escaped", "The hold must persist its escaped position") for (let i = 0; i < 20; i++) { await page.evaluate(() => (window as any).fixture.delta("A", "After hold.\n\n"))