From b56b6211950576199f3882196609499670feb94b Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Thu, 2 Jul 2026 15:28:21 -0400 Subject: [PATCH] fix(desktop): stop timeline cap from evicting scrolled-in history mid-read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A channel timeline cache holding more than MAX_TIMELINE_MESSAGES (2000) content events — i.e. the user paged back to an old day in a busy channel — was silently trimmed back to the newest 2000 by the next capped merge: any live event, optimistic send, or fresh-history revalidation. The rows being read vanished out from under the reader, appearing as randomly missing messages for old dates (all present right after scrollback, partially or fully gone seconds later). Scrollback merges were already uncapped (isOlderHistoryPage special case), which is what made the loss intermittent instead of consistent. Fix: no merge into a potentially-on-screen timeline applies the cap. - mergeTimelineHistoryMessages: sort + dedupe only (drops the isOlderHistoryPage split — both branches now behave identically). - mergeTimelineCacheMessages (live/optimistic/single-event): same. - The DOM bound is enforced where nothing is rendered from the cache: the cold-snapshot paint (already capped) and a new trim-on-leave in useChannelSubscription cleanup, which re-applies normalizeTimelineMessages when the user leaves the channel. Regression test: expanded 2537-event cache + fresh newest-page revalidation merge — all old rows survive. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/src/features/messages/hooks.ts | 32 +++++++++++--- .../messages/lib/messageQueryKeys.test.mjs | 30 +++++++++++++ .../features/messages/lib/messageQueryKeys.ts | 42 ++++++------------- 3 files changed, 70 insertions(+), 34 deletions(-) diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index 81feccd0c..7510651bc 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -113,15 +113,21 @@ export function mergeMessages( return mergeMessagesWithNormalizer(current, incoming, sortMessages); } +/** + * Merge one incoming event (live subscription, optimistic send, edit overlay) + * into the timeline cache. Sort + dedupe only — deliberately NOT capped. + * + * This merge fires constantly while the user may be scrolled back; capping it + * evicted paged-in history out from under the reader (the "June 12 messages + * flicker away" bug). The MAX_TIMELINE_MESSAGES bound is applied instead when + * the channel is left (see useChannelSubscription's cleanup) and on cold + * snapshot paints — moments when no scrolled-back timeline is on screen. + */ export function mergeTimelineCacheMessages( current: RelayEvent[], incoming: RelayEvent, ): RelayEvent[] { - return mergeMessagesWithNormalizer( - current, - incoming, - normalizeTimelineMessages, - ); + return mergeMessages(current, incoming); } export function createOptimisticMessage( @@ -319,6 +325,21 @@ export function useChannelSubscription(channel: Channel | null) { } }); + // Leaving the channel is the safe moment to enforce the timeline cap: + // nothing is rendered from this cache anymore, so trimming to the newest + // MAX_TIMELINE_MESSAGES window cannot evict rows out from under a + // scrolled-back reader. Merges while the channel is open are deliberately + // uncapped for the same reason. + const trimTimelineOnLeave = useEffectEvent((leftChannelId: string) => { + queryClient.setQueryData( + channelMessagesKey(leftChannelId), + (current) => + current && current.length > 0 + ? normalizeTimelineMessages(current) + : current, + ); + }); + useEffect(() => { if (!channelId || channelType === "forum") { return; @@ -371,6 +392,7 @@ export function useChannelSubscription(channel: Channel | null) { if (cleanup) { void cleanup(); } + trimTimelineOnLeave(channelId); }; }, [channelId, channelType]); } diff --git a/desktop/src/features/messages/lib/messageQueryKeys.test.mjs b/desktop/src/features/messages/lib/messageQueryKeys.test.mjs index 0448c0a83..8b67fd8b4 100644 --- a/desktop/src/features/messages/lib/messageQueryKeys.test.mjs +++ b/desktop/src/features/messages/lib/messageQueryKeys.test.mjs @@ -263,3 +263,33 @@ test("sortMessages tiebreaks same-second events on id, order-independent", () => assert.deepEqual(forward, reverse); assert.deepEqual(forward, [a.id, b.id, c.id]); }); + +test("fresh-newest revalidation over an expanded cache never evicts scrolled-in history", () => { + // Regression: a channel holding more content events than the timeline cap + // (the user paged back to an old day), then a background revalidation + // merges the newest history window over it. The old rows must survive — + // capping this merge evicted the day being read (the "June 12 messages + // flicker away" bug). + const expandedCache = []; + for (let index = 0; index < 37; index += 1) { + expandedCache.push( + event({ id: id("jun", index), createdAt: 1_000 + index }), + ); + } + for (let index = 0; index < 2_500; index += 1) { + expandedCache.push( + event({ id: id("mid", index), createdAt: 10_000 + index }), + ); + } + const freshNewestPage = []; + for (let index = 0; index < 60; index += 1) { + freshNewestPage.push( + event({ id: id("fresh", index), createdAt: 50_000 + index }), + ); + } + + const merged = mergeTimelineHistoryMessages(expandedCache, freshNewestPage); + + assert.equal(merged.filter((item) => item.id.startsWith("jun")).length, 37); + assert.equal(merged.filter((item) => item.kind === 9).length, 2_597); +}); diff --git a/desktop/src/features/messages/lib/messageQueryKeys.ts b/desktop/src/features/messages/lib/messageQueryKeys.ts index 2a247b1b1..1d46ec4e8 100644 --- a/desktop/src/features/messages/lib/messageQueryKeys.ts +++ b/desktop/src/features/messages/lib/messageQueryKeys.ts @@ -96,37 +96,21 @@ export function normalizeTimelineMessages(messages: RelayEvent[]) { return capNewestTimelineMessages(sortMessages(messages)); } -function isOlderHistoryPage(current: RelayEvent[], history: RelayEvent[]) { - if (current.length === 0 || history.length === 0) { - return false; - } - - const sortedCurrent = sortMessages(current); - const sortedHistory = sortMessages(history); - const newestHistory = sortedHistory[sortedHistory.length - 1]?.created_at; - const oldestCurrent = sortedCurrent[0]?.created_at; - - if (newestHistory === undefined || oldestCurrent === undefined) { - return false; - } - - return newestHistory <= oldestCurrent; -} - -function normalizeTimelineHistoryMessages( - current: RelayEvent[], - history: RelayEvent[], -) { - return sortMessages([...current, ...history]); -} - +/** + * Merge a batch of events (older scrollback page, reconnect/revalidation + * window, live gap-fill) into the timeline cache. Sort + dedupe only — the + * {@link MAX_TIMELINE_MESSAGES} cap is deliberately NOT applied here. + * + * Capping merges into an already-painted timeline evicts history out from + * under a reader: page back to an old day in a channel holding more than the + * cap, and the next capped merge (live append, revalidation) silently deletes + * the rows being read. The cap is applied only by + * {@link normalizeTimelineMessages} at moments when nothing is rendered — + * the cold-snapshot paint and the channel-leave trim in `hooks.ts`. + */ export function mergeTimelineHistoryMessages( current: RelayEvent[], history: RelayEvent[], ) { - if (isOlderHistoryPage(current, history)) { - return normalizeTimelineHistoryMessages(current, history); - } - - return normalizeTimelineMessages([...current, ...history]); + return sortMessages([...current, ...history]); }