fix(messages): end the New messages hold when no repair can sample

docs/buzz-help.md

@@ -409,11 +409,15 @@ revokes all of them, and a pairing unused for 30 days expires.
   you enter. Once established, the divider stays in place for that visit as you
   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. 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
+  among the rows loaded on entry, below a message you read by reaching the
+  bottom of the channel or marking the channel read. When the unread messages
+  start at the first loaded row or further up, or the messages above them were
+  only read one by one as you scrolled past, there is no divider for that visit,
+  even if older rows load later. While Buzz is still checking such a channel's
+  older history for unread messages, the channel waits to mark anything read,
+  so opening it then does not hide its own divider; if that check fails, there
+  is no divider for that visit and reading resumes. 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

docs/unread.md

@@ -142,21 +142,28 @@ viewport):
   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. 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.
+  Later repairs, refreshes included, leave `sampled` set. Until it is set,
+  `sampling` says a repair that can set it is reading, about to restart, or not
+  yet asked for; one that fails, overflows or is abandoned (a dropped
+  connection, an access change) without a restart clears it until the next
+  starts. 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 one that a
+  channel-wide read reaches (`attention()` reports `prefixRead`: the channel
+  frontier, its `activity:` catch-up mark or the retention floor), because each
+  of those also reads every older ordinary row. While it waits with `sampling`
+  set, 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. Once
+  `sampled` is set the divider is placed and reading starts at once; if
+  `sampling` clears first, the visit settles with no divider and reading resumes
+  without waiting for a retry. A batch that filled its limit can still stop
+  short of the true oldest unread message. `prefixRead` rules that out for
+  ordinary rows, but not for mentions, broadcasts, broadcast replies or messages
+  marked unread, which catch-up's `activity:` mark never reads: a short sample
+  can hide an unread one of those above the window, and the divider then lands
+  below it. Rows above read only by their own `msg:` or `thread:` marks, as
+  dwell short of the bottom leaves them, place no divider.
 - 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

src/features/messages/ChannelTimeline.restore.test.tsx

