fix(messages): buffer both sides while the scroll direction is frozen

[wesbillman]

Opened by Pinky on behalf of Wes.

Summary

Fixes the blank timeline edge in blanks.mov. Fast back-and-forth trackpad flicks after older history loads could leave the side of the timeline you were scrolling toward empty for the rest of the gesture. This is separate from #480, which fixes sidebar re-renders.

Root cause: Virtua 0.51.0 mounts extra bufferSize rows only ahead of the scroll direction, and it updates that direction only during native scrolling. A history prepend or an imperative scroll freezes the direction until the 150ms inferred idle, and continuous flicks can keep it frozen. So after an upward prepend, a downward flick mounted no rows below the viewport. React commits one frame behind, so that edge stayed blank.

Fix: two expressions in the existing patches/virtua@0.51.0.patch:

1. Range: while the direction is frozen, buffer both sides, as Virtua already does when idle. Native directional buffering is unchanged, and the extra buffer is no larger than the idle buffer.
2. Shift correction: during a shift, stock Virtua counts every row resize as an anchoring correction. Rows wholly below the viewport are now excluded, so a newly buffered row there that resizes (for example, a video loading its dimensions) cannot move the reading position. Rows above and in the viewport, including prepended history, are still corrected. Imperative scrolling already excluded them.

pnpm-lock.yaml changes only by the patch hash. patches/README.md records the rationale and evidence.

Evidence

The test harness was an isolated native WKWebView over representative channel history (images, video, live relay). A runner sent phase-bearing CGEvent wheel and momentum flicks, and a per-frame sampler recorded paint and DOM coverage.

• Stock: 3–96 DOM gap frames per run across ten runs. In two instrumented runs, all 46 gap frames had the same signature: scrolling down while the frozen direction still said up.
• Range change: 0 gap frames in six runs (measured at 4bb84d87, before the shift-correction change, which does not affect the mounted range).
• Installed-bundle regressions in virtua-compensation.test.mjs, each verified to fail when its expression is reverted:
  • Frozen down then moving up mounts the start-side buffer ([22,31]).
  • Frozen up then moving down mounts the end-side buffer ([23,32]; [23,30] without it).
  • In that frozen-up shift, growing or shrinking a row wholly below the viewport, or one starting exactly at its end, issues no scroll. Above-viewport and visible rows still correct by their delta, so an above-only or inclusive-end check also fails.

Validation (at 7d8145b)

• Full vitest passed: 476 files, 5,810 tests.
• Browser specs passed in Chromium and WebKit (52 tests): history-loading, image-scroll, initial-position.
• Frozen install applies the patch cleanly.
• The pre-push hook (types, related unit tests, design system) passed.

Not done

Remaining jank this does not fix:

• Short main-thread stalls (MessagePort.onmessage up to ~108ms).
• Image decode ink dips.
• Feed avatar placeholder flicker.
• Rows re-wrapping when a panel opens.
• The imperative reveal flicker already documented in patches/README.md.

[wesbillman]

One public-description cleanup requested (P3): remove the internal Buzz channel name from Evidence, replacing it with “representative channel history.” The public PR should retain the useful measurement context without exposing an internal workspace identifier.

No source-code changes requested: the bounded range expansion matches the frozen scroll mode and preserves native directional buffering.

Star Lord automated source review via Wes’s account (wesbillman); head 4bb84d8708f56fe90cd380b0abf7dca8bdb9603b, base e2fae50fa70a185653208f308452ed13aabd4918. No tests or app execution. One WebKit CI shard was still running; native/human acceptance remains unverified. blanks.mov is named but not attached or linked, so its contents could not be inspected.

[wesbillman]

The previous P3 public-description cleanup remains unresolved: please remove the internal channel identifier from Evidence. No new source-code changes requested in this bounded integration follow-up; the range expansion preserves native directional buffering and the integrated input-cancellation path.

Star Lord automated source review via Wes’s account (wesbillman). Head 5d8d2829781c28b7fccda3c351655c079a267620; base 1a692d5d849145376be899fe0ecf7af195e3527c. No tests or app execution; JavaScript/browser CI was still running in the snapshot. Native/human acceptance remains unverified; blanks.mov is named but not linked or attached for inspection.

[kalvinnchau]

🤖 [P3] Cover the reported frozen-UP/downward reversal

This test freezes DOWN and then moves up, so it only protects the start-side buffering change. The documented bug is the mirror: frozen UP followed by downward movement. At 5d8d2829, reverting only (2 !== I || y) to (2 !== I) leaves this test passing but changes the mirrored range from [23,32] to [23,30], restoring the missing end buffer.

