From 40edd6f5819d7446172976327922e36c0a9d1765 Mon Sep 17 00:00:00 2001 From: mtoohey Date: Mon, 9 Nov 2026 03:47:00 +0000 Subject: [PATCH 1/2] fix(messages): show "New messages" in channels longer than one page Read a channel with more than 20 top-level messages to the bottom, switch away, let three messages arrive and come back: the new rows were there but nothing marked where you stopped. The channel window loads 20 rows on entry, so such a channel always enters with `hasMore`, and the entry boundary refused to settle on anything when history was incomplete. Channels under one page showed the line; busy channels never did. The guard existed because the first unread loaded row need not be the first unread row when unloaded history sits above the window. The timeline now places the line with history above the entry rows only when the unread store can vouch for it, through two optional channel-snapshot fields in unread.ts: - `oldestUnreadId` names the oldest unread message the timeline shows (a top-level post or broadcast reply, in rendered order). It ignores own messages, quiet agent rows and non-broadcast replies, follows reads and clears when nothing is unread. It comes from the same bounded evidence as `observedCount`, so missing history can make a later message the oldest one known. - `sampled` says a repair has read every channel in unread scope since evidence was last emptied (cache clear or capacity overflow) or a channel entered that scope. Later repairs (reconnect, backstop, refresh) only add evidence and leave it set, so they publish no extra snapshots. It lives on the store's snapshot because the store owns the repair; `sync()` reports the read-state module. `equal()` compares both so subscribers see them change. When entry history is incomplete, ChannelTimeline waits for `sampled`, then settles with no line unless `oldestUnreadId` is a row loaded on entry with a read entry row above it. Entry history is complete only when the visit captured a ready, verified, idle window with no more history, so visits that entered a cached, refreshing or still-paging window take this path too: a short channel entered from cache could not show the line before and now can under these rules. The line still stays put for the visit as older pages load, rows arrive and the reader scrolls. This narrows a race; it does not close it. Every relay read, the window's own page included, feeds the store and marks its evidence observed, so the store's oldest known unread message can sit in the window only because it has seen nothing above it. Two guards cover different parts of that: - The `sampled` wait covers the store not having repaired yet since startup, a cache clear, an overflow or a channel becoming accessible. A Chromium probe that held the repair read and re-entered a 25-message channel drew the line at message 5 while messages 0-4 were unread too. With the wait in place and the read-row guard disabled, the same probe showed no line while the repair was held and none after it returned (oldest unread: message 0). A cache clear used to leave a connected session without a repair until the 15-minute backstop or a reconnect; clear() now starts one at once, as a newly accessible channel does, so the line, unread counts and Inbox activity come back promptly. An overflow still waits for the next recovery tick. The wait does nothing about a repair that finished with a short sample. The line can now appear a moment after entry, and a host that never runs the repair never places it with history above the window. - The read-row guard covers a repair batch that filled its 500-row limit and stopped inside the window, leaving the first loaded row the oldest unread message known, but only for ordinary rows: catch-up's `activity:` mark reads all of them up to a timestamp. It never reads mentions, broadcasts, broadcast replies or messages marked unread; only their own `msg:` or `thread:` marks do. A short sample can still hide an unread one of those above the window, and the line then lands below it. A row read only by its own mark says nothing about the unloaded rows above it either. docs/unread.md records this residual. The cost is no line when the unread messages begin exactly at the first loaded row. Complete-history visits still place the line at the first unread entry row, but no longer wait on `hasMore` or `historyLimited`: a window that gains `hasMore` after entry (live arrivals trimming it) now settles from the entry rows rather than staying pending. The pending boundary now filters rows to the entry set before classifying them, so rows from older pages are not looked up on every render. `UnreadSnapshot` is re-exported through the FOUNDATION type entry `src/plugins/author.ts`. This adds two optional fields in unread.ts and does not edit author.ts, the same shape as the earlier `row` field. Help text now describes the rule instead of the old limitation, and docs/unread.md documents both fields with the bounded-evidence caveat. Tests: the restore fixture's snapshot derives `oldestUnreadId` from every row it built, loaded or not, and reports `sampled` unless a test clears it. New restore cases place the line inside incomplete history (presented and late mount, stable while older rows page in) and omit it at a first loaded row with no read row before it. A repair-wait case pins the guard's limits: with the oldest unread in the window below a receipt-read row and the repair unfinished there is no line; once the repair finishes there is none if it found an older unread row, and the line sits at the loaded row if it did not, which the store cannot tell from a sample that stopped short. The pinned "outside the entry window" case keeps two variants; its third flips: a remote read that lands before the timeline is presented and leaves a loaded row the oldest unread now places the line there. That depends on mount timing: the same read after the timeline has settled (the second variant) leaves no line, because the boundary is captured once per visit. unread.test.ts covers `oldestUnreadId` and `sampled`: absent with only window rows and during the first repair, kept with no snapshot change through a refresh, dropped by a cache clear and restored by the repair the clear starts, and dropped for a channel entering scope until its repair finishes; when that happens mid-repair, subscribers see the reset at once and it lasts until the restarted repair finishes. unread-startup.test.ts checks an overflow clears it. The clear cases of three startup tests pinned that nothing is read after a clear; they now expect the restart and still check that the abandoned repair dispatches and publishes nothing more. An Inbox test that expected activity to stay hidden after a clear now checks it is not reported as nothing to read before the repair returns, and that the repair brings it back. The idle-feed Refresh test passed for the wrong reason once the restart returned the mention before Refresh; it now shows that mention returning on its own while the cleared feed stays idle and offers Refresh instead of a spinner, and a feed-only reaction row returning only after Refresh. A browser case seeds 25 messages, reads to the bottom, receives three from a peer and asserts the line sits above the first; it fails on main and passes here. Co-Authored-By: Claude Opus 5.5 Signed-off-by: mtoohey Signed-off-by: Matt Toohey --- docs/buzz-help.md | 12 +- docs/unread.md | 21 ++ src/bundled/inbox/InboxPage.test.tsx | 38 +++- .../messages/ChannelTimeline.restore.test.tsx | 194 +++++++++++++++++- src/features/messages/ChannelTimeline.tsx | 37 +++- src/features/relay/unread-startup.test.ts | 37 +++- src/features/relay/unread.test.ts | 183 +++++++++++++++++ src/features/relay/unread.ts | 52 ++++- tests/browser/new-message-boundary.spec.mjs | 73 +++++++ 9 files changed, 608 insertions(+), 39 deletions(-) diff --git a/docs/buzz-help.md b/docs/buzz-help.md index 5cad4d8354..5c019887ef 100644 --- a/docs/buzz-help.md +++ b/docs/buzz-help.md @@ -321,11 +321,13 @@ revokes all of them, and a pairing unused for 30 days expires. Further reactions during that hold do not change the icon; history loading does not trigger it. - **New messages** marks the first known unread incoming channel message when you enter. Once established, the divider stays in place for that visit as you - read, reveal another message, or receive new messages. It is absent for a visit - that starts with incomplete or unverified history, even if older rows load - later. It may also be absent when unread evidence is incomplete, or when read - intent or the read evidence needed to locate the boundary changes before it is - established. + read, reveal another message, or receive new messages. In a channel with more + history than loads on entry, it appears when the oldest unread message is + among the rows loaded on entry, below a message you have read; when the unread + messages start at the first loaded row or further up, there is no divider for + that visit, even if older rows load later. It may also be absent when unread + evidence is incomplete, or when read intent or the read evidence needed to + locate the boundary changes before it is established. - **Opening a channel or DM** while Buzz is visible and focused clears its sidebar row: the bold name, the mention dot and the thread dot. Switching away from Buzz pauses this: new messages can light the row even while that conversation stays selected. The macOS Dock dot follows the rows. Nothing is marked diff --git a/docs/unread.md b/docs/unread.md index 371663b758..cfae992be1 100644 --- a/docs/unread.md +++ b/docs/unread.md @@ -104,6 +104,27 @@ viewport): reader's entry evidence outlives them. A channel read covers the channel mark and reading a message covers its receipt; retention keeps row marks last and never treats them as override ancestry. +- The channel snapshot's `oldestUnreadId` names the oldest unread message its + timeline shows (a top-level post or broadcast reply), from the same bounded + evidence as `observedCount`: missing history can make a later message the + oldest one known. Every relay read feeds that evidence, a channel window's + page included, so `sampled` says a repair has read every channel in unread + scope since evidence was last emptied (a cache clear or capacity overflow) or + a channel entered that scope: evidence then reaches past what windows loaded, + up to each batch's 500-row limit. A cache clear or newly accessible channel + starts that repair at once; an overflow waits for the next recovery tick. + Later repairs, refreshes included, leave `sampled` set. When entry did not + capture a verified, idle window with all of its history (rows sit above it, or + it was cached, refreshing or still paging), the **New messages** divider waits + for `sampled`, then appears only at `oldestUnreadId` among the rows loaded on + entry and only below a read one. A batch that filled its limit can still stop + short of the true oldest unread message, and the read-row condition covers + only ordinary rows: catch-up's `activity:` mark reads all of them up to a + timestamp, but never mentions, broadcasts, broadcast replies or messages + marked unread, which only their own `msg:` or `thread:` marks read. A short + sample can hide an unread one of those above the window, and the divider then + lands below it. A row above read only by its own mark says nothing about older + rows either. - Both keys use verified event timestamps, never wall time. Neither enters generic channel/thread inheritance or remote override baselines. Channel catch-up ends a local manual unread on the channel itself; message and thread manual-unread diff --git a/src/bundled/inbox/InboxPage.test.tsx b/src/bundled/inbox/InboxPage.test.tsx index 331f899fd7..c16837ec98 100644 --- a/src/bundled/inbox/InboxPage.test.tsx +++ b/src/bundled/inbox/InboxPage.test.tsx @@ -797,15 +797,22 @@ it("retries the card's connection, activity and read-state failures", async () = act(() => h.failSync()); expect(screen.queryByRole("alert")).not.toBeInTheDocument(); - // Cleared activity is not reported as nothing to read. - await act(async () => { - await h.owner.clearCache(); + // Cleared activity is not reported as nothing to read while the repair the + // clear starts observes it again, and that repair brings it back. + let clearing: Promise | undefined; + act(() => { + clearing = h.owner.clearCache(); }); expect( screen.queryByRole("list", { name: "Recent activity" }), ).not.toBeInTheDocument(); expect(screen.queryByText("No recent activity")).not.toBeInTheDocument(); expect(screen.queryByText("Nothing unread")).not.toBeInTheDocument(); + await act(async () => { + await clearing; + }); + await waitFor(() => expect(previewRows()).toHaveLength(2)); + expect(screen.queryByText("No recent activity")).not.toBeInTheDocument(); view.unmount(); act(() => h.failConnection("relay offline")); @@ -3035,8 +3042,24 @@ it("waits for roster recovery before explicit evidence refresh", async () => { it("cache clear shows explicit refresh instead of a spinner with no pending work", async () => { const h = fixture(); + const root = h.root; + if (!root) throw new Error("Missing fixture root"); + // Only the feed reads reactions; unread evidence never includes them. + h.addEvent( + signed(h.alice, { + kind: 7, + content: "❤️", + created_at: 30, + tags: [ + ["e", root.id], + ["p", h.viewer.pubkey], + ], + }), + ); + const reaction = { name: /Alice reacted with heart/ }; render(h.view); await screen.findByText("Please review this"); + await screen.findByRole("button", reaction); await waitFor(() => expect( screen.queryByText("Checking recent activity…"), @@ -3045,12 +3068,19 @@ it("cache clear shows explicit refresh instead of a spinner with no pending work await act(async () => { await h.owner.clearCache(); }); + // The repair the clear starts brings back the mention from unread evidence + // on its own, but the cleared feed stays idle: it offers Refresh rather than + // a spinner with nothing pending. + await screen.findByText("Please review this"); + expect(h.owner.session.inboxFeed.snapshot().status).toBe("idle"); expect(screen.getByText("Check recent activity.")).toBeVisible(); expect( screen.queryByText("Checking recent activity…"), ).not.toBeInTheDocument(); + expect(screen.queryByRole("button", reaction)).not.toBeInTheDocument(); fireEvent.click(screen.getByRole("button", { name: "Refresh" })); - await screen.findByText("Please review this"); + await screen.findByRole("button", reaction); + expect(h.owner.session.inboxFeed.snapshot().status).toBe("ready"); expect(h.retrySync).not.toHaveBeenCalled(); }); diff --git a/src/features/messages/ChannelTimeline.restore.test.tsx b/src/features/messages/ChannelTimeline.restore.test.tsx index 24098c2a99..af3e0d6e6e 100644 --- a/src/features/messages/ChannelTimeline.restore.test.tsx +++ b/src/features/messages/ChannelTimeline.restore.test.tsx @@ -306,9 +306,26 @@ function boundaryFixture() { list: () => channelList, subscribeList: () => () => {}, }; + // Every row built so far, loaded or not: the store's evidence, not the window. + const built: ChannelMessage[] = []; + // The store's repair has finished, so that evidence reaches past the window. + let sampled = true; const snapshot = (channelId: string) => { + const oldest = built + .filter( + (row) => + row.channelId === channelId && + !row.membership && + !row.huddle && + row.audience !== "agents" && + row.authorId !== viewer.pubkey && + (unreadIds.has(row.id) || forcedIds.has(row.id)), + ) + .sort((a, b) => a.createdAt - b.createdAt || a.id.localeCompare(b.id))[0]; return { target: { kind: "channel" as const, channelId }, + ...(oldest ? { oldestUnreadId: oldest.id } : {}), + ...(sampled ? { sampled: true as const } : {}), observedCount: unreadIds.size, attentionCount: 0, coverage: "observed" as const, @@ -356,8 +373,7 @@ function boundaryFixture() { channelId = "c", extra: Partial = {}, ): ChannelMessage { - snapshots.set(channelId, snapshot(channelId)); - return { + const result: ChannelMessage = { id: event.id, channelId, authorId: event.pubkey, @@ -370,6 +386,9 @@ function boundaryFixture() { replyCount: 0, ...extra, }; + built.push(result); + snapshots.set(channelId, snapshot(channelId)); + return result; } function tree( channelId: string, @@ -467,6 +486,9 @@ function boundaryFixture() { advanceReadRevision() { readRevision++; }, + setSampled(value: boolean) { + sampled = value; + }, publish, }; } @@ -539,13 +561,135 @@ it("waits for entry and unread evidence before selecting a verified entry bounda expect(screen.queryByText("Own message", { exact: true })).not.toBeNull(); }); -it.each([ - { remoteRead: false, presented: true }, - { remoteRead: true, presented: true }, - { remoteRead: true, presented: false }, -])( - "does not infer an entry boundary after incomplete history: %j", - async ({ remoteRead, presented }) => { +const divider = () => screen.getByRole("separator", { name: "New messages" }); +const precedes = (earlier: Node, later: Node) => + !!(earlier.compareDocumentPosition(later) & Node.DOCUMENT_POSITION_FOLLOWING); + +it.each([{ presented: true }, { presented: false }])( + "places the entry boundary inside incomplete history when the oldest unread message is loaded: %j", + async ({ presented }) => { + const h = boundaryFixture(); + const older = h.row(message(h.peer, "c", "Read above the entry window", 1)); + const read = h.row(message(h.peer, "c", "Read in loaded window", 2)); + const first = h.row(message(h.peer, "c", "First unread in window", 3)); + const later = h.row(message(h.peer, "c", "Later unread in window", 4)); + h.unreadIds.add(first.id); + h.unreadIds.add(later.id); + h.completeReadStateSnapshot(); + h.publish(); + const entryRows = [read, first, later]; + const mounted = render(h.tree("c", entryRows, true, true, presented)); + await h.completeEntry(); + if (!presented) mounted.rerender(h.tree("c", entryRows, true)); + + const placed = divider(); + expect(precedes(screen.getByText("Read in loaded window"), placed)).toBe( + true, + ); + expect(precedes(placed, screen.getByText("First unread in window"))).toBe( + true, + ); + + // Paging older history in neither moves nor removes the line. + mounted.rerender(h.tree("c", [older, ...entryRows])); + h.publish(); + expect(divider()).toBe(placed); + expect( + screen.getAllByRole("separator", { name: "New messages" }), + ).toHaveLength(1); + expect( + precedes(screen.getByText("Read above the entry window"), placed), + ).toBe(true); + expect(precedes(placed, screen.getByText("First unread in window"))).toBe( + true, + ); + mounted.unmount(); + }, +); + +it.each([{ ownFirst: false }, { ownFirst: true }])( + "does not infer an entry boundary at the first unread loaded row without a read one before it: %j", + async ({ ownFirst }) => { + // The repair's sample stopped inside the window, so the first loaded row + // is the oldest unread message the store knows. + const h = boundaryFixture(); + const own = h.row(message(h.viewer, "c", "Own message in window", 1)); + const first = h.row(message(h.peer, "c", "First loaded unread", 2)); + const later = h.row(message(h.peer, "c", "Later loaded unread", 3)); + h.unreadIds.add(first.id); + h.unreadIds.add(later.id); + h.completeReadStateSnapshot(); + h.publish(); + const entryRows = ownFirst ? [own, first, later] : [first, later]; + const mounted = render(h.tree("c", entryRows, true)); + await h.completeEntry(); + expect( + h.unread.snapshot({ kind: "channel", channelId: "c" }).oldestUnreadId, + ).toBe(first.id); + expect( + screen.queryByRole("separator", { name: "New messages" }), + ).toBeNull(); + + const older = h.row(message(h.peer, "c", "Read above the entry window", 0)); + mounted.rerender(h.tree("c", [older, ...entryRows])); + h.publish(); + expect( + screen.queryByRole("separator", { name: "New messages" }), + ).toBeNull(); + mounted.unmount(); + }, +); + +it.each([{ olderUnread: false }, { olderUnread: true }])( + "waits for the unread repair before placing a boundary inside incomplete history: %j", + async ({ olderUnread }) => { + // Before the repair returns, the store knows only the loaded rows: its + // oldest unread message sits in the window below a row read by its own + // receipt, whatever lies above the window. + const h = boundaryFixture(); + h.setSampled(false); + const read = h.row(message(h.peer, "c", "Read by its own receipt", 2)); + const first = h.row(message(h.peer, "c", "First loaded unread", 3)); + const later = h.row(message(h.peer, "c", "Later loaded unread", 4)); + h.unreadIds.add(first.id); + h.unreadIds.add(later.id); + h.completeReadStateSnapshot(); + h.publish(); + const mounted = render(h.tree("c", [read, first, later], true)); + await h.completeEntry(); + const target = { kind: "channel" as const, channelId: "c" }; + expect(h.unread.snapshot(target).oldestUnreadId).toBe(first.id); + expect( + screen.queryByRole("separator", { name: "New messages" }), + ).toBeNull(); + + // The repair returns a row above the window. + const above = h.row(message(h.peer, "c", "Above the entry window", 1)); + if (olderUnread) h.unreadIds.add(above.id); + h.setSampled(true); + h.publish(); + if (olderUnread) { + expect(h.unread.snapshot(target).oldestUnreadId).toBe(above.id); + expect( + screen.queryByRole("separator", { name: "New messages" }), + ).toBeNull(); + } else { + // The store cannot tell this from a sample that stopped short of older + // unread rows; docs/unread.md records that residual. + expect(precedes(divider(), screen.getByText("First loaded unread"))).toBe( + true, + ); + expect( + precedes(screen.getByText("Read by its own receipt"), divider()), + ).toBe(true); + } + mounted.unmount(); + }, +); + +it.each([{ remoteRead: false }, { remoteRead: true }])( + "does not infer an entry boundary when the oldest unread message is outside the entry window: %j", + async ({ remoteRead }) => { const h = boundaryFixture(); const older = h.row( message(h.peer, "c", "Older unread outside the entry window", 1), @@ -557,7 +701,7 @@ it.each([ h.completeReadStateSnapshot(); h.publish(); const revision = h.unread.revision(); - const mounted = render(h.tree("c", [read, unread], true, true, presented)); + const mounted = render(h.tree("c", [read, unread], true)); await h.completeEntry(); expect( screen.queryByRole("separator", { name: "New messages" }), @@ -577,6 +721,36 @@ it.each([ }, ); +it("places the boundary at a loaded row when a remote read leaves it the oldest unread before the timeline mounts", async () => { + const h = boundaryFixture(); + const older = h.row( + message(h.peer, "c", "Older unread outside the entry window", 1), + ); + const read = h.row(message(h.peer, "c", "Read in loaded window", 2)); + const unread = h.row(message(h.peer, "c", "Unread in loaded window", 3)); + h.unreadIds.add(older.id); + h.unreadIds.add(unread.id); + h.completeReadStateSnapshot(); + h.publish(); + const mounted = render(h.tree("c", [read, unread], true, true, false)); + await h.completeEntry(); + + // Another device reads the older row before this timeline mounts. + h.unreadIds.delete(older.id); + h.publish(); + mounted.rerender(h.tree("c", [older, read, unread])); + h.publish(); + + const placed = divider(); + expect(precedes(screen.getByText("Read in loaded window"), placed)).toBe( + true, + ); + expect(precedes(placed, screen.getByText("Unread in loaded window"))).toBe( + true, + ); + mounted.unmount(); +}); + it("does not select a stale unread row after reading advances while unread evidence is pending", async () => { const h = boundaryFixture(); const read = h.row(message(h.peer, "c", "Already read", 1)); diff --git a/src/features/messages/ChannelTimeline.tsx b/src/features/messages/ChannelTimeline.tsx index 43427b312e..e5ed5b48f7 100644 --- a/src/features/messages/ChannelTimeline.tsx +++ b/src/features/messages/ChannelTimeline.tsx @@ -421,6 +421,9 @@ function Timeline({ ? chronologicalRows .filter( (row) => + // Backdated live arrivals and older pages must neither define + // nor erase the entry boundary, so only entry rows are classified. + baseline.messageIds.has(row.id) && !row.membership && !row.huddle && row.audience !== "agents" && @@ -437,8 +440,6 @@ function Timeline({ const boundaryCandidate = (() => { if (!boundaryPending || !ownsMessageVisit || messageVisit?.active !== true) return { ready: false }; - // Later paging cannot recover entry-time read evidence for unloaded rows. - if (!messageVisit.entryHistoryComplete) return { ready: true }; if ( baseline.revision !== undefined && queries.unread.revision() !== baseline.revision @@ -449,8 +450,6 @@ function Timeline({ window.freshness !== "verified" || window.refreshing || window.loadingOlder || - window.hasMore || - window.historyLimited || unreadSync.completeness !== "snapshot" ) return { ready: false }; @@ -459,10 +458,31 @@ function Timeline({ unreadSnapshot?.freshness !== "observed" ) return { ready: false }; + // Entry history is complete only when the visit captured a ready, verified, + // idle window with no more history. Otherwise (history above the entry + // rows, or a cached, refreshing or still-paging window, so a short channel + // entered from cache lands here too) the boundary is the store's oldest + // unread message, and only when it is an entry row. Later paging cannot + // recover entry-time read evidence for rows that were not loaded. + if (!messageVisit.entryHistoryComplete) { + // Every relay read feeds the store, so until its repair has sampled + // history it may know only the loaded rows, and its oldest unread one + // can sit in the window below any read row while older ones are unread. + if (!unreadSnapshot.sampled) return { ready: false }; + const oldest = unreadSnapshot.oldestUnreadId; + if (!oldest || !baseline.messageIds.has(oldest)) return { ready: true }; + } if (!chronologicalRows.length) return { ready: noUnreadEvidence }; + // A repair batch that fills its limit can stop inside the window, leaving + // the first loaded row the oldest unread message the store knows. Requiring + // a read entry row first covers only ordinary rows: catch-up's activity: + // mark reads all of them up to a timestamp. It never reads mentions, + // broadcasts, broadcast replies or messages marked unread; only their own + // msg: or thread: marks do, so a short sample can still hide an unread one + // above the window. A row read by its own mark says nothing about the rows + // above it either. + let afterRead = messageVisit.entryHistoryComplete; for (const { row, attention } of entryAttention) { - // Backdated live arrivals must neither define nor erase the entry boundary. - if (!baseline.messageIds.has(row.id)) continue; if (attention.status === "unknown" || attention.pending) return { ready: false }; // Remote merges do not advance the local revision. Only this prefix can @@ -478,7 +498,8 @@ function Timeline({ return { ready: true }; // Ineligible means no attention category, not that an incoming row is read. if (attention.forced || attention.unread) - return { ready: true, messageId: row.id }; + return afterRead ? { ready: true, messageId: row.id } : { ready: true }; + afterRead = true; } return { ready: true }; })(); @@ -513,7 +534,7 @@ function Timeline({ if (!ownsMessageVisit || !boundaryMatchesVisit || !messageVisit?.active) return; for (const { row, attention } of entryAttention) { - if (baseline.messageIds.has(row.id) && !baseline.attention.has(row.id)) + if (!baseline.attention.has(row.id)) baseline.attention.set( row.id, attention.status === "unknown" || attention.pending diff --git a/src/features/relay/unread-startup.test.ts b/src/features/relay/unread-startup.test.ts index 18f8491988..dfaa9f722c 100644 --- a/src/features/relay/unread-startup.test.ts +++ b/src/features/relay/unread-startup.test.ts @@ -843,7 +843,7 @@ it.each(["clear", "dispose", "revoke", "join"])( else h.receive([roster(h.relay, "other", [], 20)]); held.release(); await repair; - if (boundary === "clear" || boundary === "dispose") { + if (boundary === "dispose") { expect(batches).toHaveLength(1); expect(h.snapshot().observedCount).toBeNull(); return; @@ -851,12 +851,18 @@ it.each(["clear", "dispose", "revoke", "join"])( // The abandoned read starts over against the current membership. await vi.waitFor(() => expect(batches).toHaveLength(2)); expect(h.snapshot().observedCount).toBeNull(); + // A cleared cache keeps membership, so the restart reads the first batch + // again rather than the abandoned read's later chunk. + if (boundary === "clear") expect(batches[1]).toEqual(batches[0]); fresh.release(); await h.session.unread.ensure(); const reread = batches.slice(1).flat(); if (boundary === "join") { expect(reread).toContain("new-room"); expect(h.snapshot().observedCount).toBe(1); + } else if (boundary === "clear") { + expect(reread).toEqual([...h.ids].sort()); + expect(h.snapshot().observedCount).toBe(1); } else { expect(reread).toHaveLength(128); expect(reread).not.toContain("other"); @@ -1109,6 +1115,7 @@ it("an evidence overflow is repaired on the next recovery tick", async () => { await vi.waitFor(() => expect(tickReads(h)).toEqual({ evidence: 2, snapshots: 2 }), ); + await vi.waitFor(() => expect(h.snapshot().sampled).toBe(true)); h.receive( Array.from({ length: 19 }, (_, i) => message(h.peer, "other", "x".repeat(450000), i + 20), @@ -1117,6 +1124,10 @@ it("an evidence overflow is repaired on the next recovery tick", async () => { expect(h.snapshot()).toMatchObject({ error: "Unread observation capacity reached; refresh available", }); + // The overflow emptied the sample; live evidence after it is not one. + h.receive([message(h.peer, "other", "after the overflow", 40)]); + expect(h.snapshot()).toMatchObject({ observedCount: 1 }); + expect(h.snapshot().sampled).toBeUndefined(); const before = tickReads(h).evidence; vi.setSystemTime(Date.now() + 60_000); h.recover(); @@ -1235,15 +1246,22 @@ it.each(["clear", "dispose", "revoke"])( else h.receive([roster(h.relay, "other", [], 20)]); release(); await repair; - if (boundary !== "revoke") { + if (boundary === "dispose") { expect(h.trace.map(({ kind }) => kind)).toEqual([30078]); return; } - // Only a fresh observation without the revoked channel is dispatched. + // Only a fresh observation is dispatched: after a clear it reads every + // channel once, after a revoke it leaves the revoked channel out. await h.session.unread.ensure(); const evidence = h.query.mock.calls .map(([filters]) => filters[0]) .filter((filter) => filter?.kinds?.includes(9)); + if (boundary === "clear") { + expect(evidence.flatMap((filter) => filter?.["#h"] ?? [])).toEqual( + [...h.ids].sort(), + ); + return; + } expect(evidence).toHaveLength(1); expect(evidence[0]?.["#h"]).not.toContain("other"); }, @@ -1253,12 +1271,19 @@ it("a subscriber clearing cache during progressive publication stops later chunk const h = setup(129); let cleared: Promise | undefined; h.session.unread.subscribe({ kind: "channel", channelId: "other" }, () => { - if (h.snapshot().observedCount === 1) cleared = h.clearCache(); + if (!cleared && h.snapshot().observedCount === 1) cleared = h.clearCache(); }); await h.session.unread.ensure(); await cleared; - expect(h.trace.map(({ kind }) => kind)).toEqual([30078, 9]); - expect(h.snapshot().observedCount).toBeNull(); + // The abandoned observation never dispatches its later chunk; the clear + // starts a fresh one from the first batch. + await h.session.unread.ensure(); + const batches = h.query.mock.calls + .map(([filters]) => filters[0]) + .filter((filter) => filter?.kinds?.includes(9)) + .map((filter) => filter?.["#h"]?.length); + expect(batches).toEqual([128, 128, 1]); + expect(h.snapshot().observedCount).toBe(1); }); it("explicit refresh bypasses optional profiles for both markers and evidence", async () => { diff --git a/src/features/relay/unread.test.ts b/src/features/relay/unread.test.ts index cc5012ca00..07b9311d9f 100644 --- a/src/features/relay/unread.test.ts +++ b/src/features/relay/unread.test.ts @@ -467,6 +467,189 @@ it("promotes mentions, broadcasts and participating-thread replies without promo ).toMatchObject({ status: "unknown", unread: true }); }); +it("the channel snapshot names the oldest unread message its timeline shows", async () => { + const h = setup(); + h.grant("room"); + const root = message(h.viewer, "room", "own root", 10); + const quiet = message(h.alice, "room", "agent coordination", 11, [ + ["audience", "agents"], + ]); + // Unread in the viewer's thread, but only the thread shows it. + const reply = message(h.alice, "room", "thread reply", 12, [ + ["e", root.id, "", "reply"], + ]); + const own = message(h.viewer, "room", "own later", 13); + h.emit([root, quiet, reply, own]); + expect(h.snapshot()).toMatchObject({ observedCount: 1 }); + expect(h.snapshot().oldestUnreadId).toBeUndefined(); + + const broadcast = message(h.alice, "room", "broadcast reply", 14, [ + ["e", root.id, "", "reply"], + ["broadcast", "1"], + ]); + const first = message(h.alice, "room", "first top-level", 15); + const second = message(h.alice, "room", "second top-level", 16); + h.emit([second, first, broadcast]); + expect(h.snapshot()).toMatchObject({ + observedCount: 4, + oldestUnreadId: broadcast.id, + }); + + await h.session.unread.markThrough(h.target, broadcast.id); + expect(h.snapshot()).toMatchObject({ + observedCount: 2, + oldestUnreadId: first.id, + }); + await h.session.unread.markThrough(h.target, second.id); + expect(h.snapshot()).toMatchObject({ observedCount: 0 }); + expect(h.snapshot().oldestUnreadId).toBeUndefined(); +}); + +it("the channel snapshot says sampled once a repair finishes, keeps it through later repairs and regains it after a cache clear", async () => { + const h = setup(); + const rows = Array.from({ length: 30 }, (_, i) => + message(h.alice, "room", `history ${i}`, i + 11), + ); + const cursor = rows[10]; + assert(cursor); + const elsewhere = message(h.alice, "other", "elsewhere", 50); + let hold: Promise | undefined; + let repairs = 0; + h.query.mockImplementation(async (filters) => { + const filter = filters[0]; + if (!filter?.kinds?.includes(9)) return []; + if (filter.limit === 500) { + repairs++; + await hold; + return rows; + } + if (filter["#h"]?.includes("other")) + return [ + elsewhere, + bounds(h.relay, "other", "head", { + has_more: false, + next_cursor: null, + }), + ]; + return [ + ...rows.slice(10), + bounds(h.relay, "room", "head", { + has_more: true, + next_cursor: { created_at: cursor.created_at, id: cursor.id }, + }), + ]; + }); + const repairing = () => { + let release = () => {}; + hold = new Promise((resolve) => { + release = resolve; + }); + return release; + }; + h.grant("room"); + h.session.channels.ensure("room"); + await vi.waitFor(() => + expect(h.session.channels.window("room").rows).toHaveLength(20), + ); + // The window's own rows are observed evidence, but not a sample of history. + expect(h.snapshot()).toMatchObject({ + coverage: "observed", + freshness: "observed", + observedCount: 20, + oldestUnreadId: cursor.id, + }); + expect(h.snapshot().sampled).toBeUndefined(); + + let release = repairing(); + let repair = h.session.unread.ensure(); + await flush(); + expect(h.snapshot().sampled).toBeUndefined(); + release(); + await repair; + expect(h.snapshot()).toMatchObject({ + observedCount: 30, + oldestUnreadId: rows[0]?.id, + sampled: true, + }); + + // A later repair only adds evidence: the sample stands while it runs, and + // subscribers see no change. + const sampledSnapshot = h.snapshot(); + const changed = vi.fn(); + const stop = h.session.unread.subscribe(h.target, changed); + release = repairing(); + let started = repairs; + repair = h.session.unread.refresh(); + await vi.waitFor(() => expect(repairs).toBe(started + 1)); + expect(h.snapshot().sampled).toBe(true); + release(); + await repair; + expect(h.snapshot()).toBe(sampledSnapshot); + expect(changed).not.toHaveBeenCalled(); + stop(); + + // A cleared cache forgets the sample and starts a repair at once. Until it + // returns, the reloaded window alone is not a sample. + release = repairing(); + started = repairs; + await h.clearCache(); + h.session.channels.ensure("room"); + await vi.waitFor(() => + expect(h.snapshot()).toMatchObject({ + observedCount: 20, + oldestUnreadId: cursor.id, + }), + ); + expect(repairs).toBe(started + 1); + expect(h.snapshot().sampled).toBeUndefined(); + release(); + await vi.waitFor(() => + expect(h.snapshot()).toMatchObject({ + observedCount: 30, + oldestUnreadId: rows[0]?.id, + sampled: true, + }), + ); + + // A channel entering unread scope has only its window until a repair reads it. + const other = { kind: "channel" as const, channelId: "other" }; + release = repairing(); + started = repairs; + h.grant("other"); + h.session.channels.ensure("other"); + await vi.waitFor(() => + expect(h.session.unread.snapshot(other)).toMatchObject({ + coverage: "observed", + observedCount: 1, + oldestUnreadId: elsewhere.id, + }), + ); + expect(repairs).toBe(started + 1); + expect(h.session.unread.snapshot(other).sampled).toBeUndefined(); + release(); + await vi.waitFor(() => + expect(h.session.unread.snapshot(other).sampled).toBe(true), + ); + + // Access gained while a repair is out: subscribers see the reset at once, + // and the sample returns only when the repair restarted after it finishes. + const stopRoom = h.session.unread.subscribe(h.target, () => {}); + release = repairing(); + started = repairs; + void h.session.unread.refresh(); + await vi.waitFor(() => expect(repairs).toBe(started + 1)); + expect(h.snapshot().sampled).toBe(true); + h.grant("third"); + expect(h.snapshot().sampled).toBeUndefined(); + release(); + release = repairing(); + await vi.waitFor(() => expect(repairs).toBe(started + 2)); + expect(h.snapshot().sampled).toBeUndefined(); + release(); + await vi.waitFor(() => expect(h.snapshot().sampled).toBe(true)); + stopRoom(); +}); + it("counts replies only in the viewer's conversations and threads; others stay quiet until joined or mentioned", async () => { const h = setup(); h.grant("room"); diff --git a/src/features/relay/unread.ts b/src/features/relay/unread.ts index c2f5e33e57..d5ce50f967 100644 --- a/src/features/relay/unread.ts +++ b/src/features/relay/unread.ts @@ -34,6 +34,16 @@ export type UnreadSnapshot = Readonly<{ target: ReadTarget; /** Latest verified content evidence, including read and self-authored messages. */ latestMessage?: Readonly<{ id: string; createdAt: number }>; + /** Channel targets: the id of the oldest unread message the channel's + * timeline shows (a top-level post or broadcast reply), in rendered order. + * Bounded like `observedCount`: missing history can leave a later message + * the oldest one known. Absent when none is unread. */ + oldestUnreadId?: string; + /** Channel targets: a repair has read every channel in unread scope since + * evidence was last emptied or a channel entered that scope, so evidence + * reaches past the rows channel windows loaded, up to the repair's per-batch + * limit. Later repairs keep it. */ + sampled?: true; /** null means unobserved/denied, never a fabricated zero or an exact relay total. */ observedCount: number | null; attentionCount: number | null; @@ -312,6 +322,11 @@ export function createUnread({ // Snapshot listeners last saw: a direct inbox() read must not hide the change. let inboxBase: InboxSnapshot | undefined; let inboxLoading = false; + /** A repair read every batch since evidence was last emptied (cache clear, + * overflow) or a channel entered repair scope. Until then, evidence can hold + * only what channel windows and live traffic delivered. A later repair only + * adds evidence, so this stays set while it runs. */ + let sampled = false; const handles = new Set<() => void>(); const views = new Map< () => void, @@ -990,7 +1005,7 @@ export function createUnread({ const rowThreads = new Set(); const dm = isDm(target.channelId); indexEvidence(); - let latest: RelayEvent | undefined; + let latest: RelayEvent | undefined, oldest: RelayEvent | undefined; for (const entry of byChannel.get(target.channelId) ?? []) { const event = entry.event; if ( @@ -1005,6 +1020,12 @@ export function createUnread({ if (!isUnread(entry, state, dm)) continue; count++; if (priority(entry, dm)) attention++; + if ( + target.kind === "channel" && + (!entry.parentId || entry.broadcast) && + (!oldest || order(event, oldest) < 0) + ) + oldest = event; if ( target.kind !== "channel" || !isUnread(entry, state, dm, rowCut(entry, state, dm)) @@ -1033,6 +1054,10 @@ export function createUnread({ ...(latest ? { latestMessage: { id: latest.id, createdAt: latest.created_at } } : {}), + ...(evidence && oldest ? { oldestUnreadId: oldest.id } : {}), + ...(evidence && target.kind === "channel" && sampled + ? { sampled: true as const } + : {}), observedCount: evidence ? count : null, attentionCount: evidence ? attention : null, coverage: evidence ? "observed" : "unknown", @@ -1052,6 +1077,8 @@ export function createUnread({ } const equal = (a: UnreadSnapshot, b: UnreadSnapshot) => a.latestMessage?.id === b.latestMessage?.id && + a.oldestUnreadId === b.oldestUnreadId && + a.sampled === b.sampled && a.observedCount === b.observedCount && a.attentionCount === b.attentionCount && a.row?.observedCount === b.row?.observedCount && @@ -1651,12 +1678,16 @@ export function createUnread({ if (changed.size || archiveChanged) publish(changed); } else { accessKey = next; - purge(); // An initial observation made against a display-only roster still owes // evidence when membership becomes fresh, including during an active repair. // A newly accessible channel has no evidence yet, and the epoch bump // abandons any in-flight observation, so both also observe again. - if (requested && (confirmed || gained || refresh)) { + // Until that repair finishes, the channels it adds hold only what their + // windows and live traffic delivered; purge() publishes that. + const observe = requested && (confirmed || gained || !!refresh); + if (observe) sampled = false; + purge(); + if (observe) { repairAgain = true; if (!refresh) void repair(); } @@ -1975,6 +2006,7 @@ export function createUnread({ freshness = "observed"; owing = false; error = undefined; + sampled = true; return; } try { @@ -2005,8 +2037,9 @@ export function createUnread({ ], { signal, priority }, ); - // An overflow or refresh while this read was out discarded what the - // mark stands for: an overflow waits for refresh, a refresh reads again. + // An overflow, refresh or cache clear while this read was out + // discarded what the mark stands for: an overflow waits for refresh, + // a refresh or cache clear reads again. if (!current()) return; // Accepting notifies subscribers (the Inbox after its window), and one // may refresh from there. @@ -2030,6 +2063,7 @@ export function createUnread({ freshness = "observed"; owing = false; error = undefined; + sampled = true; publish(); void findHomes(generation); } catch (cause) { @@ -2180,6 +2214,7 @@ export function createUnread({ indexed = false; forget(); known.clear(); + sampled = false; bytes = 0; error = "Unread observation capacity reached; refresh available"; freshness = "stale"; @@ -3043,11 +3078,15 @@ export function createUnread({ epoch++; channelReadGenerations.clear(); resetLookups(); - repairAgain = false; + // Every channel loses its evidence, so a session that observes unread + // observes again, as for a newly accessible channel: now, or once the + // repair the epoch bump abandons settles. + repairAgain = requested; indexed = false; events.clear(); forget(); known.clear(); + sampled = false; forcedMessages.clear(); entered.clear(); openChannels.clear(); @@ -3056,6 +3095,7 @@ export function createUnread({ freshness = "unknown"; error = undefined; publish(); + if (requested && !refresh) void repair(); }, dispose() { closed = true; diff --git a/tests/browser/new-message-boundary.spec.mjs b/tests/browser/new-message-boundary.spec.mjs index 5373fa5462..2b83de13ae 100644 --- a/tests/browser/new-message-boundary.spec.mjs +++ b/tests/browser/new-message-boundary.spec.mjs @@ -581,3 +581,76 @@ appTest.describe("pending entry membership", () => { }, ); }); + +// Crosses the window page size against the real store's history read: the +// restore tests model which rows that read leaves the unread store knowing. +appTest.describe("history longer than one page", () => { + appTest.use({ historyCounts: { alpha: 25, beta: 1 } }); + appTest( + "marks messages that arrive after reading to the bottom of a channel with unloaded history", + async ({ page, app }) => { + const seeded = app.histories.get("primary/alpha"); + const entryWindow = () => + page.evaluate(() => { + const history = window.fixtureRelay + .snapshot() + .session.channels.window("alpha"); + return { + status: history.status, + freshness: history.freshness, + hasMore: history.hasMore, + }; + }); + await open(page, app); + await expect.poll(entryWindow).toEqual({ + status: "ready", + freshness: "verified", + hasMore: true, + }); + // Read to the bottom: the channel catch-up read covers every seeded row. + await reachEdge(page, 1); + await composer(page).focus(); + await expect + .poll( + async () => + (await readJournal(page))?.state.frontiers["activity:alpha"] ?? 0, + ) + .toBeGreaterThanOrEqual(seeded.at(-1).created_at); + + await page.getByRole("button", { name: /^Beta/ }).click(); + await expect( + page.getByRole("textbox", { name: "Message #Beta", exact: true }), + ).toBeVisible(); + const arrivals = [1, 2, 3].map((index) => + app.append("primary", "alpha", `Later message ${index}`, true, false), + ); + await expect + .poll(() => + page.evaluate( + (ids) => + ids.every( + (id) => + window.fixtureRelay + .snapshot() + .session.unread.attention("alpha", id).unread, + ), + arrivals.map((event) => event.id), + ), + ) + .toBe(true); + + await page.getByRole("button", { name: /^Alpha/ }).click(); + await expect(composer(page)).toBeVisible(); + await expect.poll(entryWindow).toEqual({ + status: "ready", + freshness: "verified", + hasMore: true, + }); + await expect(separator(page)).toHaveCount(1); + expect(await messageAfter(separator(page))).toBe(arrivals[0].id); + await expect( + history(page).locator(`[data-message-id="${seeded.at(-1).id}"]`), + ).toBeVisible(); + }, + ); +}); From b7934cb80ac5d4fcc227f219988092fd5b2009d9 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 10 Nov 2026 03:13:10 +1100 Subject: [PATCH 2/2] fix(messages): hold the channel's own reading while "New messages" waits for the repair The entry boundary waits for the unread store's `sampled` flag when entry history is incomplete. Meanwhile the reader is already at the bottom, so after the 300 ms dwell catch-up read every loaded row and observe read the unread ones. Either advanced the read revision, and the boundary's revision check then settled the visit with no line before the repair could place one. The reads also erased the oldest-unread evidence the boundary waits for, so removing the revision check alone would not have helped: the boundary has to settle first and reading follow. This is the normal startup state, where Buzz restores a cached channel while the startup repair is still running, so the divider was lost on nearly every launch. The `sampled` branch of the boundary candidate now reports `awaitingRepair`, and the timeline passes `enabled: marksRead && !awaitingRepair` to `useReading`. That branch is reached only after every earlier gate passed, so the hold applies exactly when the repair is the sole blocker; re-enabling runs the hook's effect, which schedules dwell at once. A failed repair or an evidence overflow marks the snapshot stale, which an earlier gate catches, so the hold ends with the repair either way; a read-intent change from elsewhere still settles the visit with no line and releases reading. The store only repairs once asked, so the timeline calls `ensure()` while holding: in the desktop shell the sidebar, Inbox and Channels pages already asked and a finished repair answers at once, and a host that mounts a timeline alone no longer holds reading forever. Tradeoff: rows dwelt on during the hold and left before it lifts are not marked read by that dwell; the next dwell reads them. Tests: the restore fixture gains an opt-in `reading` knob (a frontier-sync read handle whose catch-up and observe mocks clear the unread rows, advance the revision and publish, as the store does), a stubbed `ensure`, `setFreshness`, and a focused-reader dwell recipe. Three new cases enter a channel with history above the window while the repair is unfinished: the hold leaves catch-up and observe uncalled and the line absent, then places the line and catches up once `sampled` is set; a stale repair releases reading with no line; a revision change from elsewhere settles with no line and releases reading. All three fail on the previous commit, where catch-up fires at 300 ms and the line never appears. Docs: docs/unread.md records the hold and when it ends; the help guide says the divider is settled before the channel marks anything read. Signed-off-by: Matt Toohey Co-Authored-By: Claude Fable 5.1 --- docs/buzz-help.md | 8 +- docs/unread.md | 19 +- .../messages/ChannelTimeline.restore.test.tsx | 163 +++++++++++++++++- src/features/messages/ChannelTimeline.tsx | 20 ++- src/features/messages/use-reading.ts | 3 +- 5 files changed, 196 insertions(+), 17 deletions(-) diff --git a/docs/buzz-help.md b/docs/buzz-help.md index 5c019887ef..d43621f111 100644 --- a/docs/buzz-help.md +++ b/docs/buzz-help.md @@ -325,9 +325,11 @@ revokes all of them, and a pairing unused for 30 days expires. history than loads on entry, it appears when the oldest unread message is among the rows loaded on entry, below a message you have read; when the unread messages start at the first loaded row or further up, there is no divider for - that visit, even if older rows load later. It may also be absent when unread - evidence is incomplete, or when read intent or the read evidence needed to - locate the boundary changes before it is established. + that visit, even if older rows load later. The divider is settled before the + channel marks anything read, so opening a channel does not hide its own + divider. It may also be absent when unread evidence is incomplete, or when + read intent or the read evidence needed to locate the boundary changes + elsewhere before it is established. - **Opening a channel or DM** while Buzz is visible and focused clears its sidebar row: the bold name, the mention dot and the thread dot. Switching away from Buzz pauses this: new messages can light the row even while that conversation stays selected. The macOS Dock dot follows the rows. Nothing is marked diff --git a/docs/unread.md b/docs/unread.md index cfae992be1..149d9e88e0 100644 --- a/docs/unread.md +++ b/docs/unread.md @@ -117,14 +117,17 @@ viewport): capture a verified, idle window with all of its history (rows sit above it, or it was cached, refreshing or still paging), the **New messages** divider waits for `sampled`, then appears only at `oldestUnreadId` among the rows loaded on - entry and only below a read one. A batch that filled its limit can still stop - short of the true oldest unread message, and the read-row condition covers - only ordinary rows: catch-up's `activity:` mark reads all of them up to a - timestamp, but never mentions, broadcasts, broadcast replies or messages - marked unread, which only their own `msg:` or `thread:` marks read. A short - sample can hide an unread one of those above the window, and the divider then - lands below it. A row above read only by its own mark says nothing about older - rows either. + entry and only below a read one. While it waits, the timeline holds its own + reading (dwell and bottom catch-up), which would otherwise read the loaded + rows and erase the evidence it waits for; the hold ends when the repair + finishes or fails, and reading then starts at once. A batch that filled its + limit can still stop short of the true oldest unread message, and the + read-row condition covers only ordinary rows: catch-up's `activity:` mark + reads all of them up to a timestamp, but never mentions, broadcasts, + broadcast replies or messages marked unread, which only their own `msg:` or + `thread:` marks read. A short sample can hide an unread one of those above + the window, and the divider then lands below it. A row above read only by + its own mark says nothing about older rows either. - Both keys use verified event timestamps, never wall time. Neither enters generic channel/thread inheritance or remote override baselines. Channel catch-up ends a local manual unread on the channel itself; message and thread manual-unread diff --git a/src/features/messages/ChannelTimeline.restore.test.tsx b/src/features/messages/ChannelTimeline.restore.test.tsx index af3e0d6e6e..ca87cd4d71 100644 --- a/src/features/messages/ChannelTimeline.restore.test.tsx +++ b/src/features/messages/ChannelTimeline.restore.test.tsx @@ -257,7 +257,8 @@ function mount(bottom = false, revealOnMount = false, anchorLoaded = true) { }; } -function boundaryFixture() { +/** `reading`: this list's own dwell can mark rows read, as the real store would. */ +function boundaryFixture({ reading = false } = {}) { const viewer = keypair(), peer = keypair(), relay = keypair(); @@ -273,7 +274,9 @@ function boundaryFixture() { listeners = new Set<() => void>(); const syncListeners = new Set<() => void>(); let readRevision = owner.session.unread.revision(); - let syncSnapshot = owner.session.unread.sync(); + let syncSnapshot = reading + ? { ...owner.session.unread.sync(), capability: "frontier-sync" as const } + : owner.session.unread.sync(); const entries: { promise: Promise; resolve: () => void; @@ -310,6 +313,8 @@ function boundaryFixture() { const built: ChannelMessage[] = []; // The store's repair has finished, so that evidence reaches past the window. let sampled = true; + let freshness: "observed" | "stale" = "observed"; + let error: string | undefined; const snapshot = (channelId: string) => { const oldest = built .filter( @@ -329,10 +334,25 @@ function boundaryFixture() { observedCount: unreadIds.size, attentionCount: 0, coverage: "observed" as const, - freshness: "observed" as const, + freshness, manual: "none" as const, + ...(error ? { error } : {}), }; }; + // A repair the timeline asks for; the real one reads the relay. + const ensure = vi.fn(() => Promise.resolve()); + // Catch-up reads every loaded row, which advances read intent and leaves + // nothing unread in the channel; observe reads the rows it is given. + const catchUp = vi.fn(async () => { + unreadIds.clear(); + readRevision++; + publish(); + }); + const observe = vi.fn(async (ids: readonly string[]) => { + for (const id of ids) unreadIds.delete(id); + readRevision++; + publish(); + }); const snapshots = new Map([ ["c", snapshot("c")], ["other", snapshot("other")], @@ -366,6 +386,17 @@ function boundaryFixture() { }; }, revision: () => readRevision, + ensure, + ...(reading + ? { + reading: () => ({ + view: () => {}, + dispose: () => {}, + observe, + catchUp, + }), + } + : {}), }; const session = { ...owner.session, channels, unread }; function row( @@ -489,6 +520,41 @@ function boundaryFixture() { setSampled(value: boolean) { sampled = value; }, + setFreshness(value: "observed" | "stale", cause?: string) { + freshness = value; + error = cause; + }, + ensure, + catchUp, + observe, + /** A focused reader at the bottom, with the dwell clock under test control. */ + async focusReader() { + vi.spyOn(document, "hasFocus").mockReturnValue(true); + vi.spyOn(document, "visibilityState", "get").mockReturnValue("visible"); + vi.spyOn(HTMLElement.prototype, "scrollHeight", "get").mockReturnValue( + 600, + ); + vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue( + new DOMRect(0, 0, 800, 500), + ); + vi.spyOn(HTMLElement.prototype, "getClientRects").mockReturnValue([ + new DOMRect(0, 0, 800, 500), + ] as unknown as DOMRectList); + await frame(); + await frame(); + const history = screen.getByRole("region", { + name: "Channel message history", + }); + await act(async () => { + history.focus(); + await Promise.resolve(); + }); + }, + async dwell() { + await act(async () => { + vi.advanceTimersByTime(300); + }); + }, publish, }; } @@ -687,6 +753,97 @@ it.each([{ olderUnread: false }, { olderUnread: true }])( }, ); +/** Rows above the window, the store's repair unfinished: the oldest unread + * row it knows sits below a receipt-read row, and the reader is at the bottom. */ +async function heldReader() { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + const h = boundaryFixture({ reading: true }); + h.setSampled(false); + const read = h.row(message(h.peer, "c", "Read by its own receipt", 2)); + const first = h.row(message(h.peer, "c", "First loaded unread", 3)); + const later = h.row(message(h.peer, "c", "Later loaded unread", 4)); + h.unreadIds.add(first.id); + h.unreadIds.add(later.id); + h.completeReadStateSnapshot(); + h.publish(); + const mounted = render(h.tree("c", [read, first, later], true)); + await h.completeEntry(); + await h.focusReader(); + await h.dwell(); + return { h, mounted, read, first, later }; +} + +it("holds this list's reading while the boundary waits for the unread repair, then reads once it is placed", async () => { + const { h, mounted, later } = await heldReader(); + try { + // Without the hold, catch-up's read would advance read intent and erase + // the oldest unread row before the repair could vouch for it: no line. + expect(h.catchUp).not.toHaveBeenCalled(); + expect(h.observe).not.toHaveBeenCalled(); + expect( + screen.queryByRole("separator", { name: "New messages" }), + ).toBeNull(); + expect(h.ensure).toHaveBeenCalledTimes(1); + + h.setSampled(true); + h.publish(); + expect(precedes(divider(), screen.getByText("First loaded unread"))).toBe( + true, + ); + expect( + precedes(screen.getByText("Read by its own receipt"), divider()), + ).toBe(true); + expect(h.catchUp).not.toHaveBeenCalled(); + + // The hold lifting starts dwell on its own. + await h.dwell(); + expect(h.catchUp).toHaveBeenCalledWith(later.id, undefined); + expect(h.ensure).toHaveBeenCalledTimes(1); + expect(precedes(divider(), screen.getByText("First loaded unread"))).toBe( + true, + ); + } finally { + mounted.unmount(); + vi.useRealTimers(); + } +}); + +it("resumes this list's reading with no boundary when the unread repair fails", async () => { + const { h, mounted, later } = await heldReader(); + try { + expect(h.catchUp).not.toHaveBeenCalled(); + h.setFreshness("stale", "Unread observation failed"); + h.publish(); + await h.dwell(); + expect(h.catchUp).toHaveBeenCalledWith(later.id, undefined); + expect( + screen.queryByRole("separator", { name: "New messages" }), + ).toBeNull(); + } finally { + mounted.unmount(); + vi.useRealTimers(); + } +}); + +it("a read-intent change from elsewhere during the held wait still settles with no boundary and releases reading", async () => { + const { h, mounted, later } = await heldReader(); + try { + expect(h.catchUp).not.toHaveBeenCalled(); + h.advanceReadRevision(); + h.publish("other"); + h.setSampled(true); + h.publish(); + expect( + screen.queryByRole("separator", { name: "New messages" }), + ).toBeNull(); + await h.dwell(); + expect(h.catchUp).toHaveBeenCalledWith(later.id, undefined); + } finally { + mounted.unmount(); + vi.useRealTimers(); + } +}); + it.each([{ remoteRead: false }, { remoteRead: true }])( "does not infer an entry boundary when the oldest unread message is outside the entry window: %j", async ({ remoteRead }) => { diff --git a/src/features/messages/ChannelTimeline.tsx b/src/features/messages/ChannelTimeline.tsx index e5ed5b48f7..60cccbc71f 100644 --- a/src/features/messages/ChannelTimeline.tsx +++ b/src/features/messages/ChannelTimeline.tsx @@ -24,6 +24,7 @@ import type { RelaySession } from "../relay/session"; import type { MessageAttention } from "../relay/unread"; import { useCallback, + useEffect, useLayoutEffect, useMemo, useRef, @@ -468,7 +469,10 @@ function Timeline({ // Every relay read feeds the store, so until its repair has sampled // history it may know only the loaded rows, and its oldest unread one // can sit in the window below any read row while older ones are unread. - if (!unreadSnapshot.sampled) return { ready: false }; + // This is the only gate that holds this list's own reading: every + // earlier one has passed, so the repair alone is awaited. + if (!unreadSnapshot.sampled) + return { ready: false, awaitingRepair: true }; const oldest = unreadSnapshot.oldestUnreadId; if (!oldest || !baseline.messageIds.has(oldest)) return { ready: true }; } @@ -503,6 +507,12 @@ function Timeline({ } return { ready: true }; })(); + // The store repairs only once something has asked it to. The desktop shell + // asks on startup; a host that mounts a timeline alone must not hold reading + // forever. A finished repair answers at once, so this adds no relay reads. + useEffect(() => { + if (boundaryCandidate.awaitingRepair) void queries.unread.ensure(); + }, [boundaryCandidate.awaitingRepair, queries.unread]); if (!boundaryMatchesVisit) setNewMessageBoundary( retainedVisit?.boundary ?? { @@ -1103,7 +1113,13 @@ function Timeline({ latestMessageId: window.rows .filter((row) => !row.membership && !row.huddle) .at(-1)?.id, - enabled: marksRead, + // Reading before the entry boundary settles would erase the evidence it + // waits for: catch-up reads every loaded row and observe the unread ones, + // and either advances the read revision that settles it with no line. + // Hold reading while the boundary waits for the repair; re-enabling + // schedules dwell at once. A failed repair marks freshness stale, which + // an earlier gate catches, so the hold ends with the repair either way. + enabled: marksRead && !boundaryCandidate.awaitingRepair, }); // Shift keeps the distance from the end, so it holds the reader only while // the rows from their first visible item to the end are kept. Relay pages diff --git a/src/features/messages/use-reading.ts b/src/features/messages/use-reading.ts index 4dd7d8643f..69d4419d6b 100644 --- a/src/features/messages/use-reading.ts +++ b/src/features/messages/use-reading.ts @@ -25,7 +25,8 @@ type Reading = { /** True while the list shows its latest row in place of the reader's own, * still loading: the rows shown are read, but that bottom is not caught up. */ restoring?: (() => boolean) | undefined; - /** False: this list marks nothing read; its owner decides what was read. */ + /** False: this list marks nothing read; its owner decides what was read, or + * when reading may start. Turning it on schedules dwell at once. */ enabled?: boolean | undefined; }; /** `useReading` for an owner that renders its own `MessageEditScope`. */