From a85fa30e0a42218f79ca596069fa14386a3bd33f Mon Sep 17 00:00:00 2001 From: Wes Date: Fri, 14 Aug 2026 14:33:39 -0600 Subject: [PATCH] perf(desktop): paint warm channels from cache Use authoritative channel-window provenance to distinguish warm empty channels from cold live-seeded caches. Mount each channel timeline with its route-matched cached snapshot and revalidate the retained scrollback extent atomically so background catch-up cannot erase the reader's anchor. Co-authored-by: Carl Signed-off-by: Wes --- .../src/features/channels/ui/ChannelPane.tsx | 1 + .../features/channels/ui/ChannelScreen.tsx | 1 + desktop/src/features/messages/hooks.ts | 97 ++++++++++++++++++- .../lib/projectChannelWindow.test.mjs | 60 +++++++++++- .../lib/timelineLoadingState.test.mjs | 27 ++++++ .../messages/lib/timelineLoadingState.ts | 8 ++ .../features/messages/ui/MessageTimeline.tsx | 30 ++---- .../ui/timelineSnapshotProjection.test.mjs | 23 ++++- 8 files changed, 216 insertions(+), 31 deletions(-) diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 1ec6cee95..cee459338 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -568,6 +568,7 @@ export const ChannelPane = React.memo(function ChannelPane({ > {isHuddleTranscript ? null : header} 0, dataLength: messagesQuery.data?.length ?? null, }, hasSettledThisChannel, diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index 8b457a7ad..4eeae402a 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -55,6 +55,8 @@ import type { Channel, Identity, RelayEvent } from "@/shared/api/types"; // from the on-render overlay. import { applyEditTagOverlay } from "@/features/messages/lib/applyEditTagOverlay.mjs"; import { + appendOlderChannelWindow, + compareRelayOrder, emptyChannelWindowStore, mapChannelWindowEvents, mergeLiveChannelWindowEvent, @@ -245,21 +247,97 @@ export function reconcileFetchedChannelWindow( events: Awaited>, previousMessages: RelayEvent[], signal: AbortSignal, +): RelayEvent[] { + return reconcileFetchedChannelWindowPages( + queryClient, + channelId, + [parseChannelWindowResponse(events, channelId, null)], + previousMessages, + signal, + ); +} + +export function reconcileFetchedChannelWindowPages( + queryClient: QueryClient, + channelId: string, + pages: ReturnType[], + previousMessages: RelayEvent[], + signal: AbortSignal, ): RelayEvent[] { // Tauri invokes cannot be canceled after dispatch. A replacement refetch can // therefore win while this older request is still in flight. Never let that // canceled request commit its stale page into the authoritative window. signal.throwIfAborted(); const windowKey = channelWindowKey(channelId); - const page = parseChannelWindowResponse(events, channelId, null); const current = queryClient.getQueryData(windowKey) ?? emptyChannelWindowStore(); - const next = replaceNewestChannelWindow(current, page); + let next = replaceNewestChannelWindow(current, pages[0]); + for (const page of pages.slice(1)) { + next = appendOlderChannelWindow(next, page); + } queryClient.setQueryData(windowKey, next); return reconcileChannelWindowMessages(next, previousMessages); } +const CHANNEL_WINDOW_PAGE_SIZE = 50; +const CHANNEL_WINDOW_MAX_REQUEST_ROWS = 200; + +async function getRefreshedChannelWindowPages( + channelId: string, + retainedWindow: ChannelWindowStore | undefined, +) { + const retainedRowCount = + retainedWindow?.pages.reduce( + (count, page) => count + page.rows.length, + 0, + ) ?? 0; + const retainedOldest = retainedWindow?.pages.at(-1)?.rows.at(-1)?.event; + const targetRows = Math.max(CHANNEL_WINDOW_PAGE_SIZE, retainedRowCount); + const firstEvents = await getChannelWindowEvents( + channelId, + null, + Math.min(targetRows, CHANNEL_WINDOW_MAX_REQUEST_ROWS), + ); + const firstPage = parseChannelWindowResponse(firstEvents, channelId, null); + const pages = [firstPage]; + let rowCount = firstPage.rows.length; + + const coversRetainedOldest = () => { + if (!retainedOldest) return true; + const refreshedOldest = pages.at(-1)?.rows.at(-1)?.event; + return ( + refreshedOldest !== undefined && + compareRelayOrder(refreshedOldest, retainedOldest) >= 0 + ); + }; + + while ( + pages.at(-1)?.hasMore && + (rowCount < targetRows || !coversRetainedOldest()) + ) { + const tail = pages.at(-1); + if (!tail?.nextCursor) break; + const nextEvents = await getChannelWindowEvents( + channelId, + tail.nextCursor, + Math.min( + Math.max(CHANNEL_WINDOW_PAGE_SIZE, targetRows - rowCount), + CHANNEL_WINDOW_MAX_REQUEST_ROWS, + ), + ); + const nextPage = parseChannelWindowResponse( + nextEvents, + channelId, + tail.nextCursor, + ); + pages.push(nextPage); + rowCount += nextPage.rows.length; + } + + return pages; +} + export function useChannelMessagesQuery(channel: Channel | null) { const queryClient = useQueryClient(); const queryKey = channelMessagesKey(channel?.id ?? "none"); @@ -271,11 +349,20 @@ export function useChannelMessagesQuery(channel: Channel | null) { if (!channel) throw new Error("No channel selected."); const previousMessages = queryClient.getQueryData(queryKey) ?? []; - const events = await getChannelWindowEvents(channel.id); - return reconcileFetchedChannelWindow( + const retainedWindow = queryClient.getQueryData( + channelWindowKey(channel.id), + ); + // A subscription/reconnect catch-up must cover the retained page extent. + // Refetching only the default head page would replace a multi-page window + // and delete the rows (and anchor) the reader is currently parked on. + const pages = await getRefreshedChannelWindowPages( + channel.id, + retainedWindow, + ); + return reconcileFetchedChannelWindowPages( queryClient, channel.id, - events, + pages, previousMessages, signal, ); diff --git a/desktop/src/features/messages/lib/projectChannelWindow.test.mjs b/desktop/src/features/messages/lib/projectChannelWindow.test.mjs index 14ec110ad..8890e610a 100644 --- a/desktop/src/features/messages/lib/projectChannelWindow.test.mjs +++ b/desktop/src/features/messages/lib/projectChannelWindow.test.mjs @@ -2,7 +2,10 @@ import assert from "node:assert/strict"; import test from "node:test"; import { QueryClient, QueryObserver } from "@tanstack/react-query"; -import { reconcileFetchedChannelWindow } from "../hooks.ts"; +import { + reconcileFetchedChannelWindow, + reconcileFetchedChannelWindowPages, +} from "../hooks.ts"; import { channelMessagesKey, channelWindowKey } from "./messageQueryKeys.ts"; import { appendOlderChannelWindow, @@ -287,6 +290,61 @@ test("test_live_projection_retains_pending_send_and_non_broadcast_thread_reply", ]); }); +test("test_catch_up_atomically_replaces_the_full_retained_extent", () => { + const harness = createHarness(); + const older = event("older", 50); + const firstCursor = { createdAt: 100, eventId: event("initial", 100).id }; + const loaded = appendOlderChannelWindow( + replaceNewestChannelWindow(emptyChannelWindowStore(), { + ...newestPage([event("initial", 100)]), + nextCursor: firstCursor, + hasMore: true, + }), + { + startCursor: firstCursor, + rows: [{ event: older, thread: null }], + aux: [], + nextCursor: null, + hasMore: false, + }, + ); + harness.client.setQueryData(harness.windowKey, loaded); + harness.client.setQueryData(harness.messagesKey, [ + older, + event("initial", 100), + ]); + + const refreshedHead = { + startCursor: null, + rows: [event("gap", 110), event("initial", 100)].map((item) => ({ + event: item, + thread: null, + })), + aux: [], + nextCursor: firstCursor, + hasMore: true, + }; + const refreshedTail = { + startCursor: firstCursor, + rows: [{ event: older, thread: null }], + aux: [], + nextCursor: null, + hasMore: false, + }; + + const projected = reconcileFetchedChannelWindowPages( + harness.client, + harness.channelId, + [refreshedHead, refreshedTail], + harness.client.getQueryData(harness.messagesKey), + new AbortController().signal, + ); + harness.client.setQueryData(harness.messagesKey, projected); + + assert.equal(harness.client.getQueryData(harness.windowKey).pages.length, 2); + assert.deepEqual(contents(harness), ["older", "initial", "gap"]); +}); + test("test_canceled_stale_fetch_cannot_overwrite_catch_up_window", async () => { const harness = createHarness(); const requests = []; diff --git a/desktop/src/features/messages/lib/timelineLoadingState.test.mjs b/desktop/src/features/messages/lib/timelineLoadingState.test.mjs index fe6960f74..41f924833 100644 --- a/desktop/src/features/messages/lib/timelineLoadingState.test.mjs +++ b/desktop/src/features/messages/lib/timelineLoadingState.test.mjs @@ -31,6 +31,33 @@ test("stale placeholder while refetching is loading", () => { ); }); +test("authoritative warm empty stays visible while revalidating", () => { + assert.equal( + selectTimelineLoadingState({ + ...settled, + isFetching: true, + hasAuthoritativePage: true, + dataLength: 0, + }), + false, + ); +}); + +test("authoritative warm rows stay visible before the local latch settles", () => { + assert.equal( + selectTimelineLoadingState( + { + ...settled, + isFetching: true, + hasAuthoritativePage: true, + dataLength: 12, + }, + false, + ), + false, + ); +}); + test("subscription-seeded empty cache while fetching is loading", () => { // The live subscription's setQueryData seeds [] before history settles, so // data is defined but empty and a fetch is still in flight. diff --git a/desktop/src/features/messages/lib/timelineLoadingState.ts b/desktop/src/features/messages/lib/timelineLoadingState.ts index ea46168d2..1a4df4605 100644 --- a/desktop/src/features/messages/lib/timelineLoadingState.ts +++ b/desktop/src/features/messages/lib/timelineLoadingState.ts @@ -12,6 +12,8 @@ export type TimelineQueryStatus = { isPending: boolean; isFetching: boolean; isPlaceholderData: boolean; + /** True once this channel has an authoritative page, including an empty one. */ + hasAuthoritativePage?: boolean; dataLength: number | null; }; @@ -19,6 +21,12 @@ export function selectTimelineLoadingState( status: TimelineQueryStatus, hasSettled = true, ): boolean { + // Page provenance, not row count, distinguishes a warm empty channel from a + // cold cache seeded by the live subscription. Once a page exists, every + // subsequent fetch is background revalidation and must not cover the cache. + if (status.hasAuthoritativePage) { + return false; + } if (status.isPending) { return true; } diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index f8c5395b1..097aef80f 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -146,12 +146,6 @@ type TimelineSnapshot = { historyExhausted: boolean; }; -const EMPTY_TIMELINE_SNAPSHOT: TimelineSnapshot = { - channelId: null, - messages: EMPTY_MESSAGES, - historyExhausted: false, -}; - const MessageTimelineBase = React.forwardRef< MessageTimelineHandle, MessageTimelineProps @@ -232,28 +226,16 @@ const MessageTimelineBase = React.forwardRef< [scrollContainerRef, virtualizerScrollParent], ); - // Gate the heavy timeline render (each row runs a synchronous - // react-markdown parse) behind React concurrency. `useDeferredValue` lets the - // commit that rebuilds the message list yield to higher-priority work, so the - // main thread stops freezing and the OS no longer shows the busy cursor when - // entering a channel. We pass `initialValue: []` so even the FIRST render on - // channel entry stays light — the heavy list streams in on a deferred commit - // rather than blocking the initial paint. We deliberately drive BOTH the - // scroll manager and the rendered list off the same deferred value — - // scroll/autoscroll/deep-link logic reads the DOM (`scrollIntoView`, - // ResizeObserver on the content), so it must stay consistent with what's - // actually painted. You can't scroll to a row that hasn't committed yet. - // Channel id travels with the deferred message snapshot. Without that guard, a - // route change can paint the previous channel's deferred rows for a frame even - // though the sidebar/header already moved to the new channel. + // The timeline itself is keyed by channel, so this is the selected channel's + // route-matched cache on mount. Warm channels paint it immediately; cold + // channels still initialize cheaply because their snapshot is empty. Keeping + // rows and history provenance in the same initial value also prevents scroll + // and deep-link logic from observing a different generation than the DOM. const liveSnapshot = React.useMemo( () => ({ channelId: channelId ?? null, messages, historyExhausted }), [channelId, historyExhausted, messages], ); - const deferredSnapshot = React.useDeferredValue( - liveSnapshot, - EMPTY_TIMELINE_SNAPSHOT, - ); + const deferredSnapshot = React.useDeferredValue(liveSnapshot, liveSnapshot); const deferredMessages = deferredSnapshot.messages; const imagePreloadStateRef = React.useRef({ activeImages: new Set(), diff --git a/desktop/src/features/messages/ui/timelineSnapshotProjection.test.mjs b/desktop/src/features/messages/ui/timelineSnapshotProjection.test.mjs index cd43c1de3..021a07d6d 100644 --- a/desktop/src/features/messages/ui/timelineSnapshotProjection.test.mjs +++ b/desktop/src/features/messages/ui/timelineSnapshotProjection.test.mjs @@ -219,7 +219,7 @@ function samePair(a, b) { function makeHarness(records, scroller) { return function Harness({ snapshot }) { - const deferredSnapshot = React.useDeferredValue(snapshot, EMPTY_SNAPSHOT); + const deferredSnapshot = React.useDeferredValue(snapshot, snapshot); const buffered = useBufferedTimelineMessages({ channelId: deferredSnapshot.channelId, isAtBottom: false, // reader is scrolled up — the tear's regime @@ -272,6 +272,27 @@ async function mount(Comp, snapshot) { // ── Tests ──────────────────────────────────────────────────────────────────── +test("warm snapshot is the first committed render, including its provenance", async () => { + const warm = { + channelId: "chan-warm", + messages: rows(["cached-a", "cached-b"]), + historyExhausted: true, + }; + const records = []; + const handle = await mount(makeHarness(records, makeFakeScroller()), warm); + + assert.deepEqual( + { + count: records[0].count, + firstId: records[0].firstId, + exhausted: records[0].exhausted, + }, + { count: 2, firstId: "cached-a", exhausted: true }, + ); + + await handle.unmount(); +}); + test("pass-1 landing: exhaustion proof can never pair with the stale row array", async () => { const CHANNEL = "chan-tear"; // Snapshot A: 100 rows loaded, more history exists (mid-pagination).