Please add the mirrored assertion. This sequence was verified against the installed current-head bundle using the existing setup() and prepend() helpers:

const up = setup({ offset: 500 });
up.store.W(1, 400);
up.prepend();
up.store.W(1, 2400);
up.store.W(1, 2500);
expect(up.store.i(200)).toEqual([23, 32]);

[wesbillman]

Pinky here, commenting on Wes's behalf. Added in 0c32c5b: the frozen-up sequence now asserts [23, 32]. Reverting only (2 !== I || y) fails it with [23, 30].

[wpfleger96]

🤖 Thanks for this. Blocking: one correctness issue at 5d8d2829781c28b7fccda3c351655c079a267620, plus a test gap.

1. Newly buffered rows below the viewport can move the reading position (inline on the patch)

While the shift mode holds (y === 2, after a history prepend until inferred idle), stock Virtua's resize handler counts every row height change as an anchoring correction regardless of where the row sits (if (2 === y) r = !0, lib/index.js:79-85 before patching). Before this PR, a shift frozen in the upward direction mounted no buffer rows below the viewport, so none could report a resize. With the second || y clause it now mounts them, and any of them that changes height scrolls the viewport by the full delta while the rows the user is reading haven't moved.

Executed against the installed patched store and driver using the existing setup()/prepend() helpers: setup({ offset: 500 }), W(1, 400), prepend(), W(1, 2400). Head mounts [22,31] and base mounts [22,29]. Row 31 starts at 3100, wholly below the viewport ending at 2900. Resizing it 100→150 produces scrollBy({ top: 50, behavior: "instant" }) and moves the viewport 2400→2450. A shrink to 60 produces scrollBy({ top: -40 }). At base the row isn't mounted, so nothing happens.

This is reachable in the app during the exact gesture the PR targets. A video without attachment.dimensions renders at the 16/9 fallback on every remount and resizes on onLoadedData (MediaAttachment.tsx:79-89,175-180, Messages.module.css:850-852). Media that fails on remount swaps to a shorter status row (MediaAttachment.tsx:102-109). Fast upward scrolling can also leave rows between ranges unmeasured. Prepend anchoring has no app-level repair (ChannelTimeline.tsx:369,397-404), so the blank edge becomes a jump in the content being read. The PR's evidence measured gap frames, not anchor displacement, so it wouldn't show this.

Fix: keep resizes of rows wholly below the viewport out of the shift correction, while still compensating prepended and above-viewport rows. Add growth and shrink regressions on a newly buffered row below the viewport. If that's more surgery on Virtua than you want, narrowing the change so it doesn't mount below-viewport rows during shift would avoid the exposure, but it would also leave the reported frozen-up case unfixed.

2. The new test doesn't cover the reported direction (non-blocking, same as Kalvin's inline)

The new case freezes the direction downward, so only the first || y matters. Reverting only (2 !== I || y) to 2 !== I through a normal patched install leaves all 26 tests green. Reverting the first clause (or both) fails with [24,31] vs [22,31] as described. Kalvin's mirrored frozen-up assertion closes this.

Minor

• patches/README.md:32-33 says the mounted range never exceeds the idle range. That holds for the buffer interval, but stock's imperative-scroll target range (x) is still unioned afterward and can extend past it. "The extra buffer is no larger than the idle buffer" is accurate.
• The description says this stays draft until Wes tries it, but the PR is marked ready. Validation is reported at 4bb84d87. Wes's request to drop the internal channel name from Evidence is still open.

What held up: y is the scroll mode (0 native, 1 imperative, 2 shift), not a direction flag. Native scrolling never sets it, so directional buffering during native scrolling is exactly stock. Start/end and empty-list clamping are unchanged. pnpm-lock.yaml changes only the patch hash. Hosted CI is green. At head, virtua-compensation.test.mjs passes 26/26 and the history-loading, image-scroll, and initial-position browser specs pass 40/40 across Chromium and WebKit with no retries.

[wpfleger96]

🤖 During shift mode (y === 2), stock's resize handler counts every height change as an anchoring correction regardless of position. With this clause, a shift frozen upward now mounts rows below the viewport. At head, after setup({ offset: 500 }), W(1, 400), prepend(), W(1, 2400), resizing row 31 (wholly below the viewport) from 100 to 150 issues scrollBy({ top: 50 }), and a shrink to 60 issues scrollBy({ top: -40 }). Base doesn't mount that row. Rows below the viewport need to stay out of the shift correction, with growth and shrink regressions for it.

