From 3c858d003cfa1168131dd8dff20a1e74685fb036 Mon Sep 17 00:00:00 2001 From: Michael Chapman Date: Tue, 17 Mar 2026 21:19:36 -0500 Subject: [PATCH] fix: repair pinned message navigation for large sessions (#187) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Navigating to a pinned message in a long session (ascending sort, oldest first) would silently fail — the session would open but the viewport would not reach the target message. Root cause: TanStack Virtual's scrollToIndex reconcile loop exits after just 1 stable frame (STABLE_FRAMES=1). When the target item is outside the current render window, scrollToIndex scrolls to an estimated position. The reconcile then fires once, sees that the estimated offset is unchanged (because ResizeObserver measurements are delayed behind bumpVersion's setTimeout(0)), declares the position "stable", and exits. The viewport lands at the wrong position with no further correction. A secondary issue: the virtualizer's postUpdate callback (which preserves scroll position during infinite-scroll prepends) would call scrollToOffset(currentScrollTop), resetting scrollState.index to null and breaking any in-flight reconcile loop early. Fix 1 (MessageList.svelte): after calling scrollToIndex for an unrendered target, schedule a retry in 2 animation frames. By then ResizeObserver and bumpVersion's setTimeout have fired, updating measurementsCache with real item sizes. Each retry either finds the item rendered and snaps to its exact measured offset via getOffsetForIndex, or calls scrollToIndex again with a more accurate estimate. A reqId guard cancels stale retries if a new navigation request arrives before the loop completes. Up to 10 render retries (~320 ms) are allowed. Fix 2 (createVirtualizer.svelte.ts): skip the postUpdate position-preserve when scrollState.index is non-null, i.e. when TanStack has an active scrollToIndex reconcile in progress. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../lib/components/content/MessageList.svelte | 57 +++++++++++++------ .../lib/virtual/createVirtualizer.svelte.ts | 10 ++++ 2 files changed, 51 insertions(+), 16 deletions(-) diff --git a/frontend/src/lib/components/content/MessageList.svelte b/frontend/src/lib/components/content/MessageList.svelte index e8605cb9..2dada0c9 100644 --- a/frontend/src/lib/components/content/MessageList.svelte +++ b/frontend/src/lib/components/content/MessageList.svelte @@ -146,7 +146,10 @@ function scrollToDisplayIndex( index: number, attempt: number = 0, + reqId: number = lastScrollRequest, ) { + if (reqId !== lastScrollRequest) return; + const v = virtualizer.instance; if (!v) return; @@ -157,28 +160,50 @@ (virtualCount !== desiredCount || index >= virtualCount) ) { requestAnimationFrame(() => { - scrollToDisplayIndex(index, attempt + 1); + scrollToDisplayIndex(index, attempt + 1, reqId); }); return; } - // TanStack's scrollToIndex may continuously re-seek - // in dynamic mode. Use one offset seek to avoid - // visible scroll "fight." - const offsetAndAlign = - v.getOffsetForIndex(index, "start"); - if (offsetAndAlign) { - const [offset] = offsetAndAlign; - v.scrollToOffset( - Math.round(offset), - { align: "start" }, - ); + // If the item is already rendered (in the current virtual window), + // use its exact measured offset. Predecessor sizes are known so + // getOffsetForIndex is accurate. + const virtualItems = v.getVirtualItems(); + const isRendered = virtualItems.some( + (vi) => vi.index === index, + ); + if (isRendered) { + const offsetAndAlign = + v.getOffsetForIndex(index, "start"); + if (offsetAndAlign) { + const [offset] = offsetAndAlign; + v.scrollToOffset( + Math.round(offset), + { align: "start" }, + ); + } return; } - // Item not yet measured — use scrollToIndex which will - // estimate and then correct once measured. + // Item not yet in render window. scrollToIndex scrolls to an + // estimated position, but TanStack's reconcile loop exits after + // 1 stable frame — before ResizeObserver measurements (delayed + // by bumpVersion's setTimeout(0)) have updated the offsets. The + // scroll stops at an estimated position rather than the real one. + // + // Retry in 2 frames: by then ResizeObserver + bumpVersion have + // fired, measurements are updated, and the next attempt either + // finds the item rendered (for an exact offset scroll) or repeats + // with a more accurate estimate. Limit to 10 render retries + // (~320 ms) to avoid looping forever. v.scrollToIndex(index, { align: "start" }); + if (attempt < 15) { + requestAnimationFrame(() => { + requestAnimationFrame(() => { + scrollToDisplayIndex(index, attempt + 1, reqId); + }); + }); + } } function raf(): Promise { @@ -195,7 +220,7 @@ const idx = ui.sortNewestFirst ? displayItemsAsc.length - 1 - idxAsc : idxAsc; - scrollToDisplayIndex(idx); + scrollToDisplayIndex(idx, 0, reqId); return; } @@ -217,7 +242,7 @@ const loadedIdx = ui.sortNewestFirst ? displayItemsAsc.length - 1 - loadedIdxAsc : loadedIdxAsc; - scrollToDisplayIndex(loadedIdx); + scrollToDisplayIndex(loadedIdx, 0, reqId); } export function scrollToOrdinal(ordinal: number) { diff --git a/frontend/src/lib/virtual/createVirtualizer.svelte.ts b/frontend/src/lib/virtual/createVirtualizer.svelte.ts index 725bd2af..b3da1d50 100644 --- a/frontend/src/lib/virtual/createVirtualizer.svelte.ts +++ b/frontend/src/lib/virtual/createVirtualizer.svelte.ts @@ -144,6 +144,16 @@ export function createVirtualizer( scrollEl.scrollTop = 0; return; } + // Don't override an active scrollToIndex reconcile loop. + // scrollState.index is non-null while TanStack is iterating + // toward the target index. Calling scrollToOffset here would + // reset scrollState.index to null, breaking the loop and + // leaving the viewport at the wrong position (observed as + // pinned-message navigation stopping mid-scroll in ascending + // sort when the target item is not yet rendered). + // scrollState is typed private but is a plain public field at runtime. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + if ((instance as any).scrollState?.index != null) return; if (scrollEl.scrollTop > 0) { instance.scrollToOffset(scrollEl.scrollTop); }