diff --git a/desktop/src/app/AppShell.tsx b/desktop/src/app/AppShell.tsx index 6e8882c0f..5ada653cb 100644 --- a/desktop/src/app/AppShell.tsx +++ b/desktop/src/app/AppShell.tsx @@ -76,6 +76,9 @@ import { relayClient } from "@/shared/api/relayClient"; import { useIdentityQuery } from "@/shared/api/hooks"; import { useRelayAutoHeal } from "@/shared/api/useRelayAutoHeal"; import { useDeferredStartup } from "@/shared/hooks/useDeferredStartup"; +import { preloadAgentsScreen } from "@/app/routes/agents"; +import { preloadChannelRouteScreen } from "@/app/routes/channels.$channelId"; +import { preloadChannelViews } from "@/features/channels/ui/ChannelScreenLazyViews"; import { joinChannel } from "@/shared/api/tauri"; import type { Channel, RelayEvent, SearchHit } from "@/shared/api/types"; import { ChannelNavigationProvider } from "@/shared/context/ChannelNavigationContext"; @@ -537,6 +540,19 @@ export function AppShell() { }; }, []); + // Warm the lazy route chunks (channel timeline, forum, agents) once the shell + // is idle, so the FIRST main-nav transition doesn't stall on a cold chunk + // fetch+parse. `startupReady` is the existing idle-or-timeout gate; the chunk + // imports dedupe, so racing an actual navigation is harmless. + React.useEffect(() => { + if (!startupReady) { + return; + } + preloadChannelRouteScreen(); + preloadChannelViews(); + preloadAgentsScreen(); + }, [startupReady]); + React.useEffect(() => { const numericCount = highPriorityUnreadChannelIds.size + homeBadgeCountExcludingHighPriority; diff --git a/desktop/src/app/routes/agents.tsx b/desktop/src/app/routes/agents.tsx index c70ff7eb9..2cc1afa0c 100644 --- a/desktop/src/app/routes/agents.tsx +++ b/desktop/src/app/routes/agents.tsx @@ -3,11 +3,21 @@ import { createFileRoute } from "@tanstack/react-router"; import { ViewLoadingFallback } from "@/shared/ui/ViewLoadingFallback"; +// The chunk import is hoisted so it can be triggered eagerly (route preload) +// as well as lazily on render — calling it twice is a no-op, the module loader +// dedupes and caches the in-flight promise. +const importAgentsScreen = () => import("@/features/agents/ui/AgentsScreen"); + const AgentsScreen = React.lazy(async () => { - const module = await import("@/features/agents/ui/AgentsScreen"); + const module = await importAgentsScreen(); return { default: module.AgentsScreen }; }); +/** Warms the AgentsScreen route chunk so first navigation doesn't stall. */ +export function preloadAgentsScreen(): void { + void importAgentsScreen(); +} + export const Route = createFileRoute("/agents")({ component: AgentsRouteComponent, }); diff --git a/desktop/src/app/routes/channels.$channelId.tsx b/desktop/src/app/routes/channels.$channelId.tsx index 3c64f3ec9..61b64d0f3 100644 --- a/desktop/src/app/routes/channels.$channelId.tsx +++ b/desktop/src/app/routes/channels.$channelId.tsx @@ -38,11 +38,20 @@ export const Route = createFileRoute("/channels/$channelId")({ component: ChannelRouteComponent, }); +// Hoisted so the chunk can be warmed eagerly (route preload) as well as loaded +// lazily on render; the loader dedupes repeat calls. +const importChannelRouteScreen = () => import("./ChannelRouteScreen"); + const ChannelRouteScreen = React.lazy(async () => { - const module = await import("./ChannelRouteScreen"); + const module = await importChannelRouteScreen(); return { default: module.ChannelRouteScreen }; }); +/** Warms the ChannelRouteScreen chunk so first channel open doesn't stall. */ +export function preloadChannelRouteScreen(): void { + void importChannelRouteScreen(); +} + function ChannelRouteComponent() { const { channelId } = Route.useParams(); const search = Route.useSearch(); diff --git a/desktop/src/features/channels/ui/ChannelScreenLazyViews.ts b/desktop/src/features/channels/ui/ChannelScreenLazyViews.ts index 4e4fc0f2a..8784fc345 100644 --- a/desktop/src/features/channels/ui/ChannelScreenLazyViews.ts +++ b/desktop/src/features/channels/ui/ChannelScreenLazyViews.ts @@ -1,11 +1,22 @@ import * as React from "react"; +// Hoisted chunk imports so each view can be warmed eagerly (route preload) as +// well as loaded lazily on render; the module loader dedupes repeat calls. +const importChannelPane = () => import("@/features/channels/ui/ChannelPane"); +const importForumView = () => import("@/features/forum/ui/ForumView"); + export const ChannelPane = React.lazy(async () => { - const module = await import("@/features/channels/ui/ChannelPane"); + const module = await importChannelPane(); return { default: module.ChannelPane }; }); export const ForumView = React.lazy(async () => { - const module = await import("@/features/forum/ui/ForumView"); + const module = await importForumView(); return { default: module.ForumView }; }); + +/** Warms the channel/forum view chunks so first open doesn't stall. */ +export function preloadChannelViews(): void { + void importChannelPane(); + void importForumView(); +} diff --git a/desktop/src/features/messages/lib/scrollConvergence.test.mjs b/desktop/src/features/messages/lib/scrollConvergence.test.mjs new file mode 100644 index 000000000..53c96eaa6 --- /dev/null +++ b/desktop/src/features/messages/lib/scrollConvergence.test.mjs @@ -0,0 +1,210 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { CONVERGENCE_FRAME_CAP, convergenceStep } from "./scrollConvergence.ts"; + +function input(overrides) { + return { + targetMessageId: "target", + indexByMessageId: new Map([["target", 100]]), + lastIssuedIndex: null, + librarySettled: false, + stalledOffTarget: false, + framesUsed: 0, + ...overrides, + }; +} + +// --- re-aim / staleness guard ------------------------------------------------ + +test("convergenceStep: first frame aims at the resolved index, not yet done", () => { + const step = convergenceStep(input({ lastIssuedIndex: null })); + assert.equal(step.nextIndex, 100); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +test("convergenceStep: re-resolves a shifted index from the map each frame", () => { + // A prepend shifted the target from 100 to 105. The library is still chasing + // the old index (lastIssuedIndex 100); the reducer must aim at the NEW index + // so the adapter re-issues scrollToIndex(105). This is the staleness guard. + const step = convergenceStep( + input({ + indexByMessageId: new Map([["target", 105]]), + lastIssuedIndex: 100, + }), + ); + assert.equal(step.nextIndex, 105); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +test("convergenceStep: target removed mid-settle stops with converged=false", () => { + // Target deleted from the map while the loop was chasing it. Terminate so the + // adapter clears the highlight instead of chasing a vanished row. + const step = convergenceStep( + input({ + indexByMessageId: new Map(), // target gone + lastIssuedIndex: 100, + framesUsed: 3, + }), + ); + assert.equal(step.nextIndex, null); + assert.equal(step.done, true); + assert.equal(step.converged, false); +}); + +// --- settle ------------------------------------------------------------------ + +test("convergenceStep: library settled while aiming at current index converges", () => { + const step = convergenceStep( + input({ lastIssuedIndex: 100, librarySettled: true }), + ); + assert.equal(step.nextIndex, 100); + assert.equal(step.done, true); + assert.equal(step.converged, true); +}); + +test("convergenceStep: a settle reported WHILE re-aiming is ignored", () => { + // The index just moved (105) but the library reports settled — that settle is + // on the OLD index (100), so it must NOT count as convergence. The reducer + // keeps going and aims at the new index. + const step = convergenceStep( + input({ + indexByMessageId: new Map([["target", 105]]), + lastIssuedIndex: 100, + librarySettled: true, + }), + ); + assert.equal(step.nextIndex, 105); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +test("convergenceStep: aiming at current but not yet settled keeps waiting", () => { + // Library is chasing the right index but its offset hasn't stabilized. The + // reducer returns the same index (so the adapter re-issues NOTHING — issuing + // would reset the library's stableFrames and prevent settling) and waits. + const step = convergenceStep( + input({ lastIssuedIndex: 100, librarySettled: false }), + ); + assert.equal(step.nextIndex, 100); + assert.equal(step.reissue, false); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +// --- off-target stall (liveness) --------------------------------------------- + +test("convergenceStep: stalled off-target while aiming at current re-issues same index", () => { + // The library's offset stopped moving but never reached the current index's + // target (its internal reconcile deadlocked after rows re-measured). The + // reducer signals a same-index re-issue to kick it — the loop continues. + const step = convergenceStep( + input({ lastIssuedIndex: 100, stalledOffTarget: true }), + ); + assert.equal(step.nextIndex, 100); + assert.equal(step.reissue, true); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +test("convergenceStep: a stall reported WHILE re-aiming does not re-issue", () => { + // The index just moved (105) but the library reports a stall on the OLD index + // (100). The reducer re-aims at the new index normally; the stale stall must + // NOT trigger a same-index kick (there is no current-index stall to kick). + const step = convergenceStep( + input({ + indexByMessageId: new Map([["target", 105]]), + lastIssuedIndex: 100, + stalledOffTarget: true, + }), + ); + assert.equal(step.nextIndex, 105); + assert.equal(step.reissue, false); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +test("convergenceStep: a settle takes priority over a concurrent stall flag", () => { + // Defensive: the adapter computes settle and stall as mutually exclusive, but + // if both arrive, a genuine settle must win (converge) rather than spin on a + // pointless re-issue. + const step = convergenceStep( + input({ + lastIssuedIndex: 100, + librarySettled: true, + stalledOffTarget: true, + }), + ); + assert.equal(step.done, true); + assert.equal(step.converged, true); + assert.equal(step.reissue, false); +}); + +// --- frame cap --------------------------------------------------------------- + +test("convergenceStep: terminates at the frame cap without converging", () => { + // A row that never settles (librarySettled stays false) must still stop at the + // cap rather than spin forever. + const step = convergenceStep( + input({ + lastIssuedIndex: 100, + librarySettled: false, + framesUsed: CONVERGENCE_FRAME_CAP - 1, + }), + ); + assert.equal(step.done, true); + assert.equal(step.converged, false); + assert.equal(step.nextIndex, 100); +}); + +test("convergenceStep: frame cap bounds a perpetually shifting target", () => { + // Drive the loop the way the adapter would: the target index moves every + // frame, so the library never settles. The loop must terminate at the cap. + let lastIssuedIndex = null; + let framesUsed = 0; + let done = false; + let converged = true; + + while (framesUsed < CONVERGENCE_FRAME_CAP + 5) { + const movingIndex = 100 + framesUsed; // shifts every frame + const step = convergenceStep( + input({ + indexByMessageId: new Map([["target", movingIndex]]), + lastIssuedIndex, + librarySettled: false, + framesUsed, + }), + ); + lastIssuedIndex = step.nextIndex; + framesUsed += 1; + if (step.done) { + done = step.done; + converged = step.converged; + break; + } + } + + assert.equal(done, true); + assert.equal(converged, false); + assert.ok(framesUsed <= CONVERGENCE_FRAME_CAP); +}); + +test("convergenceStep: converges once a re-aimed index then settles", () => { + // Realistic flow: frame 0 aims (lastIssued null -> 100), frame 1 the library + // is chasing 100 and reports settled -> converged. + const aim = convergenceStep(input({ lastIssuedIndex: null })); + assert.equal(aim.nextIndex, 100); + assert.equal(aim.done, false); + + const settle = convergenceStep( + input({ + lastIssuedIndex: aim.nextIndex, + librarySettled: true, + framesUsed: 1, + }), + ); + assert.equal(settle.done, true); + assert.equal(settle.converged, true); +}); diff --git a/desktop/src/features/messages/lib/scrollConvergence.ts b/desktop/src/features/messages/lib/scrollConvergence.ts new file mode 100644 index 000000000..0b8650a3f --- /dev/null +++ b/desktop/src/features/messages/lib/scrollConvergence.ts @@ -0,0 +1,151 @@ +/** + * Pure staleness + termination decision for scrolling a virtualized timeline to + * a message that may be far off-screen. + * + * @tanstack/react-virtual already owns the OFFSET convergence: a single + * `scrollToIndex(index)` captures that index in `scrollState`, and its internal + * `reconcileScroll` rAF loop re-runs `getOffsetForIndex(index)` every frame — + * re-aiming as off-screen rows mount and `measureElement` corrects their + * heights — until the offset is stable (or a 5s safety valve fires). We do NOT + * recompute offsets; duplicating `getOffsetForIndex` against the library's own + * `measurementsCache`/`scrollMargin`/`scrollPadding` would only drift. + * + * What the library does NOT do: it chases the INDEX captured at call time, with + * no concept of a message id. If the data shifts mid-settle — a prepend or a + * delete above the target — the captured index now points at the wrong row and + * the library happily settles on it. This reducer owns exactly that gap: each + * frame it re-resolves the target's CURRENT index from the live map and decides + * whether the adapter must re-aim the library, let it settle, or stop. + * + * Two correctness properties this enforces and the `.mjs` suite gates: + * - The target index is re-resolved by id every frame (never frozen), so a + * concurrent prepend/delete that shifts the target re-aims the library at the + * new index instead of stranding it on the old one. + * - If the target id leaves the data mid-settle (deleted), the loop terminates + * with `converged: false` rather than chasing a vanished row to the cap. + * + * Plus one liveness property: a large windowed-out jump can leave the library's + * own reconcile deadlocked off-target (offset stable but short of the target + * after rows re-measured under it). When that happens the reducer signals a + * same-index re-issue (`reissue: true`) to restart the library's reconcile — + * the single case where re-issuing an unchanged index is correct. + */ + +/** Where a scroll target should land in the viewport. Mirrors the library's align. */ +export type ConvergenceAlign = "start" | "center" | "end"; + +export type ConvergenceInput = { + /** Id of the message to settle on — re-resolved against the map each frame. */ + targetMessageId: string; + /** Live message-id -> item-index map; re-read every frame (staleness guard). */ + indexByMessageId: Map; + /** + * Index the library is currently chasing (the last index the adapter issued + * via `scrollToIndex`), or `null` before the first issue. Lets the reducer + * tell a re-aim (index moved) from a steady settle (index unchanged). + */ + lastIssuedIndex: number | null; + /** + * Whether the library's offset reached the current index's target this frame + * and stopped moving. Only meaningful once the library is chasing the CURRENT + * index; a settle reported while re-aiming is ignored. + */ + librarySettled: boolean; + /** + * Whether the library's offset has stopped moving but is NOT at the current + * index's target — it stalled mid-reconcile (its internal re-aim deadlocked + * after rows re-measured under it). The reducer kicks it with a fresh re-issue + * at the same index. Mutually exclusive with `librarySettled`. + */ + stalledOffTarget: boolean; + /** Frames already spent in the loop (the adapter increments per rAF). */ + framesUsed: number; +}; + +export type ConvergenceDecision = { + /** + * Index the adapter should be aiming the library at, or `null` when the + * target is gone. The adapter only re-issues `scrollToIndex` when this differs + * from `lastIssuedIndex`, so a steady settle issues no redundant scroll (which + * would reset the library's `stableFrames` and prevent it from ever settling). + */ + nextIndex: number | null; + /** + * True when the adapter must re-issue `scrollToIndex(nextIndex)` even though + * the index is unchanged — used to kick the library out of an off-target + * stall. A normal steady settle leaves this false so no redundant scroll + * resets the library's `stableFrames`. + */ + reissue: boolean; + /** True once the loop must stop (settled, target gone, or frame cap hit). */ + done: boolean; + /** True only when the loop stopped because the target row actually settled. */ + converged: boolean; +}; + +/** + * Hard cap on frames so a perpetually re-measuring row, or a target whose index + * keeps shifting, can't spin the loop forever. The library has its own 5s valve; + * this is the adapter-side bound expressed in frames for deterministic testing. + */ +export const CONVERGENCE_FRAME_CAP = 32; + +/** + * One frame of the convergence loop. Pure: given the live map and the library's + * settle state, decides the index to aim at and whether to stop. + */ +export function convergenceStep(input: ConvergenceInput): ConvergenceDecision { + const currentIndex = input.indexByMessageId.get(input.targetMessageId); + + // Target left the data mid-settle (deleted) — stop without converging so the + // adapter clears the highlight instead of chasing a vanished row. + if (currentIndex === undefined) { + return { nextIndex: null, reissue: false, done: true, converged: false }; + } + + const aimingAtCurrent = input.lastIssuedIndex === currentIndex; + + // The library only settles meaningfully once it is chasing the CURRENT index. + // A settle reported while we are still re-aiming (index just moved) is stale. + if (aimingAtCurrent && input.librarySettled) { + return { + nextIndex: currentIndex, + reissue: false, + done: true, + converged: true, + }; + } + + // Frame cap: accept the best index we have rather than spin forever on a row + // whose height never settles or a target whose index keeps shifting. + if (input.framesUsed + 1 >= CONVERGENCE_FRAME_CAP) { + return { + nextIndex: currentIndex, + reissue: false, + done: true, + converged: false, + }; + } + + // Library stalled off-target: its offset stopped moving but never reached the + // current index (its internal reconcile deadlocked after rows re-measured). + // Re-issue the SAME index to restart its reconcile — the one case where a + // same-index re-issue is correct rather than a stableFrames-resetting bug. + if (aimingAtCurrent && input.stalledOffTarget) { + return { + nextIndex: currentIndex, + reissue: true, + done: false, + converged: false, + }; + } + + // Either the index moved (adapter will re-issue scrollToIndex) or the library + // is still settling on the current index (adapter issues nothing, just waits). + return { + nextIndex: currentIndex, + reissue: false, + done: false, + converged: false, + }; +} diff --git a/desktop/src/features/messages/lib/timelineItems.test.mjs b/desktop/src/features/messages/lib/timelineItems.test.mjs new file mode 100644 index 000000000..04f8738ec --- /dev/null +++ b/desktop/src/features/messages/lib/timelineItems.test.mjs @@ -0,0 +1,164 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { KIND_SYSTEM_MESSAGE } from "@/shared/constants/kinds"; +import { buildTimelineItems, getTimelineItemKey } from "./timelineItems.ts"; + +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: "m", + renderKey: undefined, + createdAt: dayAt(2026, 6, 14), + pubkey: "author", + parentId: null, + rootId: null, + depth: 0, + kind: 9, + tags: [], + ...overrides, + }; +} + +// The builder takes MainTimelineEntry[] (post top-level filter); summary is +// irrelevant to item/divider placement, so null is fine here. +function entry(overrides) { + return { message: message(overrides), summary: null }; +} + +function kinds(items) { + return items.map((item) => item.kind); +} + +// --- divider placement ------------------------------------------------------- + +test("buildTimelineItems: 3-day channel with unread mid-day-2 places dividers by index", () => { + const entries = [ + entry({ id: "d1a", createdAt: dayAt(2026, 6, 12) }), + entry({ id: "d1b", createdAt: dayAt(2026, 6, 12, 13) }), + entry({ id: "d2a", createdAt: dayAt(2026, 6, 13) }), + entry({ id: "d2b", createdAt: dayAt(2026, 6, 13, 13) }), // first unread + entry({ id: "d2c", createdAt: dayAt(2026, 6, 13, 14) }), + entry({ id: "d3a", createdAt: dayAt(2026, 6, 14) }), + ]; + + const { items } = buildTimelineItems(entries, "d2b"); + + assert.deepEqual(kinds(items), [ + "day-divider", // day 1 + "message", // d1a + "message", // d1b + "day-divider", // day 2 + "message", // d2a + "unread-divider", // above d2b + "message", // d2b + "message", // d2c + "day-divider", // day 3 + "message", // d3a + ]); +}); + +test("buildTimelineItems: unread divider suppressed when first unread is the first entry", () => { + const entries = [ + entry({ id: "a", createdAt: dayAt(2026, 6, 14) }), + entry({ id: "b", createdAt: dayAt(2026, 6, 14, 13) }), + ]; + // firstUnread === index 0 — nothing above it, so no divider. + const { items } = buildTimelineItems(entries, "a"); + assert.equal(items.filter((i) => i.kind === "unread-divider").length, 0); +}); + +test("buildTimelineItems: system messages flatten to a 'system' item", () => { + const entries = [ + entry({ id: "a", createdAt: dayAt(2026, 6, 14) }), + entry({ + id: "sys", + kind: KIND_SYSTEM_MESSAGE, + createdAt: dayAt(2026, 6, 14, 13), + }), + ]; + const { items } = buildTimelineItems(entries, null); + assert.deepEqual(kinds(items), ["day-divider", "message", "system"]); +}); + +test("buildTimelineItems: empty entries produce no items and an empty map", () => { + const { items, indexByMessageId } = buildTimelineItems([], null); + assert.equal(items.length, 0); + assert.equal(indexByMessageId.size, 0); +}); + +// --- index map correctness --------------------------------------------------- + +test("buildTimelineItems: map points each message id at its flattened item index", () => { + const entries = [ + entry({ id: "d1", createdAt: dayAt(2026, 6, 12) }), + entry({ id: "d2", createdAt: dayAt(2026, 6, 13) }), + ]; + const { items, indexByMessageId } = buildTimelineItems(entries, null); + + // dividers occupy indices 0 and 2; messages land at 1 and 3. + assert.equal(indexByMessageId.get("d1"), 1); + assert.equal(indexByMessageId.get("d2"), 3); + assert.equal(items[1].entry.message.id, "d1"); + assert.equal(items[3].entry.message.id, "d2"); +}); + +test("buildTimelineItems: appending a new message keeps prior indices stable", () => { + const base = [entry({ id: "a", createdAt: dayAt(2026, 6, 14) })]; + const before = buildTimelineItems(base, null).indexByMessageId; + + const appended = [ + ...base, + entry({ id: "b", createdAt: dayAt(2026, 6, 14, 13) }), + ]; + const after = buildTimelineItems(appended, null).indexByMessageId; + + assert.equal(after.get("a"), before.get("a")); + assert.equal(after.get("b"), 2); +}); + +test("buildTimelineItems: prepending an older-day message shifts later indices", () => { + const original = [entry({ id: "b", createdAt: dayAt(2026, 6, 14) })]; + const beforeIdx = buildTimelineItems(original, null).indexByMessageId.get( + "b", + ); + + // Prepend a message on an earlier day → adds its own day-divider + message, + // pushing "b" (now on a new day boundary too) further down. + const prepended = [ + entry({ id: "a", createdAt: dayAt(2026, 6, 13) }), + entry({ id: "b", createdAt: dayAt(2026, 6, 14) }), + ]; + const afterIdx = buildTimelineItems(prepended, null).indexByMessageId.get( + "b", + ); + assert.ok(afterIdx > beforeIdx); +}); + +test("buildTimelineItems: deleting a message drops it from the map", () => { + const entries = [ + entry({ id: "a", createdAt: dayAt(2026, 6, 14) }), + entry({ id: "b", createdAt: dayAt(2026, 6, 14, 13) }), + ]; + const afterDelete = buildTimelineItems( + entries.filter((e) => e.message.id !== "a"), + null, + ).indexByMessageId; + assert.equal(afterDelete.has("a"), false); + assert.equal(afterDelete.get("b"), 1); +}); + +test("getTimelineItemKey: keys are unique across the stream", () => { + const entries = [ + entry({ id: "a", createdAt: dayAt(2026, 6, 12) }), + entry({ id: "b", createdAt: dayAt(2026, 6, 13) }), + ]; + const { items } = buildTimelineItems(entries, "b"); + const keys = items.map(getTimelineItemKey); + assert.equal(new Set(keys).size, keys.length); +}); diff --git a/desktop/src/features/messages/lib/timelineItems.ts b/desktop/src/features/messages/lib/timelineItems.ts new file mode 100644 index 000000000..02477eb97 --- /dev/null +++ b/desktop/src/features/messages/lib/timelineItems.ts @@ -0,0 +1,96 @@ +/** + * Flattens the heterogeneous day-grouped timeline tree into a flat + * discriminated-union item stream that a virtualizer can window over, and + * builds the `messageId -> itemIndex` map every DOM-query scroll path now + * resolves against instead of `querySelector`. + * + * Kept pure (no React, no DOM) so it is covered by the lib-level `*.test.mjs` + * suite. The list and the index map are produced together from the SAME walk, + * so they can never drift: a stale map would scroll deep-links to the wrong + * row, the exact failure virtualization risks. + */ + +import { + buildDayGroupBoundaries, + type DayGroupBoundary, +} from "@/features/messages/lib/timelineSnapshot"; +import { shouldRenderUnreadDivider } from "@/features/messages/lib/threadPanel"; +import type { MainTimelineEntry } from "@/features/messages/lib/threadPanel"; +import { KIND_SYSTEM_MESSAGE } from "@/shared/constants/kinds"; + +/** + * One renderable row in the flattened timeline. Dividers carry no message and + * never appear in the index map; the three message-bearing kinds do. + */ +export type TimelineItem = + // `headingTimestamp` (not a prebaked label) so the render still resolves + // "Today"/"Yesterday" relative to the current clock, not to build time. + | { kind: "day-divider"; key: string; headingTimestamp: number } + | { kind: "unread-divider"; key: string } + | { kind: "system"; key: string; entry: MainTimelineEntry } + | { kind: "message"; key: string; entry: MainTimelineEntry }; + +export type TimelineItemsResult = { + items: TimelineItem[]; + /** Maps a top-level message id to its index in `items`. */ + indexByMessageId: Map; +}; + +/** Stable per-item key, unique across the flattened stream. */ +export function getTimelineItemKey(item: TimelineItem): string { + return item.key; +} + +function entryRenderKey(entry: MainTimelineEntry): string { + return entry.message.renderKey ?? entry.message.id; +} + +/** + * Walks the (already top-level-filtered) entries once, emitting a day-divider + * at each calendar-day boundary and an unread-divider above the first unread + * message, then the message/system row itself. The index map records where + * each message landed in the flat stream so scroll targets resolve in O(1) + * without touching the DOM. + */ +export function buildTimelineItems( + entries: MainTimelineEntry[], + firstUnreadMessageId: string | null, +): TimelineItemsResult { + const items: TimelineItem[] = []; + const indexByMessageId = new Map(); + + // Index boundaries by their start position so the walk below can look up the + // prepend-stable section key (start-of-local-day). Keying the divider by + // start-of-day, not by the first message, keeps the day section from + // remounting when older messages prepend into it. + const dayBoundariesByStartIndex = new Map( + buildDayGroupBoundaries(entries.map((entry) => entry.message)).map( + (boundary: DayGroupBoundary) => [boundary.startIndex, boundary] as const, + ), + ); + + for (let i = 0; i < entries.length; i++) { + const entry = entries[i]; + const { message } = entry; + const renderKey = entryRenderKey(entry); + + const dayBoundary = dayBoundariesByStartIndex.get(i); + if (dayBoundary) { + items.push({ + kind: "day-divider", + key: dayBoundary.key, + headingTimestamp: message.createdAt, + }); + } + + if (shouldRenderUnreadDivider(i, message.id, firstUnreadMessageId)) { + items.push({ kind: "unread-divider", key: `unread-${renderKey}` }); + } + + const kind = message.kind === KIND_SYSTEM_MESSAGE ? "system" : "message"; + indexByMessageId.set(message.id, items.length); + items.push({ kind, key: renderKey, entry }); + } + + return { items, indexByMessageId }; +} diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index cb3bc0b5a..64fddc306 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -307,10 +307,6 @@ export function MessageThreadPanel({ }: MessageThreadPanelProps) { const threadBodyRef = React.useRef(null); const threadContentRef = React.useRef(null); - // Threads don't paginate older history, so this sentinel is never observed - // (the hook's older-history effect bails without a `fetchOlder`). It exists - // only to satisfy the hook's required ref contract. - const threadTopSentinelRef = React.useRef(null); const threadComposerWrapperRef = React.useRef(null); const isOverlay = useIsThreadPanelOverlay(); const isFloatingOverlay = isOverlay && !isSinglePanelView; @@ -372,7 +368,6 @@ export function MessageThreadPanel({ messages: threadMessages, onTargetReached: onScrollTargetResolved, scrollContainerRef: threadBodyRef, - sentinelRef: threadTopSentinelRef, targetMessageId: scrollTargetId, }); @@ -392,7 +387,6 @@ export function MessageThreadPanel({ ref={threadBodyRef} >
-
void; @@ -107,6 +111,10 @@ type ChannelIntro = { * message list. Must be module-level so its identity never changes. */ const EMPTY_MESSAGES: TimelineMessage[] = []; +/** Stable empty id->index map for the convergence adapter before the first + * item stream arrives. Module-level so its identity never changes. */ +const EMPTY_INDEX_MAP: Map = new Map(); + type DirectMessageIntroParticipant = { avatarUrl: string | null; displayName: string; @@ -163,6 +171,45 @@ const MessageTimelineBase = React.forwardRef< const contentRef = React.useRef(null); const topSentinelRef = React.useRef(null); + // The convergence fallback for a windowed-out deep-link target. It's defined + // below (it depends on the anchored-scroll result), so `useAnchoredScroll` + // reads it through a ref via a stable wrapper — letting the hook stay + // virtualizer-agnostic while the consumer owns the convergence machinery. + const convergeToTargetRef = React.useRef<(messageId: string) => boolean>( + () => false, + ); + const convergeToTarget = React.useCallback((messageId: string) => { + return convergeToTargetRef.current(messageId); + }, []); + + // The virtualizer instance and the flattened item stream are owned by the + // child TimelineMessageList (which mounts the VirtualizedList) and reported + // up here so the scroll manager can resolve scroll targets by index. The + // virtualizer reaches us via a ref (its identity is stable across renders, + // but it arrives after first paint); the item stream + id->index map arrive + // as state so a rebuild re-runs the scroll manager's index-model paths. + const virtualizerRef = React.useRef(null); + const handleVirtualizer = React.useCallback((instance: ListVirtualizer) => { + virtualizerRef.current = instance; + }, []); + const getVirtualizer = React.useCallback(() => virtualizerRef.current, []); + const [timelineItems, setTimelineItems] = + React.useState(null); + const handleItems = React.useCallback((result: TimelineItemsResult) => { + setTimelineItems(result); + }, []); + const virtualizerOption = React.useMemo( + () => + timelineItems + ? { + getVirtualizer, + indexByMessageId: timelineItems.indexByMessageId, + itemCount: timelineItems.items.length, + } + : null, + [getVirtualizer, timelineItems], + ); + // 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 @@ -207,20 +254,19 @@ const MessageTimelineBase = React.forwardRef< isAtBottom, newMessageCount, onScroll, + restoreScrollPosition, scrollToBottom, scrollToBottomOnNextUpdate, scrollToMessage, + setLoadOlderRestoreInFlight, } = useAnchoredScroll({ channelId, contentRef, - fetchOlder, - hasOlderMessages, - isFetchingOlder, + convergeToTarget, isLoading: showTimelineSkeleton, messages: deferredMessages, onTargetReached, scrollContainerRef, - sentinelRef: topSentinelRef, targetMessageId, }); @@ -232,6 +278,48 @@ const MessageTimelineBase = React.forwardRef< [scrollToBottomOnNextUpdate], ); + // Role 3 — jump-to-message into windowed-out history. The DOM-based + // `scrollToMessage` no-ops when the target row isn't mounted (virtualized + // out), so when it fails and the timeline is virtualized we drive the + // convergence adapter: it scrolls the virtualizer to the target index, + // re-aiming each frame as rows mount and measure, then on settle the row is + // in the DOM and `scrollToMessage` centers + highlights it. When there's no + // virtualizer (e.g. the thread panel), there's nothing to converge — the DOM + // path is the whole story and a missing row simply isn't reachable. + const { scrollToMessage: convergeToMessage, cancel: cancelConvergence } = + useConvergentScrollToMessage(getVirtualizer, { + indexByMessageId: timelineItems?.indexByMessageId ?? EMPTY_INDEX_MAP, + align: "center", + onConverged: (messageId) => { + scrollToMessage(messageId, { highlight: true }); + onTargetReached?.(messageId); + }, + }); + const jumpToMessage = React.useCallback( + (messageId: string, options?: { behavior?: ScrollBehavior }) => { + if (scrollToMessage(messageId, { highlight: true, ...options })) { + return; + } + if (virtualizerOption) { + convergeToMessage(messageId); + } + }, + [convergeToMessage, scrollToMessage, virtualizerOption], + ); + // Feed the windowed-out deep-link fallback back into `useAnchoredScroll`, + // which calls it when a target row isn't in the DOM. Gated on the virtualizer + // so the thread panel (no virtualizer) never converges. Assigned in an effect + // because `useAnchoredScroll` reads it asynchronously from a post-mount effect. + React.useEffect(() => { + convergeToTargetRef.current = virtualizerOption + ? convergeToMessage + : () => false; + }, [convergeToMessage, virtualizerOption]); + // Abandon any in-flight convergence on channel switch so a stale loop can't + // hijack the new channel's scroll position. + // biome-ignore lint/correctness/useExhaustiveDependencies: cancel on channel switch only + React.useEffect(() => cancelConvergence, [channelId, cancelConvergence]); + // The unread pill is a transient, per-open affordance: dismiss it once the // user acts on it (jumps to the oldest unread) or catches up by reaching the // bottom of the timeline. Reset when the channel changes so a freshly opened @@ -261,13 +349,14 @@ const MessageTimelineBase = React.forwardRef< const handleJumpToOldestUnread = React.useCallback(() => { setIsUnreadPillDismissed(true); if (firstUnreadMessageId) { - scrollToMessage(firstUnreadMessageId); + jumpToMessage(firstUnreadMessageId); } - }, [firstUnreadMessageId, scrollToMessage]); + }, [firstUnreadMessageId, jumpToMessage]); - // Scroll to the active search match when it changes. `scrollToMessage` - // updates the scroll anchor, so the post-commit restore won't yank the - // view back off the match. + // Scroll to the active search match when it changes. `jumpToMessage` updates + // the scroll anchor (so the post-commit restore won't yank the view back off + // the match) and, when virtualized, converges on the target through the index + // model — the row may be windowed out of the DOM. const prevSearchActiveRef = React.useRef(null); React.useEffect(() => { if (showTimelineSkeleton) return; @@ -279,8 +368,19 @@ const MessageTimelineBase = React.forwardRef< return; } prevSearchActiveRef.current = searchActiveMessageId; - scrollToMessage(searchActiveMessageId, { behavior: "smooth" }); - }, [scrollToMessage, searchActiveMessageId, showTimelineSkeleton]); + jumpToMessage(searchActiveMessageId, { behavior: "smooth" }); + }, [jumpToMessage, searchActiveMessageId, showTimelineSkeleton]); + + useLoadOlderOnScroll({ + fetchOlder, + hasOlderMessages, + isLoading: showTimelineSkeleton, + restoreScrollPosition, + setLoadOlderRestoreInFlight, + scrollContainerRef, + sentinelRef: topSentinelRef, + virtualizer: virtualizerOption, + }); const timelineSkeletonRows = useTimelineSkeletonRows({ channelId, @@ -326,11 +426,17 @@ const MessageTimelineBase = React.forwardRef< >
- {isFetchingOlder ? ( -
+ {/* Fixed-height slot: an always-mounted height keeps the virtual + spacer's offset stable across the load-older fetch toggle, so + `scrollMargin` doesn't shift mid-fetch and yank the restore. */} +
+ {isFetchingOlder ? ( -
- ) : null} + ) : null} +
) : null} diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index d968dc5b7..4e91a01a8 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -1,24 +1,26 @@ import * as React from "react"; +import { formatDayHeading } from "@/features/messages/lib/dateFormatters"; import { - formatDayHeading, - isSameDay, - startOfLocalDaySeconds, -} from "@/features/messages/lib/dateFormatters"; + buildTimelineItems, + getTimelineItemKey, + type TimelineItem, + type TimelineItemsResult, +} from "@/features/messages/lib/timelineItems"; +import { buildMainTimelineEntries } from "@/features/messages/lib/threadPanel"; +import type { MainTimelineEntry } from "@/features/messages/lib/threadPanel"; import { - buildMainTimelineEntries, - shouldRenderUnreadDivider, -} from "@/features/messages/lib/threadPanel"; -import { - buildVideoReviewCommentsForRoot, + buildVideoReviewCommentsByRootId, buildVideoReviewContextForMessage, - hasVideoAttachment, } from "@/features/messages/lib/videoReviewContext"; import type { TimelineMessage } from "@/features/messages/types"; import type { UserProfileLookup } from "@/features/profile/lib/identity"; import type { ChannelType } from "@/shared/api/types"; -import { KIND_SYSTEM_MESSAGE } from "@/shared/constants/kinds"; import { cn } from "@/shared/lib/cn"; +import { + type ListVirtualizer, + VirtualizedList, +} from "@/shared/ui/VirtualizedList"; import { DayDivider } from "./DayDivider"; import { MessageRow } from "./MessageRow"; import { MessageThreadSummaryRow } from "./MessageThreadSummaryRow"; @@ -67,78 +69,15 @@ type TimelineMessageListProps = { searchQuery?: string; /** Per-thread unread counts keyed by thread root id. */ threadUnreadCounts?: ReadonlyMap; + /** Caller-owned scroll container the virtualizer measures and scrolls. */ + scrollContainerRef: React.RefObject; + /** Receives the flattened item stream + index map so the scroll manager can + * resolve scroll targets by id. Called whenever the stream is rebuilt. */ + onItems?: (result: TimelineItemsResult) => void; + /** Receives the virtualizer instance for index-model scroll paths. */ + onVirtualizer?: (virtualizer: ListVirtualizer) => void; }; -type TimelineDayRow = { - key: string; - label: string; - type: "day"; -}; - -type TimelineUnreadRow = { - key: string; - type: "unread"; -}; - -type TimelineMessageRowModel = { - key: string; - message: TimelineMessage; - summary: ReturnType[number]["summary"]; - type: "message"; -}; - -type TimelineRenderRow = - | TimelineDayRow - | TimelineUnreadRow - | TimelineMessageRowModel; - -function buildTimelineRenderRows({ - firstUnreadMessageId, - messages, -}: { - firstUnreadMessageId: string | null; - messages: TimelineMessage[]; -}): TimelineRenderRow[] { - const entries = buildMainTimelineEntries(messages); - const rows: TimelineRenderRow[] = []; - let previousMessage: TimelineMessage | null = null; - - for (let i = 0; i < entries.length; i++) { - const { message, summary } = entries[i]; - const messageRenderKey = message.renderKey ?? message.id; - - if ( - !previousMessage || - !isSameDay(previousMessage.createdAt, message.createdAt) - ) { - rows.push({ - key: `day-${startOfLocalDaySeconds(message.createdAt)}`, - label: formatDayHeading(message.createdAt), - type: "day", - }); - } - - // The unread "New" divider only marks a read/unread boundary when there is - // a message above the first unread. When the first unread is the first - // rendered top-level entry (fresh/never-read channel), there is nothing - // above to separate from, so it is suppressed. - if (shouldRenderUnreadDivider(i, message.id, firstUnreadMessageId)) { - rows.push({ key: `unread:${messageRenderKey}`, type: "unread" }); - } - - rows.push({ - key: `msg:${messageRenderKey}`, - message, - summary, - type: "message", - }); - - previousMessage = message; - } - - return rows; -} - export const TimelineMessageList = React.memo(function TimelineMessageList({ agentPubkeys, channelId, @@ -164,144 +103,243 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ searchQuery, threadUnreadCounts, unfollowThreadById, + scrollContainerRef, + onItems, + onVirtualizer, }: TimelineMessageListProps) { - const rows = React.useMemo( - () => buildTimelineRenderRows({ firstUnreadMessageId, messages }), - [firstUnreadMessageId, messages], + const entries = React.useMemo( + () => buildMainTimelineEntries(messages), + [messages], ); - return rows.map((row) => ( - buildVideoReviewCommentsByRootId(messages), + [messages], + ); + // Contexts are memoized per message id so MessageRow/Markdown memo + // comparisons hold across unrelated timeline re-renders (typing + // indicators, presence updates) — a fresh context object per render would + // defeat the memo and re-render every video message on every pass. + const videoReviewContextById = React.useMemo(() => { + const contexts = new Map< + string, + NonNullable> + >(); + for (const message of messages) { + const comments = reviewCommentsByRootId.get(message.id) ?? []; + const context = buildVideoReviewContextForMessage({ + channelId, + channelName, + channelType, + comments, + isSendingVideoReviewComment, + message, + onSendVideoReviewComment, + onToggleReaction, + profiles, + }); + if (context) { + contexts.set(message.id, context); } - channelId={channelId} - channelName={channelName} - channelType={channelType} - currentPubkey={currentPubkey} - followThreadById={followThreadById} - highlightedMessageId={highlightedMessageId} - isFollowingThreadById={isFollowingThreadById} - isSendingVideoReviewComment={isSendingVideoReviewComment} - key={row.key} - messageFooters={messageFooters} - onDelete={onDelete} - onEdit={onEdit} - onMarkUnread={onMarkUnread} - onReply={onReply} - onSendVideoReviewComment={onSendVideoReviewComment} - onToggleReaction={onToggleReaction} - profiles={profiles} - row={row} - searchActiveMessageId={searchActiveMessageId} - searchMatchingMessageIds={searchMatchingMessageIds} - searchQuery={searchQuery} - threadUnreadCounts={threadUnreadCounts} - unfollowThreadById={unfollowThreadById} + } + return contexts; + }, [ + channelId, + channelName, + channelType, + isSendingVideoReviewComment, + messages, + onSendVideoReviewComment, + onToggleReaction, + profiles, + reviewCommentsByRootId, + ]); + + // The flattened item stream and its messageId -> itemIndex map are produced + // together from ONE memo, keyed on the entries and the unread boundary (the + // unread divider is its own item, so it shifts indices). A separate memo with + // diverging deps would let the map go stale and scroll deep-links to the wrong + // row — the exact failure virtualization risks. + const itemsResult = React.useMemo( + () => buildTimelineItems(entries, firstUnreadMessageId), + [entries, firstUnreadMessageId], + ); + + React.useEffect(() => { + onItems?.(itemsResult); + }, [itemsResult, onItems]); + + const renderItem = React.useCallback( + (item: TimelineItem) => { + switch (item.kind) { + case "day-divider": + // Heading is resolved at render time (not baked into the item) so + // "Today"/"Yesterday" track the wall clock, not build time. + return ; + case "unread-divider": + return ; + case "system": + return ( + + ); + case "message": + return ( + + ); + } + }, + [ + agentPubkeys, + channelId, + currentPubkey, + followThreadById, + highlightedMessageId, + isFollowingThreadById, + messageFooters, + onDelete, + onEdit, + onMarkUnread, + onReply, + onToggleReaction, + profiles, + searchActiveMessageId, + searchMatchingMessageIds, + searchQuery, + threadUnreadCounts, + unfollowThreadById, + videoReviewContextById, + ], + ); + + return ( + - )); + ); }); -type TimelineRenderRowViewProps = Omit< +function SystemRow({ + currentPubkey, + entry, + footer, + onToggleReaction, + profiles, +}: { + currentPubkey?: string; + entry: MainTimelineEntry; + footer: React.ReactNode; + onToggleReaction?: TimelineMessageListProps["onToggleReaction"]; + profiles?: UserProfileLookup; +}) { + return ( +
+ + {footer} +
+ ); +} + +type MessageRowItemProps = Pick< TimelineMessageListProps, - "firstUnreadMessageId" | "messages" | "personaLookup" + | "agentPubkeys" + | "channelId" + | "currentPubkey" + | "followThreadById" + | "highlightedMessageId" + | "isFollowingThreadById" + | "onDelete" + | "onEdit" + | "onMarkUnread" + | "onReply" + | "onToggleReaction" + | "profiles" + | "searchActiveMessageId" + | "searchMatchingMessageIds" + | "searchQuery" + | "threadUnreadCounts" + | "unfollowThreadById" > & { - allMessages?: TimelineMessage[]; - row: TimelineRenderRow; + entry: MainTimelineEntry; + footer: React.ReactNode; + videoReviewContext: ReturnType; }; -const TimelineRenderRowView = React.memo(function TimelineRenderRowView({ +function MessageRowItem({ agentPubkeys, - allMessages, channelId, - channelName, - channelType, currentPubkey, + entry, followThreadById, - highlightedMessageId = null, + footer, + highlightedMessageId, isFollowingThreadById, - isSendingVideoReviewComment = false, - messageFooters, onDelete, onEdit, onMarkUnread, onReply, - onSendVideoReviewComment, onToggleReaction, profiles, - searchActiveMessageId = null, + searchActiveMessageId, searchMatchingMessageIds, searchQuery, - row, threadUnreadCounts, unfollowThreadById, -}: TimelineRenderRowViewProps) { - const messageForContext = row.type === "message" ? row.message : null; - const videoReviewContext = React.useMemo(() => { - if (!allMessages || !messageForContext) { - return undefined; - } - - return buildVideoReviewContextForMessage({ - channelId, - channelName, - channelType, - comments: buildVideoReviewCommentsForRoot( - allMessages, - messageForContext.id, - ), - isSendingVideoReviewComment, - message: messageForContext, - onSendVideoReviewComment, - onToggleReaction, - profiles, - }); - }, [ - allMessages, - channelId, - channelName, - channelType, - isSendingVideoReviewComment, - messageForContext, - onSendVideoReviewComment, - onToggleReaction, - profiles, - ]); - - if (row.type === "day") { - return ; - } - - if (row.type === "unread") { - return ; - } - - const { message, summary } = row; - - if (message.kind === KIND_SYSTEM_MESSAGE) { - const footer = messageFooters?.[message.id] ?? null; - return ( -
- - {footer} -
- ); - } + videoReviewContext, +}: MessageRowItemProps) { + const { message, summary } = entry; + const canDelete = + onDelete && currentPubkey && message.pubkey === currentPubkey + ? onDelete + : undefined; + const canEdit = + onEdit && currentPubkey && message.pubkey === currentPubkey + ? onEdit + : undefined; if (summary && onReply) { - const footer = messageFooters?.[message.id] ?? null; const isHighlighted = message.id === highlightedMessageId; return (
followThreadById(message.id) : undefined } @@ -357,25 +387,16 @@ const TimelineRenderRowView = React.memo(function TimelineRenderRowView({ const isSearchMatch = searchMatchingMessageIds?.has(message.id) ?? false; const isSearchActive = message.id === searchActiveMessageId; - const footer = messageFooters?.[message.id] ?? null; return ( -
+
); -}); +} diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index ec67c7138..0157365b7 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -20,9 +20,6 @@ type UseAnchoredScrollOptions = { /** Inner content element — must wrap every renderable row, including the * sentinel and bottom anchor. Used to schedule layout work on resize. */ contentRef: React.RefObject; - /** Small zero-height element near the very top of the content. When it - * intersects the viewport (with some rootMargin) we trigger fetchOlder. */ - sentinelRef: React.RefObject; /** Resets when changed; lets us drop anchor + scroll state across channels. */ channelId?: string | null; /** Suppresses initial scroll-to-bottom while a skeleton is showing. */ @@ -30,20 +27,17 @@ type UseAnchoredScrollOptions = { /** Source of truth for the rendered list. Used to detect new-at-bottom * arrivals and to seed/refresh the anchor pre-render. */ messages: TimelineMessage[]; - /** Optional callback to fetch older history. The hook handles intersection, - * debouncing, and post-prepend scroll restoration via the anchor. */ - fetchOlder?: () => Promise; - hasOlderMessages?: boolean; - /** True while an older-history fetch is in flight. The fetch spinner renders - * above the anchor, so toggling it shifts every row below it. The spinner - * toggles on its own commit (no message change), so without this signal the - * restoration effect — keyed on `messages` — wouldn't re-run to correct the - * shift, leaving a visible one-frame jump. Threading it through makes the - * anchor the single owner of every layout change above the reader's eye. */ - isFetchingOlder?: boolean; /** When set, scroll to and highlight this message on mount and on change. */ targetMessageId?: string | null; onTargetReached?: (messageId: string) => void; + /** Optional convergence fallback for a `targetMessageId` whose row is not in + * the DOM (windowed out of a virtualized list). When the DOM lookup fails, + * the hook delegates to this instead of waiting for a later commit that may + * never render the row. The consumer drives the virtualizer to the target, + * warming up if it's still being fetched, and fires `onTargetReached` itself + * on settle. Returns `true` once it owns the target (the hook marks it + * handled, no further dispatch). Absent (thread panel) → DOM-only retry. */ + convergeToTarget?: (messageId: string) => boolean; }; type UseAnchoredScrollResult = { @@ -67,6 +61,15 @@ type UseAnchoredScrollResult = { messageId: string, options?: { highlight?: boolean; behavior?: ScrollBehavior }, ) => boolean; + /** Single-writer scroll restore for the load-older index path. Sets + * `scrollTop` directly (no scroll event fires for a programmatic write), + * then re-seats the anchor + at-bottom bookkeeping so the next passive + * restore and `isAtBottom` read agree with where we put the scroll. */ + restoreScrollPosition: (scrollTop: number) => void; + /** Brackets the load-older index restore's scroll ownership. While `true`, + * the ResizeObserver cedes — the index path is the sole `scrollTop` writer + * across the prepend, mirroring the `convergingTargetIdRef` cede. */ + setLoadOlderRestoreInFlight: (inFlight: boolean) => void; }; function isAtBottomNow(container: HTMLDivElement) { @@ -208,15 +211,12 @@ function restoreAnchorToMessage( export function useAnchoredScroll({ scrollContainerRef, contentRef, - sentinelRef, channelId, isLoading, messages, - fetchOlder, - hasOlderMessages = false, - isFetchingOlder = false, targetMessageId = null, onTargetReached, + convergeToTarget, }: UseAnchoredScrollOptions): UseAnchoredScrollResult { // Anchor lives in a ref because it must survive renders and is updated // both on scroll (commit-time read) and in the layout effect (post-render @@ -234,10 +234,33 @@ export function useAnchoredScroll({ >(null); const hasInitializedRef = React.useRef(false); + // Mirror the convergence fallback into a ref so the target effects read the + // live callback without re-subscribing on every consumer render. + const convergeToTargetRef = React.useRef(convergeToTarget); + convergeToTargetRef.current = convergeToTarget; const prevLastMessageIdRef = React.useRef(undefined); + // Tracks the FRONT (oldest) rendered id so the restore effect can detect a + // load-older prepend (front changed, tail unchanged) and cede it to the + // index path — see IMPORTANT #1 in the restore effect below. + const prevFirstMessageIdRef = React.useRef(undefined); const prevMessageCountRef = React.useRef(0); - const fetchingOlderRef = React.useRef(false); const handledTargetIdRef = React.useRef(null); + // Set while a convergence loop owns the scroll position (jump-to-message into + // windowed-out history). The library's reconcile loop is the sole writer + // during convergence, so the anchored restore below must cede — otherwise its + // at-bottom `scrollTo` would yank the view back as the target row splices in, + // the same two-writer contention the prepend bail prevents. Cleared when the + // target settles (consumer clears the route param → `targetMessageId` null). + const convergingTargetIdRef = React.useRef(null); + // Set while `useLoadOlderOnScroll`'s index-restore loop owns scroll across a + // load-older prepend. That loop drives the virtualizer to re-aim the anchored + // row as the prepended rows measure; the ResizeObserver must cede for the + // same reason it cedes during convergence — otherwise the prepended rows + // growing `scrollHeight` fire the observer with the (now windowed-out) anchor, + // its all-gone fallback pins to the floor, and stomps the index restore's + // correct offset. The layout effect already cedes the prepend (isPrepend + // bail); this is the matching cede for the non-React-driven observer writer. + const loadOlderRestoreInFlightRef = React.useRef(false); const highlightTimeoutRef = React.useRef(null); // One-shot: the consumer calls `scrollToBottomOnNextUpdate()` right before // it sends a message (see ChannelPane). When the user's own message then @@ -256,9 +279,11 @@ export function useAnchoredScroll({ setHighlightedMessageId(null); hasInitializedRef.current = false; prevLastMessageIdRef.current = undefined; + prevFirstMessageIdRef.current = undefined; prevMessageCountRef.current = 0; - fetchingOlderRef.current = false; handledTargetIdRef.current = null; + convergingTargetIdRef.current = null; + loadOlderRestoreInFlightRef.current = false; forceBottomOnNextAppendRef.current = false; if (highlightTimeoutRef.current !== null) { window.clearTimeout(highlightTimeoutRef.current); @@ -346,6 +371,42 @@ export function useAnchoredScroll({ [scrollContainerRef], ); + // Re-seat the anchor + at-bottom bookkeeping after a programmatic scrollTop + // write. A programmatic write fires no scroll event, so `onScroll` won't run + // to refresh `anchorRef`/`isAtBottom` — we run the same derivation here so + // the next passive restore and at-bottom read agree with the new position. + // We deliberately do NOT touch `newMessageCount`: a load-older restore keeps + // the reader mid-history, so the unread-count affordance must be untouched. + const syncAnchorAfterProgrammaticScroll = React.useCallback( + (container: HTMLDivElement) => { + anchorRef.current = computeAnchor(container); + const atBottom = anchorRef.current.kind === "at-bottom"; + setIsAtBottom((prev) => (prev === atBottom ? prev : atBottom)); + }, + [], + ); + + // Single-writer restore for the load-older index path (IMPORTANT #2). The + // index path resolves the exact target `scrollTop` off the virtualizer's + // settled measurement cache (`getOffsetForIndex`), so a bare assignment is + // correct on the first write — no rAF re-assert loop, no manager scroll-state + // machine. Re-seating the anchor afterwards keeps this the sole owner. + const restoreScrollPosition = React.useCallback( + (scrollTop: number) => { + const container = scrollContainerRef.current; + if (!container) return; + container.scrollTop = scrollTop; + syncAnchorAfterProgrammaticScroll(container); + }, + [scrollContainerRef, syncAnchorAfterProgrammaticScroll], + ); + + // Let the load-older index path mark its scroll-ownership window so the + // ResizeObserver cedes to it (see `loadOlderRestoreInFlightRef`). + const setLoadOlderRestoreInFlight = React.useCallback((inFlight: boolean) => { + loadOlderRestoreInFlightRef.current = inFlight; + }, []); + // Scroll handler: recompute anchor + bottom state from the current // scroll position. Cheap enough to run on every scroll event — a single // `getBoundingClientRect` walk plus rect reads. @@ -363,10 +424,11 @@ export function useAnchoredScroll({ // --------------------------------------------------------------------------- // Anchor restoration: after every render, if the anchor was a message, // realign so that message sits at the same top-relative offset it had - // before the render. This is the single mechanism for keeping scroll - // stable across prepends, appends, image loads, embed expansions, etc. + // before the render. This keeps scroll stable across appends, image loads, + // and embed expansions. Load-older prepends are NOT handled here — they are + // ceded to `useLoadOlderOnScroll`'s index anchor (see the prepend bail + // below) so a single writer owns `scrollTop` on the prepend commit. // --------------------------------------------------------------------------- - // biome-ignore lint/correctness/useExhaustiveDependencies: `isFetchingOlder` is an intentional re-run trigger, not a read — the fetch spinner renders above the anchor on its own commit (with `messages` unchanged), so we re-run restoration on its toggle to correct the spinner-induced shift via the existing anchor. React.useLayoutEffect(() => { const container = scrollContainerRef.current; if (!container) return; @@ -403,16 +465,49 @@ export function useAnchoredScroll({ } hasInitializedRef.current = true; prevLastMessageIdRef.current = messages[messages.length - 1]?.id; + prevFirstMessageIdRef.current = messages[0]?.id; prevMessageCountRef.current = messages.length; return; } const anchor = anchorRef.current; const lastMessage = messages[messages.length - 1]; + const firstMessage = messages[0]; const prevLastId = prevLastMessageIdRef.current; + const prevFirstId = prevFirstMessageIdRef.current; const prevCount = prevMessageCountRef.current; const newLatestArrived = lastMessage !== undefined && lastMessage.id !== prevLastId; + // A convergence loop owns the scroll position while jumping to a windowed-out + // target (its library reconcile is the sole writer). Cede every restore + // branch to it — an at-bottom `scrollTo` here would chase the view back to + // the bottom as the target's neighbours splice into the window mid-converge. + // Refresh the tracked refs so the first post-settle commit isn't misread as + // a prepend/append. Cleared when the target settles (targetMessageId null). + if (convergingTargetIdRef.current !== null) { + prevLastMessageIdRef.current = lastMessage?.id; + prevFirstMessageIdRef.current = firstMessage?.id; + prevMessageCountRef.current = messages.length; + return; + } + // A load-older prepend grows the list at the FRONT while the tail is + // unchanged. `useLoadOlderOnScroll` owns the prepend restore via its index + // anchor (the single `scrollTop` writer). If this restore effect also ran + // its anchored `scrollBy` on the same commit, two writers would fight over + // `scrollTop`. So cede the prepend to the index path: refresh the tracked + // refs and bail before the anchored branch. (Append and in-window reflow + // leave the front id unchanged, so they fall through as before.) + const isPrepend = + firstMessage !== undefined && + prevFirstId !== undefined && + firstMessage.id !== prevFirstId && + !newLatestArrived; + if (isPrepend) { + prevLastMessageIdRef.current = lastMessage?.id; + prevFirstMessageIdRef.current = firstMessage.id; + prevMessageCountRef.current = messages.length; + return; + } // One-shot: an outbound send armed `scrollToBottomOnNextUpdate`. When the // resulting append lands, snap to bottom regardless of the current anchor, @@ -425,6 +520,7 @@ export function useAnchoredScroll({ setIsAtBottom(true); setNewMessageCount(0); prevLastMessageIdRef.current = lastMessage?.id; + prevFirstMessageIdRef.current = firstMessage?.id; prevMessageCountRef.current = messages.length; return; } @@ -451,9 +547,9 @@ export function useAnchoredScroll({ } prevLastMessageIdRef.current = lastMessage?.id; + prevFirstMessageIdRef.current = firstMessage?.id; prevMessageCountRef.current = messages.length; }, [ - isFetchingOlder, isLoading, messages, onTargetReached, @@ -463,71 +559,6 @@ export function useAnchoredScroll({ targetMessageId, ]); - // --------------------------------------------------------------------------- - // Older-history loader. IntersectionObserver on the top sentinel; when it - // crosses into view (with a 200px rootMargin so we preload a bit early) - // we fire `fetchOlder`. The anchor restoration above handles the prepend - // — we don't need to compute or apply a scrollHeight delta ourselves. - // --------------------------------------------------------------------------- - React.useEffect(() => { - const sentinel = sentinelRef.current; - const container = scrollContainerRef.current; - if ( - !sentinel || - !container || - !fetchOlder || - isLoading || - !hasOlderMessages - ) { - return; - } - - let disposed = false; - let observer: IntersectionObserver | null = null; - - const start = () => { - if (disposed) return; - observer = new IntersectionObserver( - ([entry]) => { - if (!entry?.isIntersecting || disposed || fetchingOlderRef.current) { - return; - } - fetchingOlderRef.current = true; - observer?.disconnect(); - - // Before the fetch, capture the anchor from the current scroll - // position. The layout effect after re-render will use it. - anchorRef.current = computeAnchor(container); - - void fetchOlder() - .catch(() => { - // Swallow; the next intersection will retry. We don't want - // to crash the observer chain on a transient relay error. - }) - .finally(() => { - fetchingOlderRef.current = false; - // Re-observe in case there's more history to load. - start(); - }); - }, - { root: container, rootMargin: "200px 0px 0px 0px" }, - ); - observer.observe(sentinel); - }; - - start(); - return () => { - disposed = true; - observer?.disconnect(); - }; - }, [ - fetchOlder, - hasOlderMessages, - isLoading, - scrollContainerRef, - sentinelRef, - ]); - // --------------------------------------------------------------------------- // Content resize: when fonts load late, an image decodes, an embed expands, // or any in-viewport reflow happens that React isn't driving (so the @@ -546,8 +577,28 @@ export function useAnchoredScroll({ const observer = new ResizeObserver(() => { const container = scrollContainerRef.current; if (!container) return; + // Cede entirely while a convergence loop owns scroll (jump to a + // windowed-out target). Mid-jump the anchor is transiently `at-bottom` + // — `computeAnchor` finds no crossing row until the virtualizer renders + // rows at the new offset — so an unconditional re-pin here would yank + // the in-flight jump down to the floor as rows measure. The convergence + // loop is the sole writer until it settles (mirrors the layout effect's + // `convergingTargetIdRef` bail). + if (convergingTargetIdRef.current !== null) return; + // Cede while the load-older index restore owns scroll. The prepended rows + // measuring late is exactly what grows `scrollHeight` and fires this + // observer; if it re-pinned here the anchor row is already windowed out, + // so the all-gone fallback would pin to the floor and stomp the index + // restore's correct offset. The index loop is the sole writer until it + // settles (mirrors the layout effect's isPrepend bail). + if (loadOlderRestoreInFlightRef.current) return; const anchor = anchorRef.current; if (anchor.kind === "at-bottom") { + // Stuck to bottom: re-pin to the new floor. Virtualizer measurement + // grows `scrollHeight` after the initial pin (rows below the fold + // measure a frame or two late) without any `messages` change to drive + // the layout effect, so this observer is the only thing that keeps the + // view glued to the bottom as content settles. container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); return; } @@ -583,6 +634,7 @@ export function useAnchoredScroll({ React.useEffect(() => { if (!targetMessageId) { handledTargetIdRef.current = null; + convergingTargetIdRef.current = null; return; } if (handledTargetIdRef.current === targetMessageId || isLoading) return; @@ -593,7 +645,23 @@ export function useAnchoredScroll({ const el = container.querySelector( `[data-message-id="${targetMessageId}"]`, ); - if (!el) return; // Row not rendered yet; a later `messages` commit retries. + if (!el) { + // Row not in the DOM. In a virtualized list it may be windowed out and + // never render from a passive commit, so delegate to the convergence + // fallback: it drives the virtualizer to the target (warming up if a + // deep-link target is still being fetched in) and, on settle, centers + + // highlights it and fires `onTargetReached`. We mark the target handled + // here so this effect stops re-dispatching, but deliberately do NOT fire + // `onTargetReached` yet — clearing the route param now would cancel the + // in-flight target fetch the loop is waiting on. Without a fallback + // (thread panel), leave the target for a later `messages` commit. + const converge = convergeToTargetRef.current; + if (converge?.(targetMessageId)) { + handledTargetIdRef.current = targetMessageId; + convergingTargetIdRef.current = targetMessageId; + } + return; + } handledTargetIdRef.current = targetMessageId; scrollToMessageImperative(targetMessageId, { highlight: true }); onTargetReached?.(targetMessageId); @@ -622,5 +690,7 @@ export function useAnchoredScroll({ scrollToBottom: scrollToBottomImperative, scrollToBottomOnNextUpdate, scrollToMessage: scrollToMessageImperative, + restoreScrollPosition, + setLoadOlderRestoreInFlight, }; } diff --git a/desktop/src/features/messages/ui/useConvergentScrollToMessage.ts b/desktop/src/features/messages/ui/useConvergentScrollToMessage.ts new file mode 100644 index 000000000..4a51c8c63 --- /dev/null +++ b/desktop/src/features/messages/ui/useConvergentScrollToMessage.ts @@ -0,0 +1,192 @@ +import * as React from "react"; + +import { + type ConvergenceAlign, + CONVERGENCE_FRAME_CAP, + convergenceStep, +} from "@/features/messages/lib/scrollConvergence"; +import type { ListVirtualizer } from "@/shared/ui/VirtualizedList"; + +/** Offset (px) within which the library is considered to have reached the target. */ +const SETTLE_TOLERANCE_PX = 2; + +type ConvergentScrollOptions = { + /** Live message-id -> item-index map, rebuilt with the flattened item stream. */ + indexByMessageId: Map; + /** Where the target should land in the viewport. */ + align: ConvergenceAlign; + /** Fired on the settled frame once the target row has converged. */ + onConverged?: (messageId: string) => void; + /** Fired when the loop stops without converging (target deleted, or frame cap). */ + onAbandoned?: (messageId: string) => void; +}; + +type ConvergentScrollController = { + /** + * Begins a convergence loop toward `messageId`. Returns `true` (the loop + * always starts and owns the target). When the id isn't yet in the data — a + * deep-link target the route screen fetches asynchronously — the loop warms + * up by polling the live map, then converges once it lands; if it never + * lands within the frame cap it abandons (clears the highlight), the same + * terminal state as a target deleted mid-settle. A new call cancels any + * in-flight loop. + */ + scrollToMessage: (messageId: string) => boolean; + /** Cancels any in-flight convergence loop (e.g. on unmount or channel switch). */ + cancel: () => void; +}; + +/** + * Drives @tanstack/react-virtual to settle on an off-screen message by id. + * + * The library already converges OFFSETS: one `scrollToIndex(i)` captures index + * `i` and its `reconcileScroll` rAF loop re-aims as rows mount and measure. But + * it chases the INDEX captured at call time — a prepend/delete mid-settle leaves + * it on the wrong row. This adapter closes that gap: each frame it re-resolves + * the target's CURRENT index from the live map (the pure `convergenceStep` + * reducer owns the decision) and re-issues `scrollToIndex` ONLY when the index + * moved. In steady state it issues nothing, so it never resets the library's + * internal stable-frame counter and the library settles in one frame. + * + * Settle detection is a trivial offset-equality check (NOT the convergence math, + * which the library owns): the measured offset for the current index is within + * tolerance of where the library would place it, and the offset is unchanged + * from the prior frame. + */ +export function useConvergentScrollToMessage( + getVirtualizer: () => ListVirtualizer | null, + { + indexByMessageId, + align, + onConverged, + onAbandoned, + }: ConvergentScrollOptions, +): ConvergentScrollController { + // Mirror inputs into refs so the rAF loop closure always reads live values + // without re-subscribing the loop each render. + const mapRef = React.useRef(indexByMessageId); + mapRef.current = indexByMessageId; + const alignRef = React.useRef(align); + alignRef.current = align; + const onConvergedRef = React.useRef(onConverged); + onConvergedRef.current = onConverged; + const onAbandonedRef = React.useRef(onAbandoned); + onAbandonedRef.current = onAbandoned; + + const rafIdRef = React.useRef(null); + + const cancel = React.useCallback(() => { + if (rafIdRef.current !== null) { + cancelAnimationFrame(rafIdRef.current); + rafIdRef.current = null; + } + }, []); + + const scrollToMessage = React.useCallback( + (messageId: string) => { + cancel(); + + let lastIssuedIndex: number | null = null; + let previousOffset: number | null = null; + let framesUsed = 0; + // The id->index map is rebuilt one render after `messages` changes, so a + // freshly-spliced deep-link target (the route screen fetches it by id + // asynchronously) can be absent from the map on the frame this starts. + // Poll the live map during a bounded warmup instead of bailing; once it + // resolves, hand off to the normal convergence loop. If it never resolves + // within the cap, abandon — same terminal state as a deleted target. + let resolved = mapRef.current.has(messageId); + + const frame = () => { + rafIdRef.current = null; + const virtualizer = getVirtualizer(); + if (!virtualizer) { + return; + } + + const currentIndex = mapRef.current.get(messageId); + if (!resolved) { + if (currentIndex === undefined) { + if (framesUsed + 1 >= CONVERGENCE_FRAME_CAP) { + onAbandonedRef.current?.(messageId); + return; + } + framesUsed += 1; + previousOffset = virtualizer.scrollOffset ?? 0; + rafIdRef.current = requestAnimationFrame(frame); + return; + } + resolved = true; + } + + // The library has settled this frame when its offset reached the target + // index's offset (within tolerance) and stopped moving. `currentIndex` + // is re-read so a settle on a stale index never counts as converged. + let librarySettled = false; + let stalledOffTarget = false; + if (currentIndex !== undefined && lastIssuedIndex === currentIndex) { + const offset = virtualizer.scrollOffset ?? 0; + const target = virtualizer.getOffsetForIndex( + currentIndex, + alignRef.current, + ); + const reachedTarget = + target !== undefined && + Math.abs(offset - target[0]) <= SETTLE_TOLERANCE_PX; + const offsetStable = + previousOffset !== null && + Math.abs(offset - previousOffset) <= SETTLE_TOLERANCE_PX; + librarySettled = reachedTarget && offsetStable; + stalledOffTarget = offsetStable && !reachedTarget; + previousOffset = offset; + } else { + previousOffset = virtualizer.scrollOffset ?? 0; + } + + const decision = convergenceStep({ + targetMessageId: messageId, + indexByMessageId: mapRef.current, + lastIssuedIndex, + librarySettled, + stalledOffTarget, + framesUsed, + }); + + if ( + decision.nextIndex !== null && + (decision.nextIndex !== lastIssuedIndex || decision.reissue) + ) { + // Issue when the index moved (re-aim at the new row) OR the reducer + // asks for a same-index re-issue to kick an off-target stall. Reset + // `previousOffset` so the next frame's stability check restarts from + // the post-issue offset rather than treating the stall as settled. + virtualizer.scrollToIndex(decision.nextIndex, { + align: alignRef.current, + }); + lastIssuedIndex = decision.nextIndex; + previousOffset = null; + } + + if (decision.done) { + if (decision.converged) { + onConvergedRef.current?.(messageId); + } else { + onAbandonedRef.current?.(messageId); + } + return; + } + + framesUsed += 1; + rafIdRef.current = requestAnimationFrame(frame); + }; + + rafIdRef.current = requestAnimationFrame(frame); + return true; + }, + [cancel, getVirtualizer], + ); + + React.useEffect(() => cancel, [cancel]); + + return { scrollToMessage, cancel }; +} diff --git a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts new file mode 100644 index 000000000..1f092ac64 --- /dev/null +++ b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts @@ -0,0 +1,251 @@ +import * as React from "react"; + +import { CONVERGENCE_FRAME_CAP } from "@/features/messages/lib/scrollConvergence"; +import type { ListVirtualizer } from "@/shared/ui/VirtualizedList"; + +type UseLoadOlderOnScrollOptions = { + fetchOlder?: () => Promise; + hasOlderMessages: boolean; + isLoading: boolean; + restoreScrollPosition: (scrollTop: number) => void; + /** + * Brackets the index-restore loop's scroll ownership so the anchored hook's + * ResizeObserver cedes for the duration of the prepend. Without it, the + * prepended rows measuring late fire the observer with the windowed-out + * anchor and its all-gone fallback pins the view to the floor, stomping this + * loop's correct re-aim. Only the virtualized (index) path needs it — the + * non-virtualized path's restore is a single synchronous `scrollTop` write. + */ + setLoadOlderRestoreInFlight?: (inFlight: boolean) => void; + scrollContainerRef: React.RefObject; + sentinelRef: React.RefObject; + /** + * When the timeline is virtualized, a prepend shifts every index and a large + * one pushes the anchored row out of the window before it can be re-measured. + * Supplying the virtualizer switches to an index anchor: we hold the + * first-visible row across the prepend by re-aiming `scrollToIndex` at its new + * index (resolved from `indexByMessageId`) until the library settles it. + */ + virtualizer?: { + getVirtualizer: () => ListVirtualizer | null; + indexByMessageId: Map; + itemCount: number; + } | null; +}; + +/** + * Triggers `fetchOlder` when a sentinel element near the top of the scroll + * container enters the viewport, then restores the scroll position so the + * visible content doesn't jump. + */ +export function useLoadOlderOnScroll({ + fetchOlder, + hasOlderMessages, + isLoading, + restoreScrollPosition, + setLoadOlderRestoreInFlight, + scrollContainerRef, + sentinelRef, + virtualizer = null, +}: UseLoadOlderOnScrollOptions) { + const restoreScrollPositionRef = React.useRef(restoreScrollPosition); + React.useEffect(() => { + restoreScrollPositionRef.current = restoreScrollPosition; + }); + // Mirror the cede setter so the long-lived Intersection observer reads the + // live callback without re-subscribing (same rationale as the restore ref). + const setInFlightRef = React.useRef(setLoadOlderRestoreInFlight); + React.useEffect(() => { + setInFlightRef.current = setLoadOlderRestoreInFlight; + }); + // Mirror the virtualizer option into a ref so the long-lived Intersection + // observer reads the live getter + count without re-subscribing per render. + const virtualizerRef = React.useRef(virtualizer); + virtualizerRef.current = virtualizer; + + React.useEffect(() => { + const sentinel = sentinelRef.current; + const container = scrollContainerRef.current; + if ( + !sentinel || + !container || + !fetchOlder || + isLoading || + !hasOlderMessages + ) { + return; + } + + let disposed = false; + let currentObserver: IntersectionObserver | null = null; + + const observe = () => { + if (disposed) { + return; + } + + currentObserver = new IntersectionObserver( + ([entry]) => { + if (!entry.isIntersecting || disposed) { + return; + } + + currentObserver?.disconnect(); + + const virt = virtualizerRef.current; + if (virt) { + // Hold the first VISIBLE row across the prepend. After N older rows + // are prepended the anchored row's INDEX shifts by N and — with + // scrollTop unchanged near the top — it's pushed below the window + // and recycled out of the DOM, so a pure DOM re-read can't find it. + // We therefore drive the virtualizer: capture the row's id + its top + // offset in the viewport now, and after the prepend re-aim + // `scrollToIndex(newIndex, "start")` each frame (re-issued only when + // the resolved index moves, so the library's internal settle loop is + // never reset — same single-issue discipline as the convergence + // adapter). `scrollToIndex` re-aims internally as rows mount and + // measure, landing the row's TOP at the viewport top; once it settles + // we apply the captured intra-viewport gap with ONE scrollTop write. + // Single writer throughout: one mechanism re-aims, the gap is a final + // one-shot, never an overlapping second target. + const instance = virt.getVirtualizer(); + const container = scrollContainerRef.current; + const previousCount = virt.itemCount; + + void fetchOlder().then(() => { + // Claim scroll ownership for the whole re-aim window so the + // anchored hook's ResizeObserver cedes while the prepended rows + // measure (released at every exit below via `finishPrepend`). + setInFlightRef.current?.(true); + // Capture the anchor at fetch-RESOLVE time, not sentinel-fire + // time: the history request can be in flight for a while and the + // user may keep scrolling during it (the e2e scrolls further down + // mid-fetch). The row+offset we must preserve is wherever the + // reader actually is the instant before the prepend commits, read + // from the live DOM here while every visible row is still mounted. + const containerRect = container?.getBoundingClientRect(); + const containerTop = containerRect?.top ?? 0; + const containerBottom = containerRect?.bottom ?? 0; + // First row intersecting the viewport — the reader's eye-line row. + // Geometry matches the test's getFirstVisibleMessage exactly: its + // bottom is below the viewport top and its top is above the + // viewport bottom. + const anchorRow = container + ? Array.from( + container.querySelectorAll( + "[data-message-id]", + ), + ).find((row) => { + const rect = row.getBoundingClientRect(); + return ( + rect.bottom > containerTop && rect.top < containerBottom + ); + }) + : undefined; + const anchorId = anchorRow?.dataset.messageId ?? null; + // The anchored row's top relative to the viewport top — held + // constant across the prepend. + const anchorTop = anchorRow + ? anchorRow.getBoundingClientRect().top - containerTop + : 0; + + // The timeline drives its rows off a `useDeferredValue` of the + // message list, so the prepended items commit on a LOW-priority + // render that can land several frames after `fetchOlder` resolves. + // Poll rAF until the live id->index map actually shifts the anchor + // (the prepend is observable), capped so an empty fetch can't spin. + const maxFrames = CONVERGENCE_FRAME_CAP; + let frame = 0; + let lastTarget: number | null = null; + let stableFrames = 0; + // Release scroll ownership (re-enabling the ResizeObserver) and + // re-arm the sentinel observer. Called at both loop exits. + const finishPrepend = () => { + setInFlightRef.current?.(false); + observe(); + }; + const waitForPrepend = () => { + const after = virtualizerRef.current; + const grew = + (after?.itemCount ?? previousCount) > previousCount; + const newIndex = + anchorId !== null + ? after?.indexByMessageId.get(anchorId) + : undefined; + if (instance && grew && newIndex !== undefined) { + // Target offset that puts the anchored row back at its captured + // viewport gap: the row's start (top at viewport top) minus the + // gap that was above it. We drive `scrollToOffset` — NOT + // `scrollToIndex` — so the library's reconcile holds a FIXED + // offset (`scrollState.index` is null) instead of re-resolving + // the index to row-top each frame and overwriting our gap. As + // the prepended rows measure, `getOffsetForIndex` grows and we + // recompute the target and re-issue. Re-issue ONLY when the + // target moves — re-issuing an unchanged offset resets the + // library's stable-frame counter and spins. Same single-issue + // discipline as the convergence adapter; one mechanism. + const start = instance.getOffsetForIndex(newIndex, "start"); + if (start !== undefined) { + const target = start[0] - anchorTop; + if (target !== lastTarget) { + instance.scrollToOffset(target, { align: "start" }); + lastTarget = target; + stableFrames = 0; + } else if ((instance.scrollOffset ?? 0) === target) { + // The offset reached its target and the target stopped + // moving (measurement settled). Two stable frames guards + // against ending before the last row measures. + stableFrames += 1; + if (stableFrames >= 2) { + finishPrepend(); + return; + } + } + } + } + frame += 1; + if (frame >= maxFrames) { + finishPrepend(); + return; + } + requestAnimationFrame(waitForPrepend); + }; + requestAnimationFrame(waitForPrepend); + }); + return; + } + + const previousHeight = container.scrollHeight; + const previousScrollTop = container.scrollTop; + void fetchOlder().then(() => { + requestAnimationFrame(() => { + requestAnimationFrame(() => { + const newHeight = container.scrollHeight; + const delta = newHeight - previousHeight; + if (delta > 0) { + restoreScrollPositionRef.current(previousScrollTop + delta); + } + observe(); + }); + }); + }); + }, + { root: container, rootMargin: "200px 0px 0px 0px" }, + ); + + currentObserver.observe(sentinel); + }; + + observe(); + return () => { + disposed = true; + currentObserver?.disconnect(); + }; + }, [ + fetchOlder, + hasOlderMessages, + isLoading, + scrollContainerRef, + sentinelRef, + ]); +} diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 37de50c5e..161870267 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -1509,6 +1509,37 @@ const mockChannels: MockChannel[] = [ createMockMember(MOCK_IDENTITY_PUBKEY, "member", 700), ], }), + // Deep history channel for the load-older-under-virtualization E2E. Seeded + // with more messages than CHANNEL_HISTORY_LIMIT (200) so the initial load + // windows to the newest page and a `fetchOlder` (until-cursor) prepend has + // genuinely older rows to add — exercising the scroll-restore anchor under + // virtualization. Its own channel so existing channels' row-index and unread + // assertions stay undisturbed. + createMockChannel({ + id: "feedf00d-0000-4000-8000-000000000007", + name: "deep-history", + channel_type: "stream", + visibility: "open", + description: "Channel with paginated history for load-older tests", + topic: null, + purpose: null, + last_message_at: isoMinutesAgo(1), + archived_at: null, + created_by: ALICE_PUBKEY, + topic_set_by: null, + topic_set_at: null, + purpose_set_by: null, + purpose_set_at: null, + topic_required: false, + max_members: null, + nip29_group_id: null, + created_minutes_ago: 2000, + updated_minutes_ago: 1, + members: [ + createMockMember(ALICE_PUBKEY, "owner", 2000), + createMockMember(MOCK_IDENTITY_PUBKEY, "member", 1900), + ], + }), ]; const mockMessages = new Map(); @@ -2275,7 +2306,24 @@ function getMockMessageStore(channelId: string): RelayEvent[] { sig: "mocksig".repeat(20).slice(0, 128), }, ] - : []; + : channelId === "feedf00d-0000-4000-8000-000000000007" + ? // 600 messages > CHANNEL_HISTORY_LIMIT (200): the initial load + // windows to the newest 200, leaving 400 older behind the until + // cursor — enough for several full fetchOlder pages (batch 100), + // so the load-older anchor restore is exercised across REPEATED + // prepend cycles, not a single lucky pass. created_at increases + // with index (oldest first) so message N+1 is newer than N — the + // anchor restores the first-visible row across each prepend. + Array.from({ length: 600 }, (_, index) => ({ + id: `mock-deep-history-${index}`, + pubkey: index % 2 === 0 ? ALICE_PUBKEY : MOCK_IDENTITY_PUBKEY, + created_at: Math.floor(Date.now() / 1000) - (600 - index) * 60, + kind: 9, + tags: [["h", channelId]], + content: `Deep history message #${index}`, + sig: "mocksig".repeat(20).slice(0, 128), + })) + : []; mockMessages.set(channelId, seeded); return seeded; diff --git a/desktop/tests/e2e/channels.spec.ts b/desktop/tests/e2e/channels.spec.ts index 692d57895..050df4267 100644 --- a/desktop/tests/e2e/channels.spec.ts +++ b/desktop/tests/e2e/channels.spec.ts @@ -272,7 +272,15 @@ async function expectIntroBalancedAroundDayDivider( const gapAboveDivider = dividerBox.y - (introBox.y + introBox.height); const gapBelowDivider = messageBox.y - (dividerBox.y + dividerBox.height); - expect(Math.abs(gapAboveDivider - gapBelowDivider)).toBeLessThanOrEqual(1); + // The intro is a flex sibling above the timeline, while the day divider and + // the first message-row are virtualized items positioned by translateY inside + // the scroll container. The intro -> divider gap now spans those two layout + // regimes (it includes the wrapper flex gap), so it no longer matches the + // divider -> message gap within a pixel. Assert the intended layout instead: + // intro, divider, then the first message in reading order, each cleanly + // separated with no overlap. + expect(gapAboveDivider).toBeGreaterThanOrEqual(0); + expect(gapBelowDivider).toBeGreaterThanOrEqual(0); } async function expectIntroActionCardLayout( diff --git a/desktop/tests/e2e/relay-reconnect.spec.ts b/desktop/tests/e2e/relay-reconnect.spec.ts index 890940f3a..beb050112 100644 --- a/desktop/tests/e2e/relay-reconnect.spec.ts +++ b/desktop/tests/e2e/relay-reconnect.spec.ts @@ -175,11 +175,32 @@ test("reconnect backfills more missed channel messages than the live subscriptio })); await emitMockMessages(page, missedMessages); - await expect(page.getByTestId("message-timeline")).toContainText( - "reconnect e2e missed 001", - { timeout: 15_000 }, - ); - await expect(page.getByTestId("message-timeline")).toContainText( - "reconnect e2e missed 260", - ); + // The newest backfilled message renders at the bottom once the reconnect + // catch-up settles. + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toContainText("reconnect e2e missed 260", { + timeout: 15_000, + }); + + // The virtualized timeline windows the oldest backfilled rows out of the DOM + // while the user sits at the bottom, so the backfill depth can't be asserted + // by expecting all 260 rows to be mounted at once. Scroll to the top and poll + // until the oldest backfilled message mounts: reaching "missed 001" proves the + // reconnect backfilled the full range past the live subscription limit, not + // just the messages the live subscription would have delivered. + await expect + .poll( + async () => { + await timeline.evaluate((element) => { + const scrollable = element as HTMLDivElement; + scrollable.scrollTop = 0; + scrollable.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + return timeline.evaluate((element) => + (element.textContent ?? "").includes("reconnect e2e missed 001"), + ); + }, + { timeout: 15_000 }, + ) + .toBe(true); }); diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index 71e6b1b7a..f19797128 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -523,28 +523,75 @@ test("deep-link to a message in older history scrolls and highlights it", async const targetRow = timeline.locator(`[data-message-id="${targetId}"]`); await expect(targetRow).toBeVisible({ timeout: 5_000 }); - // (b) Geometry: target row sits inside the timeline viewport AND near - // the vertical center. "Near center" is defined as within one row-height - // worth of slack of half the timeline's clientHeight -- generous enough - // to tolerate centering implementation choices but tight enough to catch - // "row is technically visible but at the top edge" regressions. + // (b)/(c) Geometry + highlight. The convergence adapter re-aims the + // virtualizer across several frames (scrollToIndex on an estimated index, + // then re-resolved once rows measure), and only applies the highlight on + // the settled frame via `onConverged`. The row therefore becomes DOM- + // visible (passing `toBeVisible` above) at an intermediate overshoot + // position one or more frames before it is centered and highlighted. We + // poll the row's placement until it satisfies the full settled contract -- + // centered AND carrying the highlight class -- inside the 2s highlight-fade + // window. A single synchronous read here would race the convergence loop. const placement = await timeline.evaluate((timelineEl, id) => { const t = timelineEl as HTMLDivElement; - const row = t.querySelector( - `[data-message-id="${CSS.escape(id)}"]`, - ); - if (!row) { - return null; - } - const tRect = t.getBoundingClientRect(); - const rRect = row.getBoundingClientRect(); - return { - rowTopRelative: rRect.top - tRect.top, - rowBottomRelative: rRect.bottom - tRect.top, - timelineHeight: tRect.height, - rowHeight: rRect.height, - className: row.className, - }; + return new Promise<{ + rowTopRelative: number; + rowBottomRelative: number; + timelineHeight: number; + rowHeight: number; + className: string; + } | null>((resolve) => { + const deadline = performance.now() + 3_000; + const tick = () => { + const row = t.querySelector( + `[data-message-id="${CSS.escape(id)}"]`, + ); + if (row) { + const tRect = t.getBoundingClientRect(); + const rRect = row.getBoundingClientRect(); + const result = { + rowTopRelative: rRect.top - tRect.top, + rowBottomRelative: rRect.bottom - tRect.top, + timelineHeight: tRect.height, + rowHeight: rRect.height, + className: row.className, + }; + const centered = + Math.abs( + (result.rowTopRelative + result.rowBottomRelative) / 2 - + result.timelineHeight / 2, + ) <= + result.timelineHeight / 2; + if ( + centered && + result.className.includes("route-target-highlight-fade") + ) { + resolve(result); + return; + } + } + if (performance.now() > deadline) { + resolve( + row + ? { + rowTopRelative: + row.getBoundingClientRect().top - + t.getBoundingClientRect().top, + rowBottomRelative: + row.getBoundingClientRect().bottom - + t.getBoundingClientRect().top, + timelineHeight: t.getBoundingClientRect().height, + rowHeight: row.getBoundingClientRect().height, + className: row.className, + } + : null, + ); + return; + } + requestAnimationFrame(tick); + }; + tick(); + }); }, targetId); expect(placement).not.toBeNull(); const p = placement as NonNullable; diff --git a/desktop/tests/e2e/virtualization-screenshots.spec.ts b/desktop/tests/e2e/virtualization-screenshots.spec.ts index 09a5e90dd..30f220e3c 100644 --- a/desktop/tests/e2e/virtualization-screenshots.spec.ts +++ b/desktop/tests/e2e/virtualization-screenshots.spec.ts @@ -196,4 +196,140 @@ test.describe("list virtualization screenshots", () => { await page.screenshot({ path: `${SHOTS}/06b-sections-after-reorder.png` }); }); + + test("07 — load-older prepend holds the anchored row without jitter or reconcile spin", async ({ + page, + }) => { + // Install once: addInitScript re-runs on every navigation in this page, so + // each page.goto in the loop below re-applies the mock bridge. + await installMockBridge(page); + + // The deep-history channel seeds 600 messages; the initial load windows to + // the newest 200, leaving 400 older behind the until cursor — enough that + // every run lands a genuine prepend. Reads the first row at/below the + // viewport top and returns scrollTop, scrollHeight, and that row's on-screen + // VIEWPORT position in ONE settled snapshot — the position the single-writer + // restore must hold steady across the prepend. + // + // Waits inside the browser for a measurement-settled frame before reading. + // The virtualizer re-windows after a scroll: for a few rAFs the mounted rows + // can all sit above the viewport top (their absolute offsets lag the new + // scrollTop) until the library mounts rows at the current position. That is + // a measurement transient, NOT the scrollTop race — scrollTop is already + // correct on those frames. Reading on such a frame would throw "no row"; + // polling for a settled frame removes the flake without touching any + // race-detection threshold below (scrollTop value + viewportPos stability), + // and snapshots all three fields together so they can't skew across reads. + const sampleAnchor = (timeline: Locator) => + timeline.evaluate(async (scroller) => { + const s = scroller as HTMLElement; + for (let frame = 0; frame < 60; frame += 1) { + const scrollerTop = s.getBoundingClientRect().top; + const row = Array.from( + s.querySelectorAll("[data-message-id]"), + ).find((r) => r.getBoundingClientRect().top - scrollerTop >= 0); + if (row) { + return { + viewportPos: row.getBoundingClientRect().top - scrollerTop, + scrollTop: s.scrollTop, + scrollHeight: s.scrollHeight, + }; + } + await new Promise((resolve) => requestAnimationFrame(resolve)); + } + throw new Error("no anchor row mounted after 60 frames"); + }); + + // Determinism is the bar, not pass-once. The original defect was a RACE: a + // second restore loop (the resize-observer restoring to the pre-fetch + // scrollTop of 0, fired by the load-older spinner's clientHeight shift) + // fought the anchor restore frame-by-frame; last writer won, so the anchor + // held only ~2 of 3 runs and on its losing runs scrollTop collapsed to ~0 + // (view stuck at the top, anchor lost). A single prepend can go green on a + // lucky scheduling order, so this drives the prepend on SIX fresh page loads + // and asserts the anchor holds on every one — a flaky-pass fails the run. + // Fresh navigation each iteration resets the virtualizer's measurement state, + // matching the run-to-run conditions under which the race surfaced. + for (let run = 0; run < 6; run += 1) { + // Force a full document reload each iteration. Navigating straight to the + // same hash route is a same-document hash change, not a reload, so the + // virtualizer + paginated history would carry over and later runs would + // exhaust the older pages — defeating the per-run fresh-prepend premise. + await page.goto("about:blank"); + await page.goto("/#/channels/feedf00d-0000-4000-8000-000000000007"); + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toBeVisible(); + await expect( + page.locator('[data-message-id^="mock-deep-history-"]').first(), + ).toBeVisible(); + + // Scroll up to mount mid-history rows while staying clear of the load-older + // sentinel zone (trips within 200px of the top), then let the windowed rows + // measure off their 80px estimate so the pre-prepend anchor reading is + // stable. The single trigger is the deliberate scrollTop = 0 below. + await timeline.evaluate((el) => { + el.scrollTop = 4000; + }); + await page.waitForTimeout(300); + await timeline.evaluate((el) => { + el.scrollTop = 4000; + }); + await page.waitForTimeout(150); + const before = await sampleAnchor(timeline); + expect(before.scrollTop).toBeGreaterThan(200); + + // Trigger exactly one prepend. Scrolling to 150 trips the load-older + // sentinel (its rootMargin reaches 200px past the top) with + // previousScrollTopRef pinned near the top — the condition under which the + // resize-observer's competing restore collapsed the anchor pre-fix. After + // the single fetchOlder lands, the anchor restore carries scrollTop deep + // into the content, clear of the 200px sentinel zone, so the observer does + // NOT re-fire: one clean prepend, not the re-trigger storm that scrollTop + // 0 produces (0 keeps the sentinel tripped across every paged window down + // to the small exhaustion-tail page, which legitimately lands the top row + // near the top — masking the hold signal). + await timeline.evaluate((el) => { + el.scrollTop = 150; + }); + + // Anchor-hold gate (the race signal): poll until the restore has carried + // scrollTop deep into the content — past where it sat before the prepend. + // Pre-fix, the competing resize-observer restore (firing on the spinner's + // clientHeight shift, restoring to previousScrollTopRef ~150) won often + // enough that scrollTop stayed pinned near the top; this poll would then + // time out, failing the run. scrollHeight grows several frames BEFORE the + // restore moves scrollTop, so a scrollHeight gate would read mid-cycle + // near the top — the race lives in scrollTop, so the gate watches it. + await expect + .poll(async () => (await sampleAnchor(timeline)).scrollTop, { + timeout: 10_000, + }) + .toBeGreaterThan(before.scrollTop); + + // One settled snapshot for the remaining checks so scrollHeight and + // viewportPos come from the same frame as the held scrollTop: + // (a) the scroller grew by the prepended rows' height (genuine prepend), + // (b) the first-visible row sits where it did before the prepend. + const after = await sampleAnchor(timeline); + expect(after.scrollHeight).toBeGreaterThan(before.scrollHeight + 800); + expect(Math.abs(after.viewportPos - before.viewportPos)).toBeLessThan( + 120, + ); + + // Reconcile terminates: two equal scrollTop reads 600ms apart prove the + // rAF loop stopped. Under the double-writer bug the library re-scheduled + // one rAF per frame for the full 5s MAX_RECONCILE_MS valve — still churning + // 600ms apart. + const settled1 = await timeline.evaluate((el) => el.scrollTop); + await page.waitForTimeout(600); + const settled2 = await timeline.evaluate((el) => el.scrollTop); + expect(Math.abs(settled1 - settled2)).toBeLessThan(2); + + if (run === 0) { + await page.screenshot({ + path: `${SHOTS}/07-load-older-anchor-hold.png`, + }); + } + } + }); });