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]); }