From 6d5c3e9b01539ee8b950bec629aa9393c4267f0c Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 20 Jun 2026 11:27:58 -0600 Subject: [PATCH 1/9] desktop: fix scrollback paging and visible history depth Fix the scrollback regressions as one coherent pagination contract: - Keep backward-paged history in the cache instead of applying the newest-window cap to older roots, and gate the channel intro on the rendered timeline being caught up with the live cache before declaring the true channel start reached. - Fetch historical channel rows with content kinds only, then backfill reactions, edits, and deletions by `#e` reference so page limits buy visible history depth without rendering stale overlays. - Share one older-history pager between cold load and scroll-up. It targets 30 top-level rendered rows, uses 200-event pages, caps one pass at three pages, and starts cold history at 300 events. - Drop in-flight observer triggers instead of queueing retries, and require a parked top sentinel to leave/re-enter before another fetch once the timeline is scrollable. This removes the spinner-flash/flood pattern. - Keep the older-history spinner pinned in the timeline viewport while loading. Adds regression coverage for the timeline cap, aux backfill, content-kind parity, non-overlapping history fetches, viewport-visible loading state, and fixtures large enough to exercise pagination beyond the larger cold window. Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/src/features/messages/hooks.ts | 32 +- .../messages/lib/auxBackfill.test.mjs | 55 +++ .../src/features/messages/lib/auxBackfill.ts | 74 ++++ .../lib/formatTimelineMessages.test.mjs | 82 ++++ .../messages/lib/formatTimelineMessages.ts | 2 +- .../messages/lib/messageQueryKeys.test.mjs | 69 +++- .../features/messages/lib/messageQueryKeys.ts | 66 +++- .../messages/lib/pageOlderMessages.ts | 132 +++++++ .../messages/lib/timelineSnapshot.test.mjs | 39 ++ .../features/messages/lib/timelineSnapshot.ts | 12 + .../features/messages/ui/MessageTimeline.tsx | 24 +- .../features/messages/ui/useAnchoredScroll.ts | 69 ++-- .../messages/useFetchOlderMessages.ts | 79 +--- desktop/src/shared/api/relayChannelFilters.ts | 104 +++++ desktop/src/shared/api/relayClientSession.ts | 101 ++--- .../src/shared/api/relayClientShared.test.mjs | 28 +- desktop/src/shared/api/relayClientShared.ts | 11 +- .../shared/api/relayReconnectReplay.test.mjs | 5 +- desktop/src/shared/constants/kinds.ts | 35 ++ desktop/src/testing/e2eBridge.ts | 19 +- desktop/tests/e2e/scroll-history.spec.ts | 356 +++++++++++++++++- 21 files changed, 1187 insertions(+), 207 deletions(-) create mode 100644 desktop/src/features/messages/lib/auxBackfill.test.mjs create mode 100644 desktop/src/features/messages/lib/auxBackfill.ts create mode 100644 desktop/src/features/messages/lib/pageOlderMessages.ts create mode 100644 desktop/src/shared/api/relayChannelFilters.ts diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index 57db8b13c..4151b1b0b 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -31,6 +31,12 @@ import type { Channel, Identity, RelayEvent } from "@/shared/api/types"; // Same .mjs the renderer uses, so the cache-update projection can't drift // from the on-render overlay. import { applyEditTagOverlay } from "@/features/messages/lib/applyEditTagOverlay.mjs"; +import { backfillAuxForMessages } from "@/features/messages/lib/auxBackfill"; +import { countTopLevelTimelineRows } from "@/features/messages/lib/formatTimelineMessages"; +import { + MIN_TOP_LEVEL_ROWS_PER_FETCH, + pageOlderMessagesUntilRowFloor, +} from "@/features/messages/lib/pageOlderMessages"; import { KIND_STREAM_MESSAGE, KIND_SYSTEM_MESSAGE, @@ -42,7 +48,7 @@ type MessageQueryContext = { queryKey: ReturnType; }; -const CHANNEL_HISTORY_LIMIT = 200; +const CHANNEL_HISTORY_LIMIT = 300; function getLocalRenderKey(message: RelayEvent) { return message.localKey ?? message.id; @@ -186,7 +192,25 @@ export function useChannelMessagesQuery(channel: Channel | null) { history, ); - return mergedHistory; + // Paint messages immediately; backfill their reactions/edits/deletions + // by `#e` in the background (it self-merges into the same cache key). + void backfillAuxForMessages(queryClient, channel.id, history); + + // Seed the cache, then — only if the cold window renders thinner than a + // normal scroll page — top it up to the same visible-row floor. A + // reply-heavy channel's 300-message cold load can be ~12 rows; a normal + // channel already clears the floor and skips the extra fetch entirely. + queryClient.setQueryData(queryKey, mergedHistory); + if ( + countTopLevelTimelineRows(mergedHistory) < MIN_TOP_LEVEL_ROWS_PER_FETCH + ) { + await pageOlderMessagesUntilRowFloor( + queryClient, + channel.id, + () => true, + ); + } + return queryClient.getQueryData(queryKey) ?? mergedHistory; }, staleTime: 5 * 60 * 1_000, gcTime: 5 * 60 * 1_000, @@ -211,6 +235,8 @@ export function useChannelSubscription(channel: Channel | null) { channelMessagesKey(channelId), (current = []) => mergeTimelineHistoryMessages(current, history), ); + + void backfillAuxForMessages(queryClient, channelId, history); }); const appendMessage = useEffectEvent((event: RelayEvent) => { @@ -278,7 +304,7 @@ export function useChannelSubscription(channel: Channel | null) { cleanup = dispose; // No post-subscribe history refetch: useChannelMessagesQuery already - // loaded the latest CHANNEL_HISTORY_LIMIT (200) events, and the live + // loaded the latest CHANNEL_HISTORY_LIMIT (300) events, and the live // subscription itself backfills up to 50 most-recent events via its // initial REQ (buildChannelFilter(id, 50)). Both write into the same // channelMessagesKey cache, so any window between the two REQs is diff --git a/desktop/src/features/messages/lib/auxBackfill.test.mjs b/desktop/src/features/messages/lib/auxBackfill.test.mjs new file mode 100644 index 000000000..df305c922 --- /dev/null +++ b/desktop/src/features/messages/lib/auxBackfill.test.mjs @@ -0,0 +1,55 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { collectMessageIdsForAuxBackfill } from "./auxBackfill.ts"; + +const CHANNEL_ID = "36411e44-0e2d-4cfe-bd6e-567eb169db9f"; + +function event(id, kind) { + return { + id, + pubkey: "a".repeat(64), + kind, + created_at: 1_700_000_000, + content: "", + tags: [["h", CHANNEL_ID]], + sig: "sig", + }; +} + +function hex(char) { + return char.repeat(64); +} + +test("collects content-kind message ids (stream, v2, diff, system, jobs)", () => { + const events = [ + event(hex("1"), 9), // stream message + event(hex("2"), 40002), // v2 stream message + event(hex("3"), 40008), // diff (own row) + event(hex("4"), 40099), // system message + event(hex("5"), 43001), // job request + ]; + assert.deepEqual(collectMessageIdsForAuxBackfill(events), [ + hex("1"), + hex("2"), + hex("3"), + hex("4"), + hex("5"), + ]); +}); + +test("excludes auxiliary kinds (reactions, edits, deletions)", () => { + const events = [ + event(hex("1"), 9), // message — kept + event(hex("2"), 7), // reaction — excluded + event(hex("3"), 40003), // edit — excluded + event(hex("4"), 5), // NIP-09 deletion — excluded + event(hex("5"), 9005), // Buzz-native deletion — excluded + ]; + assert.deepEqual(collectMessageIdsForAuxBackfill(events), [hex("1")]); +}); + +test("returns empty for a window of only auxiliary events", () => { + const events = [event(hex("2"), 7), event(hex("3"), 40003)]; + assert.deepEqual(collectMessageIdsForAuxBackfill(events), []); +}); diff --git a/desktop/src/features/messages/lib/auxBackfill.ts b/desktop/src/features/messages/lib/auxBackfill.ts new file mode 100644 index 000000000..c3ca7afd7 --- /dev/null +++ b/desktop/src/features/messages/lib/auxBackfill.ts @@ -0,0 +1,74 @@ +import type { QueryClient } from "@tanstack/react-query"; + +import { + channelMessagesKey, + sortMessages, +} from "@/features/messages/lib/messageQueryKeys"; +import { relayClient } from "@/shared/api/relayClient"; +import type { RelayEvent } from "@/shared/api/types"; +import { CHANNEL_TIMELINE_CONTENT_KINDS } from "@/shared/constants/kinds"; + +const TIMELINE_CONTENT_KINDS: ReadonlySet = new Set( + CHANNEL_TIMELINE_CONTENT_KINDS, +); + +/** + * Extract the ids of the visible content messages from a freshly-fetched + * history window. Auxiliary events (reactions, edits, deletions) are then + * backfilled by `#e` reference over exactly these ids. Pure so it can be + * unit-tested without a relay or query client. + */ +export function collectMessageIdsForAuxBackfill( + historyEvents: RelayEvent[], +): string[] { + return historyEvents + .filter((event) => TIMELINE_CONTENT_KINDS.has(event.kind)) + .map((event) => event.id); +} + +/** + * After a content-kinds-only history fetch, pull the auxiliary events + * (reactions, edits, deletions) that reference the loaded messages — keyed by + * `#e` over their ids, not by a time window — and merge them into the same + * channel cache. + * + * History fetches request content kinds only so the `limit` budget buys + * visible message depth (a reaction-heavy 200-event window was only ~136 + * messages). The cost is that an edit/deletion for a visible message can fall + * outside any fetched time window — so aux must be pulled by reference, or a + * visible message renders stale (un-edited / not-deleted). + * + * Best-effort: failures are logged but never reject, so a flaky overlay fetch + * can't blank the freshly-loaded messages. + */ +export async function backfillAuxForMessages( + queryClient: QueryClient, + channelId: string, + historyEvents: RelayEvent[], +): Promise { + const messageIds = collectMessageIdsForAuxBackfill(historyEvents); + if (messageIds.length === 0) { + return; + } + + try { + const auxEvents = await relayClient.fetchAuxEventsForMessages( + channelId, + messageIds, + ); + if (auxEvents.length === 0) { + return; + } + + queryClient.setQueryData( + channelMessagesKey(channelId), + (current = []) => sortMessages([...current, ...auxEvents]), + ); + } catch (error) { + console.error( + "Failed to backfill auxiliary events for channel", + channelId, + error, + ); + } +} diff --git a/desktop/src/features/messages/lib/formatTimelineMessages.test.mjs b/desktop/src/features/messages/lib/formatTimelineMessages.test.mjs index 6526d19f8..8af5aaa10 100644 --- a/desktop/src/features/messages/lib/formatTimelineMessages.test.mjs +++ b/desktop/src/features/messages/lib/formatTimelineMessages.test.mjs @@ -4,7 +4,12 @@ import test from "node:test"; import { countTopLevelTimelineRows, formatTimelineMessages, + isTimelineContentEvent, } from "./formatTimelineMessages.ts"; +import { + CHANNEL_AUX_EVENT_KINDS, + CHANNEL_TIMELINE_CONTENT_KINDS, +} from "@/shared/constants/kinds"; const HEX64_A = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; @@ -45,6 +50,64 @@ function deletionEvent(kind, targetId, overrides = {}) { }; } +function streamEdit(targetId, content, overrides = {}) { + return { + id: HEX64_B, + pubkey: PUBKEY_A, + kind: 40003, + created_at: 1_700_000_001, + content, + tags: [ + ["h", CHANNEL_ID], + ["e", targetId], + ], + sig: "sig", + ...overrides, + }; +} + +// --------------------------------------------------------------------------- +// Keystone regression: aux events (edits/deletions) apply by `#e` reference, +// NOT by time-window overlap. This is the invariant the split-query + +// `#e`-backfill fix depends on: an edit/deletion can be loaded long after the +// message it targets — even with a far-future `created_at` — and must still +// apply. If the reducer ever gated aux application on timestamp proximity, a +// late edit/delete for a visible old message would silently render stale. +// --------------------------------------------------------------------------- + +test("a far-future edit still rewrites the body of an old message", () => { + const old = streamMessage({ created_at: 1_700_000_000 }); + const lateEdit = streamEdit(HEX64_A, "edited body", { + created_at: 1_900_000_000, + }); + const out = formatTimelineMessages([old, lateEdit], null, undefined, null); + assert.equal(out.length, 1, "the message should still render"); + assert.equal( + out[0].body, + "edited body", + "the far-future edit must overlay the old message's body regardless of the time gap", + ); + assert.equal(out[0].edited, true, "the message must be marked edited"); +}); + +test("a far-future deletion still hides an old message", () => { + const old = streamMessage({ created_at: 1_700_000_000 }); + const lateDeletion = deletionEvent(9005, HEX64_A, { + created_at: 1_900_000_000, + }); + const out = formatTimelineMessages( + [old, lateDeletion], + null, + undefined, + null, + ); + assert.equal( + out.length, + 0, + "the far-future deletion must filter out the old message regardless of the time gap", + ); +}); + test("kind:5 (NIP-09) deletion hides the target message", () => { const events = [streamMessage(), deletionEvent(5, HEX64_A)]; const out = formatTimelineMessages(events, null, undefined, null); @@ -194,3 +257,22 @@ test("countTopLevelTimelineRows ignores non-content kinds (reactions)", () => { }; assert.equal(countTopLevelTimelineRows([message(hex64("1")), reaction]), 1); }); + +// Guardrail: the history fetch requests exactly CHANNEL_TIMELINE_CONTENT_KINDS, +// so that set must stay in lockstep with isTimelineContentEvent. Drift would +// silently drop a content kind from history (fetched but never rendered) or +// fetch an aux kind as content. Assert parity in both directions. +test("CHANNEL_TIMELINE_CONTENT_KINDS matches isTimelineContentEvent", () => { + for (const kind of CHANNEL_TIMELINE_CONTENT_KINDS) { + assert.ok( + isTimelineContentEvent({ kind }), + `content kind ${kind} must be a timeline content event`, + ); + } + for (const kind of CHANNEL_AUX_EVENT_KINDS) { + assert.ok( + !isTimelineContentEvent({ kind }), + `aux kind ${kind} must not be a timeline content event`, + ); + } +}); diff --git a/desktop/src/features/messages/lib/formatTimelineMessages.ts b/desktop/src/features/messages/lib/formatTimelineMessages.ts index 5cb474c0d..d56811449 100644 --- a/desktop/src/features/messages/lib/formatTimelineMessages.ts +++ b/desktop/src/features/messages/lib/formatTimelineMessages.ts @@ -42,7 +42,7 @@ import { applyEditTagOverlay } from "@/features/messages/lib/applyEditTagOverlay const HEX_RE = /^[0-9a-f]+$/i; -function isTimelineContentEvent(event: RelayEvent) { +export function isTimelineContentEvent(event: RelayEvent) { return ( event.kind === KIND_STREAM_MESSAGE || event.kind === KIND_STREAM_MESSAGE_V2 || diff --git a/desktop/src/features/messages/lib/messageQueryKeys.test.mjs b/desktop/src/features/messages/lib/messageQueryKeys.test.mjs index 6c954f34e..34c535b5a 100644 --- a/desktop/src/features/messages/lib/messageQueryKeys.test.mjs +++ b/desktop/src/features/messages/lib/messageQueryKeys.test.mjs @@ -5,7 +5,6 @@ import { mergeTimelineHistoryMessages, normalizeTimelineMessages, } from "./messageQueryKeys.ts"; -import { mergeTimelineCacheMessages } from "../hooks.ts"; const CHANNEL_ID = "timeline-window-test"; const PUBKEY = "a".repeat(64); @@ -140,10 +139,32 @@ test("normalizeTimelineMessages still caps old visible content", () => { ); }); -test("timeline history and live cache merges retain the same visible content regardless of order", () => { +test("timeline history merge preserves freshly fetched older content roots", () => { + const current = []; + const olderPage = []; + + for (let index = 0; index < 2_000; index += 1) { + current.push(event({ id: id("new", index), createdAt: 10_000 + index })); + } + for (let index = 0; index < 100; index += 1) { + olderPage.push(event({ id: id("old", index), createdAt: 1_000 + index })); + } + + const merged = mergeTimelineHistoryMessages(current, olderPage); + const mergedContent = merged + .filter((item) => item.kind === 9) + .map((item) => item.id); + + assert.equal(mergedContent.length, 2_100); + assert.equal(mergedContent[0], id("old", 0)); + assert.equal(mergedContent[99], id("old", 99)); + assert.equal(mergedContent[100], id("new", 0)); + assert.equal(mergedContent.at(-1), id("new", 1_999)); +}); + +test("timeline history merge preserves the older window despite auxiliary events", () => { const seedMessages = []; const olderPage = []; - const liveMessage = event({ id: id("liv", 0), createdAt: 20_000 }); for (let index = 0; index < 700; index += 1) { seedMessages.push( @@ -181,24 +202,32 @@ test("timeline history and live cache merges retain the same visible content reg olderPage.push(event({ id: id("old", index), createdAt: 1_000 + index })); } - const historyThenLive = mergeTimelineCacheMessages( - mergeTimelineHistoryMessages(seedMessages, olderPage), - liveMessage, - ); - const liveThenHistory = mergeTimelineHistoryMessages( - mergeTimelineCacheMessages(seedMessages, liveMessage), - olderPage, - ); - const historyThenLiveContent = historyThenLive - .filter((item) => item.kind === 9) - .map((item) => item.id); - const liveThenHistoryContent = liveThenHistory + const merged = mergeTimelineHistoryMessages(seedMessages, olderPage); + const mergedContent = merged .filter((item) => item.kind === 9) .map((item) => item.id); - assert.equal(historyThenLiveContent.length, 2_000); - assert.equal(liveThenHistoryContent.length, 2_000); - assert.deepEqual(liveThenHistoryContent, historyThenLiveContent); - assert.equal(historyThenLiveContent[0], id("old", 201)); - assert.equal(historyThenLiveContent.at(-1), liveMessage.id); + assert.equal(mergedContent.length, 2_200); + assert.equal(mergedContent[0], id("old", 0)); + assert.equal(mergedContent[1_499], id("old", 1_499)); + assert.equal(mergedContent[1_500], id("new", 0)); + assert.equal(mergedContent.at(-1), id("new", 699)); + assert.equal(merged.filter((item) => item.kind === 5).length, 1_303); + assert.equal(merged.filter((item) => item.kind === 7).length, 231); +}); + +test("sortMessages tiebreaks same-second events on id, order-independent", () => { + // Three events sharing one created_at, fed in two different input orders. + // The (created_at, id) sort must produce the same sequence both ways, so a + // history-then-live merge and a live-then-history merge can't shuffle a + // same-second message to a different visible position. + const a = event({ id: id("aaa", 1), createdAt: 5_000 }); + const b = event({ id: id("bbb", 1), createdAt: 5_000 }); + const c = event({ id: id("ccc", 1), createdAt: 5_000 }); + + const forward = normalizeTimelineMessages([a, b, c]).map((m) => m.id); + const reverse = normalizeTimelineMessages([c, b, a]).map((m) => m.id); + + assert.deepEqual(forward, reverse); + assert.deepEqual(forward, [a.id, b.id, c.id]); }); diff --git a/desktop/src/features/messages/lib/messageQueryKeys.ts b/desktop/src/features/messages/lib/messageQueryKeys.ts index 0ea36739f..2e4c5c65b 100644 --- a/desktop/src/features/messages/lib/messageQueryKeys.ts +++ b/desktop/src/features/messages/lib/messageQueryKeys.ts @@ -37,9 +37,16 @@ export function dedupeMessagesById(messages: RelayEvent[]) { } export function sortMessages(messages: RelayEvent[]) { - return dedupeMessagesById(messages).sort( - (left, right) => left.created_at - right.created_at, - ); + return dedupeMessagesById(messages).sort((left, right) => { + if (left.created_at !== right.created_at) { + return left.created_at - right.created_at; + } + // Tiebreak same-second events on id so the merge order is deterministic. + // Without this, two events sharing a created_at can land in a different + // position depending on which REQ (history vs live-sub) delivered them + // first — reading as a "missing"/shuffled message at a fixed scroll offset. + return left.id < right.id ? -1 : left.id > right.id ? 1 : 0; + }); } function isTimelineWindowContentEvent(event: RelayEvent) { @@ -57,17 +64,7 @@ function isTimelineWindowContentEvent(event: RelayEvent) { ); } -/** - * Sort, dedupe, and cap the timeline at {@link MAX_TIMELINE_MESSAGES} visible - * content events so de-virtualized rendering does not grow into an unbounded - * DOM during long-lived channel sessions. - * - * Auxiliary events (reactions, edits, tombstones) are kept in cache so they can - * still apply to retained or later-loaded content, but they must not consume the - * visible message window and evict older loaded roots. - */ -export function normalizeTimelineMessages(messages: RelayEvent[]) { - const normalized = sortMessages(messages); +function capNewestTimelineMessages(normalized: RelayEvent[]) { const contentEvents = normalized.filter(isTimelineWindowContentEvent); if (contentEvents.length <= MAX_TIMELINE_MESSAGES) { @@ -84,9 +81,50 @@ export function normalizeTimelineMessages(messages: RelayEvent[]) { ); } +/** + * Sort, dedupe, and cap the timeline at {@link MAX_TIMELINE_MESSAGES} visible + * content events so de-virtualized rendering does not grow into an unbounded + * DOM during long-lived channel sessions. + * + * Auxiliary events (reactions, edits, tombstones) are kept in cache so they can + * still apply to retained or later-loaded content, but they must not consume the + * visible message window and evict older loaded roots. + */ +export function normalizeTimelineMessages(messages: RelayEvent[]) { + return capNewestTimelineMessages(sortMessages(messages)); +} + +function isOlderHistoryPage(current: RelayEvent[], history: RelayEvent[]) { + if (current.length === 0 || history.length === 0) { + return false; + } + + const sortedCurrent = sortMessages(current); + const sortedHistory = sortMessages(history); + const newestHistory = sortedHistory[sortedHistory.length - 1]?.created_at; + const oldestCurrent = sortedCurrent[0]?.created_at; + + if (newestHistory === undefined || oldestCurrent === undefined) { + return false; + } + + return newestHistory <= oldestCurrent; +} + +function normalizeTimelineHistoryMessages( + current: RelayEvent[], + history: RelayEvent[], +) { + return sortMessages([...current, ...history]); +} + export function mergeTimelineHistoryMessages( current: RelayEvent[], history: RelayEvent[], ) { + if (isOlderHistoryPage(current, history)) { + return normalizeTimelineHistoryMessages(current, history); + } + return normalizeTimelineMessages([...current, ...history]); } diff --git a/desktop/src/features/messages/lib/pageOlderMessages.ts b/desktop/src/features/messages/lib/pageOlderMessages.ts new file mode 100644 index 000000000..47fdaf00e --- /dev/null +++ b/desktop/src/features/messages/lib/pageOlderMessages.ts @@ -0,0 +1,132 @@ +import type { QueryClient } from "@tanstack/react-query"; + +import { countTopLevelTimelineRows } from "@/features/messages/lib/formatTimelineMessages"; +import { backfillAuxForMessages } from "@/features/messages/lib/auxBackfill"; +import { + channelMessagesKey, + mergeTimelineHistoryMessages, +} from "@/features/messages/lib/messageQueryKeys"; +import { relayClient } from "@/shared/api/relayClient"; +import type { RelayEvent } from "@/shared/api/types"; + +const OLDER_MESSAGES_BATCH_SIZE = 200; + +// One fetch should advance the timeline by a predictable, *visible* amount. +// Thread replies collapse into their parent and non-content events never render, +// so a single batch can add far fewer rows than that — page in more batches +// until at least this many top-level rows are added (or history runs out). +// Counting rows, not messages, keeps a reply-heavy window from feeling like the +// fetch did nothing. The cold load and scrollback share this floor so the first +// page is the same size as later ones. +export const MIN_TOP_LEVEL_ROWS_PER_FETCH = 30; + +// Hard ceiling on relay pages fetched in one pass. On reply-heavy channels a +// batch yields only a few visible rows, so the row floor alone could dig through +// hundreds of messages behind one spinner and commit them at once (a sudden +// "flood" with a tiny scrollbar). Capping per-pass keeps each fetch a bounded +// page; the scroll observer re-arms to page further while in view. +const MAX_BATCHES_PER_FETCH = 3; + +// Yield a frame so the rows just merged paint before the next round-trip; +// otherwise a multi-batch pass renders as one bulk commit at the loop's end. +function yieldToPaint(): Promise { + return new Promise((resolve) => { + if (typeof requestAnimationFrame === "function") { + requestAnimationFrame(() => resolve()); + } else { + setTimeout(resolve, 0); + } + }); +} + +export type PageOlderResult = { + /** False once a short relay page proves history is exhausted. */ + hasOlderMessages: boolean; +}; + +/** + * Page older history into the channel cache until the timeline has gained + * {@link MIN_TOP_LEVEL_ROWS_PER_FETCH} visible rows, history runs out, or the + * {@link MAX_BATCHES_PER_FETCH} ceiling is hit. Shared by the cold-load query + * and the scroll-up loader so both produce the same visible page size. + * + * `shouldContinue` lets the caller bail mid-pass (e.g. channel switch). Returns + * whether more history is believed to remain. + */ +export async function pageOlderMessagesUntilRowFloor( + queryClient: QueryClient, + channelId: string, + shouldContinue: () => boolean, +): Promise { + const queryKey = channelMessagesKey(channelId); + const baseline = queryClient.getQueryData(queryKey) ?? []; + if (baseline.length === 0) { + return { hasOlderMessages: false }; + } + + const baselineRowCount = countTopLevelTimelineRows(baseline); + let hasOlderMessages = true; + let batchesFetched = 0; + + while (hasOlderMessages && shouldContinue()) { + const before = queryClient.getQueryData(queryKey) ?? []; + if (before.length === 0) { + break; + } + + // `until` is inclusive — the relay returns the boundary message again, but + // sortMessages dedupes by id. Subtracting 1 risks skipping same-second + // messages. + const oldestTimestamp = before[0].created_at; + const olderMessages = await relayClient.fetchChannelHistoryBefore( + channelId, + oldestTimestamp, + OLDER_MESSAGES_BATCH_SIZE, + ); + batchesFetched += 1; + + // A full page means more likely remains; a short page is the only signal + // of true exhaustion. An *empty* page is ambiguous (transient relay + // pressure returns []), so don't end paging on it — let the progress guard + // below stop this pass instead. + if ( + olderMessages.length > 0 && + olderMessages.length < OLDER_MESSAGES_BATCH_SIZE + ) { + hasOlderMessages = false; + } + + if (olderMessages.length > 0) { + queryClient.setQueryData(queryKey, (current = []) => + mergeTimelineHistoryMessages(current, olderMessages), + ); + void backfillAuxForMessages(queryClient, channelId, olderMessages); + } + + // Progress guard, not exhaustion: if the oldest timestamp didn't move back + // (empty page, or all-duplicate), stop this pass to avoid re-fetching the + // same `until`. + const oldestAfterMerge = (queryClient.getQueryData( + queryKey, + ) ?? [])[0]?.created_at; + if (oldestAfterMerge === undefined || oldestAfterMerge >= oldestTimestamp) { + break; + } + + const rowsGained = + countTopLevelTimelineRows( + queryClient.getQueryData(queryKey) ?? [], + ) - baselineRowCount; + if (rowsGained >= MIN_TOP_LEVEL_ROWS_PER_FETCH) { + break; + } + + if (batchesFetched >= MAX_BATCHES_PER_FETCH) { + break; + } + + await yieldToPaint(); + } + + return { hasOlderMessages }; +} diff --git a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs index 22a3cfe2c..54217ec28 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs +++ b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs @@ -6,6 +6,7 @@ import { buildDayGroupBoundaries, isDeferredTimelineSnapshotStale, isNearBottomMetrics, + isRenderedTimelineBehindHistoryPrepend, resolveDeepLinkTarget, selectDeferredListRenderState, selectLatestMessageAutoScrollBehavior, @@ -518,3 +519,41 @@ test("timeline-intro-surface: no intro without an intro model", () => { null, ); }); + +test("isRenderedTimelineBehindHistoryPrepend: false when both empty", () => { + assert.equal(isRenderedTimelineBehindHistoryPrepend([], []), false); +}); + +test("isRenderedTimelineBehindHistoryPrepend: false during initial empty-to-loaded settle", () => { + // Rendered still empty while the live cache filled on open: not a prepend lag, + // so a freshly opened short channel can still show its intro. + assert.equal( + isRenderedTimelineBehindHistoryPrepend([], [{ id: "a" }]), + false, + ); +}); + +test("isRenderedTimelineBehindHistoryPrepend: false when rendered matches live", () => { + const a = { id: "a" }; + const b = { id: "b" }; + assert.equal(isRenderedTimelineBehindHistoryPrepend([a, b], [a, b]), false); +}); + +test("isRenderedTimelineBehindHistoryPrepend: true when rendered trails a live prepend", () => { + const older = { id: "older" }; + const a = { id: "a" }; + const b = { id: "b" }; + // Live cache gained an older root; rendered still starts at `a` and is shorter. + assert.equal( + isRenderedTimelineBehindHistoryPrepend([a, b], [older, a, b]), + true, + ); +}); + +test("isRenderedTimelineBehindHistoryPrepend: false when rendered oldest already matches live oldest", () => { + const a = { id: "a" }; + const b = { id: "b" }; + // Rendered shorter than live but oldest unchanged (e.g. a newer live append): + // not behind an older-history prepend. + assert.equal(isRenderedTimelineBehindHistoryPrepend([a], [a, b]), false); +}); diff --git a/desktop/src/features/messages/lib/timelineSnapshot.ts b/desktop/src/features/messages/lib/timelineSnapshot.ts index 9e3ac823c..50a3a29c4 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.ts +++ b/desktop/src/features/messages/lib/timelineSnapshot.ts @@ -217,6 +217,18 @@ export function isDeferredTimelineSnapshotStale({ return deferredSnapshot.channelId !== liveSnapshot.channelId; } +// True when an older page merged into the live cache but the deferred render +// hasn't painted it yet; false on the initial empty-to-loaded settle. +export function isRenderedTimelineBehindHistoryPrepend( + rendered: TimelineMessage[], + live: TimelineMessage[], +): boolean { + if (rendered.length === 0 || rendered.length >= live.length) { + return false; + } + return rendered[0]?.id !== live[0]?.id; +} + export type TimelineIntroSurface = | "direct-message-intro" | "channel-intro" diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index ae364daea..1e6adfbb4 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -3,6 +3,7 @@ import { Hash } from "lucide-react"; import { isDeferredTimelineSnapshotStale, + isRenderedTimelineBehindHistoryPrepend, selectTimelineBodySurface, selectTimelineIntroSurface, } from "@/features/messages/lib/timelineSnapshot"; @@ -222,7 +223,6 @@ const MessageTimelineBase = React.forwardRef< const { highlightedMessageId, isAtBottom, - hasReachedTop, newMessageCount, onScroll, scrollToBottom, @@ -246,7 +246,8 @@ const MessageTimelineBase = React.forwardRef< hasChannelIntro: channelIntro !== null && directMessageIntro === null, hasDirectMessageIntro: directMessageIntro !== null, hasReachedChannelStart: - (!hasOlderMessages && hasReachedTop) || messages.length === 0, + !isRenderedTimelineBehindHistoryPrepend(deferredMessages, messages) && + (messages.length === 0 || (!hasOlderMessages && !isFetchingOlder)), isSkeletonVisible: showTimelineSkeleton, }); const showDirectMessageIntro = @@ -342,6 +343,19 @@ const MessageTimelineBase = React.forwardRef< /> ) : null} + {isFetchingOlder ? ( +
+ + + +
+ ) : null}
- {isFetchingOlder ? ( -
- -
- ) : null} -
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. */ + /** True while an older-history fetch is in flight. Threaded in as a + * restoration re-run trigger so the anchor reasserts itself around the + * prepend on the fetch-state toggle, not only on the `messages` change. */ isFetchingOlder?: boolean; /** When set, scroll to and highlight this message on mount and on change. */ targetMessageId?: string | null; @@ -52,8 +48,6 @@ type UseAnchoredScrollResult = { onScroll: () => void; /** True when the user is within `AT_BOTTOM_THRESHOLD_PX` of the bottom. */ isAtBottom: boolean; - /** True once the user has reached the top of the currently loaded timeline. */ - hasReachedTop: boolean; /** Number of new messages that have arrived while the user is not at the * bottom. Cleared when the user returns to the bottom. */ newMessageCount: number; @@ -79,18 +73,6 @@ function isAtBottomNow(container: HTMLDivElement) { ); } -function isAtTopNow(container: HTMLDivElement) { - return container.scrollTop <= AT_TOP_THRESHOLD_PX; -} - -function isAtTimelineStartNow(container: HTMLDivElement) { - const scrollableDistance = Math.max( - 0, - container.scrollHeight - container.clientHeight, - ); - return scrollableDistance <= AT_TOP_THRESHOLD_PX || isAtTopNow(container); -} - /** * Pick an anchor for the current scroll position. * @@ -243,7 +225,6 @@ export function useAnchoredScroll({ // layout effect below so the read is consistent with what's in the DOM. const messagesRef = React.useRef(messages); const [isAtBottom, setIsAtBottom] = React.useState(true); - const [hasReachedTop, setHasReachedTop] = React.useState(false); const [newMessageCount, setNewMessageCount] = React.useState(0); const [highlightedMessageId, setHighlightedMessageId] = React.useState< string | null @@ -268,7 +249,6 @@ export function useAnchoredScroll({ React.useLayoutEffect(() => { anchorRef.current = { kind: "at-bottom" }; setIsAtBottom(true); - setHasReachedTop(false); setNewMessageCount(0); setHighlightedMessageId(null); hasInitializedRef.current = false; @@ -372,7 +352,6 @@ export function useAnchoredScroll({ anchorRef.current = computeAnchor(container); const atBottom = anchorRef.current.kind === "at-bottom"; setIsAtBottom((prev) => (prev === atBottom ? prev : atBottom)); - setHasReachedTop((current) => current || isAtTimelineStartNow(container)); if (atBottom) { setNewMessageCount(0); } @@ -384,7 +363,7 @@ export function useAnchoredScroll({ // before the render. This is the single mechanism for keeping scroll // stable across prepends, appends, image loads, embed expansions, etc. // --------------------------------------------------------------------------- - // 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. + // biome-ignore lint/correctness/useExhaustiveDependencies: `isFetchingOlder` is an intentional re-run trigger, not a read. It re-runs restoration on fetch-state toggles so the anchor reasserts itself around the prepend; the correction is a no-op when nothing above the anchor moved. React.useLayoutEffect(() => { const container = scrollContainerRef.current; if (!container) return; @@ -420,7 +399,6 @@ export function useAnchoredScroll({ scrollToBottomImperative("auto"); } hasInitializedRef.current = true; - setHasReachedTop((current) => current || isAtTimelineStartNow(container)); prevLastMessageIdRef.current = messages[messages.length - 1]?.id; prevMessageCountRef.current = messages.length; return; @@ -471,7 +449,6 @@ export function useAnchoredScroll({ prevLastMessageIdRef.current = lastMessage?.id; prevMessageCountRef.current = messages.length; - setHasReachedTop((current) => current || isAtTimelineStartNow(container)); }, [ isFetchingOlder, isLoading, @@ -504,14 +481,32 @@ export function useAnchoredScroll({ let disposed = false; let observer: IntersectionObserver | null = null; + let rearmFrame = 0; + // Once the timeline is scrollable, a parked sentinel must not keep + // re-firing: require it to actually leave and re-enter the preload band + // (a real scroll) before the next fetch. Without this, re-observing a + // still-intersecting sentinel synthesizes back-to-back fetches — the + // "spinner flashes a few times then a burst of rows" on reply-heavy + // channels. Auto-fill of a not-yet-scrollable page bypasses the gate. + let mustExitBandBeforeFetch = false; const start = () => { if (disposed) return; observer = new IntersectionObserver( ([entry]) => { - if (!entry?.isIntersecting || disposed || fetchingOlderRef.current) { + if (!entry?.isIntersecting) { + mustExitBandBeforeFetch = false; return; } + if (disposed || fetchingOlderRef.current || mustExitBandBeforeFetch) { + return; + } + + // One older fetch at a time. While a scroll-up is in flight, drop + // further triggers outright rather than queueing retries — fast + // scrolling otherwise stacks several sequential page loads. The + // post-fetch re-arm fires the next page only when the sentinel is + // still (or again) in the preload band. fetchingOlderRef.current = true; observer?.disconnect(); @@ -526,8 +521,18 @@ export function useAnchoredScroll({ }) .finally(() => { fetchingOlderRef.current = false; - // Re-observe in case there's more history to load. - start(); + // If the prepend made the timeline scrollable, require a real + // scroll (sentinel leaving the band) before the next fetch. + // A still-too-short page keeps auto-filling. + mustExitBandBeforeFetch = + container.scrollHeight - container.clientHeight > + AT_BOTTOM_THRESHOLD_PX; + // Re-observe next frame so the fresh observer's callback sees the + // post-prepend intersection state. + rearmFrame = window.requestAnimationFrame(() => { + rearmFrame = 0; + start(); + }); }); }, { root: container, rootMargin: "200px 0px 0px 0px" }, @@ -538,6 +543,9 @@ export function useAnchoredScroll({ start(); return () => { disposed = true; + if (rearmFrame !== 0) { + window.cancelAnimationFrame(rearmFrame); + } observer?.disconnect(); }; }, [ @@ -637,7 +645,6 @@ export function useAnchoredScroll({ return { onScroll, isAtBottom, - hasReachedTop, newMessageCount, highlightedMessageId, scrollToBottom: scrollToBottomImperative, diff --git a/desktop/src/features/messages/useFetchOlderMessages.ts b/desktop/src/features/messages/useFetchOlderMessages.ts index ad4d39d9f..50ad7ca9d 100644 --- a/desktop/src/features/messages/useFetchOlderMessages.ts +++ b/desktop/src/features/messages/useFetchOlderMessages.ts @@ -1,24 +1,10 @@ import { useCallback, useRef, useState } from "react"; import { useQueryClient } from "@tanstack/react-query"; -import { countTopLevelTimelineRows } from "@/features/messages/lib/formatTimelineMessages"; -import { - channelMessagesKey, - mergeTimelineHistoryMessages, -} from "@/features/messages/lib/messageQueryKeys"; -import { relayClient } from "@/shared/api/relayClient"; +import { channelMessagesKey } from "@/features/messages/lib/messageQueryKeys"; +import { pageOlderMessagesUntilRowFloor } from "@/features/messages/lib/pageOlderMessages"; import type { Channel, RelayEvent } from "@/shared/api/types"; -const OLDER_MESSAGES_BATCH_SIZE = 100; - -// One scroll-up should advance the timeline by a predictable, *visible* amount. -// Because thread replies collapse into their parent and non-content events -// never render, a single 100-message batch can add far fewer rows than that — -// so we page in additional batches until at least this many top-level rows have -// been added (or history runs out). Counting rows, not messages, keeps a -// reply-heavy window from feeling like the fetch did nothing. -const MIN_TOP_LEVEL_ROWS_PER_FETCH = 10; - export function useFetchOlderMessages(channel: Channel | null) { const queryClient = useQueryClient(); const channelId = channel?.id ?? null; @@ -54,60 +40,15 @@ export function useFetchOlderMessages(channel: Channel | null) { isFetchingOlderRef.current = true; setIsFetchingOlder(true); - - // Page in batches until the timeline has gained at least - // MIN_TOP_LEVEL_ROWS_PER_FETCH *visible* rows or history is exhausted. - // A single batch is fetched first; only reply-heavy windows that fall short - // of the floor loop again. Each batch re-reads the oldest timestamp from the - // cache so successive `until` values keep walking backward without gaps. - const baselineRowCount = countTopLevelTimelineRows(currentMessages); try { - while (hasOlderMessagesRef.current) { - const messagesBeforeFetch = - queryClient.getQueryData(queryKey) ?? []; - if (messagesBeforeFetch.length === 0) { - break; - } - - // Use the oldest timestamp directly — `until` is inclusive so the relay - // will return the boundary message again, but `sortMessages` - // deduplicates by id. Subtracting 1 risks skipping messages that share - // the same second. - const oldestTimestamp = messagesBeforeFetch[0].created_at; - const olderMessages = await relayClient.fetchChannelHistoryBefore( - channelId, - oldestTimestamp, - OLDER_MESSAGES_BATCH_SIZE, - ); - - if (olderMessages.length < OLDER_MESSAGES_BATCH_SIZE) { - hasOlderMessagesRef.current = false; - setHasOlderMessages(false); - } - - if (olderMessages.length > 0) { - queryClient.setQueryData(queryKey, (current = []) => - mergeTimelineHistoryMessages(current, olderMessages), - ); - - const updatedMessages = - queryClient.getQueryData(queryKey) ?? []; - if ( - updatedMessages.length > 0 && - updatedMessages[0].created_at === oldestTimestamp - ) { - hasOlderMessagesRef.current = false; - setHasOlderMessages(false); - } - } - - const rowsGained = - countTopLevelTimelineRows( - queryClient.getQueryData(queryKey) ?? [], - ) - baselineRowCount; - if (rowsGained >= MIN_TOP_LEVEL_ROWS_PER_FETCH) { - break; - } + const { hasOlderMessages: more } = await pageOlderMessagesUntilRowFloor( + queryClient, + channelId, + () => previousChannelIdRef.current === channelId, + ); + if (!more) { + hasOlderMessagesRef.current = false; + setHasOlderMessages(false); } } catch (error) { console.error("Failed to fetch older messages", channelId, error); diff --git a/desktop/src/shared/api/relayChannelFilters.ts b/desktop/src/shared/api/relayChannelFilters.ts new file mode 100644 index 000000000..2f80b506f --- /dev/null +++ b/desktop/src/shared/api/relayChannelFilters.ts @@ -0,0 +1,104 @@ +import { + CHANNEL_AUX_EVENT_KINDS, + CHANNEL_EVENT_KINDS, + CHANNEL_TIMELINE_CONTENT_KINDS, + HOME_MENTION_EVENT_KINDS, +} from "@/shared/constants/kinds"; +import type { RelaySubscriptionFilter } from "@/shared/api/relayClientShared"; + +// Auxiliary-event backfill: `#e` filters reference loaded message ids to pull +// their reactions/edits/deletions. Chunk the ids so each REQ stays within +// relay filter limits, and let each chunk return up to the relay's WS cap — +// a single reaction-heavy message can have many aux events. +export const AUX_BACKFILL_CHUNK_SIZE = 100; +export const MAX_HISTORICAL_LIMIT = 10_000; + +/** + * Live-subscription filter for an open channel: the broad + * {@link CHANNEL_EVENT_KINDS} set so the tail delivers reactions/edits/ + * deletions for future messages as well as new message rows. + */ +export function buildChannelFilter( + channelId: string, + limit: number, + until?: number, +): RelaySubscriptionFilter { + const filter: RelaySubscriptionFilter = { + kinds: [...CHANNEL_EVENT_KINDS], + "#h": [channelId], + limit, + }; + + if (until !== undefined) { + filter.until = until; + } + + return filter; +} + +/** + * History filter for cold-load and scrollback: message kinds *only*, so the + * `limit` budget buys visible message depth. Auxiliary events (reactions, + * edits, deletions) are backfilled separately by `#e` reference via + * {@link buildChannelAuxFilter}, and arrive for future messages through the + * live subscription ({@link buildChannelFilter}, which keeps the broad + * {@link CHANNEL_EVENT_KINDS} set). + */ +export function buildChannelHistoryFilter( + channelId: string, + limit: number, + until?: number, +): RelaySubscriptionFilter { + const filter: RelaySubscriptionFilter = { + kinds: [...CHANNEL_TIMELINE_CONTENT_KINDS], + "#h": [channelId], + limit, + }; + + if (until !== undefined) { + filter.until = until; + } + + return filter; +} + +/** + * Aux-backfill filter for one chunk of loaded message ids: pulls reactions/ + * edits/deletions ({@link CHANNEL_AUX_EVENT_KINDS}) that reference those ids + * by `#e`. Keyed by reference, not time, so a late edit/deletion for an old + * visible message still applies — see {@link buildChannelHistoryFilter}. + */ +export function buildChannelAuxFilter( + channelId: string, + messageIds: string[], +): RelaySubscriptionFilter { + return { + kinds: [...CHANNEL_AUX_EVENT_KINDS], + "#h": [channelId], + "#e": messageIds, + limit: MAX_HISTORICAL_LIMIT, + }; +} + +export function buildGlobalStreamFilter( + limit: number, +): RelaySubscriptionFilter { + return { + kinds: [...CHANNEL_EVENT_KINDS], + limit, + }; +} + +export function buildChannelMentionFilter( + channelId: string, + pubkey: string, + limit: number, +): RelaySubscriptionFilter { + return { + kinds: [...HOME_MENTION_EVENT_KINDS], + "#h": [channelId], + "#p": [pubkey], + limit, + since: Math.floor(Date.now() / 1_000), + }; +} diff --git a/desktop/src/shared/api/relayClientSession.ts b/desktop/src/shared/api/relayClientSession.ts index 1d14265a4..583ab9b6e 100644 --- a/desktop/src/shared/api/relayClientSession.ts +++ b/desktop/src/shared/api/relayClientSession.ts @@ -7,8 +7,6 @@ import { } from "@/shared/api/tauri"; import type { PresenceStatus, RelayEvent } from "@/shared/api/types"; import { - CHANNEL_EVENT_KINDS, - HOME_MENTION_EVENT_KINDS, KIND_STREAM_MESSAGE, KIND_TYPING_INDICATOR, KIND_USER_STATUS, @@ -21,6 +19,14 @@ import { type RelaySubscription, type RelaySubscriptionFilter, } from "@/shared/api/relayClientShared"; +import { + AUX_BACKFILL_CHUNK_SIZE, + buildChannelAuxFilter, + buildChannelFilter, + buildChannelHistoryFilter, + buildChannelMentionFilter, + buildGlobalStreamFilter, +} from "@/shared/api/relayChannelFilters"; import { replayLiveSubscriptions } from "@/shared/api/relayReconnectReplay"; import { RelayConnectionStateEmitter } from "@/shared/api/relayConnectionStateEmitter"; import { @@ -148,7 +154,7 @@ export class RelayClient { } async fetchChannelHistory(channelId: string, limit = 50) { - return this.fetchHistory(this.buildChannelFilter(channelId, limit)); + return this.fetchHistory(buildChannelHistoryFilter(channelId, limit)); } async fetchChannelHistoryBefore( @@ -156,7 +162,49 @@ export class RelayClient { before: number, limit = 50, ) { - return this.fetchHistory(this.buildChannelFilter(channelId, limit, before)); + return this.fetchHistory( + buildChannelHistoryFilter(channelId, limit, before), + ); + } + + /** + * Fetch the auxiliary events (reactions, edits, deletions) that + * reference a set of already-loaded message ids, keyed by `#e` rather than a + * time window. + * + * History fetches deliberately request message kinds only, so the `limit` + * budget buys visible message depth instead of being diluted by aux events + * (on a reaction-heavy channel a 200-event window was only ~136 messages). + * The trade-off is that an edit/deletion for a visible message can fall + * outside any message time window — so we must pull aux by reference, not by + * time, or a visible message would render stale (un-edited / not-deleted). + * + * Batched: `#e` filters can grow large, so message ids are chunked to keep + * each REQ within relay filter limits. Results across chunks are merged. + */ + async fetchAuxEventsForMessages( + channelId: string, + messageIds: string[], + ): Promise { + if (messageIds.length === 0) { + return []; + } + + await this.ensureConnected(); + + const chunks: string[][] = []; + for (let i = 0; i < messageIds.length; i += AUX_BACKFILL_CHUNK_SIZE) { + chunks.push(messageIds.slice(i, i + AUX_BACKFILL_CHUNK_SIZE)); + } + + const batches: RelayEvent[][] = []; + for (const ids of chunks) { + batches.push( + await this.requestHistory(buildChannelAuxFilter(channelId, ids)), + ); + } + + return batches.flat(); } async fetchEvents(filter: RelaySubscriptionFilter): Promise { @@ -269,7 +317,7 @@ export class RelayClient { channelId: string, onEvent: (event: RelayEvent) => void, ) { - return this.subscribe(this.buildChannelFilter(channelId, 50), onEvent); + return this.subscribe(buildChannelFilter(channelId, 50), onEvent); } /** @@ -357,7 +405,7 @@ export class RelayClient { } async subscribeToAllStreamMessages(onEvent: (event: RelayEvent) => void) { - return this.subscribe(this.buildGlobalStreamFilter(50), onEvent); + return this.subscribe(buildGlobalStreamFilter(50), onEvent); } async subscribeLive( @@ -373,7 +421,7 @@ export class RelayClient { onEvent: (event: RelayEvent) => void, ) { return this.subscribe( - this.buildChannelMentionFilter(channelId, pubkey, 50), + buildChannelMentionFilter(channelId, pubkey, 50), onEvent, ); } @@ -487,45 +535,6 @@ export class RelayClient { this.emitReconnectIfNeeded(); } - private buildChannelFilter( - channelId: string, - limit: number, - until?: number, - ): RelaySubscriptionFilter { - const filter: RelaySubscriptionFilter = { - kinds: [...CHANNEL_EVENT_KINDS], - "#h": [channelId], - limit, - }; - - if (until !== undefined) { - filter.until = until; - } - - return filter; - } - - private buildGlobalStreamFilter(limit: number): RelaySubscriptionFilter { - return { - kinds: [...CHANNEL_EVENT_KINDS], - limit, - }; - } - - private buildChannelMentionFilter( - channelId: string, - pubkey: string, - limit: number, - ): RelaySubscriptionFilter { - return { - kinds: [...HOME_MENTION_EVENT_KINDS], - "#h": [channelId], - "#p": [pubkey], - limit, - since: Math.floor(Date.now() / 1_000), - }; - } - private async subscribe( filter: RelaySubscriptionFilter, onEvent: (event: RelayEvent) => void, diff --git a/desktop/src/shared/api/relayClientShared.test.mjs b/desktop/src/shared/api/relayClientShared.test.mjs index de3180e8a..0bbf6ba19 100644 --- a/desktop/src/shared/api/relayClientShared.test.mjs +++ b/desktop/src/shared/api/relayClientShared.test.mjs @@ -1,7 +1,33 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { isRelayConnectionDegraded } from "./relayClientShared.ts"; +import { isRelayConnectionDegraded, sortEvents } from "./relayClientShared.ts"; + +function event(id, createdAt) { + return { + id, + pubkey: "pubkey", + created_at: createdAt, + kind: 9, + tags: [], + content: "", + sig: "sig", + }; +} + +test("sortEvents — same-second events sort by id, order-independent", () => { + const a = event("aaa", 100); + const b = event("bbb", 100); + const c = event("ccc", 101); + + const forward = sortEvents([a, b, c]).map((e) => e.id); + const shuffled = sortEvents([c, b, a]).map((e) => e.id); + + // Stable (created_at, id) order regardless of input order, matching the + // cache sort (sortMessages) and the relay's id-ASC same-second tiebreak. + assert.deepEqual(forward, ["aaa", "bbb", "ccc"]); + assert.deepEqual(shuffled, ["aaa", "bbb", "ccc"]); +}); test("isRelayConnectionDegraded — healthy states are not degraded", () => { assert.equal(isRelayConnectionDegraded("idle"), false); diff --git a/desktop/src/shared/api/relayClientShared.ts b/desktop/src/shared/api/relayClientShared.ts index 996c54154..379f3b530 100644 --- a/desktop/src/shared/api/relayClientShared.ts +++ b/desktop/src/shared/api/relayClientShared.ts @@ -63,7 +63,16 @@ export type PendingEvent = { export type RelaySubscription = HistorySubscription | LiveSubscription; export function sortEvents(events: RelayEvent[]) { - return [...events].sort((left, right) => left.created_at - right.created_at); + return [...events].sort((left, right) => { + if (left.created_at !== right.created_at) { + return left.created_at - right.created_at; + } + // Same (created_at, id) tiebreak as the cache sort (sortMessages) so a + // history REQ resolves same-second events in a stable, relay-matching + // order. Currently every consumer re-sorts downstream, but keeping the + // two sorts on one invariant avoids a latent ordering drift. + return left.id < right.id ? -1 : left.id > right.id ? 1 : 0; + }); } export function getTextPayload(message: unknown) { diff --git a/desktop/src/shared/api/relayReconnectReplay.test.mjs b/desktop/src/shared/api/relayReconnectReplay.test.mjs index 4f1c2731e..a58aa7f25 100644 --- a/desktop/src/shared/api/relayReconnectReplay.test.mjs +++ b/desktop/src/shared/api/relayReconnectReplay.test.mjs @@ -5,7 +5,7 @@ import { buildReconnectReplayFilter, replayLiveSubscriptions, } from "./relayReconnectReplay.ts"; -import { RelayClient } from "./relayClientSession.ts"; +import { buildChannelFilter } from "./relayChannelFilters.ts"; function replayFilter(filter, since, until) { return buildReconnectReplayFilter(filter, since, until); @@ -111,8 +111,7 @@ test("channel reconnect replay pages the missed window until a short page", asyn eventRange("middle", 1002, 500), eventRange("oldest", 995, 8), ]; - const client = new RelayClient(); - const filter = client.buildChannelFilter("channel-1", 50); + const filter = buildChannelFilter("channel-1", 50); const subscriptions = new Map([ [ "live-1", diff --git a/desktop/src/shared/constants/kinds.ts b/desktop/src/shared/constants/kinds.ts index 89b41c3e0..3cb325ff3 100644 --- a/desktop/src/shared/constants/kinds.ts +++ b/desktop/src/shared/constants/kinds.ts @@ -65,6 +65,41 @@ export const CHANNEL_EVENT_KINDS = [ KIND_SYSTEM_MESSAGE, // 40099 — system messages (join, leave, etc.) ] as const; +// Auxiliary (non-row) timeline kinds: events that overlay onto or hide an +// existing message rather than rendering their own row — reactions, edits, and +// deletions. History fetches request the visible content kinds only, so the +// `limit` budget buys visible message depth instead of being diluted by these +// (on a reaction-heavy channel a 200-event window was only ~136 messages). +// They are backfilled separately by `#e` reference over the loaded message ids +// — by reference, not by time window, so a late edit/delete for a visible old +// message still applies. NOTE: kind:40008 (diff) renders its OWN row, so it is +// a content kind, not aux. +export const CHANNEL_AUX_EVENT_KINDS = [ + KIND_DELETION, // 5 — NIP-09 event deletions + KIND_REACTION, // 7 — NIP-25 reactions + KIND_NIP29_DELETE_EVENT, // 9005 — NIP-29 / Buzz-native deletions + KIND_STREAM_MESSAGE_EDIT, // 40003 — message edits +] as const; + +// Visible content kinds the main timeline renders as their own rows. Mirrors +// `isTimelineContentEvent` in formatTimelineMessages.ts — keep the two in sync. +// This is the kind set the history fetch requests so the `limit` budget maps +// to visible rows; auxiliary overlays (CHANNEL_AUX_EVENT_KINDS) are fetched +// separately by `#e` reference. Forum kinds (45001/45003) are excluded: forum +// channels use a different query path, not this timeline. +export const CHANNEL_TIMELINE_CONTENT_KINDS = [ + KIND_STREAM_MESSAGE, // 9 + KIND_STREAM_MESSAGE_V2, // 40002 + KIND_STREAM_MESSAGE_DIFF, // 40008 — diff messages (own row) + KIND_SYSTEM_MESSAGE, // 40099 — system rows (join/leave/channel-created) + KIND_JOB_REQUEST, // 43001 + KIND_JOB_ACCEPTED, // 43002 + KIND_JOB_PROGRESS, // 43003 + KIND_JOB_RESULT, // 43004 + KIND_JOB_CANCEL, // 43005 + KIND_JOB_ERROR, // 43006 +] as const; + // Timeline kinds that are NOT conversational: relay-signed system rows // (channel-created, member-joined) and job-lifecycle events. These render in // the timeline but must not count toward the channel's unread pill — a freshly diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 37de50c5e..5fa2c2095 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -466,6 +466,7 @@ type MockSubscription = { type MockFilter = { "#d"?: string[]; + "#e"?: string[]; "#h"?: string[]; authors?: string[]; kinds?: number[]; @@ -2370,8 +2371,22 @@ function emitMockHistory( // `history-` subscriptions — the prefix `relayClientSession` uses for // older-message pagination — so live/initial subscriptions stay instant. const delayMs = getConfig()?.mock?.historyDelayMs ?? 0; - if (delayMs > 0 && subId.startsWith("history-")) { - window.setTimeout(emit, delayMs); + const isVisibleOlderHistoryPage = + subId.startsWith("history-") && filter.until !== undefined && !filter["#e"]; + if (delayMs > 0 && isVisibleOlderHistoryPage) { + const probe = window as unknown as { + __HISTORY_INFLIGHT__?: number; + __HISTORY_INFLIGHT_PEAK__?: number; + }; + probe.__HISTORY_INFLIGHT__ = (probe.__HISTORY_INFLIGHT__ ?? 0) + 1; + probe.__HISTORY_INFLIGHT_PEAK__ = Math.max( + probe.__HISTORY_INFLIGHT_PEAK__ ?? 0, + probe.__HISTORY_INFLIGHT__, + ); + window.setTimeout(() => { + probe.__HISTORY_INFLIGHT__ = (probe.__HISTORY_INFLIGHT__ ?? 1) - 1; + emit(); + }, delayMs); return; } diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index 71e6b1b7a..55076402a 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -81,7 +81,7 @@ test("preserves user scroll while older channel history loads", async ({ } window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ channelName: "general", - count: 250, + count: 600, lineCount: 3, }); }); @@ -207,7 +207,7 @@ test("does not teleport upward when user abandons fetch by jumping to bottom", a } window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ channelName: "general", - count: 250, + count: 600, lineCount: 3, }); }); @@ -465,18 +465,16 @@ test("deep-link to a message in older history scrolls and highlights it", async } const events = window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ channelName: "general", - count: 250, + count: 600, lineCount: 3, }); return (events ?? []).map((event) => event.id); }); expect(prependedIds.length).toBeGreaterThanOrEqual(100); - // Pick a target from the OLDER half of the prepended block. The initial - // history slice on channel open is limited to ~50 events; anything in - // the older half is guaranteed to be outside the first render window. - // prependedIds are emitted in chronological order (older first), so the - // first quarter is reliably old. + // Pick a target from the older part of the prepended block, well outside + // the 300-event cold history window. prependedIds are chronological (older + // first), so an early index remains unloaded on channel open. const targetId = prependedIds[Math.floor(prependedIds.length / 8)]; expect(targetId).toBeTruthy(); @@ -1048,3 +1046,345 @@ test("in-viewport reflow above the anchor row does not push it down", async ({ ) .toBeLessThanOrEqual(2); }); + +// Regression: the channel intro ("header") must never flash mid-content while +// an older-history page is still loading. Reproduces the reported bug where the +// header appeared in the middle of history during pagination. The companion +// "scrollable channel ... hides intro actions until top" test in channels.spec +// already proves the positive (intro shows once at the true, exhausted top); +// this proves the negative (it stays hidden while a fetch is in flight). +// +// Uses real wheel input, not `scrollTop = 0`, because the timeline is +// virtualized (Virtuoso) and re-asserts its tracked scroll position after a +// raw scrollTop write -- see the abandon test above for the same rationale. +test("channel intro stays hidden while older history is loading", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", + ); + + await page.evaluate(() => { + for (let index = 0; index < 40; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `visible current ${index}\nsecond line ${index}`, + }); + } + window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ + channelName: "general", + count: 600, + lineCount: 3, + }); + }); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + const timeline = page.getByTestId("message-timeline"); + await expect(timeline.locator("[data-message-id]").first()).toBeVisible(); + + // Pace the older fetch so it stays in flight long enough to assert against. + await page.evaluate(() => { + window.__BUZZ_E2E__ = { + ...window.__BUZZ_E2E__, + mock: { + ...window.__BUZZ_E2E__?.mock, + historyDelayMs: 5_000, + }, + }; + }); + + await page.waitForFunction(() => { + const element = document.querySelector( + '[data-testid="message-timeline"]', + ) as HTMLDivElement | null; + return element ? element.scrollHeight > element.clientHeight + 1000 : false; + }); + + // At the bottom on initial open: intro hidden. + await expect(page.getByTestId("message-channel-intro")).toHaveCount(0); + + // Wheel up to the top to trigger the (5s-delayed) older fetch. + await timeline.hover(); + for (let attempt = 0; attempt < 32; attempt += 1) { + const metrics = await getTimelineMetrics(page); + if (metrics.scrollTop < 500) { + break; + } + await page.mouse.wheel(0, -2000); + } + await page.waitForTimeout(150); + + // We are near the top of the loaded window AND the prepend has not resolved + // (history is still in flight). The intro MUST stay hidden -- this is the + // mid-load flash the fix prevents. + const duringFetch = await getTimelineMetrics(page); + expect(duringFetch.scrollTop).toBeLessThan(500); + await expect(page.getByTestId("message-channel-intro")).toHaveCount(0); + + // It must remain hidden while the fetch indicator is visible. Once the + // delayed fetch completes, the channel may legitimately be at the true start + // and show the intro. + await expect + .poll( + async () => { + const introCount = await page + .getByTestId("message-channel-intro") + .count(); + const indicatorCount = await page + .getByTestId("message-timeline-fetching-older") + .count(); + if (indicatorCount > 0 && introCount > 0) { + return "intro-appeared-during-fetch"; + } + return indicatorCount === 0 ? "resolved" : "pending"; + }, + { timeout: 7_000 }, + ) + .toBe("resolved"); +}); + +// Regression for the architectural bug Wes reported: in a channel with MORE +// content events than the timeline window cap (MAX_TIMELINE_MESSAGES = 2000), +// scrolling back must keep paginating older history WITHOUT ever flashing the +// channel intro ("header") mid-content. The old code inferred exhaustion from a +// post-merge oldest-timestamp compare; once the cap evicted a freshly-prepended +// older root, that compare wrongly fired `hasOlderMessages=false`, popping the +// header and then flooding in a batch. This test crosses the cap on purpose and +// asserts the header stays hidden while genuine older history still loads. +test("channel intro stays hidden while paginating past the timeline cap", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", + ); + + // Seed well past the 2000-event cap: a current window plus ~2600 older roots. + // Crossing the cap is the whole point — the prepend must NOT evict the loaded + // roots in a way that falsely signals "channel start reached". + await page.evaluate(() => { + for (let index = 0; index < 60; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `recent ${index}`, + }); + } + window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ + channelName: "general", + count: 2600, + lineCount: 2, + }); + }); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + const timeline = page.getByTestId("message-timeline"); + await expect(timeline.locator("[data-message-id]").first()).toBeVisible(); + + // At the bottom on open: intro hidden. + await expect(page.getByTestId("message-channel-intro")).toHaveCount(0); + + // Scroll back until pagination reaches the true channel start (the oldest + // seeded root, "mock older 0") or clearly stalls. Each pass triggers an + // older-history fetch; we wait for it to settle (oldest rendered id advances) + // before the next wheel. The intro must never appear while older roots remain. + // + // With the old newest-2000 cap, the freshly-prepended older roots are evicted + // once the cache crosses MAX_TIMELINE_MESSAGES, so the oldest rendered row + // cannot advance past the window boundary — the loop stalls well above index + // 0. The fixed cap retains backward-paged roots, so it reaches the start. + const oldestRenderedIndex = async () => + page.getByTestId("message-timeline").evaluate((element) => { + const rows = (element as HTMLDivElement).querySelectorAll( + "[data-message-id]", + ); + let min = Number.POSITIVE_INFINITY; + for (const row of rows) { + const match = row.textContent?.match(/mock older (\d+)/); + if (match) min = Math.min(min, Number(match[1])); + } + return Number.isFinite(min) ? min : null; + }); + + const isIntroHeaderInViewport = async () => + page.getByTestId("message-timeline").evaluate((timelineElement) => { + const icon = timelineElement.querySelector( + '[data-testid="message-channel-intro-icon"]', + ); + if (!icon) return false; + const timelineRect = timelineElement.getBoundingClientRect(); + const iconRect = icon.getBoundingClientRect(); + return ( + iconRect.bottom > timelineRect.top && iconRect.top < timelineRect.bottom + ); + }); + + await timeline.hover(); + let deepest = Number.POSITIVE_INFINITY; + let stallStreak = 0; + for (let attempt = 0; attempt < 200 && deepest > 0; attempt += 1) { + await page.mouse.wheel(0, -4000); + await page.waitForTimeout(80); + + const current = await oldestRenderedIndex(); + if ((current ?? Number.POSITIVE_INFINITY) > 50) { + expect(await isIntroHeaderInViewport()).toBe(false); + } + + if (current !== null && current < deepest) { + deepest = current; + stallStreak = 0; + } else { + stallStreak += 1; + // Already at the top of the rendered window and not advancing: give the + // in-flight fetch a beat to prepend, then bail if it never progresses. + await page.waitForTimeout(200); + if (stallStreak > 25) break; + } + } + + // Reached deep history (near the true start) without the cap evicting the + // backward-paged roots. < 50 leaves slack for virtualization not rendering + // the literal index-0 row at the exact top. + expect(deepest).toBeLessThan(50); +}); + +// Regression for the flood Wes reported: scrolling back while an older-history +// fetch is already in flight must NOT queue a second concurrent visible page +// fetch. Aux backfills also use `history-*` subscriptions, so the e2e bridge +// probe counts only visible older-page requests (`until` and not `#e`). +test("older-history fetches never overlap (no concurrent in-flight requests)", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", + ); + + await page.evaluate(() => { + for (let index = 0; index < 60; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `recent ${index}`, + }); + } + window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ + channelName: "general", + count: 1200, + lineCount: 2, + }); + ( + window as unknown as { __HISTORY_INFLIGHT_PEAK__?: number } + ).__HISTORY_INFLIGHT_PEAK__ = 0; + window.__BUZZ_E2E__ = { + ...window.__BUZZ_E2E__, + mock: { ...window.__BUZZ_E2E__?.mock, historyDelayMs: 400 }, + }; + }); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + const timeline = page.getByTestId("message-timeline"); + await expect(timeline.locator("[data-message-id]").first()).toBeVisible(); + + await timeline.hover(); + await timeline.evaluate((element) => { + const timelineElement = element as HTMLDivElement; + timelineElement.scrollTop = 150; + timelineElement.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + await expect(page.getByTestId("message-timeline-fetching-older")).toBeVisible( + { timeout: 2_000 }, + ); + + for (let attempt = 0; attempt < 40; attempt += 1) { + await page.mouse.wheel(0, -3000); + await page.waitForTimeout(50); + } + await page.waitForTimeout(800); + + const peak = await page.evaluate( + () => + (window as unknown as { __HISTORY_INFLIGHT_PEAK__?: number }) + .__HISTORY_INFLIGHT_PEAK__ ?? 0, + ); + expect(peak).toBeLessThanOrEqual(1); +}); + +// Regression for Wes's "I only see the loading indicator sometimes": the +// older-fetch spinner used to be an in-flow element at content-top, so it +// scrolled out of view the moment the reader was anywhere but the very top. +// It must be pinned to the viewport so it's visible while a backward fetch is +// in flight regardless of scroll position. +test("older-history spinner stays visible in viewport while fetching mid-scroll", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", + ); + + await page.evaluate(() => { + for (let index = 0; index < 60; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `recent ${index}`, + }); + } + window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ + channelName: "general", + count: 1200, + lineCount: 2, + }); + window.__BUZZ_E2E__ = { + ...window.__BUZZ_E2E__, + mock: { ...window.__BUZZ_E2E__?.mock, historyDelayMs: 2_000 }, + }; + }); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + const timeline = page.getByTestId("message-timeline"); + await expect(timeline.locator("[data-message-id]").first()).toBeVisible(); + + await timeline.hover(); + await timeline.evaluate((element) => { + const timelineElement = element as HTMLDivElement; + timelineElement.scrollTop = 150; + timelineElement.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + + const indicator = page.getByTestId("message-timeline-fetching-older"); + await expect(indicator).toBeVisible({ timeout: 2_000 }); + + const { scrollTop } = await getTimelineMetrics(page); + expect(scrollTop).toBeGreaterThan(8); + + const withinViewport = await page.evaluate(() => { + const timelineEl = document.querySelector( + '[data-testid="message-timeline"]', + ); + const spinnerEl = document.querySelector( + '[data-testid="message-timeline-fetching-older"]', + ); + if (!timelineEl || !spinnerEl) return false; + const t = timelineEl.getBoundingClientRect(); + const s = spinnerEl.getBoundingClientRect(); + return s.top >= t.top - 1 && s.bottom <= t.bottom + 1; + }); + expect(withinViewport).toBe(true); +}); From 85028d7f3608132f7d1bc97b1adf9ceb03982ed9 Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 20 Jun 2026 11:34:14 -0600 Subject: [PATCH 2/9] desktop: keep scrollback spinner up until prepended rows paint The older-history spinner was bound to isFetchingOlder, which clears the moment the fetch resolves. Rows render off a deferred (concurrent) snapshot a frame or two later, so the spinner vanished before the new rows appeared. Hold it visible while the rendered timeline is still behind the history prepend. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/src/features/messages/ui/MessageTimeline.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 1e6adfbb4..47efee247 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -343,7 +343,10 @@ const MessageTimelineBase = React.forwardRef< />
) : null} - {isFetchingOlder ? ( + {/* `isFetchingOlder` clears on fetch resolve, but rows paint a frame + later off the deferred snapshot — keep the spinner up until then. */} + {isFetchingOlder || + isRenderedTimelineBehindHistoryPrepend(deferredMessages, messages) ? (
Date: Sat, 20 Jun 2026 11:43:43 -0600 Subject: [PATCH 3/9] desktop: hold the skeleton through the whole cold load First channel load showed skeleton, then briefly the older-fetch spinner, then messages. The cold-load query seeds rows before its row-floor top-up finishes, so the loading latch settled (dataLength>0) while the fetch was still running, dropping the skeleton and exposing the spinner. Keep the timeline in its loading state while the initial fetch is in flight (before the channel has settled once), so the skeleton stays up until the first load is fully done. The per-channel settle latch still owns later refetch blips, so no skeleton bounce on revisit. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- .../features/channels/ui/ChannelScreen.tsx | 23 ++++++++++------- .../lib/timelineLoadingState.test.mjs | 25 +++++++++++++++++++ .../messages/lib/timelineLoadingState.ts | 11 ++++++-- 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 7ffd1d380..14ae4203e 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -511,18 +511,23 @@ export function ChannelScreen({ }); // `data !== undefined` is not "loaded": the cache is seeded early by stale // placeholders and the live subscription. Wait for the history fetch to settle. - const timelineLoadingNow = - activeChannel !== null && - activeChannel.channelType !== "forum" && - selectTimelineLoadingState({ - isPending: messagesQuery.isPending, - isFetching: messagesQuery.isFetching, - isPlaceholderData: messagesQuery.isPlaceholderData, - dataLength: messagesQuery.data?.length ?? null, - }); // Latch loaded per channel so a later background refetch can't flip back to // the skeleton — that re-flip is the "skeleton bouncing up and down" on entry. const settledChannelIdRef = React.useRef(null); + const hasSettledThisChannel = + activeChannelId !== null && settledChannelIdRef.current === activeChannelId; + const timelineLoadingNow = + activeChannel !== null && + activeChannel.channelType !== "forum" && + selectTimelineLoadingState( + { + isPending: messagesQuery.isPending, + isFetching: messagesQuery.isFetching, + isPlaceholderData: messagesQuery.isPlaceholderData, + dataLength: messagesQuery.data?.length ?? null, + }, + hasSettledThisChannel, + ); const { settledChannelId, isLoading: isTimelineLoading } = resolveTimelineLoadingLatch( settledChannelIdRef.current, diff --git a/desktop/src/features/messages/lib/timelineLoadingState.test.mjs b/desktop/src/features/messages/lib/timelineLoadingState.test.mjs index 7359fbbd9..bb32831c3 100644 --- a/desktop/src/features/messages/lib/timelineLoadingState.test.mjs +++ b/desktop/src/features/messages/lib/timelineLoadingState.test.mjs @@ -72,6 +72,31 @@ test("background refetch of a populated channel is not loading", () => { ); }); +test("initial load holds the skeleton while the cold-load top-up fetches", () => { + // Cold load seeds rows before its row-floor top-up finishes, so dataLength>0 + // while isFetching is still true. Before settle, hold the skeleton — dropping + // it here is what exposed the older-fetch spinner on first load. + assert.equal( + selectTimelineLoadingState( + { ...settled, isFetching: true, dataLength: 8 }, + false, + ), + true, + ); +}); + +test("settled channel with rows mid-refetch is not loading", () => { + // Same query shape, but after first settle: the latch owns refetch blips, so + // present rows mean loaded. + assert.equal( + selectTimelineLoadingState( + { ...settled, isFetching: true, dataLength: 8 }, + true, + ), + false, + ); +}); + import { resolveTimelineLoadingLatch } from "./timelineLoadingState.ts"; test("latch: loading on first entry to a channel", () => { diff --git a/desktop/src/features/messages/lib/timelineLoadingState.ts b/desktop/src/features/messages/lib/timelineLoadingState.ts index f34a87ced..001c06341 100644 --- a/desktop/src/features/messages/lib/timelineLoadingState.ts +++ b/desktop/src/features/messages/lib/timelineLoadingState.ts @@ -17,12 +17,19 @@ export type TimelineQueryStatus = { export function selectTimelineLoadingState( status: TimelineQueryStatus, + hasSettled = true, ): boolean { if (status.isPending) { return true; } - // A fetch is in flight; keep loading while what we'd show is a placeholder or - // still empty. Once real rows are present we are loaded, even mid-refetch. + // Before the first settle, hold the skeleton for the whole cold load — the + // row-floor top-up keeps `isFetching` true after the cache already has rows, + // and dropping the skeleton there exposes the older-fetch spinner on first + // load. After settle, the latch protects against refetch blips, so once real + // rows are present we are loaded even mid-refetch. + if (!hasSettled) { + return status.isFetching; + } return ( status.isFetching && (status.isPlaceholderData || (status.dataLength ?? 0) === 0) From 22a1a390616f89102d830a61424f81a9e43c6723 Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 20 Jun 2026 12:15:08 -0600 Subject: [PATCH 4/9] desktop: cover first-load skeleton during history top-up Add a scroll-history regression for sparse/reply-heavy cold loads: while the initial history query is still topping up to the visible-row floor, the timeline must keep the skeleton visible and must not expose the older-history spinner. Validation: - bin/pnpm --dir desktop exec playwright test tests/e2e/scroll-history.spec.ts --workers=1 - bin/pnpm --dir desktop exec biome check tests/e2e/scroll-history.spec.ts Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/tests/e2e/scroll-history.spec.ts | 48 ++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index 55076402a..013a3fb0c 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -61,6 +61,54 @@ async function getMessagePosition( }, messageId); } +test("first channel load holds skeleton instead of showing older-history spinner", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", + ); + + await page.evaluate(() => { + const root = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: "cold-load root", + createdAt: 1_700_000_000, + }); + if (!root) throw new Error("Failed to seed cold-load root"); + + for (let index = 0; index < 360; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `cold-load reply ${index}`, + parentEventId: root.id, + createdAt: 1_700_000_001 + index, + }); + } + + window.__BUZZ_E2E__ = { + ...window.__BUZZ_E2E__, + mock: { ...window.__BUZZ_E2E__?.mock, historyDelayMs: 1_500 }, + }; + }); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + const timeline = page.getByTestId("message-timeline"); + await expect(timeline.locator(".t-skel-bar").first()).toBeVisible(); + await expect(page.getByTestId("message-timeline-fetching-older")).toHaveCount( + 0, + ); + + await expect(timeline.locator("[data-message-id]").first()).toBeVisible({ + timeout: 5_000, + }); +}); + test("preserves user scroll while older channel history loads", async ({ page, }) => { From 61a65d42f1edd9a366437639c2520e95a1f1a297 Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 20 Jun 2026 12:38:16 -0600 Subject: [PATCH 5/9] desktop: fix scroll e2e CI failures Fix the two PR #1153 smoke failures seen in CI: - Trim the scroll-history cap-crossing fixture from 2600 to 2100 older roots, still above the 2000-row cap but below the 30s CI timeout cliff. - Update the small-channel content test for the new correct behavior: a tiny fully-loaded channel is genuinely at its start, so the channel intro is visible immediately while welcome-only actions remain hidden. Validation: - CI=1 bin/pnpm --dir desktop exec playwright test tests/e2e/channels.spec.ts --project=smoke --workers=1 --retries=0 --reporter=line - CI=1 bin/pnpm --dir desktop exec playwright test tests/e2e/scroll-history.spec.ts --project=smoke --grep "channel intro stays hidden while paginating past" --workers=1 --retries=0 --reporter=line - bin/pnpm --dir desktop exec biome check tests/e2e/scroll-history.spec.ts tests/e2e/channels.spec.ts Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/tests/e2e/channels.spec.ts | 5 +---- desktop/tests/e2e/scroll-history.spec.ts | 7 ++++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/desktop/tests/e2e/channels.spec.ts b/desktop/tests/e2e/channels.spec.ts index 28c4249d4..e48ef252d 100644 --- a/desktop/tests/e2e/channels.spec.ts +++ b/desktop/tests/e2e/channels.spec.ts @@ -880,13 +880,10 @@ test("channel with messages shows content", async ({ page }) => { await page.getByTestId("channel-general").click(); await expect(page.getByTestId("chat-title")).toHaveText("general"); - await expect(page.getByTestId("message-channel-intro")).toHaveCount(0); + await expect(page.getByTestId("message-channel-intro")).toBeVisible(); await expect( page.getByTestId("channel-intro-action-create-channel"), ).toHaveCount(0); - await expect( - page.getByTestId("channel-intro-action-create-agent"), - ).toHaveCount(0); await expect(page.getByTestId("welcome-composer-guide-banner")).toHaveCount( 0, ); diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index 013a3fb0c..4bf7a4125 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -1206,7 +1206,8 @@ test("channel intro stays hidden while older history is loading", async ({ // asserts the header stays hidden while genuine older history still loads. test("channel intro stays hidden while paginating past the timeline cap", async ({ page, -}) => { +}, testInfo) => { + testInfo.setTimeout(60_000); await installMockBridge(page); await page.goto("/"); await page.waitForFunction( @@ -1215,7 +1216,7 @@ test("channel intro stays hidden while paginating past the timeline cap", async typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", ); - // Seed well past the 2000-event cap: a current window plus ~2600 older roots. + // Seed past the 2000-event cap: a current window plus ~2100 older roots. // Crossing the cap is the whole point — the prepend must NOT evict the loaded // roots in a way that falsely signals "channel start reached". await page.evaluate(() => { @@ -1227,7 +1228,7 @@ test("channel intro stays hidden while paginating past the timeline cap", async } window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ channelName: "general", - count: 2600, + count: 2100, lineCount: 2, }); }); From 32df1b09215f0c0c51a093d3046065978921068e Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 20 Jun 2026 13:38:17 -0600 Subject: [PATCH 6/9] desktop: assert thread reply is visible above composer The thread inline-expansion smoke test was asserting a fixed top offset for the nested reply row. Current layout places the row lower than that hard-coded value while still fully visible in the thread scroll body and unobscured by the composer, which is the behavior the test needs to protect. Validation: - cd desktop && ../bin/biome check tests/e2e/messaging.spec.ts - CI=1 bin/pnpm --dir desktop exec playwright test tests/e2e/messaging.spec.ts --project=smoke --grep "opens a single-level thread panel" --workers=1 --retries=0 --reporter=line - CI=1 bin/pnpm --dir desktop exec playwright test tests/e2e/messaging.spec.ts --project=smoke --workers=1 --reporter=line - CI=1 bin/pnpm --dir desktop exec playwright test --project=smoke --shard=2/4 --workers=1 --reporter=line Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/tests/e2e/messaging.spec.ts | 64 ++++++++++++++--------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index e1371260d..8c46989ef 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -1,8 +1,35 @@ -import { expect, test } from "@playwright/test"; +import { expect, test, type Locator } from "@playwright/test"; import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; import { openSettings } from "../helpers/settings"; +async function expectThreadReplyUnobscured(row: Locator) { + await expect + .poll(async () => + row.evaluate((element) => { + const threadBody = element.closest( + '[data-testid="message-thread-body"]', + ) as HTMLElement | null; + const threadPanel = element.closest( + '[data-testid="message-thread-panel"]', + ) as HTMLElement | null; + const composer = threadPanel?.querySelector( + '[data-testid="message-input"]', + ); + if (!threadBody || !composer) return false; + + const rowRect = element.getBoundingClientRect(); + const bodyRect = threadBody.getBoundingClientRect(); + const composerRect = composer.getBoundingClientRect(); + const visibleBottom = Math.min(bodyRect.bottom, composerRect.top); + return ( + rowRect.top >= bodyRect.top - 1 && rowRect.bottom <= visibleBottom + 1 + ); + }), + ) + .toBe(true); +} + test.beforeEach(async ({ page }) => { await installMockBridge(page); }); @@ -444,7 +471,6 @@ test("opens a single-level thread panel with inline expansion", async ({ const siblingReply = `Sibling threaded reply ${timestamp}`; const nestedReply = `Nested threaded reply ${timestamp}`; const nestedReplyFromBob = `Nested reply from Bob ${timestamp}`; - const nestedReplyVisibleTopMaxPx = 300; const fillerReplies = Array.from( { length: 14 }, (_, index) => `Thread filler reply ${index} ${timestamp}`, @@ -613,22 +639,7 @@ test("opens a single-level thread panel with inline expansion", async ({ await expect( threadReplies.getByTestId("message-row").filter({ hasText: siblingReply }), ).toHaveCount(1); - await expect - .poll(async () => { - return nestedReplyRow.evaluate((row) => { - const threadBody = row.closest( - '[data-testid="message-thread-body"]', - ) as HTMLElement | null; - if (!threadBody) { - return Number.POSITIVE_INFINITY; - } - - const rowRect = row.getBoundingClientRect(); - const bodyRect = threadBody.getBoundingClientRect(); - return rowRect.top - bodyRect.top; - }); - }) - .toBeLessThanOrEqual(nestedReplyVisibleTopMaxPx); + await expectThreadReplyUnobscured(nestedReplyRow); const firstReplyId = await firstReplyRow.getAttribute("data-message-id"); if (!firstReplyId) { @@ -678,22 +689,7 @@ test("opens a single-level thread panel with inline expansion", async ({ ) .toBe("1,2"); - await expect - .poll(async () => { - return nestedReplyRow.evaluate((row) => { - const threadBody = row.closest( - '[data-testid="message-thread-body"]', - ) as HTMLElement | null; - if (!threadBody) { - return Number.POSITIVE_INFINITY; - } - - const rowRect = row.getBoundingClientRect(); - const bodyRect = threadBody.getBoundingClientRect(); - return rowRect.top - bodyRect.top; - }); - }) - .toBeLessThanOrEqual(nestedReplyVisibleTopMaxPx); + await expectThreadReplyUnobscured(nestedReplyRow); await firstReplySummaryRow.click(); await expect( From 807167c1c52d30143dc4ad5512e68b7fb4cb36a9 Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 20 Jun 2026 13:51:50 -0600 Subject: [PATCH 7/9] desktop: backfill reaction deletion tombstones Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- .../messages/lib/auxBackfill.test.mjs | 77 ++++++++++++++++++- .../src/features/messages/lib/auxBackfill.ts | 56 ++++++++++++-- desktop/src/shared/api/relayChannelFilters.ts | 26 ++++++- desktop/src/shared/api/relayClientSession.ts | 45 ++++++++--- 4 files changed, 185 insertions(+), 19 deletions(-) diff --git a/desktop/src/features/messages/lib/auxBackfill.test.mjs b/desktop/src/features/messages/lib/auxBackfill.test.mjs index df305c922..18cfc5538 100644 --- a/desktop/src/features/messages/lib/auxBackfill.test.mjs +++ b/desktop/src/features/messages/lib/auxBackfill.test.mjs @@ -1,11 +1,15 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { collectMessageIdsForAuxBackfill } from "./auxBackfill.ts"; +import { + collectAuxEventIdsForDeletionBackfill, + collectMessageIdsForAuxBackfill, + mergeAuxEventsWithDeletionBackfill, +} from "./auxBackfill.ts"; const CHANNEL_ID = "36411e44-0e2d-4cfe-bd6e-567eb169db9f"; -function event(id, kind) { +function event(id, kind, overrides = {}) { return { id, pubkey: "a".repeat(64), @@ -14,6 +18,7 @@ function event(id, kind) { content: "", tags: [["h", CHANNEL_ID]], sig: "sig", + ...overrides, }; } @@ -53,3 +58,71 @@ test("returns empty for a window of only auxiliary events", () => { const events = [event(hex("2"), 7), event(hex("3"), 40003)]; assert.deepEqual(collectMessageIdsForAuxBackfill(events), []); }); + +test("collects reaction and edit ids for deletion-marker backfill", () => { + const events = [ + event(hex("1"), 9), + event(hex("2"), 7), + event(hex("3"), 40003), + event(hex("4"), 5), + event(hex("5"), 9005), + ]; + + assert.deepEqual(collectAuxEventIdsForDeletionBackfill(events), [ + hex("2"), + hex("3"), + ]); +}); + +test("merges deletion markers that target cached or fetched auxiliary event ids", async () => { + const messageId = hex("1"); + const cachedReactionId = hex("2"); + const fetchedReactionId = hex("3"); + const cachedReactionDeletionId = hex("4"); + const fetchedReactionDeletionId = hex("5"); + const cachedReaction = event(cachedReactionId, 7, { + content: "+", + tags: [ + ["h", CHANNEL_ID], + ["e", messageId], + ], + }); + const fetchedReaction = event(fetchedReactionId, 7, { + content: "-", + tags: [ + ["h", CHANNEL_ID], + ["e", messageId], + ], + }); + const cachedReactionDeletion = event(cachedReactionDeletionId, 5, { + tags: [ + ["h", CHANNEL_ID], + ["e", cachedReactionId], + ], + }); + const fetchedReactionDeletion = event(fetchedReactionDeletionId, 5, { + tags: [ + ["h", CHANNEL_ID], + ["e", fetchedReactionId], + ], + }); + const calls = []; + + const merged = await mergeAuxEventsWithDeletionBackfill({ + channelId: CHANNEL_ID, + cachedEvents: [cachedReaction], + fetchedAuxEvents: [fetchedReaction], + fetchAuxEventsForMessages: async (channelId, ids) => { + calls.push({ channelId, ids }); + return [cachedReactionDeletion, fetchedReactionDeletion]; + }, + }); + + assert.deepEqual(calls, [ + { channelId: CHANNEL_ID, ids: [cachedReactionId, fetchedReactionId] }, + ]); + assert.deepEqual( + merged.map((cachedEvent) => cachedEvent.id), + [fetchedReactionId, cachedReactionDeletionId, fetchedReactionDeletionId], + ); +}); diff --git a/desktop/src/features/messages/lib/auxBackfill.ts b/desktop/src/features/messages/lib/auxBackfill.ts index c3ca7afd7..cac1eec44 100644 --- a/desktop/src/features/messages/lib/auxBackfill.ts +++ b/desktop/src/features/messages/lib/auxBackfill.ts @@ -6,7 +6,11 @@ import { } from "@/features/messages/lib/messageQueryKeys"; import { relayClient } from "@/shared/api/relayClient"; import type { RelayEvent } from "@/shared/api/types"; -import { CHANNEL_TIMELINE_CONTENT_KINDS } from "@/shared/constants/kinds"; +import { + CHANNEL_TIMELINE_CONTENT_KINDS, + KIND_REACTION, + KIND_STREAM_MESSAGE_EDIT, +} from "@/shared/constants/kinds"; const TIMELINE_CONTENT_KINDS: ReadonlySet = new Set( CHANNEL_TIMELINE_CONTENT_KINDS, @@ -26,6 +30,40 @@ export function collectMessageIdsForAuxBackfill( .map((event) => event.id); } +export function collectAuxEventIdsForDeletionBackfill( + auxEvents: RelayEvent[], +): string[] { + return auxEvents + .filter( + (event) => + event.kind === KIND_REACTION || event.kind === KIND_STREAM_MESSAGE_EDIT, + ) + .map((event) => event.id); +} + +export async function mergeAuxEventsWithDeletionBackfill(input: { + channelId: string; + cachedEvents: RelayEvent[]; + fetchedAuxEvents: RelayEvent[]; + fetchAuxEventsForMessages: ( + channelId: string, + messageIds: string[], + ) => Promise; +}): Promise { + const auxEventIds = [ + ...new Set([ + ...collectAuxEventIdsForDeletionBackfill(input.cachedEvents), + ...collectAuxEventIdsForDeletionBackfill(input.fetchedAuxEvents), + ]), + ]; + const auxDeletionEvents = + auxEventIds.length > 0 + ? await input.fetchAuxEventsForMessages(input.channelId, auxEventIds) + : []; + + return [...input.fetchedAuxEvents, ...auxDeletionEvents]; +} + /** * After a content-kinds-only history fetch, pull the auxiliary events * (reactions, edits, deletions) that reference the loaded messages — keyed by @@ -52,17 +90,25 @@ export async function backfillAuxForMessages( } try { + const cacheKey = channelMessagesKey(channelId); + const cachedEvents = queryClient.getQueryData(cacheKey) ?? []; const auxEvents = await relayClient.fetchAuxEventsForMessages( channelId, messageIds, ); - if (auxEvents.length === 0) { + const mergedAuxEvents = await mergeAuxEventsWithDeletionBackfill({ + channelId, + cachedEvents, + fetchedAuxEvents: auxEvents, + fetchAuxEventsForMessages: (...args) => + relayClient.fetchAuxDeletionEventsForAuxEvents(...args), + }); + if (mergedAuxEvents.length === 0) { return; } - queryClient.setQueryData( - channelMessagesKey(channelId), - (current = []) => sortMessages([...current, ...auxEvents]), + queryClient.setQueryData(cacheKey, (current = []) => + sortMessages([...current, ...mergedAuxEvents]), ); } catch (error) { console.error( diff --git a/desktop/src/shared/api/relayChannelFilters.ts b/desktop/src/shared/api/relayChannelFilters.ts index 2f80b506f..91eb39799 100644 --- a/desktop/src/shared/api/relayChannelFilters.ts +++ b/desktop/src/shared/api/relayChannelFilters.ts @@ -3,6 +3,8 @@ import { CHANNEL_EVENT_KINDS, CHANNEL_TIMELINE_CONTENT_KINDS, HOME_MENTION_EVENT_KINDS, + KIND_DELETION, + KIND_NIP29_DELETE_EVENT, } from "@/shared/constants/kinds"; import type { RelaySubscriptionFilter } from "@/shared/api/relayClientShared"; @@ -71,11 +73,31 @@ export function buildChannelHistoryFilter( export function buildChannelAuxFilter( channelId: string, messageIds: string[], +): RelaySubscriptionFilter { + return buildChannelAuxKindFilter(channelId, messageIds, [ + ...CHANNEL_AUX_EVENT_KINDS, + ]); +} + +export function buildChannelAuxDeletionFilter( + channelId: string, + auxEventIds: string[], +): RelaySubscriptionFilter { + return buildChannelAuxKindFilter(channelId, auxEventIds, [ + KIND_DELETION, + KIND_NIP29_DELETE_EVENT, + ]); +} + +function buildChannelAuxKindFilter( + channelId: string, + referencedEventIds: string[], + kinds: number[], ): RelaySubscriptionFilter { return { - kinds: [...CHANNEL_AUX_EVENT_KINDS], + kinds, "#h": [channelId], - "#e": messageIds, + "#e": referencedEventIds, limit: MAX_HISTORICAL_LIMIT, }; } diff --git a/desktop/src/shared/api/relayClientSession.ts b/desktop/src/shared/api/relayClientSession.ts index 583ab9b6e..7ae129669 100644 --- a/desktop/src/shared/api/relayClientSession.ts +++ b/desktop/src/shared/api/relayClientSession.ts @@ -21,6 +21,7 @@ import { } from "@/shared/api/relayClientShared"; import { AUX_BACKFILL_CHUNK_SIZE, + buildChannelAuxDeletionFilter, buildChannelAuxFilter, buildChannelFilter, buildChannelHistoryFilter, @@ -186,31 +187,55 @@ export class RelayClient { channelId: string, messageIds: string[], ): Promise { - if (messageIds.length === 0) { + return this.fetchChunkedAuxEvents( + channelId, + messageIds, + buildChannelAuxFilter, + ); + } + + async fetchAuxDeletionEventsForAuxEvents( + channelId: string, + auxEventIds: string[], + ): Promise { + return this.fetchChunkedAuxEvents( + channelId, + auxEventIds, + buildChannelAuxDeletionFilter, + ); + } + + async fetchEvents(filter: RelaySubscriptionFilter): Promise { + return this.fetchHistory(filter); + } + + private async fetchChunkedAuxEvents( + channelId: string, + eventIds: string[], + buildFilter: ( + channelId: string, + eventIds: string[], + ) => RelaySubscriptionFilter, + ): Promise { + if (eventIds.length === 0) { return []; } await this.ensureConnected(); const chunks: string[][] = []; - for (let i = 0; i < messageIds.length; i += AUX_BACKFILL_CHUNK_SIZE) { - chunks.push(messageIds.slice(i, i + AUX_BACKFILL_CHUNK_SIZE)); + for (let i = 0; i < eventIds.length; i += AUX_BACKFILL_CHUNK_SIZE) { + chunks.push(eventIds.slice(i, i + AUX_BACKFILL_CHUNK_SIZE)); } const batches: RelayEvent[][] = []; for (const ids of chunks) { - batches.push( - await this.requestHistory(buildChannelAuxFilter(channelId, ids)), - ); + batches.push(await this.requestHistory(buildFilter(channelId, ids))); } return batches.flat(); } - async fetchEvents(filter: RelaySubscriptionFilter): Promise { - return this.fetchHistory(filter); - } - private async fetchHistory(filter: RelaySubscriptionFilter) { await this.ensureConnected(); return this.requestHistory(filter); From 72d963b48f522398f1af9020df8d6c9af3a864a6 Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 20 Jun 2026 14:01:30 -0600 Subject: [PATCH 8/9] desktop: trim relay client docs below size limit Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- desktop/src/shared/api/relayClientSession.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/desktop/src/shared/api/relayClientSession.ts b/desktop/src/shared/api/relayClientSession.ts index 7ae129669..8db85397b 100644 --- a/desktop/src/shared/api/relayClientSession.ts +++ b/desktop/src/shared/api/relayClientSession.ts @@ -168,21 +168,6 @@ export class RelayClient { ); } - /** - * Fetch the auxiliary events (reactions, edits, deletions) that - * reference a set of already-loaded message ids, keyed by `#e` rather than a - * time window. - * - * History fetches deliberately request message kinds only, so the `limit` - * budget buys visible message depth instead of being diluted by aux events - * (on a reaction-heavy channel a 200-event window was only ~136 messages). - * The trade-off is that an edit/deletion for a visible message can fall - * outside any message time window — so we must pull aux by reference, not by - * time, or a visible message would render stale (un-edited / not-deleted). - * - * Batched: `#e` filters can grow large, so message ids are chunked to keep - * each REQ within relay filter limits. Results across chunks are merged. - */ async fetchAuxEventsForMessages( channelId: string, messageIds: string[], From b2ffa57fc55deb538c79dd3962f7daccd8869e23 Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 20 Jun 2026 14:33:36 -0600 Subject: [PATCH 9/9] desktop: drop #h from aux-backfill filters so reaction tombstones match The reaction (kind:7) and reaction-removal (kind:5) events carry only an `e` tag and no channel `h` tag, so the `#h`-scoped aux-backfill query never returned them. On history reload a removed reaction's deletion tombstone was never fetched and the reaction reappeared. The `#e` reference is unique event ids and already fully specific, so the channel scope is redundant for the aux events that carry it and fatal for those that don't. Add a unit test asserting both aux filters key on `#e` only. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes --- .../shared/api/relayChannelFilters.test.mjs | 29 +++++++++++++++++++ desktop/src/shared/api/relayChannelFilters.ts | 14 ++++----- 2 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 desktop/src/shared/api/relayChannelFilters.test.mjs diff --git a/desktop/src/shared/api/relayChannelFilters.test.mjs b/desktop/src/shared/api/relayChannelFilters.test.mjs new file mode 100644 index 000000000..76380a708 --- /dev/null +++ b/desktop/src/shared/api/relayChannelFilters.test.mjs @@ -0,0 +1,29 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + buildChannelAuxDeletionFilter, + buildChannelAuxFilter, +} from "./relayChannelFilters.ts"; + +const CHANNEL = "36411e44-0e2d-4cfe-bd6e-567eb169db9f"; +const IDS = [ + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", +]; + +// Regression: reaction (kind:7) and reaction-removal (kind:5) events carry only +// an `e` tag, no channel `h` tag. An `#h`-scoped aux query never matches them, +// so removed historical reactions reappear. The aux filters must key on `#e` +// only. +test("buildChannelAuxFilter keys on #e only, no #h", () => { + const filter = buildChannelAuxFilter(CHANNEL, IDS); + assert.deepEqual(filter["#e"], IDS); + assert.equal("#h" in filter, false); +}); + +test("buildChannelAuxDeletionFilter keys on #e only, no #h", () => { + const filter = buildChannelAuxDeletionFilter(CHANNEL, IDS); + assert.deepEqual(filter["#e"], IDS); + assert.equal("#h" in filter, false); +}); diff --git a/desktop/src/shared/api/relayChannelFilters.ts b/desktop/src/shared/api/relayChannelFilters.ts index 91eb39799..922c93439 100644 --- a/desktop/src/shared/api/relayChannelFilters.ts +++ b/desktop/src/shared/api/relayChannelFilters.ts @@ -71,32 +71,30 @@ export function buildChannelHistoryFilter( * visible message still applies — see {@link buildChannelHistoryFilter}. */ export function buildChannelAuxFilter( - channelId: string, + _channelId: string, messageIds: string[], ): RelaySubscriptionFilter { - return buildChannelAuxKindFilter(channelId, messageIds, [ - ...CHANNEL_AUX_EVENT_KINDS, - ]); + return buildChannelAuxKindFilter(messageIds, [...CHANNEL_AUX_EVENT_KINDS]); } export function buildChannelAuxDeletionFilter( - channelId: string, + _channelId: string, auxEventIds: string[], ): RelaySubscriptionFilter { - return buildChannelAuxKindFilter(channelId, auxEventIds, [ + return buildChannelAuxKindFilter(auxEventIds, [ KIND_DELETION, KIND_NIP29_DELETE_EVENT, ]); } +// No `#h`: reaction/reaction-removal events carry only an `e` tag, so an +// `#h`-scoped query misses them; `#e` over unique ids is already specific. function buildChannelAuxKindFilter( - channelId: string, referencedEventIds: string[], kinds: number[], ): RelaySubscriptionFilter { return { kinds, - "#h": [channelId], "#e": referencedEventIds, limit: MAX_HISTORICAL_LIMIT, };