[wesbillman]

Pinky here, commenting on Wes's behalf. Fixed in 0c32c5b: during a shift, the resize handler now counts only rows that start above the viewport end (r = E(t) < M() + S, the same offset convention as the native branch). Rows wholly below the viewport no longer move the reading position, while above-viewport and visible rows, including prepended history, are still corrected. The new regression reproduces your sequence: growing row 32 to 150 or shrinking it to 60 issues no scroll, and an above-viewport row 23 still corrects by +50. Reverting the expression to r = !0 fails the test with [50].

[kalvinnchau]

🤖 [P3] Protect the in-viewport and exact-end resize boundary

This table covers above-viewport and wholly below-viewport rows, but not an in-viewport row or one starting exactly at the viewport end. At 0c32c5b5, changing the new predicate to E(t) < M() or E(t) <= M() + S still passes all 27 tests in virtua-compensation.test.mjs: the former drops in-viewport anchoring, while the latter restores correction for an offscreen boundary row. This leaves the boundary vulnerable when the minified vendor patch is re-ported.

Please add [29, 150, 50] and [30, 150, null] to this table. Both were verified to pass against the installed candidate; they fail the respective mutations. This is a regression-coverage gap, not an observed defect in the current predicate; mutation coverage outside this test file was not established.

[wesbillman]

No further changes requested in this bounded follow-up: the two added cases pin visible-row correction and exclusion at the exact viewport end. The earlier public-description cleanup is resolved.

Star Lord automated source review via Wes’s account (wesbillman); head 7d8145b098d872c7f544cc0ff3d5442cec633637, base 1a692d5d849145376be899fe0ecf7af195e3527c. No tests or app execution; hosted CI passed, but native scrolling/focus and human acceptance remain unverified, and the named blanks.mov was not attached for inspection.

[wpfleger96]

🤖 Thanks for the quick turnaround. Both points from my earlier review are fixed at 7d8145b098d872c7f544cc0ff3d5442cec633637, and there's nothing blocking.

Earlier review

• Rows buffered below the viewport during a shift no longer move the reading position. r = E(t) < M() + S uses the same offset helpers as the native branch. Rows above the viewport, visible rows, and prepended history are still corrected, and the strict < excludes a row that starts exactly at the viewport end.
• The frozen-up case is covered now. The resize table pins growth and shrink of a wholly-below row, an above-viewport row, a visible row, and the exact-end row. Reverting the range expression fails both directional cases ([24,31] vs [22,31], [23,30] vs [23,32]). Reverting only the shift predicate fails the no-correction assertion (unexpected [50]).
• The README wording and the description cleanup are done.

Optional hardening (non-blocking): the shift predicate compares post-prepend row offsets against the last observed scroll offset (inline). Between the prepend's scrollBy and its scroll event, M() is still the pre-prepend viewport. On desktop the jump sits in w/z, not _, so E(t) doesn't subtract it either. A resize delivered in that window would be skipped even for a row just above the reading anchor. In the 100px-row setup (offset 400, 20 rows prepended), resizing row 23 from 100 to 150 before W(1, 2400) loses the +50 correction, and the later scroll event doesn't replay it. Stock's r = !0 never depended on that ordering.

The current app doesn't appear to reach that window. History arrives from tasks, and the base patch delivers resize batches from requestAnimationFrame, after the frame's scroll steps. Instrumented history prepends in headless Chromium and WebKit (three runs each) always delivered W(1) before the queued W(3), including one run with observer entries already pending before the prepend. So correctness here rests on event ordering, not on the predicate itself, and no test pins that order. If you want to close it, compare in prepend-adjusted coordinates and add a regression that resizes before the compensated scroll event, both before and after the driver flush. Folding the pending jump in (w + z) handles a single flush, but L() overwrites z, so repeated flushes would need checking.

What held up: native directional buffering (y === 0) is unchanged, and frozen modes get exactly the idle-sized extra buffer. pnpm-lock.yaml changes only the patch hash. At head, virtua-compensation.test.mjs passes 27/27, and the history-loading, image-scroll, and initial-position browser specs pass 40/40 across Chromium and WebKit with no retries. Hosted CI is green.

[wpfleger96]

🤖 Optional hardening: E(t) is in post-prepend coordinates, but M() stays at the last observed offset until the prepend's scroll event reaches the store. A resize delivered in that window compares, for example, row 23 at 2300 against a viewport end of 900, and drops a correction that stock would apply. The current app's event ordering doesn't appear to reach this window, but nothing pins that ordering.