fix(desktop): stop competing scroll writers collapsing the load-older anchor

Loading older messages under virtualization let three writers fight over
scrollTop on overlapping frames, so the anchored row jittered or collapsed
to the top (~33% of prepends) and the library's reconcile spun the full 5s
MAX_RECONCILE_MS valve. Establish a single owner of scroll position across
the whole fetch+restore window:

- useLoadOlderOnScroll restores by scrollTop ONLY (drop scrollToIndex), via
  one getOffsetForIndex(anchorIndex + prepended, "start")[0] + intra-row gap
  write. getOffsetForIndex is a pure measurement-cache read, so no library
  scrollState is set and the reconcile loop has nothing to fight.
- The viewport ResizeObserver in useTimelineScrollManager no longer runs a
  competing restore during a fetch: it skips while isFetchingOlder is true
  (the spinner's clientHeight 720->590 mount-shift fires before the lock is
  set) and otherwise defers to lockedScrollTopRef when the load-older restore
  holds it. MessageTimeline threads isFetchingOlder into the manager.

The defect was invisible to unit tests (jsdom getBoundingClientRect -> 0) and
to static traces; the new load-older E2E drives a real prepend on six fresh
page loads and asserts the anchor holds every run, the scroller genuinely
grew, and the reconcile terminates. emitMockHistory now honors the relay
filter's until/limit so the mock relay paginates like a real one, which the
E2E needs to exercise a genuine older page.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

desktop/src/features/messages/ui/MessageTimeline.tsx

