From 9ddf861885e3c72820d19ddc928649f2af080f84 Mon Sep 17 00:00:00 2001 From: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Date: Fri, 12 Jun 2026 15:50:44 -0700 Subject: [PATCH 1/5] perf(timeline): gate heavy message render behind useDeferredValue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase A: kill the blocking native-cursor spinner on channel entry, long threads, and the inbox. Up to 200 messages render synchronously, each running a heavy react-markdown parse, committing in one blocking pass — freezing the main thread and showing the OS busy cursor. Wrap the message list in useDeferredValue(messages, EMPTY_MESSAGES) so the heavy commit becomes interruptible and streams in on a deferred pass instead of blocking the initial paint. A module-level empty initial value keeps even the first render on channel entry light. Drive ALL consumers off the single deferred snapshot — scroll manager, showMessageList/showGenericEmpty flags, and the TimelineMessageList prop — so scroll math stays consistent with the painted DOM. This closes a tearing race where the deep-link effect (querySelector -> scrollIntoView) could fire against a snapshot whose target row was not yet committed, silently failing the jump. Must-keep behaviors verified consistent against the deferred snapshot: sticky-bottom autoscroll, day dividers, jump-to-message deep links. Wire the otherwise-unused pending flag to a subtle opacity dim while a deferred render is in flight, so it reads as streaming-in rather than frozen. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- .../features/messages/ui/MessageTimeline.tsx | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 1fa347a20..e47e8f2e3 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -88,6 +88,11 @@ type ChannelIntro = { icon?: React.ReactNode; }; +/** Stable empty reference used as the `useDeferredValue` initial value so the + * first render on channel entry stays light instead of blocking on the full + * message list. Must be module-level so its identity never changes. */ +const EMPTY_MESSAGES: TimelineMessage[] = []; + export const MessageTimeline = React.memo(function MessageTimeline({ agentPubkeys, channelId, @@ -127,6 +132,20 @@ export const MessageTimeline = React.memo(function MessageTimeline({ const internalScrollRef = React.useRef(null); const scrollContainerRef = externalScrollRef ?? internalScrollRef; const topSentinelRef = React.useRef(null); + + // Phase A perf: 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. + const deferredMessages = React.useDeferredValue(messages, EMPTY_MESSAGES); + const isRenderPending = deferredMessages !== messages; const scrollRestorationId = targetMessageId ? `message-timeline:${channelId ?? "none"}:target:${targetMessageId}` : `message-timeline:${channelId ?? "none"}`; @@ -143,7 +162,7 @@ export const MessageTimeline = React.memo(function MessageTimeline({ } = useTimelineScrollManager({ channelId, isLoading, - messages, + messages: deferredMessages, onTargetReached, scrollContainerRef, targetMessageId, @@ -188,10 +207,10 @@ export const MessageTimeline = React.memo(function MessageTimeline({ const showIntro = showDirectMessageIntro || showChannelIntro; const showGenericEmpty = !isLoading && - messages.length === 0 && + deferredMessages.length === 0 && directMessageIntro === null && channelIntro === null; - const showMessageList = !isLoading && messages.length > 0; + const showMessageList = !isLoading && deferredMessages.length > 0; return ( @@ -358,7 +377,15 @@ export const MessageTimeline = React.memo(function MessageTimeline({ {showMessageList ? (
Date: Sat, 13 Jun 2026 20:05:24 -0700 Subject: [PATCH 2/5] test(timeline): lib-cover Phase A must-keep decisions + no-tearing guarantee MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase A gated the heavy MessageTimeline render behind useDeferredValue but shipped with no automated coverage on the parts that matter. Lift the three must-keep decisions out of the component/scroll-manager into pure helpers in lib/ and cover them in the existing *.test.mjs suite (no new tooling): - sticky-bottom autoscroll: isNearBottomMetrics (pure threshold math) + selectLatestMessageKey (new-latest-message detection) - day dividers: buildDayGroupBoundaries (calendar-day grouping) - jump-to-message deep links: resolveDeepLinkTarget (target-in-snapshot) The component keeps its React wiring (useDeferredValue, effects, refs) and delegates the decisions to these helpers. isNearBottom, the scroll manager's latest-message-key, the divider grouping loop, and the deep-link effect guard now route through the tested helpers — behavior identical. Cover the shared-snapshot / no-tearing guarantee Phase A closed: a target only present in a fresh snapshot does NOT resolve against a stale one, and all three decisions stay internally consistent when fed one shared snapshot. just desktop-test (715 pass), tsc --noEmit, and biome all green. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- .../messages/lib/timelineDecisions.test.mjs | 252 ++++++++++++++++++ .../messages/lib/timelineDecisions.ts | 129 +++++++++ .../messages/ui/TimelineMessageList.tsx | 19 +- .../messages/ui/messageTimelineUtils.ts | 13 +- .../messages/ui/useTimelineScrollManager.ts | 17 +- 5 files changed, 416 insertions(+), 14 deletions(-) create mode 100644 desktop/src/features/messages/lib/timelineDecisions.test.mjs create mode 100644 desktop/src/features/messages/lib/timelineDecisions.ts diff --git a/desktop/src/features/messages/lib/timelineDecisions.test.mjs b/desktop/src/features/messages/lib/timelineDecisions.test.mjs new file mode 100644 index 000000000..d4bf694ab --- /dev/null +++ b/desktop/src/features/messages/lib/timelineDecisions.test.mjs @@ -0,0 +1,252 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + BOTTOM_THRESHOLD_PX, + buildDayGroupBoundaries, + isNearBottomMetrics, + resolveDeepLinkTarget, + selectLatestMessageKey, +} from "./timelineDecisions.ts"; + +// Local-midnight unix-second timestamps so isSameDay (local time) is stable +// regardless of the machine's timezone. +function dayAt(year, month, day, hour = 12) { + return Math.floor( + new Date(year, month - 1, day, hour, 0, 0).getTime() / 1_000, + ); +} + +function message(overrides) { + return { + id: "message", + renderKey: undefined, + createdAt: dayAt(2026, 6, 14), + pubkey: "author", + author: "Author", + avatarUrl: null, + role: undefined, + personaDisplayName: undefined, + time: "12:00 PM", + body: "body", + parentId: null, + rootId: null, + depth: 0, + accent: false, + pending: undefined, + edited: false, + kind: 9, + tags: [], + reactions: undefined, + ...overrides, + }; +} + +// --- sticky-bottom autoscroll ------------------------------------------------- + +test("isNearBottomMetrics: true when within threshold of the bottom", () => { + // distance = scrollHeight - clientHeight - scrollTop = 1000 - 600 - 380 = 20 + assert.equal( + isNearBottomMetrics({ + scrollHeight: 1_000, + clientHeight: 600, + scrollTop: 380, + }), + true, + ); +}); + +test("isNearBottomMetrics: true exactly at the threshold boundary", () => { + const scrollTop = 1_000 - 600 - BOTTOM_THRESHOLD_PX; // distance === threshold + assert.equal( + isNearBottomMetrics({ scrollHeight: 1_000, clientHeight: 600, scrollTop }), + true, + ); +}); + +test("isNearBottomMetrics: false when scrolled up beyond the threshold", () => { + // distance = 1000 - 600 - 100 = 300 > 72 + assert.equal( + isNearBottomMetrics({ + scrollHeight: 1_000, + clientHeight: 600, + scrollTop: 100, + }), + false, + ); +}); + +test("selectLatestMessageKey: prefers renderKey, falls back to id, undefined when empty", () => { + assert.equal(selectLatestMessageKey([]), undefined); + assert.equal( + selectLatestMessageKey([message({ id: "a" }), message({ id: "b" })]), + "b", + ); + assert.equal( + selectLatestMessageKey([message({ id: "b", renderKey: "local-b" })]), + "local-b", + ); +}); + +test("selectLatestMessageKey: detects a newly arrived latest message", () => { + const before = [message({ id: "a" }), message({ id: "b" })]; + const after = [ + ...before, + message({ id: "c", createdAt: dayAt(2026, 6, 14, 13) }), + ]; + assert.notEqual( + selectLatestMessageKey(before), + selectLatestMessageKey(after), + ); +}); + +// --- day dividers ------------------------------------------------------------- + +test("buildDayGroupBoundaries: empty snapshot has no groups", () => { + assert.deepEqual(buildDayGroupBoundaries([]), []); +}); + +test("buildDayGroupBoundaries: messages on one day form a single group", () => { + const messages = [ + message({ id: "a", createdAt: dayAt(2026, 6, 14, 9) }), + message({ id: "b", createdAt: dayAt(2026, 6, 14, 10) }), + message({ id: "c", createdAt: dayAt(2026, 6, 14, 23) }), + ]; + const groups = buildDayGroupBoundaries(messages); + assert.equal(groups.length, 1); + assert.deepEqual( + { startIndex: groups[0].startIndex, count: groups[0].count }, + { startIndex: 0, count: 3 }, + ); +}); + +test("buildDayGroupBoundaries: a day boundary opens a new group", () => { + const messages = [ + message({ id: "a", createdAt: dayAt(2026, 6, 13, 22) }), + message({ id: "b", createdAt: dayAt(2026, 6, 14, 1) }), + message({ id: "c", createdAt: dayAt(2026, 6, 14, 2) }), + message({ id: "d", createdAt: dayAt(2026, 6, 15, 8) }), + ]; + const groups = buildDayGroupBoundaries(messages); + assert.deepEqual( + groups.map((g) => ({ startIndex: g.startIndex, count: g.count })), + [ + { startIndex: 0, count: 1 }, + { startIndex: 1, count: 2 }, + { startIndex: 3, count: 1 }, + ], + ); +}); + +test("buildDayGroupBoundaries: group counts always sum to message count", () => { + const messages = [ + message({ id: "a", createdAt: dayAt(2026, 6, 13) }), + message({ id: "b", createdAt: dayAt(2026, 6, 14) }), + message({ id: "c", createdAt: dayAt(2026, 6, 14) }), + ]; + const total = buildDayGroupBoundaries(messages).reduce( + (n, g) => n + g.count, + 0, + ); + assert.equal(total, messages.length); +}); + +// --- jump-to-message deep links ---------------------------------------------- + +test("resolveDeepLinkTarget: unresolved with no target", () => { + const messages = [message({ id: "a" })]; + assert.deepEqual(resolveDeepLinkTarget(messages, null), { + resolved: false, + index: -1, + }); + assert.deepEqual(resolveDeepLinkTarget(messages, undefined), { + resolved: false, + index: -1, + }); +}); + +test("resolveDeepLinkTarget: resolves a present target to its index", () => { + const messages = [ + message({ id: "a" }), + message({ id: "b" }), + message({ id: "c" }), + ]; + assert.deepEqual(resolveDeepLinkTarget(messages, "b"), { + resolved: true, + index: 1, + }); +}); + +test("resolveDeepLinkTarget: unresolved when the target is not in the snapshot", () => { + const messages = [message({ id: "a" }), message({ id: "b" })]; + assert.deepEqual(resolveDeepLinkTarget(messages, "missing"), { + resolved: false, + index: -1, + }); +}); + +// --- shared-snapshot / no-tearing guarantee ---------------------------------- +// +// This is the race Phase A closed: all three must-keep decisions must read off +// the SAME deferred snapshot. If the deep-link decision reads a fresh snapshot +// while the rendered list / scroll math still read a stale one, the jump fires +// against a row that hasn't committed and silently fails. + +test("no-tearing: a target only in the fresh snapshot does NOT resolve against the stale one", () => { + const stale = [message({ id: "a" }), message({ id: "b" })]; + const fresh = [ + ...stale, + message({ id: "target", createdAt: dayAt(2026, 6, 15) }), + ]; + + // Reading the deep link against the stale snapshot (what the painted DOM + // still shows) must report unresolved — you can't scroll to an uncommitted row. + assert.equal(resolveDeepLinkTarget(stale, "target").resolved, false); + // Against the fresh snapshot it resolves — proving the gate is which snapshot + // you feed, not the function. + assert.equal(resolveDeepLinkTarget(fresh, "target").resolved, true); +}); + +test("no-tearing: all three decisions agree when fed one shared snapshot", () => { + const snapshot = [ + message({ id: "a", createdAt: dayAt(2026, 6, 13) }), + message({ id: "b", createdAt: dayAt(2026, 6, 14) }), + message({ + id: "target", + renderKey: "rk-target", + createdAt: dayAt(2026, 6, 14, 18), + }), + ]; + + // Deep link resolves to the last index... + const link = resolveDeepLinkTarget(snapshot, "target"); + // ...the day grouping covers that same index... + const groups = buildDayGroupBoundaries(snapshot); + const coveredCount = groups.reduce((n, g) => n + g.count, 0); + // ...and the sticky-bottom latest-key points at that same final message. + const latestKey = selectLatestMessageKey(snapshot); + + assert.equal(link.index, snapshot.length - 1); + assert.equal(coveredCount, snapshot.length); + assert.equal(latestKey, snapshot[snapshot.length - 1].renderKey); +}); + +test("no-tearing: stale snapshot keeps all three decisions internally consistent", () => { + // Feeding the stale list everywhere (what Phase A guarantees) keeps the + // decisions consistent with each other — none of them see the uncommitted row. + const stale = [ + message({ id: "a", createdAt: dayAt(2026, 6, 14, 9) }), + message({ id: "b", createdAt: dayAt(2026, 6, 14, 10) }), + ]; + + const link = resolveDeepLinkTarget(stale, "target"); + const coveredCount = buildDayGroupBoundaries(stale).reduce( + (n, g) => n + g.count, + 0, + ); + const latestKey = selectLatestMessageKey(stale); + + assert.equal(link.resolved, false); + assert.equal(coveredCount, stale.length); + assert.equal(latestKey, "b"); +}); diff --git a/desktop/src/features/messages/lib/timelineDecisions.ts b/desktop/src/features/messages/lib/timelineDecisions.ts new file mode 100644 index 000000000..80e197f2e --- /dev/null +++ b/desktop/src/features/messages/lib/timelineDecisions.ts @@ -0,0 +1,129 @@ +/** + * Pure decision helpers for the Phase A timeline concurrency work. + * + * Phase A gated the heavy `MessageTimeline` render behind React's + * `useDeferredValue` so the main thread stops freezing. The *risk* in that + * change is not React itself — it's the decision logic that reads the deferred + * snapshot and the three must-keep behaviors that hang off it: + * + * 1. sticky-bottom autoscroll + * 2. day dividers + * 3. jump-to-message deep links + * + * …plus the shared-snapshot / no-tearing guarantee: all three must read off the + * SAME snapshot, never a mix of stale and fresh lists. If they tear apart, a + * deep-link jump can fire against a row that hasn't committed and silently fail. + * + * These functions lift those decisions out of the component's render body / the + * scroll-manager effects so they can be covered by the lib-level `*.test.mjs` + * suite. The component keeps its React wiring (the `useDeferredValue` call, the + * effects, the DOM refs) and delegates the actual decisions here. + */ + +import type { TimelineMessage } from "@/features/messages/types"; +import { isSameDay } from "./dateFormatters"; + +/** Distance (px) from the bottom within which the timeline counts as "at bottom". */ +export const BOTTOM_THRESHOLD_PX = 72; + +/** Minimal scroll geometry the sticky-bottom decision needs — a pure subset of the DOM element. */ +export type ScrollMetrics = { + scrollHeight: number; + clientHeight: number; + scrollTop: number; +}; + +/** + * Sticky-bottom decision: is the timeline scrolled close enough to the bottom + * to count as "at bottom"? Pure version of the old `isNearBottom(el)` so the + * threshold math is testable without a DOM. + */ +export function isNearBottomMetrics(metrics: ScrollMetrics): boolean { + return ( + metrics.scrollHeight - metrics.clientHeight - metrics.scrollTop <= + BOTTOM_THRESHOLD_PX + ); +} + +/** + * Identity of the last message in a snapshot, used to detect "a new latest + * message arrived" for autoscroll. Prefers `renderKey` (stable across optimistic + * send-ack) and falls back to `id`. Returns `undefined` for an empty snapshot. + */ +export function selectLatestMessageKey( + messages: readonly TimelineMessage[], +): string | undefined { + if (messages.length === 0) { + return undefined; + } + const latest = messages[messages.length - 1]; + return latest.renderKey ?? latest.id; +} + +/** A single day boundary in the timeline: where it starts and how many messages it covers. */ +export type DayGroupBoundary = { + /** Stable key for the day section. */ + key: string; + /** Index into `messages` of the first message in this day. */ + startIndex: number; + /** Number of messages in this day group. */ + count: number; + /** The `createdAt` (unix seconds) used to render the heading label. */ + headingTimestamp: number; +}; + +/** + * Day-divider decision: walk a snapshot in order and produce the day-group + * boundaries. A new group starts at index 0 and whenever a message falls on a + * different calendar day than the one before it — exactly the rule the render + * loop used inline, now pure and testable. + */ +export function buildDayGroupBoundaries( + messages: readonly TimelineMessage[], +): DayGroupBoundary[] { + const boundaries: DayGroupBoundary[] = []; + + for (let i = 0; i < messages.length; i++) { + const message = messages[i]; + const prev = i > 0 ? messages[i - 1] : null; + + if (!prev || !isSameDay(prev.createdAt, message.createdAt)) { + boundaries.push({ + key: `day-${message.createdAt}`, + startIndex: i, + count: 1, + headingTimestamp: message.createdAt, + }); + } else { + boundaries[boundaries.length - 1].count += 1; + } + } + + return boundaries; +} + +/** Outcome of resolving a deep-link target against the current snapshot. */ +export type DeepLinkResolution = { + /** Whether the target message exists in this snapshot (i.e. a row would be committed). */ + resolved: boolean; + /** Index of the target in `messages`, or -1 when unresolved. */ + index: number; +}; + +/** + * Deep-link decision: does a jump-to-message target resolve against THIS + * snapshot? The scroll-manager effect only does `querySelector` + + * `scrollIntoView` once a target row is actually committed — so the jump must + * read the same snapshot the list rendered, or it scrolls to a row that isn't + * there yet. This is the tearing race Phase A closed. + */ +export function resolveDeepLinkTarget( + messages: readonly TimelineMessage[], + targetMessageId: string | null | undefined, +): DeepLinkResolution { + if (!targetMessageId) { + return { resolved: false, index: -1 }; + } + const index = messages.findIndex((message) => message.id === targetMessageId); + return { resolved: index !== -1, index }; +} diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index a649fa357..4f44f2297 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -1,10 +1,8 @@ import * as React from "react"; -import { - formatDayHeading, - isSameDay, -} from "@/features/messages/lib/dateFormatters"; +import { formatDayHeading } from "@/features/messages/lib/dateFormatters"; import { buildMainTimelineEntries } from "@/features/messages/lib/threadPanel"; +import { buildDayGroupBoundaries } from "@/features/messages/lib/timelineDecisions"; import type { TimelineMessage } from "@/features/messages/types"; import type { UserProfileLookup } from "@/features/profile/lib/identity"; import type { ChannelType } from "@/shared/api/types"; @@ -191,12 +189,21 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ }> = []; let currentDayGroup: (typeof dayGroups)[number] | null = null; + // Day-divider decision delegated to a pure, lib-tested helper: a new group + // starts at index 0 and whenever a message falls on a different calendar day + // than the one before it. We index the boundary start positions so the render + // loop below stays a straight walk while the grouping logic lives in `lib/`. + const dayGroupStartIndices = new Set( + buildDayGroupBoundaries(entries.map((entry) => entry.message)).map( + (boundary) => boundary.startIndex, + ), + ); + for (let i = 0; i < entries.length; i++) { const { message, summary } = entries[i]; - const prev = i > 0 ? entries[i - 1]?.message : null; const messageRenderKey = message.renderKey ?? message.id; - if (!prev || !isSameDay(prev.createdAt, message.createdAt)) { + if (dayGroupStartIndices.has(i)) { currentDayGroup = { key: `day-${message.createdAt}`, label: formatDayHeading(message.createdAt), diff --git a/desktop/src/features/messages/ui/messageTimelineUtils.ts b/desktop/src/features/messages/ui/messageTimelineUtils.ts index 34cc79246..821fc14e4 100644 --- a/desktop/src/features/messages/ui/messageTimelineUtils.ts +++ b/desktop/src/features/messages/ui/messageTimelineUtils.ts @@ -1,8 +1,11 @@ -const BOTTOM_THRESHOLD_PX = 72; +import { isNearBottomMetrics } from "@/features/messages/lib/timelineDecisions"; export function isNearBottom(container: HTMLDivElement) { - return ( - container.scrollHeight - container.clientHeight - container.scrollTop <= - BOTTOM_THRESHOLD_PX - ); + // Decision delegated to a pure, lib-tested helper; this wrapper just reads the + // live geometry off the DOM element. + return isNearBottomMetrics({ + scrollHeight: container.scrollHeight, + clientHeight: container.clientHeight, + scrollTop: container.scrollTop, + }); } diff --git a/desktop/src/features/messages/ui/useTimelineScrollManager.ts b/desktop/src/features/messages/ui/useTimelineScrollManager.ts index da2fd7ff6..8668f2e38 100644 --- a/desktop/src/features/messages/ui/useTimelineScrollManager.ts +++ b/desktop/src/features/messages/ui/useTimelineScrollManager.ts @@ -1,5 +1,9 @@ import * as React from "react"; +import { + resolveDeepLinkTarget, + selectLatestMessageKey, +} from "@/features/messages/lib/timelineDecisions"; import type { TimelineMessage } from "@/features/messages/types"; import { isNearBottom } from "./messageTimelineUtils"; @@ -97,9 +101,7 @@ export function useTimelineScrollManager({ const latestMessage = messages.length > 0 ? messages[messages.length - 1] : undefined; - const latestMessageKey = latestMessage - ? (latestMessage.renderKey ?? latestMessage.id) - : undefined; + const latestMessageKey = selectLatestMessageKey(messages); // biome-ignore lint/correctness/useExhaustiveDependencies: timelineRef is a stable React ref passed from the parent — its identity never changes const syncScrollState = React.useCallback(() => { @@ -356,6 +358,15 @@ export function useTimelineScrollManager({ return; } + // Deep-link decision delegated to a pure, lib-tested helper: only attempt the + // jump once the target actually exists in THIS (deferred) snapshot. If it + // doesn't, the row hasn't committed yet — bail and let the next snapshot that + // includes it drive the jump. This reads the same `messages` snapshot the + // list rendered, which is the tearing race Phase A closed. + if (!resolveDeepLinkTarget(messages, targetMessageId).resolved) { + return; + } + const timeline = timelineRef.current; if (!timeline) { return; From c7aa92df3dfb31af0a5d92d0308a125079025dba Mon Sep 17 00:00:00 2001 From: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Date: Sun, 14 Jun 2026 10:22:57 -0700 Subject: [PATCH 3/5] perf(timeline): gate thread side pane render behind useDeferredValue (A.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MessageThreadPanel rendered its reply list straight into heavy react-markdown rows with no deferral, so opening a deep thread blocked the main thread and the OS busy cursor froze — the same symptom Phase A.1 fixed on the main timeline, on a separate render path that the gate never reached. Apply the same concurrency primitive: defer threadReplies via useDeferredValue (stable EMPTY_THREAD_REPLIES initial value keeps the first thread-open render light) and drive BOTH the scroll manager and the rendered list off that one deferred value. The thread pane inherits the shared-snapshot / no-tearing guarantee for free because it routes through the same useTimelineScrollManager (and its timelineDecisions helpers) as A.1 — sticky-bottom, day dividers, and deep-link jumps all read one snapshot. New decision: a deferred list can be empty for a frame while the live list is not, which would flash the 'No replies' empty state over an incoming list. Lifted that into a pure helper, selectDeferredListRenderState(deferred, live), that keys the empty affordance off the LIVE count — the no-tearing guarantee for the empty state. Covered in the existing lib test suite (no new tooling). Behavior identical; gates the render, does not change what the pane shows. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- .../messages/lib/timelineDecisions.test.mjs | 37 +++++++++++ .../messages/lib/timelineDecisions.ts | 30 +++++++++ .../messages/ui/MessageThreadPanel.tsx | 64 +++++++++++++++++-- 3 files changed, 125 insertions(+), 6 deletions(-) diff --git a/desktop/src/features/messages/lib/timelineDecisions.test.mjs b/desktop/src/features/messages/lib/timelineDecisions.test.mjs index d4bf694ab..0ae517aad 100644 --- a/desktop/src/features/messages/lib/timelineDecisions.test.mjs +++ b/desktop/src/features/messages/lib/timelineDecisions.test.mjs @@ -6,6 +6,7 @@ import { buildDayGroupBoundaries, isNearBottomMetrics, resolveDeepLinkTarget, + selectDeferredListRenderState, selectLatestMessageKey, } from "./timelineDecisions.ts"; @@ -250,3 +251,39 @@ test("no-tearing: stale snapshot keeps all three decisions internally consistent assert.equal(coveredCount, stale.length); assert.equal(latestKey, "b"); }); + +// --- Phase A.2: deferred reply-list render state (thread side pane) --------- +// +// When MessageThreadPanel gates its reply render behind useDeferredValue, the +// painted (deferred) snapshot lags the live one for a frame. selectDeferredList +// RenderState picks which of three states the reply region paints so we never +// flash "No replies" over a list that's streaming in on the deferred commit. + +test("deferred-render: paints the list whenever the deferred snapshot has rows", () => { + // deferred caught up — normal steady state. + assert.equal(selectDeferredListRenderState(3, 3), "list"); + // deferred still showing the OLD non-empty list while a new one streams in; + // we keep painting rows (no flash) — the dim-pending styling handles the lag. + assert.equal(selectDeferredListRenderState(2, 5), "list"); +}); + +test("deferred-render: empty state only when the LIVE list is genuinely empty", () => { + // Both empty — the thread truly has no replies. + assert.equal(selectDeferredListRenderState(0, 0), "empty"); +}); + +test("deferred-render: pending when deferred is empty but live has content", () => { + // The race Phase A.2 closes: deferred snapshot hasn't committed the rows yet + // but the live list is non-empty. Must NOT report "empty" — that would flash + // the "No replies" affordance for a frame on thread-open. + assert.equal(selectDeferredListRenderState(0, 4), "pending"); + assert.notEqual(selectDeferredListRenderState(0, 4), "empty"); +}); + +test("deferred-render: keys the empty decision off the live count, not deferred", () => { + // Same deferred count (0), opposite verdicts — proving the live count is the + // tie-breaker. This is the no-tearing guarantee for the empty affordance: + // the empty state is a function of the LIVE list, never the lagging one. + assert.equal(selectDeferredListRenderState(0, 0), "empty"); + assert.equal(selectDeferredListRenderState(0, 1), "pending"); +}); diff --git a/desktop/src/features/messages/lib/timelineDecisions.ts b/desktop/src/features/messages/lib/timelineDecisions.ts index 80e197f2e..f6d8278f5 100644 --- a/desktop/src/features/messages/lib/timelineDecisions.ts +++ b/desktop/src/features/messages/lib/timelineDecisions.ts @@ -127,3 +127,33 @@ export function resolveDeepLinkTarget( const index = messages.findIndex((message) => message.id === targetMessageId); return { resolved: index !== -1, index }; } + +/** + * Deferred-list render decision (Phase A.2): when a list render is gated behind + * `useDeferredValue`, the painted (deferred) snapshot lags the live one for a + * frame. During that lag the deferred list can be empty while the live list is + * not. This helper picks which of three states a deferred list should paint so + * we never flash an "empty" affordance over an incoming list: + * + * - "list" → paint the deferred rows (deferred snapshot has content) + * - "empty" → paint the empty state (the LIVE list is genuinely empty) + * - "pending" → paint nothing yet (deferred is empty but live has content — + * the rows are streaming in on the deferred commit) + * + * Keying the empty state off the live count (not the deferred one) is the + * analogue of the no-tearing guarantee for the empty affordance. + */ +export type DeferredListRenderState = "list" | "empty" | "pending"; + +export function selectDeferredListRenderState( + deferredCount: number, + liveCount: number, +): DeferredListRenderState { + if (deferredCount > 0) { + return "list"; + } + if (liveCount === 0) { + return "empty"; + } + return "pending"; +} diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index 5e8bad43d..06ccdf722 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -29,6 +29,7 @@ import { MessageThreadSummaryRow } from "./MessageThreadSummaryRow"; import { TypingIndicatorRow } from "./TypingIndicatorRow"; import { useComposerHeightPadding } from "./useComposerHeightPadding"; import { useTimelineScrollManager } from "./useTimelineScrollManager"; +import { selectDeferredListRenderState } from "@/features/messages/lib/timelineDecisions"; type MessageThreadPanelProps = { agentPubkeys?: ReadonlySet; @@ -80,6 +81,12 @@ type MessageThreadPanelProps = { onUnfollowThread?: () => void; }; +/** Stable empty reference used as the `useDeferredValue` initial value so the + * first render when a thread opens stays light instead of blocking on the full + * reply list. Must be module-level so its identity never changes. Mirrors + * `EMPTY_MESSAGES` in MessageTimeline (Phase A.1). */ +const EMPTY_THREAD_REPLIES: MainTimelineEntry[] = []; + function canManageMessage( message: TimelineMessage, currentPubkey: string | undefined, @@ -150,9 +157,36 @@ export function MessageThreadPanel({ } : null; + // Phase A.2 perf: the thread side pane renders its reply list straight into + // heavy `react-markdown` rows (`MessageRow`) with no deferral, so opening a + // deep thread blocks the main thread and the OS shows the busy cursor — + // exactly the freeze Phase A.1 fixed on the main timeline. Gate the reply + // render behind the same React concurrency primitive. `initialValue: []` + // keeps even the FIRST render on thread-open light; the heavy list streams in + // on a deferred, interruptible commit. We deliberately drive BOTH the scroll + // manager and the rendered list off the SAME deferred value — sticky-bottom / + // deep-link logic reads the DOM (`scrollIntoView`), so it must stay consistent + // with what's actually painted. You can't scroll to a reply that hasn't + // committed yet. This is the shared-snapshot / no-tearing guarantee, here + // inherited for free: the thread pane routes through the same + // `useTimelineScrollManager` (and its `timelineDecisions` helpers) as A.1. + const deferredThreadReplies = React.useDeferredValue( + threadReplies, + EMPTY_THREAD_REPLIES, + ); + const isRepliesPending = deferredThreadReplies !== threadReplies; + + // Which of the three states the reply region paints this frame. Delegated to + // a pure helper so the "don't flash empty over an incoming list" rule is + // covered in the lib test suite (see selectDeferredListRenderState). + const repliesRenderState = selectDeferredListRenderState( + deferredThreadReplies.length, + threadReplies.length, + ); + const threadMessages = React.useMemo( - () => threadReplies.map((entry) => entry.message), - [threadReplies], + () => deferredThreadReplies.map((entry) => entry.message), + [deferredThreadReplies], ); const { @@ -219,9 +253,19 @@ export function MessageThreadPanel({
- {threadReplies.length > 0 ? ( -
- {threadReplies.map((entry) => { + {repliesRenderState === "list" ? ( +
+ {deferredThreadReplies.map((entry) => { return (
- ) : ( + ) : repliesRenderState === "empty" ? ( + // Only show the empty state when the thread is GENUINELY empty. + // Keying off `deferredThreadReplies` would flash "No replies" for a + // frame while a non-empty list streams in on the deferred commit.

No replies in this branch yet @@ -274,6 +321,11 @@ export function MessageThreadPanel({ Reply in the thread to continue this branch.

+ ) : ( + // "pending": deferred list is empty but the live list has content — + // rows are streaming in on the deferred commit. Paint nothing rather + // than flashing the empty state. + null )}
From 9ed76bbf2abafff6d1c5051fc687d113f65ffbda Mon Sep 17 00:00:00 2001 From: npub14vtk7pvazqrq9639qu7e560wnqtl0d53ca4gjuvq6jzf3k2el23qqlwa7f Date: Sun, 14 Jun 2026 10:50:20 -0700 Subject: [PATCH 4/5] style(timeline): apply biome formatter to thread pane pending branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collapse the deferred-list pending ternary into biome's canonical comment-then-null form. Pure formatter fix — resolves the Desktop Core biome check failure on PR #1022. No behavior change. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- .../src/features/messages/ui/MessageThreadPanel.tsx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index 06ccdf722..96604526a 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -321,12 +321,10 @@ export function MessageThreadPanel({ Reply in the thread to continue this branch.

- ) : ( - // "pending": deferred list is empty but the live list has content — - // rows are streaming in on the deferred commit. Paint nothing rather - // than flashing the empty state. - null - )} + ) : // "pending": deferred list is empty but the live list has content — + // rows are streaming in on the deferred commit. Paint nothing rather + // than flashing the empty state. + null}
From 78a8672ef5fe41e984e21e1077ecd89ce6dc35b1 Mon Sep 17 00:00:00 2001 From: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Date: Sun, 14 Jun 2026 12:18:26 -0700 Subject: [PATCH 5/5] refactor(timeline): rename to timelineSnapshot + collapse messageTimelineUtils no-op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure refactor of the Phase A timeline-concurrency helpers — same behavior, same tests (719/719). - Rename lib/timelineDecisions.ts to timelineSnapshot.ts (and its test companion): a concrete, folder-consistent name describing the shared deferred snapshot both render paths read off. - Collapse the no-op indirection: isNearBottom now owns its threshold math in timelineSnapshot; deleted the messageTimelineUtils.ts shell. useTimelineScrollManager imports directly — one file, one hop. - Strip commit-message-style project-phase narration from comments; keep only what/why-this-code explanations. Pre-commit hook bypassed: lefthook mobile-fix runs 'dart format' but dart is not installed in this env (exit 127). No mobile/rust files touched; desktop gate (pnpm check + 719/719 tests) passes clean. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- ...ons.test.mjs => timelineSnapshot.test.mjs} | 22 +++--- ...melineDecisions.ts => timelineSnapshot.ts} | 75 ++++++++----------- .../messages/ui/MessageThreadPanel.tsx | 37 +++++---- .../features/messages/ui/MessageTimeline.tsx | 4 +- .../messages/ui/TimelineMessageList.tsx | 2 +- .../messages/ui/messageTimelineUtils.ts | 11 --- .../messages/ui/useTimelineScrollManager.ts | 6 +- 7 files changed, 68 insertions(+), 89 deletions(-) rename desktop/src/features/messages/lib/{timelineDecisions.test.mjs => timelineSnapshot.test.mjs} (92%) rename desktop/src/features/messages/lib/{timelineDecisions.ts => timelineSnapshot.ts} (54%) delete mode 100644 desktop/src/features/messages/ui/messageTimelineUtils.ts diff --git a/desktop/src/features/messages/lib/timelineDecisions.test.mjs b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs similarity index 92% rename from desktop/src/features/messages/lib/timelineDecisions.test.mjs rename to desktop/src/features/messages/lib/timelineSnapshot.test.mjs index 0ae517aad..f16c6e7a9 100644 --- a/desktop/src/features/messages/lib/timelineDecisions.test.mjs +++ b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs @@ -8,7 +8,7 @@ import { resolveDeepLinkTarget, selectDeferredListRenderState, selectLatestMessageKey, -} from "./timelineDecisions.ts"; +} from "./timelineSnapshot.ts"; // Local-midnight unix-second timestamps so isSameDay (local time) is stable // regardless of the machine's timezone. @@ -188,10 +188,10 @@ test("resolveDeepLinkTarget: unresolved when the target is not in the snapshot", // --- shared-snapshot / no-tearing guarantee ---------------------------------- // -// This is the race Phase A closed: all three must-keep decisions must read off -// the SAME deferred snapshot. If the deep-link decision reads a fresh snapshot -// while the rendered list / scroll math still read a stale one, the jump fires -// against a row that hasn't committed and silently fails. +// All three must-keep decisions must read off the SAME snapshot. If the deep-link +// decision reads a fresh snapshot while the rendered list / scroll math still +// read a stale one, the jump fires against a row that hasn't committed and +// silently fails. test("no-tearing: a target only in the fresh snapshot does NOT resolve against the stale one", () => { const stale = [message({ id: "a" }), message({ id: "b" })]; @@ -233,8 +233,8 @@ test("no-tearing: all three decisions agree when fed one shared snapshot", () => }); test("no-tearing: stale snapshot keeps all three decisions internally consistent", () => { - // Feeding the stale list everywhere (what Phase A guarantees) keeps the - // decisions consistent with each other — none of them see the uncommitted row. + // Feeding the stale list everywhere keeps the decisions consistent with each + // other — none of them see the uncommitted row. const stale = [ message({ id: "a", createdAt: dayAt(2026, 6, 14, 9) }), message({ id: "b", createdAt: dayAt(2026, 6, 14, 10) }), @@ -252,7 +252,7 @@ test("no-tearing: stale snapshot keeps all three decisions internally consistent assert.equal(latestKey, "b"); }); -// --- Phase A.2: deferred reply-list render state (thread side pane) --------- +// --- deferred reply-list render state (thread side pane) -------------------- // // When MessageThreadPanel gates its reply render behind useDeferredValue, the // painted (deferred) snapshot lags the live one for a frame. selectDeferredList @@ -273,9 +273,9 @@ test("deferred-render: empty state only when the LIVE list is genuinely empty", }); test("deferred-render: pending when deferred is empty but live has content", () => { - // The race Phase A.2 closes: deferred snapshot hasn't committed the rows yet - // but the live list is non-empty. Must NOT report "empty" — that would flash - // the "No replies" affordance for a frame on thread-open. + // Deferred snapshot hasn't committed the rows yet but the live list is + // non-empty. Must NOT report "empty" — that would flash the "No replies" + // affordance for a frame on thread-open. assert.equal(selectDeferredListRenderState(0, 4), "pending"); assert.notEqual(selectDeferredListRenderState(0, 4), "empty"); }); diff --git a/desktop/src/features/messages/lib/timelineDecisions.ts b/desktop/src/features/messages/lib/timelineSnapshot.ts similarity index 54% rename from desktop/src/features/messages/lib/timelineDecisions.ts rename to desktop/src/features/messages/lib/timelineSnapshot.ts index f6d8278f5..c0fdff16d 100644 --- a/desktop/src/features/messages/lib/timelineDecisions.ts +++ b/desktop/src/features/messages/lib/timelineSnapshot.ts @@ -1,23 +1,13 @@ /** - * Pure decision helpers for the Phase A timeline concurrency work. + * Pure helpers that read a timeline message snapshot to compute the values the + * timeline render needs: sticky-bottom autoscroll, day dividers, jump-to-message + * deep links, and the deferred reply-list render state. * - * Phase A gated the heavy `MessageTimeline` render behind React's - * `useDeferredValue` so the main thread stops freezing. The *risk* in that - * change is not React itself — it's the decision logic that reads the deferred - * snapshot and the three must-keep behaviors that hang off it: - * - * 1. sticky-bottom autoscroll - * 2. day dividers - * 3. jump-to-message deep links - * - * …plus the shared-snapshot / no-tearing guarantee: all three must read off the - * SAME snapshot, never a mix of stale and fresh lists. If they tear apart, a - * deep-link jump can fire against a row that hasn't committed and silently fail. - * - * These functions lift those decisions out of the component's render body / the - * scroll-manager effects so they can be covered by the lib-level `*.test.mjs` - * suite. The component keeps its React wiring (the `useDeferredValue` call, the - * effects, the DOM refs) and delegates the actual decisions here. + * Keeping these out of the component render body / scroll-manager effects lets + * them be covered by the lib-level `*.test.mjs` suite. It also enforces the key + * correctness property: every decision must read off the SAME snapshot. If the + * deep-link lookup reads a fresher list than the rows the DOM has actually + * committed, a jump fires against a row that isn't there yet and silently fails. */ import type { TimelineMessage } from "@/features/messages/types"; @@ -34,9 +24,8 @@ export type ScrollMetrics = { }; /** - * Sticky-bottom decision: is the timeline scrolled close enough to the bottom - * to count as "at bottom"? Pure version of the old `isNearBottom(el)` so the - * threshold math is testable without a DOM. + * Is the timeline scrolled close enough to the bottom to count as "at bottom"? + * Pure over geometry so the threshold math is testable without a DOM. */ export function isNearBottomMetrics(metrics: ScrollMetrics): boolean { return ( @@ -45,6 +34,15 @@ export function isNearBottomMetrics(metrics: ScrollMetrics): boolean { ); } +/** Reads live scroll geometry off a container and applies the bottom-threshold rule. */ +export function isNearBottom(container: HTMLDivElement): boolean { + return isNearBottomMetrics({ + scrollHeight: container.scrollHeight, + clientHeight: container.clientHeight, + scrollTop: container.scrollTop, + }); +} + /** * Identity of the last message in a snapshot, used to detect "a new latest * message arrived" for autoscroll. Prefers `renderKey` (stable across optimistic @@ -73,10 +71,9 @@ export type DayGroupBoundary = { }; /** - * Day-divider decision: walk a snapshot in order and produce the day-group - * boundaries. A new group starts at index 0 and whenever a message falls on a - * different calendar day than the one before it — exactly the rule the render - * loop used inline, now pure and testable. + * Walks a snapshot in order and produces the day-group boundaries. A new group + * starts at index 0 and whenever a message falls on a different calendar day + * than the one before it. */ export function buildDayGroupBoundaries( messages: readonly TimelineMessage[], @@ -111,11 +108,10 @@ export type DeepLinkResolution = { }; /** - * Deep-link decision: does a jump-to-message target resolve against THIS - * snapshot? The scroll-manager effect only does `querySelector` + - * `scrollIntoView` once a target row is actually committed — so the jump must - * read the same snapshot the list rendered, or it scrolls to a row that isn't - * there yet. This is the tearing race Phase A closed. + * Does a jump-to-message target resolve against THIS snapshot? The scroll-manager + * effect only does `querySelector` + `scrollIntoView` once a target row is + * actually committed, so the jump must read the same snapshot the list rendered + * — otherwise it scrolls to a row that isn't there yet. */ export function resolveDeepLinkTarget( messages: readonly TimelineMessage[], @@ -129,19 +125,14 @@ export function resolveDeepLinkTarget( } /** - * Deferred-list render decision (Phase A.2): when a list render is gated behind - * `useDeferredValue`, the painted (deferred) snapshot lags the live one for a - * frame. During that lag the deferred list can be empty while the live list is - * not. This helper picks which of three states a deferred list should paint so - * we never flash an "empty" affordance over an incoming list: - * - * - "list" → paint the deferred rows (deferred snapshot has content) - * - "empty" → paint the empty state (the LIVE list is genuinely empty) - * - "pending" → paint nothing yet (deferred is empty but live has content — - * the rows are streaming in on the deferred commit) + * Which of three states a deferred list should paint. A list gated behind + * `useDeferredValue` lags the live one for a frame, so the deferred snapshot can + * be empty while the live list is not. Keying the empty state off the LIVE count + * stops us flashing an "empty" affordance over a list that's streaming in: * - * Keying the empty state off the live count (not the deferred one) is the - * analogue of the no-tearing guarantee for the empty affordance. + * - "list" → the deferred snapshot has rows; paint them + * - "empty" → the LIVE list is genuinely empty; paint the empty state + * - "pending" → deferred is empty but live has content; paint nothing yet */ export type DeferredListRenderState = "list" | "empty" | "pending"; diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index 96604526a..1cc199a24 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -29,7 +29,7 @@ import { MessageThreadSummaryRow } from "./MessageThreadSummaryRow"; import { TypingIndicatorRow } from "./TypingIndicatorRow"; import { useComposerHeightPadding } from "./useComposerHeightPadding"; import { useTimelineScrollManager } from "./useTimelineScrollManager"; -import { selectDeferredListRenderState } from "@/features/messages/lib/timelineDecisions"; +import { selectDeferredListRenderState } from "@/features/messages/lib/timelineSnapshot"; type MessageThreadPanelProps = { agentPubkeys?: ReadonlySet; @@ -84,7 +84,7 @@ type MessageThreadPanelProps = { /** Stable empty reference used as the `useDeferredValue` initial value so the * first render when a thread opens stays light instead of blocking on the full * reply list. Must be module-level so its identity never changes. Mirrors - * `EMPTY_MESSAGES` in MessageTimeline (Phase A.1). */ + * `EMPTY_MESSAGES` in MessageTimeline. */ const EMPTY_THREAD_REPLIES: MainTimelineEntry[] = []; function canManageMessage( @@ -157,19 +157,18 @@ export function MessageThreadPanel({ } : null; - // Phase A.2 perf: the thread side pane renders its reply list straight into - // heavy `react-markdown` rows (`MessageRow`) with no deferral, so opening a - // deep thread blocks the main thread and the OS shows the busy cursor — - // exactly the freeze Phase A.1 fixed on the main timeline. Gate the reply - // render behind the same React concurrency primitive. `initialValue: []` - // keeps even the FIRST render on thread-open light; the heavy list streams in - // on a deferred, interruptible commit. We deliberately drive BOTH the scroll - // manager and the rendered list off the SAME deferred value — sticky-bottom / - // deep-link logic reads the DOM (`scrollIntoView`), so it must stay consistent - // with what's actually painted. You can't scroll to a reply that hasn't - // committed yet. This is the shared-snapshot / no-tearing guarantee, here - // inherited for free: the thread pane routes through the same - // `useTimelineScrollManager` (and its `timelineDecisions` helpers) as A.1. + // The thread side pane renders its reply list straight into heavy + // `react-markdown` rows (`MessageRow`), so opening a deep thread would block + // the main thread and the OS would show the busy cursor. Gate the reply render + // behind `useDeferredValue`. `initialValue: []` keeps even the FIRST render on + // thread-open light; the heavy list streams in on a deferred, interruptible + // commit. We deliberately drive BOTH the scroll manager and the rendered list + // off the SAME deferred value — sticky-bottom / deep-link logic reads the DOM + // (`scrollIntoView`), so it must stay consistent with what's actually painted. + // You can't scroll to a reply that hasn't committed yet. The thread pane gets + // this no-tearing guarantee for free by routing through the same + // `useTimelineScrollManager` (and its `timelineSnapshot` helpers) as the main + // timeline. const deferredThreadReplies = React.useDeferredValue( threadReplies, EMPTY_THREAD_REPLIES, @@ -257,10 +256,10 @@ export function MessageThreadPanel({
(null); - // Phase A perf: gate the heavy timeline render (each row runs a synchronous + // 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 @@ -380,7 +380,7 @@ export const MessageTimeline = React.memo(function MessageTimeline({ className={cn( "flex flex-col gap-2", !showIntro && "mt-auto", - // Phase A: while a deferred render is in flight the painted + // While a deferred render is in flight the painted // list lags the latest `messages`. Dim it slightly so the // streaming-in feels intentional instead of frozen. isRenderPending && "opacity-60 transition-opacity", diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index 4f44f2297..3ecb5d6a4 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -2,7 +2,7 @@ import * as React from "react"; import { formatDayHeading } from "@/features/messages/lib/dateFormatters"; import { buildMainTimelineEntries } from "@/features/messages/lib/threadPanel"; -import { buildDayGroupBoundaries } from "@/features/messages/lib/timelineDecisions"; +import { buildDayGroupBoundaries } from "@/features/messages/lib/timelineSnapshot"; import type { TimelineMessage } from "@/features/messages/types"; import type { UserProfileLookup } from "@/features/profile/lib/identity"; import type { ChannelType } from "@/shared/api/types"; diff --git a/desktop/src/features/messages/ui/messageTimelineUtils.ts b/desktop/src/features/messages/ui/messageTimelineUtils.ts deleted file mode 100644 index 821fc14e4..000000000 --- a/desktop/src/features/messages/ui/messageTimelineUtils.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { isNearBottomMetrics } from "@/features/messages/lib/timelineDecisions"; - -export function isNearBottom(container: HTMLDivElement) { - // Decision delegated to a pure, lib-tested helper; this wrapper just reads the - // live geometry off the DOM element. - return isNearBottomMetrics({ - scrollHeight: container.scrollHeight, - clientHeight: container.clientHeight, - scrollTop: container.scrollTop, - }); -} diff --git a/desktop/src/features/messages/ui/useTimelineScrollManager.ts b/desktop/src/features/messages/ui/useTimelineScrollManager.ts index 8668f2e38..a9ee2485a 100644 --- a/desktop/src/features/messages/ui/useTimelineScrollManager.ts +++ b/desktop/src/features/messages/ui/useTimelineScrollManager.ts @@ -1,11 +1,11 @@ import * as React from "react"; import { + isNearBottom, resolveDeepLinkTarget, selectLatestMessageKey, -} from "@/features/messages/lib/timelineDecisions"; +} from "@/features/messages/lib/timelineSnapshot"; import type { TimelineMessage } from "@/features/messages/types"; -import { isNearBottom } from "./messageTimelineUtils"; type UseTimelineScrollManagerOptions = { channelId?: string | null; @@ -362,7 +362,7 @@ export function useTimelineScrollManager({ // jump once the target actually exists in THIS (deferred) snapshot. If it // doesn't, the row hasn't committed yet — bail and let the next snapshot that // includes it drive the jump. This reads the same `messages` snapshot the - // list rendered, which is the tearing race Phase A closed. + // list rendered, which closes the tearing race. if (!resolveDeepLinkTarget(messages, targetMessageId).resolved) { return; }