@@ -271,6 +271,8 @@ function boundaryFixture({ reading = false } = {}) {
     unknownIds = new Set<string>(),
     pendingIds = new Set<string>(),
     ineligibleIds = new Set<string>(),
+    // Read only by their own receipt, not by a channel-wide read.
+    receiptIds = new Set<string>(),
     listeners = new Set<() => void>();
   const syncListeners = new Set<() => void>();
   let readRevision = owner.session.unread.revision();
@@ -313,6 +315,8 @@ function boundaryFixture({ reading = false } = {}) {
   const built: ChannelMessage[] = [];
   // The store's repair has finished, so that evidence reaches past the window.
   let sampled = true;
+  // Unsampled: a repair that can still sample is running.
+  let repairing = true;
   let freshness: "observed" | "stale" = "observed";
   let error: string | undefined;
   const snapshot = (channelId: string) => {
@@ -330,7 +334,11 @@ function boundaryFixture({ reading = false } = {}) {
     return {
       target: { kind: "channel" as const, channelId },
       ...(oldest ? { oldestUnreadId: oldest.id } : {}),
-      ...(sampled ? { sampled: true as const } : {}),
+      ...(sampled
+        ? { sampled: true as const }
+        : repairing
+          ? { sampling: true as const }
+          : {}),
       observedCount: unreadIds.size,
       attentionCount: 0,
       coverage: "observed" as const,
@@ -383,6 +391,9 @@ function boundaryFixture({ reading = false } = {}) {
         unread: unreadIds.has(id),
         forced: forcedIds.has(id),
         viewing: false,
+        ...(!unreadIds.has(id) && !receiptIds.has(id)
+          ? { prefixRead: true as const }
+          : {}),
       };
     },
     revision: () => readRevision,
@@ -488,6 +499,7 @@ function boundaryFixture({ reading = false } = {}) {
     unknownIds,
     pendingIds,
     ineligibleIds,
+    receiptIds,
     enterChannel,
     subscribers: () => ({ unread: listeners.size, sync: syncListeners.size }),
     completeReadStateSnapshot,
@@ -520,6 +532,10 @@ function boundaryFixture({ reading = false } = {}) {
     setSampled(value: boolean) {
       sampled = value;
     },
+    /** Whether an unsampled store still has a repair that can sample. */
+    setRepairing(value: boolean) {
+      repairing = value;
+    },
     setFreshness(value: "observed" | "stale", cause?: string) {
       freshness = value;
       error = cause;
@@ -710,11 +726,11 @@ 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.
+    // oldest unread message sits in the window below a row catch-up read,
+    // 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 read = h.row(message(h.peer, "c", "Read by catch-up", 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);
@@ -740,26 +756,33 @@ it.each([{ olderUnread: false }, { olderUnread: true }])(
         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.
+      // Catch-up read every ordinary row above the read one, but a sample
+      // that stopped short could still miss an unread mention, broadcast or
+      // marked message there; 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);
+      expect(precedes(screen.getByText("Read by catch-up"), divider())).toBe(
+        true,
+      );
     }
     mounted.unmount();
   },
 );
 
-/** 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() {
+/** Rows above the window, the store unsampled: the oldest unread row it knows
+ * sits below a row catch-up read, and the reader is at the bottom. Without
+ * `failed` its repair is still running; with it, a repair already ended
+ * without sampling and live evidence has since made the store observed. */
+async function heldReader({ failed }: { failed?: string } = {}) {
   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));
+  if (failed) {
+    h.setRepairing(false);
+    h.setFreshness("observed", failed);
+  }
+  const read = h.row(message(h.peer, "c", "Read by catch-up", 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);
@@ -790,9 +813,9 @@ it("holds this list's reading while the boundary waits for the unread repair, th
     expect(precedes(divider(), screen.getByText("First loaded unread"))).toBe(
       true,
     );
-    expect(
-      precedes(screen.getByText("Read by its own receipt"), divider()),
-    ).toBe(true);
+    expect(precedes(screen.getByText("Read by catch-up"), divider())).toBe(
+      true,
+    );
     expect(h.catchUp).not.toHaveBeenCalled();
 
     // The hold lifting starts dwell on its own.
@@ -808,22 +831,82 @@ it("holds this list's reading while the boundary waits for the unread repair, th
   }
 });
 
-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");
+it.each([{ observedAgain: false }, { observedAgain: true }])(
+  "resumes this list's reading with no boundary when the unread repair fails: %j",
+  async ({ observedAgain }) => {
+    const { h, mounted, later } = await heldReader();
+    try {
+      expect(h.catchUp).not.toHaveBeenCalled();
+      h.setRepairing(false);
+      h.setFreshness("stale", "Unread observation failed");
+      h.publish();
+      // A window read or live message marks the evidence observed again
+      // before any dwell, with the error kept and nothing sampled. Dwelling
+      // while stale would instead settle the visit through its own read.
+      if (observedAgain) {
+        h.setFreshness("observed", "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.each([
+  "Unread observation failed",
+  "Unread observation capacity reached; refresh available",
+])(
+  "reads at once with no boundary when entered after a repair ended without sampling: %s",
+  async (failed) => {
+    const { h, mounted, later } = await heldReader({ failed });
+    try {
+      // No repair is coming, so waiting would hold reading until a retry.
+      expect(h.catchUp).toHaveBeenCalledWith(later.id, undefined);
+      expect(h.ensure).not.toHaveBeenCalled();
+      expect(
+        screen.queryByRole("separator", { name: "New messages" }),
+      ).toBeNull();
+    } finally {
+      mounted.unmount();
+      vi.useRealTimers();
+    }
+  },
+);
+
+it.each([{ receipt: false }, { receipt: true }])(
+  "places a boundary inside incomplete history only below a row a channel-wide read reaches: %j",
+  async ({ receipt }) => {
+    // A row read only by its own receipt says nothing about the ordinary rows
+    // above it, which a sample that stopped short may not have seen.
+    const h = boundaryFixture();
+    const read = h.row(message(h.peer, "c", "Read in loaded window", 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));
+    if (receipt) h.receiptIds.add(read.id);
+    h.unreadIds.add(first.id);
+    h.unreadIds.add(later.id);
+    h.completeReadStateSnapshot();
     h.publish();
-    await h.dwell();
-    expect(h.catchUp).toHaveBeenCalledWith(later.id, undefined);
-    expect(
-      screen.queryByRole("separator", { name: "New messages" }),
-    ).toBeNull();
-  } finally {
+    const mounted = render(h.tree("c", [read, first, later], true));
+    await h.completeEntry();
+    if (receipt)
+      expect(
+        screen.queryByRole("separator", { name: "New messages" }),
+      ).toBeNull();
+    else
+      expect(precedes(divider(), screen.getByText("First loaded unread"))).toBe(
+        true,
+      );
     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();

src/features/messages/ChannelTimeline.tsx

@@ -470,21 +470,25 @@ function Timeline({
       // 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.
       // This is the only gate that holds this list's own reading: every
-      // earlier one has passed, so the repair alone is awaited.
+      // earlier one has passed, so the repair alone is awaited. A repair that
+      // ended without sampling (failed, overflowed, abandoned) settles the
+      // visit with no line, so reading never waits for a later retry.
       if (!unreadSnapshot.sampled)
-        return { ready: false, awaitingRepair: true };
+        return unreadSnapshot.sampling
+          ? { ready: false, awaitingRepair: true }
+          : { ready: true };
       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.
+    // the first loaded row the oldest unread message the store knows. So the
+    // line needs an entry row above it that a channel-wide read reaches, which
+    // reads every older ordinary row; one read only by its own msg: or thread:
+    // mark says nothing about them. That still leaves mentions, broadcasts,
+    // broadcast replies and messages marked unread, which catch-up's activity:
+    // mark never reads, so a short sample can hide an unread one of those
+    // above the window.
     let afterRead = messageVisit.entryHistoryComplete;
     for (const { row, attention } of entryAttention) {
       if (attention.status === "unknown" || attention.pending)
@@ -503,7 +507,7 @@ function Timeline({
       // Ineligible means no attention category, not that an incoming row is read.
       if (attention.forced || attention.unread)
         return afterRead ? { ready: true, messageId: row.id } : { ready: true };
-      afterRead = true;
+      if (attention.prefixRead) afterRead = true;
     }
     return { ready: true };
   })();