@@ -245,6 +245,7 @@ const MessageTimelineBase = React.forwardRef<
     syncScrollState,
   } = useTimelineScrollManager({
     channelId,
+    isFetchingOlder,
     isLoading: showTimelineSkeleton,
     messages: deferredMessages,
     onTargetReached,

desktop/src/features/messages/ui/useLoadOlderOnScroll.ts

@@ -98,12 +98,17 @@ export function useLoadOlderOnScroll({
                     (virtualizerRef.current?.itemCount ?? previousCount) -
                     previousCount;
                   if (after && anchorIndex !== null && prepended > 0) {
-                    after.scrollToIndex(anchorIndex + prepended, {
-                      align: "start",
-                    });
-                    // scrollToIndex aligns the row's top to the viewport top;
-                    // re-apply the captured gap so the view doesn't nudge by a
-                    // partial row.
+                    // Restore by scrollTop ONLY — a single writer. Compute the
+                    // anchored row's top via getOffsetForIndex (a pure read of
+                    // the measurement cache, no scrollState) and add back the
+                    // captured intra-row gap. Calling scrollToIndex here too
+                    // would set the library's scrollState aiming at the row TOP
+                    // while this restore aims at row top + gap; the two write
+                    // scrollTop to different values on overlapping rAF frames,
+                    // so the library's reconcile never reaches approxEqual,
+                    // never re-scrolls (its target is unchanged), and spins one
+                    // rAF/frame for the full 5s MAX_RECONCILE_MS valve on every
+                    // prepend. One mechanism, no fight.
                     const target = after.getOffsetForIndex(
                       anchorIndex + prepended,
                       "start",

desktop/src/features/messages/ui/useTimelineScrollManager.ts

@@ -12,6 +12,7 @@ import { useConvergentScrollToMessage } from "./useConvergentScrollToMessage";
 
 type UseTimelineScrollManagerOptions = {
   channelId?: string | null;
+  isFetchingOlder?: boolean;
   isLoading: boolean;
   messages: TimelineMessage[];
   onTargetReached?: (messageId: string) => void;
@@ -37,6 +38,7 @@ type PinToBottomOptions = {
 
 export function useTimelineScrollManager({
   channelId,
+  isFetchingOlder = false,
   isLoading,
   messages,
   onTargetReached,
@@ -63,6 +65,12 @@ export function useTimelineScrollManager({
   // a streaming-in list is what makes the timeline thrash on entry.
   const isLoadingRef = React.useRef(isLoading);
   isLoadingRef.current = isLoading;
+  // Mirror isFetchingOlder so the viewport ResizeObserver (subscribes once) can
+  // see the live value: the load-older path owns scroll position across its
+  // whole fetch+restore window, so the observer must not run a competing
+  // restore while a fetch is in flight (see the resize handler below).
+  const isFetchingOlderRef = React.useRef(isFetchingOlder);
+  isFetchingOlderRef.current = isFetchingOlder;
   const [isAtBottom, setIsAtBottom] = React.useState(true);
   const [highlightedMessageId, setHighlightedMessageId] = React.useState<
     string | null
@@ -307,7 +315,27 @@ export function useTimelineScrollManager({
         return;
       }
 
-      restoreScrollPosition(previousScrollTopRef.current);
+      // The load-older path owns scroll position across its whole window. Two
+      // guards keep this observer from running a competing restore — without
+      // them the spinner's clientHeight 720->590 shift fires here and restores
+      // to previousScrollTopRef.current (0, since the user scrolled to the top
+      // to trigger), collapsing the anchor.
+      //
+      // Guard 1 — fetch in flight, lock not yet set: the spinner mounts BEFORE
+      // the fetch resolves and calls restoreScrollPosition, so lockedScrollTop
+      // is still null on this fire. Skip entirely; the load-older path restores
+      // once the page arrives.
+      if (isFetchingOlderRef.current) {
+        return;
+      }
+
+      // Guard 2 — restore running, lock set: a later shift (e.g. spinner
+      // unmount) can fire while restoreScrollPosition's rAF loop holds its
+      // target in lockedScrollTopRef. Defer to that target so both aim at the
+      // same scrollTop instead of fighting frame-by-frame.
+      restoreScrollPosition(
+        lockedScrollTopRef.current ?? previousScrollTopRef.current,
+      );
     });
 
     observer.observe(timeline);

desktop/src/testing/e2eBridge.ts

@@ -466,6 +466,8 @@ type MockFilter = {
   "#h"?: string[];
   authors?: string[];
   kinds?: number[];
+  limit?: number;
+  until?: number;
 };
 
 type MockSocket = {
@@ -1491,6 +1493,37 @@ const mockChannels: MockChannel[] = [
       createMockMember(MOCK_IDENTITY_PUBKEY, "member", 700),
     ],
   }),
+  // Deep history channel for the load-older-under-virtualization E2E. Seeded
+  // with more messages than CHANNEL_HISTORY_LIMIT (200) so the initial load
+  // windows to the newest page and a `fetchOlder` (until-cursor) prepend has
+  // genuinely older rows to add — exercising the scroll-restore anchor under
+  // virtualization. Its own channel so existing channels' row-index and unread
+  // assertions stay undisturbed.
+  createMockChannel({
+    id: "feedf00d-0000-4000-8000-000000000007",
+    name: "deep-history",
+    channel_type: "stream",
+    visibility: "open",
+    description: "Channel with paginated history for load-older tests",
+    topic: null,
+    purpose: null,
+    last_message_at: isoMinutesAgo(1),
+    archived_at: null,
+    created_by: ALICE_PUBKEY,
+    topic_set_by: null,
+    topic_set_at: null,
+    purpose_set_by: null,
+    purpose_set_at: null,
+    topic_required: false,
+    max_members: null,
+    nip29_group_id: null,
+    created_minutes_ago: 2000,
+    updated_minutes_ago: 1,
+    members: [
+      createMockMember(ALICE_PUBKEY, "owner", 2000),
+      createMockMember(MOCK_IDENTITY_PUBKEY, "member", 1900),
+    ],
+  }),
 ];
 
 const mockMessages = new Map<string, RelayEvent[]>();
@@ -2257,15 +2290,50 @@ function getMockMessageStore(channelId: string): RelayEvent[] {
                 sig: "mocksig".repeat(20).slice(0, 128),
               },
             ]
-          : [];
+          : channelId === "feedf00d-0000-4000-8000-000000000007"
+            ? // 600 messages > CHANNEL_HISTORY_LIMIT (200): the initial load
+              // windows to the newest 200, leaving 400 older behind the until
+              // cursor — enough for several full fetchOlder pages (batch 100),
+              // so the load-older anchor restore is exercised across REPEATED
+              // prepend cycles, not a single lucky pass. created_at increases
+              // with index (oldest first) so message N+1 is newer than N — the
+              // anchor restores the first-visible row across each prepend.
+              Array.from({ length: 600 }, (_, index) => ({
+                id: `mock-deep-history-${index}`,
+                pubkey: index % 2 === 0 ? ALICE_PUBKEY : MOCK_IDENTITY_PUBKEY,
+                created_at: Math.floor(Date.now() / 1000) - (600 - index) * 60,
+                kind: 9,
+                tags: [["h", channelId]],
+                content: `Deep history message #${index}`,
+                sig: "mocksig".repeat(20).slice(0, 128),
+              }))
+            : [];
 
   mockMessages.set(channelId, seeded);
   return seeded;
 }
 
-function emitMockHistory(socket: MockSocket, subId: string, channelId: string) {
-  const events = getMockMessageStore(channelId);
-  for (const event of events) {
+function emitMockHistory(
+  socket: MockSocket,
+  subId: string,
+  channelId: string,
+  filter?: MockFilter,
+) {
+  // Honor the relay window so load-older paginates instead of replaying the
+  // whole store. A real relay returns the newest `limit` events at or before
+  // `until` (inclusive — the client dedupes the boundary message by id). Cap at
+  // `limit` after the `until` filter so the page is genuinely older content.
+  const events = getMockMessageStore(channelId).filter(
+    (event) => filter?.until === undefined || event.created_at <= filter.until,
+  );
+  const limit = filter?.limit ?? events.length;
+  const windowed =
+    events.length > limit
+      ? [...events]
+          .sort((left, right) => right.created_at - left.created_at)
+          .slice(0, limit)
+      : events;
+  for (const event of windowed) {
     sendWsText(socket.handler, ["EVENT", subId, event]);
   }
   sendWsText(socket.handler, ["EOSE", subId]);
@@ -5847,7 +5915,7 @@ function sendToMockSocket(args: {
       return;
     }
 
-    emitMockHistory(socket, subId, channelId);
+    emitMockHistory(socket, subId, channelId, filter);
     return;
   }
 

desktop/tests/e2e/virtualization-screenshots.spec.ts

@@ -196,4 +196,136 @@ test.describe("list virtualization screenshots", () => {
 
     await page.screenshot({ path: `${SHOTS}/06b-sections-after-reorder.png` });
   });
+
+  test("07 — load-older prepend holds the anchored row without jitter or reconcile spin", async ({
+    page,
+  }) => {
+    // Install once: addInitScript re-runs on every navigation in this page, so
+    // each page.goto in the loop below re-applies the mock bridge.
+    await installMockBridge(page);
+
+    // The deep-history channel seeds 600 messages; the initial load windows to
+    // the newest 200, leaving 400 older behind the until cursor — enough that
+    // every run lands a genuine prepend. Reads the first row at/below the
+    // viewport top and returns scrollTop, scrollHeight, and that row's on-screen
+    // VIEWPORT position in ONE settled snapshot — the position the single-writer
+    // restore must hold steady across the prepend.
+    //
+    // Waits inside the browser for a measurement-settled frame before reading.
+    // The virtualizer re-windows after a scroll: for a few rAFs the mounted rows
+    // can all sit above the viewport top (their absolute offsets lag the new
+    // scrollTop) until the library mounts rows at the current position. That is
+    // a measurement transient, NOT the scrollTop race — scrollTop is already
+    // correct on those frames. Reading on such a frame would throw "no row";
+    // polling for a settled frame removes the flake without touching any
+    // race-detection threshold below (scrollTop value + viewportPos stability),
+    // and snapshots all three fields together so they can't skew across reads.
+    const sampleAnchor = (timeline: Locator) =>
+      timeline.evaluate(async (scroller) => {
+        const s = scroller as HTMLElement;
+        for (let frame = 0; frame < 60; frame += 1) {
+          const scrollerTop = s.getBoundingClientRect().top;
+          const row = Array.from(
+            s.querySelectorAll<HTMLElement>("[data-message-id]"),
+          ).find((r) => r.getBoundingClientRect().top - scrollerTop >= 0);
+          if (row) {
+            return {
+              viewportPos: row.getBoundingClientRect().top - scrollerTop,
+              scrollTop: s.scrollTop,
+              scrollHeight: s.scrollHeight,
+            };
+          }
+          await new Promise((resolve) => requestAnimationFrame(resolve));
+        }
+        throw new Error("no anchor row mounted after 60 frames");
+      });
+
+    // Determinism is the bar, not pass-once. The original defect was a RACE: a
+    // second restore loop (the resize-observer restoring to the pre-fetch
+    // scrollTop of 0, fired by the load-older spinner's clientHeight shift)
+    // fought the anchor restore frame-by-frame; last writer won, so the anchor
+    // held only ~2 of 3 runs and on its losing runs scrollTop collapsed to ~0
+    // (view stuck at the top, anchor lost). A single prepend can go green on a
+    // lucky scheduling order, so this drives the prepend on SIX fresh page loads
+    // and asserts the anchor holds on every one — a flaky-pass fails the run.
+    // Fresh navigation each iteration resets the virtualizer's measurement state,
+    // matching the run-to-run conditions under which the race surfaced.
+    for (let run = 0; run < 6; run += 1) {
+      // Force a full document reload each iteration. Navigating straight to the
+      // same hash route is a same-document hash change, not a reload, so the
+      // virtualizer + paginated history would carry over and later runs would
+      // exhaust the older pages — defeating the per-run fresh-prepend premise.
+      await page.goto("about:blank");
+      await page.goto("/#/channels/feedf00d-0000-4000-8000-000000000007");
+      const timeline = page.getByTestId("message-timeline");
+      await expect(timeline).toBeVisible();
+      await expect(
+        page.locator('[data-message-id^="mock-deep-history-"]').first(),
+      ).toBeVisible();
+
+      // Scroll up to mount mid-history rows while staying clear of the load-older
+      // sentinel zone (trips within 200px of the top), then let the windowed rows
+      // measure off their 80px estimate so the pre-prepend anchor reading is
+      // stable. The single trigger is the deliberate scrollTop = 0 below.
+      await timeline.evaluate((el) => {
+        el.scrollTop = 4000;
+      });
+      await page.waitForTimeout(300);
+      await timeline.evaluate((el) => {
+        el.scrollTop = 4000;
+      });
+      await page.waitForTimeout(150);
+      const before = await sampleAnchor(timeline);
+      expect(before.scrollTop).toBeGreaterThan(200);
+
+      // Trigger exactly one prepend. Scrolling to 150 trips the load-older
+      // sentinel (its rootMargin reaches 200px past the top) with
+      // previousScrollTopRef pinned near the top — the condition under which the
+      // resize-observer's competing restore collapsed the anchor pre-fix. After
+      // the single fetchOlder lands, the anchor restore carries scrollTop deep
+      // into the content, clear of the 200px sentinel zone, so the observer does
+      // NOT re-fire: one clean prepend, not the re-trigger storm that scrollTop
+      // 0 produces (0 keeps the sentinel tripped across every paged window down
+      // to the small exhaustion-tail page, which legitimately lands the top row
+      // near the top — masking the hold signal).
+      await timeline.evaluate((el) => {
+        el.scrollTop = 150;
+      });
+
+      // Anchor-hold gate (the race signal): poll until the restore has carried
+      // scrollTop deep into the content — past where it sat before the prepend.
+      // Pre-fix, the competing resize-observer restore (firing on the spinner's
+      // clientHeight shift, restoring to previousScrollTopRef ~150) won often
+      // enough that scrollTop stayed pinned near the top; this poll would then
+      // time out, failing the run. scrollHeight grows several frames BEFORE the
+      // restore moves scrollTop, so a scrollHeight gate would read mid-cycle
+      // near the top — the race lives in scrollTop, so the gate watches it.
+      await expect
+        .poll(async () => (await sampleAnchor(timeline)).scrollTop, {
+          timeout: 10_000,
+        })
+        .toBeGreaterThan(before.scrollTop);
+
+      // One settled snapshot for the remaining checks so scrollHeight and
+      // viewportPos come from the same frame as the held scrollTop:
+      //   (a) the scroller grew by the prepended rows' height (genuine prepend),
+      //   (b) the first-visible row sits where it did before the prepend.
+      const after = await sampleAnchor(timeline);
+      expect(after.scrollHeight).toBeGreaterThan(before.scrollHeight + 800);
+      expect(Math.abs(after.viewportPos - before.viewportPos)).toBeLessThan(120);
+
+      // Reconcile terminates: two equal scrollTop reads 600ms apart prove the
+      // rAF loop stopped. Under the double-writer bug the library re-scheduled
+      // one rAF per frame for the full 5s MAX_RECONCILE_MS valve — still churning
+      // 600ms apart.
+      const settled1 = await timeline.evaluate((el) => el.scrollTop);
+      await page.waitForTimeout(600);
+      const settled2 = await timeline.evaluate((el) => el.scrollTop);
+      expect(Math.abs(settled1 - settled2)).toBeLessThan(2);
+
+      if (run === 0) {
+        await page.screenshot({ path: `${SHOTS}/07-load-older-anchor-hold.png` });
+      }
+    }
+  });
 });