fix(desktop): eliminate upscroll realization jitter (compensation writer + estimator + certified perf gate)#1650
fix(desktop): eliminate upscroll realization jitter (compensation writer + estimator + certified perf gate)#1650tlongwell-block wants to merge 7 commits into
Conversation
Tyler's report: scrolling UP a fully-loaded channel is subtly jumpy; DOWN is smooth. Root cause (Eva H1 + L-C): rows above the opening viewport have never painted, so under content-visibility:auto they sit at estimateRowHeight()'s contain-intrinsic-size reserve (60px, utilities.css:17). Scrolling up, each realizes at its true height; the reserve-vs-true delta shifts the reading position. Shipped WKWebView has no overflow-anchor, so it corrects nothing — every delta lands raw. This adds a deterministic gate that is RED at tip 77bd0e7 and that the converged owned-compensation fix (overflow-anchor:none + same-frame scrollBy(realized-reserved)) turns green: - e2eBridge: a dedicated `jitter-corpus` mock channel (400 structurally heterogeneous rows — headings/lists/blockquotes/code fences/long prose whose true height estimateRowHeight is known to miss), its own channel so the uniform deep-history seed the load-older specs use is untouched. - upscroll-jitter.perf.ts: forces overflow-anchor:none on the scroller (mirrors shipped WKWebView; logs CSS.supports per engine), then wheels up in notches tracking a row held INSIDE the viewport (SAFE_MARGIN band from both edges, re-picked each step, scored only if the same id stayed in-band before+after). Residual = row's visual delta-top minus the scroll delta: 0 == smooth. Gate is peak <=2px, rms <=0.6px. Validity: within a single cold run the already-realized rows read residual 0.0 exactly while realizing rows drift -27..-75px — an artifact would drift on both. Negative sign shows the estimator over-reserves. At tip: peak 75px, rms 40px over 23 steps → RED. A correct fill of scrollTop absorbs the drift into the scroll delta, driving residual->0. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
…ive) Dawn found the RED gate from 33e7351 was mathematically invariant to the fix: its residual subtracted the REALIZED scrollTop delta, which includes the fix-writer's compensating scrollBy, so the writer cancelled out and the gate scored raw estimate error R (only T3 can move it), not motion smoothness. Quinn and Eva re-derived and confirmed. Co-visible and any row-vs-row differential share the disease: compensation is a uniform global offset, so it cancels out of any quantity not referenced to the input. Reframe (converged with Dawn/Quinn/Eva; ratified event 63f5e6e1): - Metric is now per-notch MOTION CONSISTENCY: the reading row's viewport motion (rowMove = after.top - before.top; NO scrollTop reference) scored as deviation from the run median. This references the row's own motion ACROSS notches (temporal self-reference), never a neighbour (spatial) or scrollTop, so the writer's scrollBy survives into the metric and is SEEN. - Actuation is a fixed synchronous step (scrollTop -= STEP) so the input delta is constant by construction, bypassing Blink's wheel-scaling (a wheel notch applies 218/220/222...; median-of-run would misread that per-notch scaling as jitter). Dawn independently confirmed even the actuated scrollTop delta is post-writer contaminated (48-75px spread under sync) — rowMove is the only clean measurable. - Anti-cheat floor: mean rowMove must be > STEP*0.75 so a frozen or half-applying scroller (near-zero variance, would false-green) is caught. - Old scrollTop-referenced residual kept as a printed NON-GATING diagnostic (= estimate error R = T3's estimator acceptance number). - Second non-gating diagnostic: one wheel-actuated pass of the same metric (Tyler's real input is a wheel), on the record every run. - RED-at-tip is a HARD gate with a VOID comment: median-of-run is only valid while the corpus produces varying realization, and the tip run being RED IS the proof of that dispersion. RED at tip: peak-dev 41px / rms 24px (gate <=2 / <=0.6). GREEN target is Dawn's real T2 writer: peak-dev 0.00 / rms 0.00, reproducible over 3 sync runs. A synthetic per-notch-varying oracle drives 74/77 notches to exactly STEP; the real writer re-pins in the same ResizeObserver cycle and closes the synthetic-only outliers. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Scrolling UP a fully-loaded channel was subtly jumpy while scrolling down was smooth. Root cause (Eva's H1): rows above the opening viewport have never painted, so they sit at their `contain-intrinsic-size` reserve under `content-visibility: auto`. Scrolling up, each row realizes at its true height the instant it enters the realization band; the delta (true - reserve) shifts content above the reading row. On shipped WKWebView there is no `overflow-anchor` to correct it, so every realization lands raw on the reading position — one lurch per row. The fix makes the conversation scroller own its reading position through a single same-frame compensation writer: - Snapshot the reading anchor (first fully-visible row + its top offset) on every scroll event, before the ResizeObserver fires. The RO runs after layout and before paint, so the "before" position can only come from the last scroll event — never from inside the RO callback. - Observe each `.timeline-row-cv` row (CV realization is a resize of the row's own box and does not surface as an ancestor resize in Blink), seeded at observe time with the browser's current layout height read from `contain-intrinsic-block-size` — the reserve for an unpainted row. Seeding from the reserve is what turns the first realization into a measurable `realized - reserve` delta instead of an unmeasurable first sighting. - On a real height change, re-pin the reading anchor to its saved offset with a same-frame `scrollBy(drift)`. A single measured correction: the layout engine already summed the above-anchor deltas into the row's top, and rows resizing below the anchor don't move it, so they're excluded for free. Re-baseline after the correction. - Force `overflow-anchor: none` on the scroller so the writer is the single scroll authority on every engine — Chromium's native anchoring can't double-correct behind the writer's back, and WKWebView has none anyway. This also aligns CI (Chromium) with macOS (WKWebView), which ran different scroll contracts — how the jitter shipped unseen. Verified against the committed T1.1 gate (upscroll-jitter.perf.ts) on jitter-corpus cold-load, sync fixed-step actuation, overflow-anchor forced off: RED without this writer (peak-dev 46 / rms 23), GREEN with it (peak-dev 0.00 / rms 0.00, reproducible), and the non-gating wheel felt-mode pass also 0.00/0.00. The non-gating estimate-error R diagnostic stays at 75 (T3's lane; this writer compensates motion, it does not shrink the reserve error). Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Co-authored-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta <d8473ee32b973aa31a21a65adddcc4b69cc2a8a4dee8121ecd51926e0cddbc02@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta <d8473ee32b973aa31a21a65adddcc4b69cc2a8a4dee8121ecd51926e0cddbc02@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta <d8473ee32b973aa31a21a65adddcc4b69cc2a8a4dee8121ecd51926e0cddbc02@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta <d8473ee32b973aa31a21a65adddcc4b69cc2a8a4dee8121ecd51926e0cddbc02@sprout-oss.stage.blox.sqprod.co>
…-contract guard Two of the deferred scroll-jitter seams, on top of the integrated fix train (T1.1 gate -> T2 writer -> T3 estimator @ 1fa4551): - columnWidthPx: measure the timeline row column once (useElementWidth) and thread it into timelineRowReserveStyle so row-height reserves use the real wrap width instead of the 64-char fallback. Pre-measure zero passes undefined to preserve the estimator's own fallback rather than tripping its min-chars floor. On jitter-corpus this shrinks estimate-error R from 75 -> 55 peak (better reserves = smaller realized-vs-reserved delta) while the motion gate stays GREEN 0.00/0.00 -- R moves only in the estimator lane, the gate only in the writer lane, cross-lane invariant intact. - Wren #4 anchor-contract guard: a companion perf test that reads the PRODUCTION computed overflow-anchor on the real conversation scroller (no test override) and asserts it resolves to none. The jitter gate forces the property for its measurement; this proves the shipped stylesheet actually ships it, so a regression that hands correction back to Chromium's native anchoring (masking a WKWebView-only on-device break) fails loudly. The onRealizedHeight seam is intentionally NOT wired: T4 ground truth (RESEARCH/GUI_SCROLL_RECLAIM_T4.md) shows the upscroll path is append-only with no unmount, so it needs no JS height cache; a per-event-id cache is only the deferred, narrowly-scoped T4b for the head-refetch case. Adding an unused callback now would be dead abstraction. Validation: - pnpm --dir desktop typecheck: clean - biome check on both changed files: clean - rowHeightEstimate.test.mjs 16/16, useAnchoredScroll.test.mjs 2/2 - perf: upscroll-jitter GREEN 0.00/0.00 (R 55), anchor-contract PASS Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
The committed gate certified only the CV-realization half of H1: it terminated at the ~600px fetchOlder sentinel band, so the pagination half — cold rows paged in ABOVE the viewport by scrollback — was proven only by Quinn's throwaway probe, not by the gate as it runs (the coverage gap Quinn [9] and Dawn [10] flagged, land-now/fast-follow ratified by Eva [11]). Root cause: when a prepend lands, scrollTop jumps UP (~100 older rows of height added above), so `appliedTop = before - after` goes negative and the loop's top-of-history break fired at the sentinel — before scoring any paged-in row. On the RED baseline this break was deterministic; on GREEN it was flaky (the prepend was scored only when the re-anchor happened to leave the tracked row in the safe band). Fix: distinguish the two causes of a non-decreasing scrollTop by mounted-count. A prepend grows it; the true top does not. On a prepend re-anchor, skip scoring that step (its motion is the multi-thousand-px jump, not a fixed STEP notch — scoring it would poison the metric) and CONTINUE; the next iteration re-baselines a fresh safe-band row in the paged-in window and scoring resumes across the prepend. Assert `prependObserved` so the gate exercises BOTH jitter sources every run. Verified on the T3 integration tip 1fa4551 (GREEN) and the T1.1 base 32e41e7 (RED), both rebuilt: - GREEN: prepend scored (50->200, walks all crossings), gate 0.00/0.00 PASS, 77-78 scored steps, deterministic across 4 runs. - RED: prepend scored (50->150), coverage assert PASSES, gate still FAILS on jitter (peak 41px > 2.0) — reds for the right reason. The dual-sided RED/GREEN contract and the anti-cheat floors are preserved; the change is scoped to the gate file only. Note: gate-run R is window-dependent (Eva/Max [8]); scoring the deeper paged-in population raises the non-gating R diagnostic from 75 to ~85. This is NOT a cross-lane leak — the T2 writer's reserve is untouched; R moved only because the sample window grew, exactly as the window-sensitivity caveat predicts. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Request changes — reviewed on behalf of Wes, verified locally at head 104f3cd.
The H1 diagnosis is right and the core mechanism works: I ran the perf gate locally at this head and got 0.00px peak / 0.00px rms motion deviation over 80 notches (wheel mode too) — the upscroll jitter is dead on the main channel timeline. The single-writer + overflow-anchor: none + per-row ResizeObserver architecture is the correct call.
But as it stands the PR trades that jitter for two regressions, and CI's red shards agree:
-
Thread panel regression —
MessageThreadPanelsharesuseAnchoredScrolland thedata-buzz-conversation-scrollselector, so it now getsoverflow-anchor: none, but it renders zero.timeline-row-cvrows, so the new per-row observer watches nothing there. The old code observed the content wrapper. Net effect: the thread panel lost native anchoring AND gets no JS compensation.timeline-no-shift.spec.ts › thread panel late row reflow keeps the reading reply stablefails at this head (CI shard 4, 3 retries; reproduced locally), passes on main. -
Main-timeline prepend + late-reflow regression —
timeline-no-shift.spec.ts › timeline prepend plus late row reflow keeps the reading row stablealso fails at this head (CI + local), passes on main. The writer doesn't yet cover the prepend-correction + late-reflow interleaving that native anchoring used to absorb in Chromium. -
The perf gate gates nothing in CI, and its T1.2 assertion doesn't reproduce —
upscroll-jitter.perf.tsis only reachable viaplaywright.perf.config.ts, which no CI workflow invokes. And running it at this head,prependObservedcame backfalseon my machine (80 steps consumed without crossing a fetchOlder prepend — viewport-height dependent), so the gate fails its own coverage assert.
Requested changes:
- (a) Give the thread panel observed rows (or a wrapper-observation fallback), or scope
overflow-anchor: none+ the row observer to the main timeline only. - (b) Fix the prepend+late-reflow case — the existing
timeline-no-shiftspec is the repro. - (c) Wire the perf gate into CI or stop calling it a gate, and make the prepend assertion deterministic (drive the walk by paged-in row count, not a fixed step budget).
The estimator lane (rowHeightEstimate) looks solid and low-risk on its own — 16/16 unit tests pass. Great bones; needs one more stop before it ships.
(Also: smoke shard 1 has channel-sort red on this run — looks like an unrelated flake, but flagging it.)
| contracts, which is how the jitter shipped unseen. Forcing it off makes the | ||
| writer the single scroll authority on every engine. See L-A/L-C findings. */ | ||
| [data-buzz-conversation-scroll] { | ||
| overflow-anchor: none; |
There was a problem hiding this comment.
This selector also matches the thread panel's scroller (MessageThreadPanel sets data-buzz-conversation-scroll), which renders no .timeline-row-cv rows — so it loses Chromium's native anchoring here while the new per-row observer in useAnchoredScroll has nothing to observe there. That's the direct cause of the timeline-no-shift.spec.ts › thread panel late row reflow failure at this head. Either scope this rule to the main timeline scroller or give the thread panel a compensation path.
| // The RO callback runs after layout, before paint, so the compensating | ||
| // `scrollBy` is same-frame invisible. | ||
| for (const row of content.querySelectorAll<HTMLElement>( | ||
| ".timeline-row-cv", |
There was a problem hiding this comment.
Observing only .timeline-row-cv rows drops the previous wrapper observation for consumers that render no CV rows — the thread panel being the live case. Its at-bottom reflow re-pin and mid-history stability both regressed (spec red at head, green on main). Consider falling back to observing content itself when no .timeline-row-cv rows exist, which restores the old behavior for non-CV consumers.
| // pass or fail. This is corpus-structural (400-row seed > 300 limit), so it | ||
| // holds on the RED baseline and the GREEN fix alike; a run that stops before | ||
| // scoring a prepend is a coverage regression, not a jitter verdict. | ||
| expect(result.prependObserved).toBe(true); |
There was a problem hiding this comment.
This assertion is not deterministic: on my machine the walk consumed all 80 steps without crossing a fetchOlder prepend (prependObserved: false), so the gate fails its own coverage assert at the PR head. The notches needed to reach the prepend depend on viewport height. Consider driving the walk until the prepend is observed (with a hard safety cap) rather than a fixed step budget — and note this file is only reachable via playwright.perf.config.ts, which no CI workflow runs, so as merged this gate certifies nothing.
What
Fixes Tyler's reported upscroll-only jitter: "scrolling up in a channel, but not down, is a little jumpy… even in fully loaded channels."
Root cause (H1, trace-proven): under
content-visibility: auto, never-painted rows enter the viewport at theircontain-intrinsic-sizeestimate and realize at their true height on entry. On WKWebView there is nooverflow-anchorsupport, so every realization delta lurches the reading row. Two sources: CV realization inside the loaded window, and coldfetchOlderpagination prepends mid-upscroll. Downscroll never hits this because rows below are already realized.The fix train (7 commits, 3 lanes, each cross-reviewed)
upscroll-jitter.perf.ts) — measures per-notch scroll motion of the reading row on a 400-row worst-case corpus. RED on the pre-fix code: peak deviation 41–48px, rms 23–24px per 220px notch. Anti-cheat floors (row count, sample count, mean-motion) prevent a trivially-green gate.useAnchoredScroll.ts+overflow-anchor: nonein production CSS) — snapshots an anchor row on scroll, seeds a ResizeObserver from the reserve, applies same-framescrollBy(drift)compensation. Gate goes GREEN: 0.00px peak / 0.00px rms — the reading row tracks the input exactly on every notch, including across a livefetchOlderprepend.rowHeightEstimate.ts) — supported-preview-only reserve (−56px false reserve on unsupported bare URLs), measured column width wired viauseElementWidth(estimate-error peak 75→55px on the test corpus). Smaller estimates = smaller realization deltas = less work for the writer, and less CV layout-shift generally.upscroll-anchor-contract.perf.ts) — asserts production computed style actually shipsoverflow-anchor: none(the gate forces it to mirror WKWebView; this guard catches stylesheet drift that would silently break on-device while staying green in CI).fetchOlderprepend inside the scored window and asserts it (prependObserved), so both jitter sources are certified per run, with a skip-and-re-baseline mechanism that cannot be gamed into skipping genuine jitter notches (skip branch is structurally unreachable when scrollTop actually moved).Numbers at this head (fresh build, same-shell rev-parse)
rowHeightEstimateunit tests 16/16 · tsc clean · biome clean · pre-push hooks green.Review trail
Every lane cross-reviewed by a non-author before integration (Quinn, Perci, Max, Dawn, Sami rotating); full trail in #buzz-gui-performance. Cross-lane invariant held throughout: gate greens only via the writer, estimate-error moves only via the estimator.
Follow-ups (deliberate, not gaps)
onRealizedHeightseam is deferred until T4b defines a real consumer).overflow-anchorforced off to mirror WKWebView; a manual smoke on macOS Tauri is the final confirmation.