From d3ec831e0cecbff347d55a236e34b27d79961503 Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 11 Aug 2026 10:37:12 -0600 Subject: [PATCH] fix(desktop): preserve fresh channel timelines (#5577) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - skip the channel subscription catch-up request when the authoritative channel window was fetched successfully within the existing five-minute freshness period - keep the responsive deferred skeleton for populated channel switches instead of briefly rendering empty-channel actions - preserve a real empty-channel intro across the first appended message only after React has committed that empty state This is intentionally narrow. It does not claim to solve the separate sidebar startup cost or general main-thread stalls found during the investigation. ### Related issue N/A — no matching open issue or PR found. ### Testing - pre-push desktop gate on `f1be6beea90b9715e04e5fc65cc5cfbe8210e0d9`: - desktop tests: 4,621 passed - desktop check: passed - desktop typecheck: passed - branch-skew: passed - focused cache/surface/lifecycle tests: 62 passed - manual diagnostic trace after rollback: - 16/16 channel revisits skipped catch-up refresh - 0 revisit refresh starts - 0 populated-channel empty/intro flashes - cached switches retained the deferred skeleton-to-list path No screenshot: the regression is a transient channel-switch state and request behavior, covered by lifecycle tests and the diagnostic trace rather than a stable visual diff. Signed-off-by: Wes Co-authored-by: Carl --- desktop/src/features/messages/hooks.ts | 6 +- .../lib/projectChannelWindow.test.mjs | 92 +++++++++++++++++++ .../messages/lib/projectChannelWindow.ts | 28 ++++++ .../messages/lib/timelineSnapshot.test.mjs | 19 +++- .../features/messages/lib/timelineSnapshot.ts | 13 +-- .../features/messages/ui/MessageTimeline.tsx | 18 +++- .../ui/useCommittedEmptyTimeline.test.mjs | 71 ++++++++++++++ .../messages/ui/useCommittedEmptyTimeline.ts | 35 +++++++ 8 files changed, 268 insertions(+), 14 deletions(-) create mode 100644 desktop/src/features/messages/ui/useCommittedEmptyTimeline.test.mjs create mode 100644 desktop/src/features/messages/ui/useCommittedEmptyTimeline.ts diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index 545500bfe..c3d09f3ad 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -17,6 +17,7 @@ import { import { projectChannelWindowMessages, refreshChannelWindowMessages, + shouldRefreshChannelWindowAfterSubscribe, } from "@/features/messages/lib/projectChannelWindow"; import { reconcileChannelWindowMessages } from "@/features/messages/lib/channelWindowReconciliation"; import { @@ -373,6 +374,9 @@ export function useChannelSubscription(channel: Channel | null) { } cleanup = dispose; + if (!shouldRefreshChannelWindowAfterSubscribe(queryClient, channelId)) { + return; + } void refreshNewestWindow().catch((error) => { if (!isDisposed) { console.error( @@ -394,7 +398,7 @@ export function useChannelSubscription(channel: Channel | null) { void cleanup(); } }; - }, [channelId, channelType]); + }, [channelId, channelType, queryClient]); } export function useSendMessageMutation( diff --git a/desktop/src/features/messages/lib/projectChannelWindow.test.mjs b/desktop/src/features/messages/lib/projectChannelWindow.test.mjs index 5c7633097..2ca235427 100644 --- a/desktop/src/features/messages/lib/projectChannelWindow.test.mjs +++ b/desktop/src/features/messages/lib/projectChannelWindow.test.mjs @@ -11,8 +11,10 @@ import { replaceNewestChannelWindow, } from "./channelWindowStore.ts"; import { + CHANNEL_WINDOW_FRESH_MS, projectChannelWindowMessages, refreshChannelWindowMessages, + shouldRefreshChannelWindowAfterSubscribe, } from "./projectChannelWindow.ts"; import { reconcileChannelWindowMessages } from "./channelWindowReconciliation.ts"; @@ -273,3 +275,93 @@ test("test_live_projection_retains_pending_send_and_non_broadcast_thread_reply", "live", ]); }); + +test("test_subscribe_refresh_skips_fresh_populated_window", () => { + const harness = createHarness(); + const updatedAt = harness.client.getQueryState( + harness.windowKey, + ).dataUpdatedAt; + + assert.equal( + shouldRefreshChannelWindowAfterSubscribe( + harness.client, + harness.channelId, + updatedAt + CHANNEL_WINDOW_FRESH_MS - 1, + ), + false, + ); +}); + +test("test_subscribe_refresh_runs_for_stale_window", () => { + const harness = createHarness(); + const updatedAt = harness.client.getQueryState( + harness.windowKey, + ).dataUpdatedAt; + + assert.equal( + shouldRefreshChannelWindowAfterSubscribe( + harness.client, + harness.channelId, + updatedAt + CHANNEL_WINDOW_FRESH_MS, + ), + true, + ); +}); + +test("test_live_cache_merge_does_not_extend_window_freshness", () => { + const harness = createHarness(); + const windowUpdatedAt = harness.client.getQueryState( + harness.windowKey, + ).dataUpdatedAt; + + harness.client.setQueryData(harness.messagesKey, (messages) => [ + ...messages, + event("live-cache-only", 110), + ]); + + assert.equal( + shouldRefreshChannelWindowAfterSubscribe( + harness.client, + harness.channelId, + windowUpdatedAt + CHANNEL_WINDOW_FRESH_MS, + ), + true, + ); +}); + +test("test_subscribe_refresh_runs_without_a_message_query", () => { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + assert.equal( + shouldRefreshChannelWindowAfterSubscribe(client, "missing-channel"), + true, + ); +}); + +test("test_subscribe_refresh_does_not_duplicate_inflight_initial_fetch", async () => { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + const channelId = "pending-channel"; + const queryKey = channelMessagesKey(channelId); + let resolveFetch; + const observer = new QueryObserver(client, { + queryKey, + queryFn: () => + new Promise((resolve) => { + resolveFetch = resolve; + }), + }); + const unsubscribe = observer.subscribe(() => {}); + + assert.equal( + shouldRefreshChannelWindowAfterSubscribe(client, channelId), + false, + ); + + resolveFetch([]); + await client.getQueryCache().find({ queryKey })?.promise; + unsubscribe(); +}); diff --git a/desktop/src/features/messages/lib/projectChannelWindow.ts b/desktop/src/features/messages/lib/projectChannelWindow.ts index 81ef3de42..2d56c096b 100644 --- a/desktop/src/features/messages/lib/projectChannelWindow.ts +++ b/desktop/src/features/messages/lib/projectChannelWindow.ts @@ -8,6 +8,34 @@ import { } from "./channelWindowStore"; import { reconcileChannelWindowMessages } from "./channelWindowReconciliation"; +export const CHANNEL_WINDOW_FRESH_MS = 5 * 60_000; + +/** + * Subscription setup closes the gap between the initial page and live events, + * but revisiting a channel with a fresh page has no gap to close. Reconnects + * still refresh unconditionally at their call site. + */ +export function shouldRefreshChannelWindowAfterSubscribe( + queryClient: QueryClient, + channelId: string, + now = Date.now(), +): boolean { + const messagesState = queryClient.getQueryState( + channelMessagesKey(channelId), + ); + if (!messagesState) return true; + if (messagesState.fetchStatus === "fetching") return false; + const windowState = queryClient.getQueryState(channelWindowKey(channelId)); + if ( + messagesState.status !== "success" || + windowState?.status !== "success" || + windowState.dataUpdatedAt === 0 + ) { + return true; + } + return now - windowState.dataUpdatedAt >= CHANNEL_WINDOW_FRESH_MS; +} + /** Keep the rendered timeline cache aligned with its authoritative window. */ export function projectChannelWindowMessages( queryClient: QueryClient, diff --git a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs index de410b5b8..a0374fbe2 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs +++ b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs @@ -420,11 +420,26 @@ test("timeline-body-surface: loading and deferred-pending both paint the single ); }); -test("timeline-body-surface: first deferred message preserves a persistent channel intro", () => { +test("timeline-body-surface: first authoritative rows wait for deferred paint", () => { + // A newly selected populated channel has already resolved live rows, but the + // deferred snapshot is still empty. It has never committed a settled empty + // surface, so showing its intro here would flash Create agent / Add people. assert.equal( selectTimelineBodySurface({ deferredCount: 0, - hasPersistentIntro: true, + preserveSettledEmptyIntro: false, + isLoading: false, + liveCount: 1, + }), + "skeleton", + ); +}); + +test("timeline-body-surface: append preserves a previously settled empty intro", () => { + assert.equal( + selectTimelineBodySurface({ + deferredCount: 0, + preserveSettledEmptyIntro: true, isLoading: false, liveCount: 1, }), diff --git a/desktop/src/features/messages/lib/timelineSnapshot.ts b/desktop/src/features/messages/lib/timelineSnapshot.ts index 14d415fb6..3bfd93494 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.ts +++ b/desktop/src/features/messages/lib/timelineSnapshot.ts @@ -185,12 +185,12 @@ export type TimelineBodySurface = "skeleton" | "empty" | "list"; export function selectTimelineBodySurface({ deferredCount, - hasPersistentIntro = false, + preserveSettledEmptyIntro = false, isLoading, liveCount, }: { deferredCount: number; - hasPersistentIntro?: boolean; + preserveSettledEmptyIntro?: boolean; isLoading: boolean; liveCount: number; }): TimelineBodySurface { @@ -200,10 +200,11 @@ export function selectTimelineBodySurface({ const renderState = selectDeferredListRenderState(deferredCount, liveCount); if (renderState === "pending") { - // A channel/DM intro is already meaningful stable content. Preserve it - // while React's deferred snapshot catches up to the first live message; - // replacing it with a skeleton makes an append look like a page reload. - return hasPersistentIntro ? "empty" : "skeleton"; + // Preserve a channel/DM intro across a new append only when this channel + // already committed an authoritative empty timeline. On first load, the + // live query can resolve before React's deferred rows commit; painting the + // intro in that gap flashes empty-channel actions over incoming messages. + return preserveSettledEmptyIntro ? "empty" : "skeleton"; } return renderState; } diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 6d9c49b75..f8c5395b1 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -16,6 +16,7 @@ import { cn } from "@/shared/lib/cn"; import { channelChrome } from "@/shared/layout/chromeLayout"; import { Spinner } from "@/shared/ui/spinner"; import { TooltipProvider } from "@/shared/ui/tooltip"; +import { useCommittedEmptyTimeline } from "./useCommittedEmptyTimeline"; import { UnreadPill, unreadCountLabel } from "@/shared/ui/UnreadPill"; import { ChannelIntroBlock, type ChannelIntro } from "./ChannelIntroBlock"; import { TimelineSkeleton, useTimelineSkeletonRows } from "./TimelineSkeleton"; @@ -285,13 +286,20 @@ const MessageTimelineBase = React.forwardRef< setTimelineVirtualizerApi(null); }, [scrollContainerRef, scrollContainerDomKey]); + const hasPersistentIntro = + channelIntro !== null || directMessageIntro !== null || pinnedIntro != null; + const timelineIsLoading = isLoading || isDeferredSnapshotStale; + const preserveSettledEmptyIntro = useCommittedEmptyTimeline({ + channelId: channelId ?? null, + deferredCount: deferredMessages.length, + hasPersistentIntro, + isLoading: timelineIsLoading, + liveCount: messages.length, + }); const timelineBodySurface = selectTimelineBodySurface({ deferredCount: deferredMessages.length, - hasPersistentIntro: - channelIntro !== null || - directMessageIntro !== null || - pinnedIntro != null, - isLoading: isLoading || isDeferredSnapshotStale, + preserveSettledEmptyIntro, + isLoading: timelineIsLoading, liveCount: messages.length, }); const showTimelineSkeleton = timelineBodySurface === "skeleton"; diff --git a/desktop/src/features/messages/ui/useCommittedEmptyTimeline.test.mjs b/desktop/src/features/messages/ui/useCommittedEmptyTimeline.test.mjs new file mode 100644 index 000000000..217f6fb5c --- /dev/null +++ b/desktop/src/features/messages/ui/useCommittedEmptyTimeline.test.mjs @@ -0,0 +1,71 @@ +import assert from "node:assert/strict"; +import { after, afterEach, before, test } from "node:test"; + +import { JSDOM } from "jsdom"; + +const dom = new JSDOM("", { + url: "http://localhost", +}); + +before(() => { + Object.assign(globalThis, { + document: dom.window.document, + HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, + window: dom.window, + }); +}); + +afterEach(async () => { + const { cleanup } = await import("@testing-library/react"); + cleanup(); +}); + +after(() => dom.window.close()); + +async function renderTimelineState(initialProps) { + const { renderHook } = await import("@testing-library/react"); + const { useCommittedEmptyTimeline } = await import( + "./useCommittedEmptyTimeline.ts" + ); + return renderHook((props) => useCommittedEmptyTimeline(props), { + initialProps, + }); +} + +const empty = { + channelId: "channel-a", + deferredCount: 0, + hasPersistentIntro: true, + isLoading: false, + liveCount: 0, +}; + +test("only preserves an intro after an empty timeline commits", async () => { + const { result, rerender } = await renderTimelineState(empty); + + assert.equal(result.current, false); + rerender({ ...empty, liveCount: 1 }); + assert.equal(result.current, true); + rerender({ ...empty, deferredCount: 1, liveCount: 1 }); + assert.equal(result.current, false); + rerender({ ...empty, liveCount: 1 }); + assert.equal(result.current, false); +}); + +test("a committed empty proof never carries across channels", async () => { + const { result, rerender } = await renderTimelineState(empty); + + rerender({ ...empty, channelId: "channel-b", liveCount: 1 }); + assert.equal(result.current, false); +}); + +test("loading and deferred-stale commits cannot establish empty proof", async () => { + const { result, rerender } = await renderTimelineState({ + ...empty, + isLoading: true, + }); + + rerender({ ...empty, liveCount: 1 }); + assert.equal(result.current, false); +}); diff --git a/desktop/src/features/messages/ui/useCommittedEmptyTimeline.ts b/desktop/src/features/messages/ui/useCommittedEmptyTimeline.ts new file mode 100644 index 000000000..cdceedcd2 --- /dev/null +++ b/desktop/src/features/messages/ui/useCommittedEmptyTimeline.ts @@ -0,0 +1,35 @@ +import * as React from "react"; + +/** Track only empty timelines that React actually committed for this channel. */ +export function useCommittedEmptyTimeline({ + channelId, + deferredCount, + hasPersistentIntro, + isLoading, + liveCount, +}: { + channelId: string | null; + deferredCount: number; + hasPersistentIntro: boolean; + isLoading: boolean; + liveCount: number; +}) { + const committedRef = React.useRef({ + channelId: null as string | null, + hasSettledEmpty: false, + }); + const preserveSettledEmptyIntro = + hasPersistentIntro && + committedRef.current.channelId === channelId && + committedRef.current.hasSettledEmpty; + + React.useLayoutEffect(() => { + if (isLoading) return; + committedRef.current = { + channelId, + hasSettledEmpty: liveCount === 0 && deferredCount === 0, + }; + }, [channelId, deferredCount, isLoading, liveCount]); + + return preserveSettledEmptyIntro; +}