From 041d72a66c31ef068c2c535f32f8695956084297 Mon Sep 17 00:00:00 2001 From: npub1jmc9dt2lyvzu3h0kxlwxt5zg4fxp9476awyxw6gwxn72g6cw7exqs64whm <96f056ad5f2305c8ddf637dc65d048aa4c12d7daeb8867690e34fca46b0ef64c@sprout-oss.stage.blox.sqprod.co> Date: Thu, 9 Jul 2026 10:40:47 -0400 Subject: [PATCH] feat(desktop): polled settle gate for mid-history prepend re-anchor (default-off) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mid-history prepend re-anchor (`scrollBy(0, drift)` in the post-commit layout effect) is main's one mid-fling scroll writer: when an older-history prepend commits while the user is flinging, it fires against live momentum and produces the walk-blind position jump. There is no event-driven "fling ended" signal to gate it on — WebKit coalesces momentum `scroll` events off the scrolling thread, so the 100ms debounce starves exactly when the fling is fastest (the mechanism pinned in Lane C). Add an opt-in, default-off polled quiet-window gate. `observeScrollSettle` polls `container.scrollTop` on requestAnimationFrame and declares settle only after k *consecutive* zero-delta frames; the re-anchor is deferred behind it and re-measures drift at settle time so it compensates the row's resting position, not a stale mid-fling read. A newer prepend, channel switch, or unmount cancels any pending correction so only the freshest anchor is re-pinned. Design edge #1 (walk-blind #1662 mechanism): WebKit freezes scrollTop reads for ~2 coalesced frames mid-fling. k MUST exceed that window or the gate reads the freeze as "settled" and fires mid-momentum, recreating the bug. Default k=3 clears the ~2-frame freeze with one frame of margin; `observeScrollSettle` clamps any caller value up to that floor so a mis-tuned k can never regress below it. Tune against the classifier, not by eye. A3 trade (for the PR body): this does not eliminate the correction, it defers it. The artifact the user feels moves from a jump mid-scroll to a small settle-time adjustment as the reading row snaps back once at rest. Default-off ⇒ byte-identical to main: no consumer wires `settleGate`, so the re-anchor runs immediately on the prepend commit as it always has. Verified: smoke behavior-contract tier (scroll-history + timeline-no-shift + virtualization, 25 tests) green; new observeScrollSettle unit tests (7) green, including the coalesced-freeze survival case; tsc + biome clean. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> --- .../messages/ui/useAnchoredScroll.test.mjs | 125 ++++++++++++- .../features/messages/ui/useAnchoredScroll.ts | 169 +++++++++++++++++- 2 files changed, 288 insertions(+), 6 deletions(-) diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.test.mjs b/desktop/src/features/messages/ui/useAnchoredScroll.test.mjs index e804ca0951..dcdf1a8eec 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.test.mjs +++ b/desktop/src/features/messages/ui/useAnchoredScroll.test.mjs @@ -1,7 +1,11 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { settleProgrammaticBottomPin } from "./useAnchoredScroll.ts"; +import { + DEFAULT_SETTLE_QUIET_FRAMES, + observeScrollSettle, + settleProgrammaticBottomPin, +} from "./useAnchoredScroll.ts"; function fakeContainer({ clientHeight, scrollHeight, scrollTop }) { const writes = []; @@ -48,3 +52,122 @@ test("settleProgrammaticBottomPin keeps settling when the floor is still out of 2, ); }); + +// --------------------------------------------------------------------------- +// observeScrollSettle — the polled quiet-window detector at the heart of the +// settle gate. A manual frame driver replaces requestAnimationFrame so each +// tick can read a scripted scrollTop, making the WebKit coalesced-freeze case +// (design edge #1) directly testable. +// --------------------------------------------------------------------------- + +/** + * Drive `observeScrollSettle` frame-by-frame. `positions` is the scrollTop the + * container reports on each successive tick. Returns { settledAt } — the + * 1-based frame index at which onSettle fired, or null if it never did within + * the scripted frames. + */ +function runSettle(positions, quietFrames) { + const queue = []; + const scheduleFrame = (cb) => { + queue.push(cb); + return queue.length; // non-zero id + }; + const cancelled = new Set(); + const cancelFrame = (id) => cancelled.add(id); + + let frame = 0; + const container = { + get scrollTop() { + // Position seen on the tick about to run. Clamp to the last scripted + // value so a still tail keeps reporting the final resting position. + return positions[Math.min(frame, positions.length - 1)]; + }, + }; + + let settledAt = null; + const cancel = observeScrollSettle( + container, + quietFrames, + () => { + settledAt = frame; + }, + scheduleFrame, + cancelFrame, + ); + + // Pump queued frames until the loop stops scheduling or we exhaust the script. + while (queue.length > 0 && frame < positions.length + quietFrames + 2) { + const cb = queue.shift(); + frame += 1; + cb(); + } + return { settledAt, cancel }; +} + +test("observeScrollSettle fires only after k consecutive still frames", () => { + // Baseline top is 100 (captured at arm time). Frames all report 100 → still + // from frame 1. With k=3, settle fires on the 3rd consecutive still frame. + const { settledAt } = runSettle([100, 100, 100, 100], 3); + assert.equal(settledAt, 3); +}); + +test("observeScrollSettle does not settle during the WebKit coalesced freeze", () => { + // Design edge #1: WebKit freezes scrollTop reads for ~2 frames mid-fling, + // then momentum resumes. A gate that fired on 2 still frames would re-anchor + // here — mid-fling — recreating the walk-blind jump. Baseline 100, frozen at + // 100 for two frames, then the fling resumes (120, 140), then truly rests. + const positions = [100, 100, 120, 140, 160, 160, 160, 160]; + const { settledAt } = runSettle(positions, DEFAULT_SETTLE_QUIET_FRAMES); + // Must NOT have fired during the 2-frame freeze (frames 1–2). + assert.ok( + settledAt === null || settledAt > 2, + `settled at frame ${settledAt}; expected to survive the 2-frame freeze`, + ); + // It should settle only once truly at rest at 160 (frames 5–8: three stills + // after the last move land the settle on frame 7). + assert.equal(settledAt, 7); +}); + +test("observeScrollSettle resets the counter when movement resumes", () => { + // One still frame, then a move, then a genuine rest. The lone still frame + // must not count toward the final quiet window. + const { settledAt } = runSettle([100, 100, 130, 130, 130, 130], 3); + // First still frame is at pos[1]; the move to 130 at pos[2] resets the + // counter. Three stills follow (pos[3..5]) → settle on frame 5, proving the + // lone earlier still did not carry over. + assert.equal(settledAt, 5); +}); + +test("observeScrollSettle cancel stops the loop and never fires onSettle", () => { + const queue = []; + const scheduleFrame = (cb) => { + queue.push(cb); + return queue.length; + }; + const cancelled = []; + const container = { scrollTop: 100 }; + let fired = false; + const cancel = observeScrollSettle( + container, + 3, + () => { + fired = true; + }, + scheduleFrame, + (id) => cancelled.push(id), + ); + // Run one still frame (counter = 1), then cancel before it can reach k. + queue.shift()(); + cancel(); + // Any queued frame must be cancelled; draining it must be a no-op. + while (queue.length > 0) queue.shift()(); + assert.equal(fired, false); + assert.ok(cancelled.length >= 1); +}); + +test("observeScrollSettle clamps sub-minimum quietFrames up to the default", () => { + // A caller passing quietFrames=1 would settle on the first still frame and + // read the freeze as a settle. The clamp forces the freeze-clearing minimum. + const { settledAt } = runSettle([100, 100, 100, 100], 1); + assert.equal(settledAt, DEFAULT_SETTLE_QUIET_FRAMES); +}); diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index 2c0176df7f..1bf77e3046 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -49,8 +49,52 @@ type UseAnchoredScrollOptions = { /** When set, scroll to and highlight this message on mount and on change. */ targetMessageId?: string | null; onTargetReached?: (messageId: string) => void; + + /** + * Opt-in, default-off polled settle gate for the mid-history prepend + * re-anchor. When `enabled`, the prepend drift-correction (`scrollBy`) is + * deferred while a user fling is in progress and applied only once the scroll + * has quiesced — see {@link SettleGateOptions}. When absent/off, the timeline + * behaves byte-identically to main: the correction runs immediately on the + * prepend commit, as it always has. + */ + settleGate?: SettleGateOptions; }; +/** + * Configuration for the polled quiet-window settle gate. + * + * The gate exists because there is no event-driven "user fling has ended" + * signal that survives WebKit's momentum-event coalescing — a debounce on + * `scroll` events starves on WebKit exactly when a fling is fastest. So we + * poll `scrollTop` on `requestAnimationFrame` and declare settle only after + * `quietFrames` *consecutive* frames of zero movement. + * + * Design edge (#1): WebKit freezes scroll-position reads for ~2 coalesced + * frames mid-fling. A gate that fires on the first still frame would read that + * freeze as "settled" and re-anchor mid-momentum — recreating the walk-blind + * jump this gate is meant to remove. `quietFrames` MUST exceed that freeze + * window; the default of 3 clears the ~2-frame freeze with one frame of margin. + * Tune against the classifier, not by eye. + */ +export type SettleGateOptions = { + /** Master switch. Off (or option omitted) ⇒ zero behavior change vs. main. */ + enabled: boolean; + /** + * Consecutive zero-delta rAF frames required to declare the scroll settled. + * Must exceed WebKit's coalesced-freeze window (~2). Defaults to + * {@link DEFAULT_SETTLE_QUIET_FRAMES}. + */ + quietFrames?: number; +}; + +/** + * Default consecutive-still-frame count for the settle gate. Chosen to clear + * WebKit's ~2-frame coalesced-freeze window (design edge #1) with a one-frame + * margin, so the gate never mistakes the freeze for a genuine settle. + */ +export const DEFAULT_SETTLE_QUIET_FRAMES = 3; + type UseAnchoredScrollResult = { /** Pass through to the scroll container's `onScroll`. */ onScroll: () => void; @@ -98,6 +142,65 @@ function isAtTrueBottom( ); } +/** + * Poll `container.scrollTop` on `requestAnimationFrame` and invoke `onSettle` + * once the position has held still for `quietFrames` *consecutive* frames. + * + * This is the settle gate's core. It exists because WebKit coalesces momentum + * `scroll` events, so an event-debounce never fires at the true end of a fling + * (design context in {@link SettleGateOptions}). Polling the position directly + * side-steps the event stream entirely: a fling is "over" when the number stops + * changing, and the consecutive-frame requirement is what makes it robust to + * WebKit's mid-fling ~2-frame position-read freeze — a single still frame is + * not enough, so the freeze cannot be mistaken for a settle. + * + * Returns a `cancel` function; call it if the observation is superseded (e.g. a + * newer prepend arrives, or the component unmounts) so the rAF loop stops and + * `onSettle` never fires for stale work. + */ +export function observeScrollSettle( + container: Pick, + quietFrames: number, + onSettle: () => void, + scheduleFrame: (cb: () => void) => number = requestAnimationFrame, + cancelFrame: (id: number) => void = cancelAnimationFrame, +): () => void { + // Guard: a non-positive frame count would settle instantly and defeat the + // gate's purpose. Clamp to the minimum that still clears the freeze window. + const required = Math.max(quietFrames, DEFAULT_SETTLE_QUIET_FRAMES); + let lastTop = container.scrollTop; + let stillFrames = 0; + let rafId: number | null = null; + let cancelled = false; + + const tick = () => { + if (cancelled) return; + const top = container.scrollTop; + if (top === lastTop) { + stillFrames += 1; + } else { + stillFrames = 0; + lastTop = top; + } + if (stillFrames >= required) { + rafId = null; + onSettle(); + return; + } + rafId = scheduleFrame(tick); + }; + + rafId = scheduleFrame(tick); + + return () => { + cancelled = true; + if (rafId !== null) { + cancelFrame(rafId); + rafId = null; + } + }; +} + /** * Pick an anchor for the current scroll position. * @@ -150,6 +253,7 @@ export function useAnchoredScroll({ targetMessageId = null, onTargetReached, + settleGate, }: UseAnchoredScrollOptions): UseAnchoredScrollResult { // Anchor lives in a ref because it must survive renders and is updated // both on scroll (commit-time read) and in the layout effect (post-render @@ -181,6 +285,17 @@ export function useAnchoredScroll({ // ignores transient gaps and keeps chasing the floor. A `ref`, not state — the // guard runs on a native scroll event, outside React's render cycle. const settlingRef = React.useRef(false); + // Cancel handle for an in-flight settle observation armed by the prepend + // re-anchor while a fling is in progress (settle gate only). Held so a newer + // prepend, a channel switch, or unmount can supersede the pending correction + // before its `onSettle` fires — otherwise a stale re-anchor could snap the + // view after the user has already flung somewhere else. + const settleObserverCancelRef = React.useRef<(() => void) | null>(null); + // Keep the latest settle-gate config in a ref so the layout effect can read + // it without adding it to the dependency array (it must not re-run the + // restoration logic just because the caller passed a new options object). + const settleGateRef = React.useRef(settleGate); + settleGateRef.current = settleGate; // Reset everything when the channel changes — the layout effect that runs // immediately after this reset is responsible for either jumping to bottom @@ -207,6 +322,10 @@ export function useAnchoredScroll({ cancelAnimationFrame(mountPinRafIdRef.current); mountPinRafIdRef.current = null; } + if (settleObserverCancelRef.current !== null) { + settleObserverCancelRef.current(); + settleObserverCancelRef.current = null; + } }, [channelId]); const scrollToBottomImperative = React.useCallback( @@ -420,17 +539,53 @@ export function useAnchoredScroll({ // observer's promise callback) because the prepended rows commit on a // deferred snapshot a few frames later, so the row's true position is only // known here. - const row = container.querySelector( - `[data-message-id="${CSS.escape(anchor.messageId)}"]`, - ); - if (row) { + // + // Re-pin measured fresh at call time: find the anchored row, read its + // current top offset, and `scrollBy` the drift back to its saved offset. + // Deferred settle callers re-run this so they compensate the row's + // *settle-time* position, not a stale mid-fling measurement. + const applyReanchor = () => { + const currentRow = container.querySelector( + `[data-message-id="${CSS.escape(anchor.messageId)}"]`, + ); + if (!currentRow) return; const currentTopOffset = - row.getBoundingClientRect().top - + currentRow.getBoundingClientRect().top - container.getBoundingClientRect().top; const drift = currentTopOffset - anchor.topOffset; if (Math.abs(drift) > 0.5) { container.scrollBy(0, drift); } + }; + + const gate = settleGateRef.current; + if (gate?.enabled) { + // Settle gate on: the prepend re-anchor is the one mid-fling scroll + // writer, and firing it against live user momentum is what produces the + // walk-blind jump. Defer it behind the polled quiet-window — apply the + // correction only once the scroll has been still for k consecutive + // frames (k clears WebKit's coalesced-freeze window; see + // `observeScrollSettle`). Supersede any correction still pending from a + // prior prepend so only the newest anchor is ever re-pinned. + // + // A3 trade (stated in the PR body): this removes the mid-fling jerk by + // *deferring* the re-anchor to settle, which means the reading row can + // drift during the fling and snap back once at rest. The artifact the + // user feels moves from "jump mid-scroll" to "small settle-time + // adjustment"; it is a trade, not a free correction. + if (settleObserverCancelRef.current !== null) { + settleObserverCancelRef.current(); + } + settleObserverCancelRef.current = observeScrollSettle( + container, + gate.quietFrames ?? DEFAULT_SETTLE_QUIET_FRAMES, + () => { + settleObserverCancelRef.current = null; + applyReanchor(); + }, + ); + } else { + applyReanchor(); } if (!isPrepend) { setNewMessageCount((current) => current + messagesArrived); @@ -523,6 +678,10 @@ export function useAnchoredScroll({ if (highlightTimeoutRef.current !== null) { window.clearTimeout(highlightTimeoutRef.current); } + if (settleObserverCancelRef.current !== null) { + settleObserverCancelRef.current(); + settleObserverCancelRef.current = null; + } }; }, []);