From c6b3293ecc5ef3cefcbb2e7bcac8beb6171909ae Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Tue, 24 Mar 2026 18:46:21 -0400 Subject: [PATCH 1/8] fix: two timeline pagination bugs affecting message ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 1. useTimelinePagination: finally-block resets fetchingRef prematurely When a paginated page returns 1–4 events and more history exists, the paginator fires a recursive continuation call (`paginate(backwards)`) before the outer `finally` block runs. The sequence was: 1. outer try: `fetchingRef.current[dir] = false; paginate(backwards);` → inner paginate runs synchronously: checks lock (false) → sets it to `true` → hits `await` → suspends 2. outer finally: `fetchingRef.current[dir] = false;` → overwrites the `true` that inner paginate just wrote This left `fetchingRef.current[dir] = false` while the inner paginate was mid-flight. The `backwardStatus === 'loading'` guard on the scroll handler masked the problem in most cases, but for a long chain of sparse pages each returning 1–4 events, multiple overlapping paginations could start. If the SDK's internal per-timeline lock was also released (or a forward-direction call raced), events could be requested from stale pagination tokens and be inserted out of order. Fix: add a `continuing` local boolean. When a recursive continuation is fired, `continuing = true` is set first. The `finally` block skips the reset when `continuing` is true, leaving the lock in the inner paginate's hands. The inner paginate's own `finally` releases it when it finishes. ## 2. getLinkedTimelines: O(N²) array copies in recursive accumulator The previous recursive `collectTimelines(tl, dir, [...acc, tl])` created N intermediate arrays for an N-timeline chain (sizes 1, 2, 3, … N), giving O(N²) total allocations. For a power user who paginates back through hundreds of pages of history, this becomes measurable GC pressure. Replace with a simple iterative loop (`while (current) { push; next }`) that builds exactly one output array in O(N) time. --- src/app/hooks/timeline/useTimelineSync.ts | 20 +++++++++++++++++++- src/app/utils/timeline.ts | 18 +++++++----------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index 395d6fc46..9c7583217 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -171,6 +171,13 @@ const useTimelinePagination = ( await to(decryptAllTimelineEvent(mx, fetchedTimeline)); } + // `continuing` tracks whether we hand the fetchingRef lock to a recursive + // continuation call below. The finally block must NOT reset the lock if + // the recursive call has already claimed it, otherwise there is a brief + // window where fetchingRef is false while the recursive paginate is in + // flight, allowing a third overlapping call to start on sparse pages. + let continuing = false; + if (alive()) { recalibratePagination(lTimelines); (backwards ? setBackwardStatus : setForwardStatus)('idle'); @@ -184,13 +191,24 @@ const useTimelinePagination = ( Direction.Backward ) === 'string'; if (stillHasToken) { + // Release lock so inner paginate can claim it, then mark continuing + // so the finally block below does NOT reset it after inner claims. fetchingRef.current[directionKey] = false; + continuing = true; paginate(backwards); + // At this point the inner paginate has synchronously set + // fetchingRef.current[directionKey] = true before hitting its own + // await. The finally below will skip the reset. } } } } finally { - fetchingRef.current[directionKey] = false; + // Only release the lock if we did NOT hand it to a recursive continuation. + // If `continuing` is true the recursive call owns the lock and will release + // it in its own finally block. + if (!continuing) { + fetchingRef.current[directionKey] = false; + } } }; }, [mx, alive, setTimeline, limit, setBackwardStatus, setForwardStatus]); diff --git a/src/app/utils/timeline.ts b/src/app/utils/timeline.ts index 4ed92ab16..0934e5b02 100644 --- a/src/app/utils/timeline.ts +++ b/src/app/utils/timeline.ts @@ -22,18 +22,14 @@ export const getFirstLinkedTimeline = ( return current; }; -const collectTimelines = ( - tl: EventTimeline | null, - dir: Direction, - acc: EventTimeline[] = [] -): EventTimeline[] => { - if (!tl) return acc; - return collectTimelines(tl.getNeighbouringTimeline(dir), dir, [...acc, tl]); -}; - export const getLinkedTimelines = (timeline: EventTimeline): EventTimeline[] => { - const firstTimeline = getFirstLinkedTimeline(timeline, Direction.Backward); - return collectTimelines(firstTimeline, Direction.Forward); + const result: EventTimeline[] = []; + let current: EventTimeline | null = getFirstLinkedTimeline(timeline, Direction.Backward); + while (current) { + result.push(current); + current = current.getNeighbouringTimeline(Direction.Forward); + } + return result; }; export const timelineToEventsCount = (t: EventTimeline) => { From 1fc0a0d80cc8413e58814df8b320e89d5b883421 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Tue, 24 Mar 2026 18:31:38 -0400 Subject: [PATCH 2/8] fix: deduplicate concurrent URL preview requests with module-level cache Multiple UrlPreviewCard instances rendering simultaneously (e.g. after a large event batch loads) each called mx.getUrlPreview(url, ts). The SDK caches by bucketed-timestamp + URL, so identical URLs from different messages produced N separate HTTP requests. Add a module-level previewRequestCache (Map) that dedups in-flight requests across all instances. Resolved promises remain cached for the session; rejected ones are evicted so later renders can retry. --- src/app/components/url-preview/UrlPreviewCard.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/app/components/url-preview/UrlPreviewCard.tsx b/src/app/components/url-preview/UrlPreviewCard.tsx index 62106a00a..a0f8f93b2 100644 --- a/src/app/components/url-preview/UrlPreviewCard.tsx +++ b/src/app/components/url-preview/UrlPreviewCard.tsx @@ -14,6 +14,13 @@ import { ImageViewer } from '../image-viewer'; const linkStyles = { color: color.Success.Main }; +// Module-level in-flight deduplication: prevents N+1 concurrent requests when a +// large event batch renders many UrlPreviewCard instances for the same URL. +// Keyed by URL only (not ts) — the same URL shows the same preview regardless +// of which message referenced it. Rejected promises are evicted so a later +// render can retry after network recovery. +const previewRequestCache = new Map>(); + const openMediaInNewTab = async (url: string | undefined) => { if (!url) { console.warn('Attempted to open an empty url'); @@ -34,7 +41,12 @@ export const UrlPreviewCard = as<'div', { url: string; ts: number; mediaType?: s const [previewStatus, loadPreview] = useAsyncCallback( useCallback(() => { if (isDirect) return Promise.resolve(null); - return mx.getUrlPreview(url, ts); + const cached = previewRequestCache.get(url); + if (cached !== undefined) return cached; + const promise = mx.getUrlPreview(url, ts); + previewRequestCache.set(url, promise); + promise.catch(() => previewRequestCache.delete(url)); + return promise; }, [url, ts, mx, isDirect]) ); From ba99ef428b488446346d372c972ff810cf0ee506 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Tue, 24 Mar 2026 18:29:41 -0400 Subject: [PATCH 3/8] fix: remove redundant startSpidering call causing N+1 sync requests --- src/client/initMatrix.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/client/initMatrix.ts b/src/client/initMatrix.ts index 18809c5d2..093093ba2 100644 --- a/src/client/initMatrix.ts +++ b/src/client/initMatrix.ts @@ -563,9 +563,6 @@ export const startClient = async (mx: MatrixClient, config?: StartClientConfig): pollTimeoutMs: slidingConfig?.pollTimeoutMs ?? SLIDING_SYNC_POLL_TIMEOUT_MS, }); manager.attach(); - // Begin background spidering so all rooms are eventually indexed. - // Not awaited — this runs incrementally in the background. - manager.startSpidering(100, 50); slidingSyncByClient.set(mx, manager); syncTransportByClient.set(mx, { transport: 'sliding', From 95b7d4ac5d138a0d78c3a4e2afba28a58f3eed8f Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Tue, 24 Mar 2026 22:03:20 -0400 Subject: [PATCH 4/8] chore: add changeset for bug fixes PR --- .changeset/fix-bug-fixes.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fix-bug-fixes.md diff --git a/.changeset/fix-bug-fixes.md b/.changeset/fix-bug-fixes.md new file mode 100644 index 000000000..aea8ed44e --- /dev/null +++ b/.changeset/fix-bug-fixes.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Fix timeline pagination lock bug, deduplicate concurrent URL preview requests, and remove redundant sliding sync spidering call causing N+1 requests. From 9db324f1e4ac311200faf006816c06dad21715fd Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Tue, 24 Mar 2026 22:11:37 -0400 Subject: [PATCH 5/8] fix: hoist 'continuing' declaration before try block for finally scope access --- src/app/hooks/timeline/useTimelineSync.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index 9c7583217..5c99bf6c2 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -147,6 +147,13 @@ const useTimelinePagination = ( (backwards ? setBackwardStatus : setForwardStatus)('loading'); } + // `continuing` tracks whether we hand the fetchingRef lock to a recursive + // continuation call below. The finally block must NOT reset the lock if + // the recursive call has already claimed it, otherwise there is a brief + // window where fetchingRef is false while the recursive paginate is in + // flight, allowing a third overlapping call to start on sparse pages. + let continuing = false; + try { const countBefore = getTimelinesEventsCount(lTimelines); @@ -171,13 +178,6 @@ const useTimelinePagination = ( await to(decryptAllTimelineEvent(mx, fetchedTimeline)); } - // `continuing` tracks whether we hand the fetchingRef lock to a recursive - // continuation call below. The finally block must NOT reset the lock if - // the recursive call has already claimed it, otherwise there is a brief - // window where fetchingRef is false while the recursive paginate is in - // flight, allowing a third overlapping call to start on sparse pages. - let continuing = false; - if (alive()) { recalibratePagination(lTimelines); (backwards ? setBackwardStatus : setForwardStatus)('idle'); From 9f45a739f688ed01c1f280b68423c509f1a7e51d Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Tue, 24 Mar 2026 23:56:01 -0400 Subject: [PATCH 6/8] fix: rename promise to urlPreview in UrlPreviewCard --- src/app/components/url-preview/UrlPreviewCard.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/components/url-preview/UrlPreviewCard.tsx b/src/app/components/url-preview/UrlPreviewCard.tsx index a0f8f93b2..edc3f0f9d 100644 --- a/src/app/components/url-preview/UrlPreviewCard.tsx +++ b/src/app/components/url-preview/UrlPreviewCard.tsx @@ -43,10 +43,10 @@ export const UrlPreviewCard = as<'div', { url: string; ts: number; mediaType?: s if (isDirect) return Promise.resolve(null); const cached = previewRequestCache.get(url); if (cached !== undefined) return cached; - const promise = mx.getUrlPreview(url, ts); - previewRequestCache.set(url, promise); - promise.catch(() => previewRequestCache.delete(url)); - return promise; + const urlPreview = mx.getUrlPreview(url, ts); + previewRequestCache.set(url, urlPreview); + urlPreview.catch(() => previewRequestCache.delete(url)); + return urlPreview; }, [url, ts, mx, isDirect]) ); From 38057ee9ce268593a6577b2ab2ce37f36a42ab53 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Wed, 25 Mar 2026 20:08:34 -0400 Subject: [PATCH 7/8] fix: reset live timeline on room navigation and reconnect The Matrix SDK for sliding sync deliberately does not call resetLiveTimeline() on limited:true (the handling is in a commented-out TODO block). This means events from a previous visit remain in the live timeline and new events get appended after them, with the backward pagination token pointing to the gap between old and new events rather than to before all events. The result is that new messages appear out of order or the gap is unreachable via the UI. Two fixes: 1. useSlidingSyncActiveRoom: reset the live timeline synchronously before the subscription request is sent on room navigation. This ensures the fresh initial:true response populates a clean timeline, not one polluted with events from the previous visit. Also removes the 100ms delay that was added as an incomplete workaround for the same issue. 2. SlidingSyncManager.onLifecycle: in RequestFinished state (fires before any room data listeners run), reset the live timeline for active-room subscriptions that receive initial:true or limited:true. This covers the reconnect/background case where the pos token expires and all active rooms get a fresh initial:true in the same sync cycle. --- src/app/hooks/useSlidingSyncActiveRoom.ts | 13 ++----------- src/client/slidingSync.ts | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/app/hooks/useSlidingSyncActiveRoom.ts b/src/app/hooks/useSlidingSyncActiveRoom.ts index 8468f4024..c86914d56 100644 --- a/src/app/hooks/useSlidingSyncActiveRoom.ts +++ b/src/app/hooks/useSlidingSyncActiveRoom.ts @@ -24,16 +24,7 @@ export const useSlidingSyncActiveRoom = (): void => { const manager = getSlidingSyncManager(mx); if (!manager) return undefined; - // Wait for the room to be initialized from list sync before subscribing - // with the full timeline limit. This prevents timeline ordering issues where - // the room might be receiving events from list expansion while we're also - // trying to load a large timeline, causing events to be added out of order. - const timeoutId = setTimeout(() => { - manager.subscribeToRoom(roomId); - }, 100); - - return () => { - clearTimeout(timeoutId); - }; + manager.subscribeToRoom(roomId); + return undefined; }, [mx, roomId]); }; diff --git a/src/client/slidingSync.ts b/src/client/slidingSync.ts index 84622b0e2..058b87cf1 100644 --- a/src/client/slidingSync.ts +++ b/src/client/slidingSync.ts @@ -8,6 +8,7 @@ import { MSC3575List, MSC3575RoomData, MSC3575RoomSubscription, + MSC3575SlidingSyncResponse, MSC3575_WILDCARD, RoomMemberEvent, SlidingSync, @@ -350,6 +351,27 @@ export class SlidingSyncManager { return; } + // Before room data is processed, reset live timelines for active rooms that + // are receiving a full refresh (initial: true) or a post-gap update + // (limited: true). The SDK deliberately does not call resetLiveTimeline() for + // sliding sync, so events from previous visits accumulate in the live + // timeline alongside new events. Resetting here — before the SDK's + // onRoomData listener runs — ensures the fresh batch lands on a clean + // timeline with a correct backward pagination token. + if (state === SlidingSyncState.RequestFinished && resp && !err) { + const rooms = (resp as MSC3575SlidingSyncResponse).rooms ?? {}; + Object.entries(rooms) + .filter(([, roomData]) => roomData.initial || roomData.limited) + .filter(([roomId]) => this.activeRoomSubscriptions.has(roomId)) + .forEach(([roomId]) => { + const room = this.mx.getRoom(roomId); + if (!room) return; + const timelineSet = room.getUnfilteredTimelineSet(); + if (timelineSet.getLiveTimeline().getEvents().length === 0) return; + timelineSet.resetLiveTimeline(); + }); + } + if (err || !resp || state !== SlidingSyncState.Complete) return; // Track what changed in this sync cycle From 7062f64e78935bf08a97c8b4b164964d3398b99f Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Wed, 25 Mar 2026 22:17:13 -0400 Subject: [PATCH 8/8] fix: always open room at bottom of timeline --- src/app/features/room/RoomTimeline.tsx | 42 +++++++++++++++++++++----- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index dd655deec..d4aaebbd9 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -207,6 +207,9 @@ export function RoomTimeline({ const topSpacerHeightRef = useRef(0); const mountScrollWindowRef = useRef(Date.now() + 3000); const hasInitialScrolledRef = useRef(false); + // Stored in a ref so eventsLength fluctuations (e.g. onLifecycle timeline reset + // firing within the window) cannot cancel it via useLayoutEffect cleanup. + const initialScrollTimerRef = useRef | undefined>(undefined); const currentRoomIdRef = useRef(room.roomId); const [isReady, setIsReady] = useState(false); @@ -273,16 +276,31 @@ export function RoomTimeline({ vListRef.current ) { vListRef.current.scrollToIndex(processedEventsRef.current.length - 1, { align: 'end' }); - const t = setTimeout(() => { - vListRef.current?.scrollToIndex(processedEventsRef.current.length - 1, { align: 'end' }); + // Store in a ref rather than a local so subsequent eventsLength changes + // (e.g. the onLifecycle timeline reset firing within 80 ms) do NOT + // cancel this timer through the useLayoutEffect cleanup. + initialScrollTimerRef.current = setTimeout(() => { + initialScrollTimerRef.current = undefined; + if (processedEventsRef.current.length > 0) { + vListRef.current?.scrollToIndex(processedEventsRef.current.length - 1, { align: 'end' }); + } setIsReady(true); }, 80); hasInitialScrolledRef.current = true; - return () => clearTimeout(t); } - return () => {}; + // No cleanup return — the timer must survive eventsLength fluctuations. + // It is cancelled on unmount by the dedicated effect below. }, [timelineSync.eventsLength, eventId, room.roomId]); + // Cancel the initial-scroll timer on unmount (the useLayoutEffect above + // intentionally does not cancel it when deps change). + useEffect( + () => () => { + if (initialScrollTimerRef.current !== undefined) clearTimeout(initialScrollTimerRef.current); + }, + [] + ); + const recalcTopSpacer = useCallback(() => { const v = vListRef.current; if (!v) return; @@ -355,6 +373,11 @@ export function RoomTimeline({ useEffect(() => { if (eventId) return; + // Guard: once the timeline is visible to the user, do not override their + // scroll position. Without this, a later timeline refresh (e.g. the + // onLifecycle reset delivering a new linkedTimelines reference) can fire + // this effect after isReady and snap the view back to the read marker. + if (isReady) return; const { readUptoEventId, inLiveTimeline, scrollTo } = unreadInfo ?? {}; if (readUptoEventId && inLiveTimeline && scrollTo) { const evtTimeline = getEventTimeline(room, readUptoEventId); @@ -366,12 +389,16 @@ export function RoomTimeline({ ) : undefined; - if (absoluteIndex !== undefined && vListRef.current) { + if (absoluteIndex !== undefined) { const processedIndex = getRawIndexToProcessedIndex(absoluteIndex); - if (processedIndex !== undefined) { + if (processedIndex !== undefined && vListRef.current) { vListRef.current.scrollToIndex(processedIndex, { align: 'start' }); - setUnreadInfo((prev) => (prev ? { ...prev, scrollTo: false } : prev)); } + // Always consume the scroll intent once the event is located in the + // linked timelines, even if its processedIndex is undefined (filtered + // event). Without this, each linkedTimelines reference change retries + // the scroll indefinitely. + setUnreadInfo((prev) => (prev ? { ...prev, scrollTo: false } : prev)); } } }, [ @@ -379,6 +406,7 @@ export function RoomTimeline({ unreadInfo, timelineSync.timeline.linkedTimelines, eventId, + isReady, getRawIndexToProcessedIndex, ]);