From 798beb4e8009a6d7577a8b9d95f0ff7153dff9fa Mon Sep 17 00:00:00 2001 From: SivanCola <32437197+SivanCola@users.noreply.github.com> Date: Thu, 3 Sep 2026 17:02:53 +0800 Subject: [PATCH] fix(frontend): absorb block-window prepends in the reader transaction Problem (#9711): scrolling up through a long Markdown answer skips the viewport thousands of pixels to another place in the document. The field diagnostics (stable d9cd713, Windows, reducedMotion=false, all 138 rows mounted) show eight upward wheel transactions where scrollHeight grows by G, scrollTop moves by exactly +G with bottomDistance unchanged, and the reader guard then fires restore-anchor and rewrites scrollTop to its pre-growth value (19140.95 reappears one frame after a 26393 sample). Root cause: the reader's transaction anchor is a row's top edge. When the answer's block window prepends older blocks inside that row above the visible blocks, MarkdownHistory compensates scrollTop by the same amount so nothing visible moves. The anchor row's top edge is now G px higher relative to the viewport and scrollTop moved against the reader, so the guard reads a reverse displacement and "restores" the anchor, which scrolls the reader G px up into the newly inserted blocks. That restore is the reported jump. The guard's positive translateY also extends the scroller's overflow, which is why extentDelta reads 2G for one frame. Fix: the arbiter measures the scrollTop delta an accepted block-window-prepend write produced and hands it to the reader hook's new absorbOffsetWrite(). The active transaction re-baselines baselineTop, lastAcceptedTop, expectedTop and anchor.offset to the compensated position and resets its extent baselines to the grown scrollHeight, so neither the scrollTop step nor the anchor-edge shift counts as displacement. Real displacements after the absorption are still detected. Verification: - transcript-reader-visual-guard-race.test.tsx replays the field shape (row starting 1,450px above the viewport, 7,252px in-row prepend with exact compensation). On the previous head it records one anomaly, one visual guard, one reader-stability correction and rewinds scrollTop to 19204; on this head no anomaly, no guard, no correction, scrollTop stays compensated, and a later 700px real displacement is still caught. - transcript-reader-extent-race, native-scrollbar-generation, anchor-compensation-race, history-prepend-race, scroll-release, markdown-history, recovery-race, test:transcript, test:motion, typecheck, test:typecheck, lint:hooks, check:scroll-writer, repolint, git diff --check pass on the tree rebased onto #9759. - Bundle on the rebased tree: gzip 463.292 KiB (budget 463.2 -> 463.4, one cross-platform decimal of headroom over an 8-byte margin), raw 2471.741 KiB (2471.0 -> 2471.8). --- .../frontend/scripts/check-bundle-budget.mjs | 9 ++- ...anscript-reader-visual-guard-race.test.tsx | 71 ++++++++++++++++++- .../lib/useTranscriptReaderExtentStability.ts | 34 ++++++++- .../src/lib/useTranscriptScrollArbiter.ts | 10 ++- 4 files changed, 117 insertions(+), 7 deletions(-) diff --git a/desktop/frontend/scripts/check-bundle-budget.mjs b/desktop/frontend/scripts/check-bundle-budget.mjs index f3953bd62c..c1d370aa77 100644 --- a/desktop/frontend/scripts/check-bundle-budget.mjs +++ b/desktop/frontend/scripts/check-bundle-budget.mjs @@ -187,7 +187,10 @@ console.log("\nbundle budgets"); // WebView2; the merged path measures 462.827 KiB. Retain one decimal step. // Generation-bound native-thumb transactions and the rebased custom-scrollbar // drag add 0.3 KiB gzip; the merged path measures 463.102 KiB. -const initialJSBudgetKiB = 463.2; +// Absorbing content-preserving block-window prepends into the active reader +// transaction adds 0.2 KiB gzip on top; the merged path measures 463.292 KiB, +// 8 bytes under the next decimal. Retain one cross-platform decimal step. +const initialJSBudgetKiB = 463.4; assertBudget("initial JavaScript gzip", initialJSGzip, initialJSBudgetKiB * 1024); assertBudget("largest initial JavaScript chunk gzip", largestInitialJS, 280 * 1024); // Render-blocking CSS is intentionally absent: styles.css loads deferred via @@ -337,6 +340,8 @@ const rawInitialBytes = [...initialJS, ...initialCSS, ...appShellCSS] // adds 0.5 KiB raw on top; the merged path measures 2469.815 KiB. // The scrollbar generation fence and drag rebase add 1.1 KiB raw; the merged // path measures 2470.932 KiB. -const rawInitialBudgetKiB = 2_471.0; +// The reader-transaction offset absorption adds 0.8 KiB raw on top; the merged +// path measures 2471.741 KiB. +const rawInitialBudgetKiB = 2_471.8; assertBudget("initial raw JavaScript and CSS", rawInitialBytes, rawInitialBudgetKiB * 1024); assertBudget("largest initial JavaScript chunk raw", largestInitialJSRaw, 1_000 * 1024); diff --git a/desktop/frontend/src/__tests__/transcript-reader-visual-guard-race.test.tsx b/desktop/frontend/src/__tests__/transcript-reader-visual-guard-race.test.tsx index 2e4a153ccf..cc101230de 100644 --- a/desktop/frontend/src/__tests__/transcript-reader-visual-guard-race.test.tsx +++ b/desktop/frontend/src/__tests__/transcript-reader-visual-guard-race.test.tsx @@ -12,7 +12,7 @@ import { JSDOM } from "jsdom"; import React, { act } from "react"; import { createRoot } from "react-dom/client"; import type { VirtuosoHandle } from "react-virtuoso"; -import type { TranscriptScrollWriteRecord } from "../lib/transcriptScrollProbe"; +import { setTranscriptScrollDiagnosticSink, type TranscriptScrollWriteRecord } from "../lib/transcriptScrollProbe"; import { useTranscriptScrollArbiter } from "../lib/useTranscriptScrollArbiter"; let passed = 0; @@ -188,6 +188,75 @@ await flushFrames(); syncItemListTransform(); check(scrollElement.dataset.transcriptReaderVisualGuard === undefined, "the applied guard releases once the correction lands"); +itemList.style.transform = "none"; + +// Field #9711 (d9cd713, Windows, all rows mounted): the reader scrolls up +// inside a long Markdown answer whose row starts above the viewport. The +// answer's block window prepends 7,252px of older blocks inside that row and +// compensates scrollTop by the same amount, so visible content does not move. +// The anchor row's top edge is now 7,252px higher relative to the viewport +// and scrollTop moved against the reader. Neither is a displacement of what +// the reader sees: the transaction must absorb the compensation instead of +// restoring the pre-prepend scrollTop and skipping the reader into the new +// blocks. +await act(async () => arbiter?.reset()); +scrollExtent = 27_812; +scrollElement.scrollTop = 19_267; +// The long answer row starts 1,450px above the viewport and spans it. +const tallRowAt = (top: number) => ({ ...rectAt(top), bottom: top + 9_000, height: 9_000 }); +rowElement.getBoundingClientRect = () => tallRowAt(-1_450 - (scrollElement.scrollTop - 19_267)); +await act(async () => arbiter?.deliverScroll()); +await act(async () => arbiter?.releaseTailFollow()); +await act(async () => arbiter?.onWheelIntent({ + ctrlKey: false, + deltaMode: 0, + deltaX: 0, + deltaY: -63.49, + target: scrollElement, +} as React.WheelEvent)); +scrollElement.scrollTop = 19_204; +await act(async () => arbiter?.deliverScroll()); +scrollByCalls = 0; +scrollWrites.length = 0; +const anomalies: Array> = []; +setTranscriptScrollDiagnosticSink((type, fields) => { + if (type === "scroll-anomaly") anomalies.push(fields); +}); +// In-row prepend: extent grows above the visible blocks, the block window +// compensates scrollTop, the row's top edge moves up by the same amount. +scrollExtent += 7_252; +let compensated = false; +await act(async () => { compensated = Boolean(arbiter?.writeOffset("block-window-prepend", scrollElement.scrollTop + 7_252)); }); +check(compensated && scrollElement.scrollTop === 19_204 + 7_252, + `the block-window prepend compensation is written through the arbiter (${scrollElement.scrollTop})`); +await act(async () => arbiter?.deliverScroll()); +check(anomalies.length === 0, + `an in-row prepend with exact compensation is not a reader anomaly (${anomalies.length} recorded)`); +check(scrollElement.dataset.transcriptReaderVisualGuard === undefined, + "an in-row prepend with exact compensation raises no visual guard"); +for (let frame = 0; frame < 4; frame += 1) await flushFrames(); +check(scrollByCalls === 0 && scrollWrites.filter((write) => write.owner === "reader-stability").length === 0, + `the reader guard does not restore the pre-prepend scrollTop (${scrollByCalls} corrections)`); +check(scrollElement.scrollTop === 19_204 + 7_252, + `the compensated scrollTop survives (${scrollElement.scrollTop})`); +// The next wheel step continues from the compensated position. +scrollElement.scrollTop -= 190; +await act(async () => arbiter?.onWheelIntent({ + ctrlKey: false, + deltaMode: 0, + deltaX: 0, + deltaY: -190.48, + target: scrollElement, +} as React.WheelEvent)); +await act(async () => arbiter?.deliverScroll()); +check(anomalies.length === 0, "continuing to scroll after the absorbed prepend stays anomaly-free"); +// A genuine reverse jump after the absorbed prepend is still caught: the +// row moves up on screen by 700px without any scrollTop change. +rowElement.getBoundingClientRect = () => tallRowAt(-1_450 - 7_252 - (scrollElement.scrollTop - 19_267) - 700); +await act(async () => arbiter?.deliverScroll()); +check(anomalies.length === 1 && Number(anomalies[0].reverseDisplacement) >= 96, + `a real displacement after the absorbed prepend is still detected (${anomalies.length})`); +setTranscriptScrollDiagnosticSink(() => {}); await act(async () => root.unmount()); dom.window.close(); diff --git a/desktop/frontend/src/lib/useTranscriptReaderExtentStability.ts b/desktop/frontend/src/lib/useTranscriptReaderExtentStability.ts index e59814ad81..db6b8a4317 100644 --- a/desktop/frontend/src/lib/useTranscriptReaderExtentStability.ts +++ b/desktop/frontend/src/lib/useTranscriptReaderExtentStability.ts @@ -590,6 +590,37 @@ export function useTranscriptReaderExtentStability({ schedule(transaction); }, [geometryCommitReadyRef, schedule]); + /** + * A content-preserving offset write (an in-row block-window prepend that + * grows the row above the reader's view and compensates scrollTop by the + * same amount) moves the native scrollTop and the anchor row's top edge + * without moving anything the reader sees. Re-baseline the transaction to + * the compensated position so neither shows up as a reverse displacement. + */ + const absorbOffsetWrite = useCallback((element: HTMLDivElement, delta: number) => { + const transaction = transactionRef.current; + if (!transaction || transaction.element !== element || delta === 0) return; + transaction.baselineTop += delta; + transaction.lastAcceptedTop += delta; + transaction.expectedTop = Math.max(0, Math.min(nativeTranscriptBottomTop(element), transaction.expectedTop + delta)); + if (transaction.anchor) transaction.anchor.offset -= delta; + transaction.baselineHeight = element.scrollHeight; + transaction.minimumHeight = element.scrollHeight; + transaction.lastHeight = element.scrollHeight; + transaction.correctionHeight = element.scrollHeight; + transaction.transientCandidateHeight = element.scrollHeight; + transaction.transientStableFrames = 0; + transaction.lastBottomDistance = nativeTranscriptDistanceFromBottom(element); + recordTranscriptScrollDiagnostic("reader-transaction", { + transactionId: transaction.id, + ownershipEpoch: transaction.ownershipEpoch, + direction: transaction.direction, + phase: transaction.phase, + result: "absorbed-offset", + extentDelta: delta, + }); + }, []); + const arm = useCallback((deltaY: number, canClaimTail: boolean) => { const element = scrollRef.current; if (!element || !Number.isFinite(deltaY) || deltaY === 0) return { started: false as const }; @@ -700,8 +731,9 @@ export function useTranscriptReaderExtentStability({ cancel, observe, holdGeometryCommit, + absorbOffsetWrite, anchorIsMounted, isActive, active: active || readerLayoutLease, - }), [active, anchorIsMounted, arm, cancel, holdGeometryCommit, readerLayoutLease, observe, isActive]); + }), [absorbOffsetWrite, active, anchorIsMounted, arm, cancel, holdGeometryCommit, readerLayoutLease, observe, isActive]); } diff --git a/desktop/frontend/src/lib/useTranscriptScrollArbiter.ts b/desktop/frontend/src/lib/useTranscriptScrollArbiter.ts index 819cd542a3..d6387c6344 100644 --- a/desktop/frontend/src/lib/useTranscriptScrollArbiter.ts +++ b/desktop/frontend/src/lib/useTranscriptScrollArbiter.ts @@ -117,6 +117,7 @@ export function useTranscriptScrollArbiter({ cancel: cancelReaderTransaction, observe: observeReaderTransaction, holdGeometryCommit: holdReaderGeometryCommit, + absorbOffsetWrite: absorbReaderOffsetWrite, anchorIsMounted: readerAnchorIsMounted, isActive: readerTransactionIsActive, active: readerTransactionActive, @@ -242,13 +243,16 @@ export function useTranscriptScrollArbiter({ case "SCROLL_TO_INDEX": writer.write({ owner: "jump", operation: "scrollToIndex", index: command.index, behavior: command.behavior, reason: writeSource, phase: "mount-anchor", expectedSurfaceGeneration: generationRef.current, expectedOwnershipEpoch: ownershipEpochRef.current, expectedGeometryRevision: geometryRevisionRef.current }); return; - case "SCROLL_TO_OFFSET": - writer.write({ owner: command.owner, operation: "scrollTo", top: command.top, behavior: command.behavior, reason: writeSource, expectedSurfaceGeneration: generationRef.current, expectedOwnershipEpoch: ownershipEpochRef.current, expectedGeometryRevision: geometryRevisionRef.current }); + case "SCROLL_TO_OFFSET": { + const before = scrollRef.current?.scrollTop ?? 0; + const written = writer.write({ owner: command.owner, operation: "scrollTo", top: command.top, behavior: command.behavior, reason: writeSource, expectedSurfaceGeneration: generationRef.current, expectedOwnershipEpoch: ownershipEpochRef.current, expectedGeometryRevision: geometryRevisionRef.current }); + if (written && scrollRef.current && command.owner === "block-window-prepend") absorbReaderOffsetWrite(scrollRef.current, scrollRef.current.scrollTop - before); return; + } case "CANCEL_RECOVERY": cancelInFlightRecovery(command.id, command.reason); } - }, [cancelInFlightRecovery, tailSettle, writer]); + }, [absorbReaderOffsetWrite, cancelInFlightRecovery, tailSettle, writer]); const dispatch = useCallback((event: TranscriptScrollEvent) => { if (