diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 86108bf6e50..4590a0653a1 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -106,6 +106,8 @@ export default defineConfig({ "**/cold-switch-longtask.perf.ts", "**/switch-settle-after-paint.spec.ts", "**/timeline-no-shift.spec.ts", + "**/thread-summary-stability.spec.ts", + "**/channel-revisit-no-skeleton.spec.ts", "**/human-edit-agent-content.spec.ts", "**/empty-edit-delete.spec.ts", "**/reaction-order.spec.ts", diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 0d15b10cb01..b569f53bfa5 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -49,6 +49,7 @@ import { resolveTimelineLoadingLatch, selectTimelineLoadingState, } from "@/features/messages/lib/timelineLoadingState"; +import { hasTimelineSettledThisSession } from "@/features/messages/lib/settledTimelineChannels"; import { useFetchOlderMessages } from "@/features/messages/useFetchOlderMessages"; import { useIndependentThreadPanel } from "@/features/messages/useIndependentThreadPanel"; import { useThreadReplies } from "@/features/messages/useThreadReplies"; @@ -596,8 +597,15 @@ export function ChannelScreen({ setThreadScrollTargetId, }); const settledChannelIdRef = React.useRef(null); + // A channel that settled at any point this session keeps its settled + // status across switches: its cache holds an authoritative window (live + // updates merge into it while away), so a stale revisit renders those rows + // stale-while-revalidate instead of flashing the skeleton for the whole + // refetch round-trip. const hasSettledThisChannel = - activeChannelId !== null && settledChannelIdRef.current === activeChannelId; + activeChannelId !== null && + (settledChannelIdRef.current === activeChannelId || + hasTimelineSettledThisSession(activeChannelId)); const timelineLoadingNow = activeChannel !== null && activeChannel.channelType !== "forum" && diff --git a/desktop/src/features/channels/useChannelSwitchTraceMarks.ts b/desktop/src/features/channels/useChannelSwitchTraceMarks.ts index bc49f6d2d6e..a7e8dcb7120 100644 --- a/desktop/src/features/channels/useChannelSwitchTraceMarks.ts +++ b/desktop/src/features/channels/useChannelSwitchTraceMarks.ts @@ -1,5 +1,6 @@ import * as React from "react"; +import { markTimelineSettledThisSession } from "@/features/messages/lib/settledTimelineChannels"; import { abandonChannelSwitchTrace, markChannelSwitchRouteCommit, @@ -48,6 +49,7 @@ export function useChannelSwitchTraceMarks({ return; } if (!isTimelineLoading) { + markTimelineSettledThisSession(activeChannelId); settleChannelSwitchTrace(activeChannelId); } }, [activeChannelId, activeChannelType, isTimelineLoading]); diff --git a/desktop/src/features/communities/useCommunityInit.ts b/desktop/src/features/communities/useCommunityInit.ts index 41555366399..4d691972d4e 100644 --- a/desktop/src/features/communities/useCommunityInit.ts +++ b/desktop/src/features/communities/useCommunityInit.ts @@ -34,6 +34,7 @@ import { resetAgentObserverStore } from "@/features/agents/observerRelayStore"; import { resetAvatarPresentations } from "@/features/profile/avatarPresentationStore"; import { resetAvatarProfileSync } from "@/features/profile/avatarProfileSync"; import { resetSidebarRelayConnectionCardState } from "@/features/sidebar/ui/useSidebarRelayConnectionCard"; +import { resetSettledTimelineChannels } from "@/features/messages/lib/settledTimelineChannels"; import { resetChannelSwitchTrace } from "@/shared/lib/channelSwitchPerf"; import { clearMarkdownNodeCache } from "@/shared/ui/markdown/nodeCache"; import { resetMessageLinkMetadataCache } from "@/shared/ui/markdown/useMessageLinkMetadata"; @@ -86,6 +87,8 @@ async function resetCommunityState({ clearSearchHitEventCache(); clearMarkdownNodeCache(); resetMessageLinkMetadataCache(); + // resetChannelSwitchTrace() runs earlier, before the first await. + resetSettledTimelineChannels(); } type CommunityInitResult = diff --git a/desktop/src/features/messages/lib/settledTimelineChannels.test.mjs b/desktop/src/features/messages/lib/settledTimelineChannels.test.mjs new file mode 100644 index 00000000000..df9f6650df6 --- /dev/null +++ b/desktop/src/features/messages/lib/settledTimelineChannels.test.mjs @@ -0,0 +1,25 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + hasTimelineSettledThisSession, + markTimelineSettledThisSession, + resetSettledTimelineChannels, +} from "./settledTimelineChannels.ts"; + +test("settled channels are remembered for the session", () => { + resetSettledTimelineChannels(); + assert.equal(hasTimelineSettledThisSession("ch-a"), false); + markTimelineSettledThisSession("ch-a"); + assert.equal(hasTimelineSettledThisSession("ch-a"), true); + assert.equal(hasTimelineSettledThisSession("ch-b"), false); +}); + +test("community reset clears every settled channel", () => { + resetSettledTimelineChannels(); + markTimelineSettledThisSession("ch-a"); + markTimelineSettledThisSession("ch-b"); + resetSettledTimelineChannels(); + assert.equal(hasTimelineSettledThisSession("ch-a"), false); + assert.equal(hasTimelineSettledThisSession("ch-b"), false); +}); diff --git a/desktop/src/features/messages/lib/settledTimelineChannels.ts b/desktop/src/features/messages/lib/settledTimelineChannels.ts new file mode 100644 index 00000000000..28396f87521 --- /dev/null +++ b/desktop/src/features/messages/lib/settledTimelineChannels.ts @@ -0,0 +1,28 @@ +/** + * Session registry of channels whose timeline has settled at least once. + * + * The per-mount loading latch (resolveTimelineLoadingLatch) resets on every + * channel switch, so a revisit past the messages query's staleTime used to + * refetch on mount with the "unsettled" skeleton branch active — flashing a + * skeleton over a fully cached timeline for the whole relay round-trip. A + * channel that settled once this session has an authoritative window in the + * query cache (live updates merge into it while away), so revisits render + * stale-while-revalidate instead. + * + * Community-scoped module state: reset via resetCommunityState() + * (useCommunityInit.ts), like every other community-scoped singleton. + */ + +const settledChannelIds = new Set(); + +export function markTimelineSettledThisSession(channelId: string): void { + settledChannelIds.add(channelId); +} + +export function hasTimelineSettledThisSession(channelId: string): boolean { + return settledChannelIds.has(channelId); +} + +export function resetSettledTimelineChannels(): void { + settledChannelIds.clear(); +} diff --git a/desktop/src/features/messages/lib/timelineLoadingState.test.mjs b/desktop/src/features/messages/lib/timelineLoadingState.test.mjs index fe6960f74c3..712ab798c4d 100644 --- a/desktop/src/features/messages/lib/timelineLoadingState.test.mjs +++ b/desktop/src/features/messages/lib/timelineLoadingState.test.mjs @@ -33,14 +33,20 @@ test("stale placeholder while refetching is loading", () => { 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. + // data is defined but empty and a fetch is still in flight. This is a + // cold-load shape: the channel has not settled this session (a channel + // that HAS settled owns an authoritative — possibly empty — window and + // paints it instead; see the settled-empty-revisit test below). assert.equal( - selectTimelineLoadingState({ - ...settled, - isFetching: true, - isPlaceholderData: false, - dataLength: 0, - }), + selectTimelineLoadingState( + { + ...settled, + isFetching: true, + isPlaceholderData: false, + dataLength: 0, + }, + false, + ), true, ); }); @@ -158,3 +164,21 @@ test("latch: no active channel passes loadingNow through untouched", () => { false, ); }); + +test("settled empty channel revisit paints the empty state, not a skeleton", () => { + // The channel settled this session with zero rows: its cache holds an + // authoritative empty window, so a stale-revisit refetch must render the + // empty state stale-while-revalidate — same contract as populated rows. + assert.equal( + selectTimelineLoadingState( + { + isPending: false, + isFetching: true, + isPlaceholderData: false, + dataLength: 0, + }, + true, + ), + false, + ); +}); diff --git a/desktop/src/features/messages/lib/timelineLoadingState.ts b/desktop/src/features/messages/lib/timelineLoadingState.ts index ea46168d254..2ef3cbc96e8 100644 --- a/desktop/src/features/messages/lib/timelineLoadingState.ts +++ b/desktop/src/features/messages/lib/timelineLoadingState.ts @@ -34,10 +34,11 @@ export function selectTimelineLoadingState( // painting those as if loaded flashes a near-empty timeline. return status.isFetching; } - return ( - status.isFetching && - (status.isPlaceholderData || (status.dataLength ?? 0) === 0) - ); + // Settled this session: the cache holds an authoritative window — possibly + // authoritatively EMPTY — so refetches paint cached content (or the empty + // state) stale-while-revalidate. Only placeholder data still skeletons: it + // is not this channel's own settled window. + return status.isFetching && status.isPlaceholderData; } /** diff --git a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs index a0374fbe2b0..b744d46fb38 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs +++ b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs @@ -489,7 +489,11 @@ test("deferred-snapshot: fresh when channel ids match", () => { ); }); -test("timeline-body-surface: stale deferred channel snapshot paints skeleton instead of old list", () => { +test("timeline-body-surface: stale deferred channel snapshot paints blank instead of old list", () => { + // Production wiring (MessageTimeline) routes deferred-snapshot divergence + // through isSwitchGap, not isLoading: the gap is a render-pipeline + // artifact, so it paints the blank surface — never the previous channel's + // rows, and not a false skeleton. const isStale = isDeferredTimelineSnapshotStale({ deferredSnapshot: { channelId: "chan-a" }, liveSnapshot: { channelId: "chan-b" }, @@ -498,10 +502,11 @@ test("timeline-body-surface: stale deferred channel snapshot paints skeleton ins assert.equal( selectTimelineBodySurface({ deferredCount: 4, - isLoading: isStale, + isLoading: false, + isSwitchGap: isStale, liveCount: 0, }), - "skeleton", + "blank", ); }); @@ -610,3 +615,29 @@ test("isRenderedTimelineBehindHistoryPrepend: false when rendered oldest already // not behind an older-history prepend. assert.equal(isRenderedTimelineBehindHistoryPrepend([a], [a, b]), false); }); + +test("timeline-body-surface: the channel-switch gap is blank, never a skeleton flash", () => { + // Deferred snapshot still holds the previous channel while the live one + // moved on. Without an authoritative load in flight this is a 1-2 frame + // render-pipeline gap — paint background, not flashing skeleton bars. + assert.equal( + selectTimelineBodySurface({ + deferredCount: 5, + isLoading: false, + isSwitchGap: true, + liveCount: 3, + }), + "blank", + ); + // A genuine authoritative load during the gap keeps the skeleton: the cold + // switch is a real loading state, not a pipeline artifact. + assert.equal( + selectTimelineBodySurface({ + deferredCount: 5, + isLoading: true, + isSwitchGap: true, + liveCount: 0, + }), + "skeleton", + ); +}); diff --git a/desktop/src/features/messages/lib/timelineSnapshot.ts b/desktop/src/features/messages/lib/timelineSnapshot.ts index 3bfd9349476..4edbec3981e 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.ts +++ b/desktop/src/features/messages/lib/timelineSnapshot.ts @@ -181,22 +181,34 @@ export function selectDeferredListRenderState( return "pending"; } -export type TimelineBodySurface = "skeleton" | "empty" | "list"; +export type TimelineBodySurface = "skeleton" | "blank" | "empty" | "list"; export function selectTimelineBodySurface({ deferredCount, preserveSettledEmptyIntro = false, isLoading, + isSwitchGap = false, liveCount, }: { deferredCount: number; preserveSettledEmptyIntro?: boolean; isLoading: boolean; + /** Deferred snapshot still holds another channel (channel-switch gap). */ + isSwitchGap?: boolean; liveCount: number; }): TimelineBodySurface { if (isLoading) { return "skeleton"; } + if (isSwitchGap) { + // The deferred snapshot lags the live one for a frame or two on every + // channel switch. Without an authoritative load in flight that gap is a + // render-pipeline artifact, not a loading state: paint plain background + // instead of flashing skeleton bars on every switch. (Painting the + // previous channel's lagging rows here is also wrong — the header and + // sidebar have already moved on.) + return "blank"; + } const renderState = selectDeferredListRenderState(deferredCount, liveCount); if (renderState === "pending") { diff --git a/desktop/src/features/messages/ui/MessageThreadSummaryRow.tsx b/desktop/src/features/messages/ui/MessageThreadSummaryRow.tsx index 083faf04db1..ae1ca0abffd 100644 --- a/desktop/src/features/messages/ui/MessageThreadSummaryRow.tsx +++ b/desktop/src/features/messages/ui/MessageThreadSummaryRow.tsx @@ -48,6 +48,10 @@ function ParticipantAvatar({ avatarUrl={participant.avatarUrl} className="h-6 w-6 text-2xs" displayName={participant.author} + // Facepile avatars start with no image URL until profiles hydrate; + // the default fallback delay would leave a blank hole for 200ms and + // then pop the initials in. Render the fallback immediately. + fallbackDelayMs={0} size="sm" /> @@ -210,7 +214,13 @@ export function MessageThreadSummaryRow({