Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion desktop/src/features/messages/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import {
projectChannelWindowMessages,
refreshChannelWindowMessages,
shouldRefreshChannelWindowAfterSubscribe,
} from "@/features/messages/lib/projectChannelWindow";
import { reconcileChannelWindowMessages } from "@/features/messages/lib/channelWindowReconciliation";
import {
Expand Down Expand Up @@ -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(
Expand All @@ -394,7 +398,7 @@ export function useChannelSubscription(channel: Channel | null) {
void cleanup();
}
};
}, [channelId, channelType]);
}, [channelId, channelType, queryClient]);
}

export function useSendMessageMutation(
Expand Down
92 changes: 92 additions & 0 deletions desktop/src/features/messages/lib/projectChannelWindow.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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();
});
28 changes: 28 additions & 0 deletions desktop/src/features/messages/lib/projectChannelWindow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
19 changes: 17 additions & 2 deletions desktop/src/features/messages/lib/timelineSnapshot.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
Expand Down
13 changes: 7 additions & 6 deletions desktop/src/features/messages/lib/timelineSnapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}
Expand Down
18 changes: 13 additions & 5 deletions desktop/src/features/messages/ui/MessageTimeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
@@ -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("<!doctype html><html><body></body></html>", {
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);
});
35 changes: 35 additions & 0 deletions desktop/src/features/messages/ui/useCommittedEmptyTimeline.ts
Original file line number Diff line number Diff line change
@@ -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;
}
Loading