fix(desktop): correct thread-unread badge staleness and phantom channel counts

Three live defects in the shipped thread-unread badge (#1069):

- Flaky badge: participatedRootIds/authoredRootIds were bare in-place-mutated
  ref Sets, so isNotifiedForThread's useCallback never re-created and a badge
  whose participation was discovered async stayed suppressed. A membershipVersion
  reducer now re-derives identity-stable snapshots for the gate while live
  consumers keep reading the mutable refs.
- Badge wouldn't clear on read: the threadUnreadCounts memo read a frozen
  open-time frontier snapshot and omitted readStateVersion from its deps. The
  snapshot now advances monotonically toward the live thread marker (never the
  latest reply, so collapsed-branch unreads survive) keyed on readStateVersion.
- Phantom channel-pill counts: the marker counted relay-signed system rows and
  job-lifecycle events. A call-site kind predicate excludes them; unreadMarker
  stays kind-agnostic. Undefined kinds are treated as conversational so pending
  rows are never dropped.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

Author: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7

desktop/src/features/channels/lib/threadBadgeCounts.ts

@@ -0,0 +1,44 @@
+import { computeThreadUnreadMarker } from "@/features/messages/lib/unreadMarker";
+import type { TimelineMessage } from "@/features/messages/types";
+
+/**
+ * Per-thread unread reply counts for the summary rows in the main timeline.
+ *
+ * Counts are computed only for threads the user has notification interest in
+ * (`isNotified`), aligning the badge display with the read-state write path,
+ * and measured against the per-root frontier snapshot rather than the live
+ * marker so badges stay stable for the session and don't flash when
+ * markChannelRead advances the channel marker. The snapshot is advanced toward
+ * the live marker on read upstream of this function, so a read thread's badge
+ * clears here once its frontier passes the last reply.
+ *
+ * @param messages Timeline messages (top-level entries plus their replies) in
+ *   chronological order.
+ * @param frontiers Per-root read frontier in unix seconds, or null/undefined
+ *   when the thread was never read (every reply counts unread).
+ * @param isNotified Whether a thread root is one the user is notified for.
+ * @param currentPubkey Replies authored by this pubkey never count as unread.
+ */
+export function computeThreadBadgeCounts(
+  messages: TimelineMessage[],
+  frontiers: ReadonlyMap<string, number | null> | undefined,
+  isNotified: (rootId: string) => boolean,
+  currentPubkey?: string,
+): Map<string, number> {
+  const counts = new Map<string, number>();
+  for (const message of messages) {
+    if (message.parentId) continue;
+    if (!isNotified(message.id)) continue;
+    const directReplies = messages.filter((m) => m.parentId === message.id);
+    if (directReplies.length === 0) continue;
+    const { unreadCount } = computeThreadUnreadMarker(
+      directReplies,
+      frontiers?.get(message.id) ?? null,
+      currentPubkey,
+    );
+    if (unreadCount > 0) {
+      counts.set(message.id, unreadCount);
+    }
+  }
+  return counts;
+}

desktop/src/features/channels/lib/threadBadgeFrontier.test.mjs

@@ -0,0 +1,91 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { nextThreadBadgeFrontier } from "./threadBadgeFrontier.ts";
+import { seedThreadBadgeFrontiers } from "./threadBadgeFrontier.ts";
+
+const msg = (id, parentId) => ({ id, parentId });
+const seedAll = () => true;
+
+test("nextThreadBadgeFrontier_unseededNullMarker_seedsNull", () => {
+  // Thread never read: snapshot seeds to null (everything unread).
+  assert.equal(nextThreadBadgeFrontier(undefined, null), null);
+});
+
+test("nextThreadBadgeFrontier_unseededWithMarker_seedsToMarker", () => {
+  assert.equal(nextThreadBadgeFrontier(undefined, 100), 100);
+});
+
+test("nextThreadBadgeFrontier_readAdvancesMarker_advancesSnapshot", () => {
+  // Snapshot frozen at open (null), user reads → live marker 200 → badge clears.
+  assert.equal(nextThreadBadgeFrontier(null, 200), 200);
+});
+
+test("nextThreadBadgeFrontier_markerNewerThanStored_advances", () => {
+  assert.equal(nextThreadBadgeFrontier(100, 250), 250);
+});
+
+test("nextThreadBadgeFrontier_markerOlderThanStored_keepsStored", () => {
+  // Monotonic: a stale lower marker never lowers the snapshot.
+  assert.equal(nextThreadBadgeFrontier(250, 100), 250);
+});
+
+test("nextThreadBadgeFrontier_markerNullAfterSeed_keepsStored", () => {
+  // Live marker reads null (never read) but snapshot already advanced — hold.
+  assert.equal(nextThreadBadgeFrontier(150, null), 150);
+});
+
+test("nextThreadBadgeFrontier_markerEqualsStored_unchanged", () => {
+  assert.equal(nextThreadBadgeFrontier(150, 150), 150);
+});
+
+test("nextThreadBadgeFrontier_storedNullMarkerZero_advancesToZero", () => {
+  // Zero is a valid frontier (epoch); null is strictly lower than any number.
+  assert.equal(nextThreadBadgeFrontier(null, 0), 0);
+});
+
+test("seedThreadBadgeFrontiers_threadWithReplies_seedsToMarker", () => {
+  const frontiers = new Map();
+  const messages = [msg("root", null), msg("r1", "root")];
+  seedThreadBadgeFrontiers(frontiers, messages, seedAll, (id) =>
+    id === "root" ? 100 : null,
+  );
+  assert.equal(frontiers.get("root"), 100);
+});
+
+test("seedThreadBadgeFrontiers_threadWithoutReplies_skipped", () => {
+  const frontiers = new Map();
+  seedThreadBadgeFrontiers(frontiers, [msg("root", null)], seedAll, () => 100);
+  assert.equal(frontiers.has("root"), false);
+});
+
+test("seedThreadBadgeFrontiers_notNotified_skipped", () => {
+  const frontiers = new Map();
+  const messages = [msg("root", null), msg("r1", "root")];
+  seedThreadBadgeFrontiers(
+    frontiers,
+    messages,
+    () => false,
+    () => 100,
+  );
+  assert.equal(frontiers.has("root"), false);
+});
+
+test("seedThreadBadgeFrontiers_replyEntry_neverSeeded", () => {
+  // A reply is never a badge root even if its id collides with a notified set.
+  const frontiers = new Map();
+  const messages = [msg("r1", "root"), msg("r2", "root")];
+  seedThreadBadgeFrontiers(frontiers, messages, seedAll, () => 100);
+  assert.equal(frontiers.size, 0);
+});
+
+test("seedThreadBadgeFrontiers_reseed_advancesMonotonically", () => {
+  const frontiers = new Map([["root", 100]]);
+  const messages = [msg("root", null), msg("r1", "root")];
+  // Re-render after the live marker advanced to 250 on read.
+  seedThreadBadgeFrontiers(frontiers, messages, seedAll, () => 250);
+  assert.equal(frontiers.get("root"), 250);
+  // A stale lower marker never lowers an already-advanced snapshot.
+  seedThreadBadgeFrontiers(frontiers, messages, seedAll, () => 100);
+  assert.equal(frontiers.get("root"), 250);
+});

desktop/src/features/channels/lib/threadBadgeFrontier.ts

@@ -0,0 +1,56 @@
+import type { TimelineMessage } from "@/features/messages/types";
+
+// Decide the next value for a thread's badge frontier snapshot. The snapshot is
+// seeded once at channel-open (reflecting "what was unread on open") and then
+// advanced monotonically toward the live thread read marker as the user reads,
+// so the badge clears after a read without waiting for channel re-entry.
+//
+// The advance target is ALWAYS the live marker (what the user actually
+// consumed), never "latest reply": a subsequent reply newer than the marker
+// re-raises the badge, and a collapsed-branch reply the marker never covered
+// stays unread. Monotonic `Math.max` guards against a stale lower marker.
+//
+// Returns the value the snapshot should hold:
+//   - `stored === undefined` (unseeded): seed to the live marker.
+//   - otherwise: the greater of the stored snapshot and the live marker, where
+//     `null` (never read) is the lowest possible frontier.
+export function nextThreadBadgeFrontier(
+  stored: number | null | undefined,
+  liveMarker: number | null,
+): number | null {
+  if (stored === undefined) {
+    return liveMarker;
+  }
+  if (liveMarker === null) {
+    return stored;
+  }
+  if (stored === null) {
+    return liveMarker;
+  }
+  return Math.max(stored, liveMarker);
+}
+
+// Seed/advance the per-root badge frontier snapshots for one channel, in place.
+// Captures only top-level notified threads that have replies; each entry is
+// seeded once at open then advanced toward the live marker on subsequent reads
+// (see nextThreadBadgeFrontier). Called during render so snapshots reflect
+// "what was unread on open," matching the openFrontierRef pattern.
+export function seedThreadBadgeFrontiers(
+  channelFrontiers: Map<string, number | null>,
+  messages: TimelineMessage[],
+  isNotified: (rootId: string) => boolean,
+  getReadAt: (rootId: string) => number | null,
+): void {
+  for (const message of messages) {
+    if (message.parentId) continue;
+    if (!isNotified(message.id)) continue;
+    if (!messages.some((m) => m.parentId === message.id)) continue;
+    channelFrontiers.set(
+      message.id,
+      nextThreadBadgeFrontier(
+        channelFrontiers.get(message.id),
+        getReadAt(message.id),
+      ),
+    );
+  }
+}

desktop/src/features/channels/ui/ChannelScreen.tsx

@@ -91,6 +91,7 @@ export function ChannelScreen({
     unfollowThread,
     isFollowingThread,
     isNotifiedForThread,
+    readStateVersion,
     setTopbarSearchHidden,
   } = useAppShell();
   const {
@@ -369,6 +370,7 @@ export function ChannelScreen({
     markChannelUnread,
     markThreadRead,
     isNotifiedForThread,
+    readStateVersion,
   });
   const editTargetMessage = React.useMemo(
     () =>

desktop/src/features/channels/ui/useChannelUnreadState.ts

@@ -8,6 +8,8 @@ import {
   subtreeMaxCreatedAt,
 } from "@/features/channels/lib/subtreeCreatedAt";
 import { computeThreadReplyUnreadCounts } from "@/features/channels/lib/threadReplyUnreadCounts";
+import { computeThreadBadgeCounts } from "@/features/channels/lib/threadBadgeCounts";
+import { seedThreadBadgeFrontiers } from "@/features/channels/lib/threadBadgeFrontier";
 import {
   buildThreadPanelDataFromIndex,
   buildThreadPanelIndex,
@@ -17,6 +19,7 @@ import {
   computeThreadUnreadMarker,
 } from "@/features/messages/lib/unreadMarker";
 import type { TimelineMessage } from "@/features/messages/types";
+import { isConversationalUnreadKind } from "@/shared/constants/kinds";
 
 import { useWelcomeInitialUnreadSuppression } from "./useWelcomeInitialUnreadSuppression";
 
@@ -33,6 +36,7 @@ type UseChannelUnreadStateOptions = {
   markChannelUnread: (channelId: string) => void;
   markThreadRead: (rootId: string, timestamp: number) => void;
   isNotifiedForThread: (rootId: string) => boolean;
+  readStateVersion: number;
 };
 
 /**
@@ -60,6 +64,7 @@ export function useChannelUnreadState({
   markChannelUnread,
   markThreadRead,
   isNotifiedForThread,
+  readStateVersion,
 }: UseChannelUnreadStateOptions) {
   // Capture the read frontier as it stood the instant this channel was opened,
   // BEFORE the mark-read effect (in ChannelScreen) advances it to latest.
@@ -166,10 +171,14 @@ export function useChannelUnreadState({
 
   // Oldest unread top-level message + count from the open-time frontier.
   // Keyed per channel so the pill/divider survive the mark-read effect.
+  // Non-conversational kinds (system rows, job-lifecycle events) are filtered
+  // out first so they don't inflate the pill; see isConversationalUnreadKind.
   const { firstUnreadMessageId, unreadCount } = React.useMemo(
     () =>
       computeChannelUnreadMarker(
-        timelineMessages,
+        timelineMessages.filter((message) =>
+          isConversationalUnreadKind(message.kind),
+        ),
         openFrontierSeconds,
         isActiveChannelForcedUnread || isActiveWelcomeInitialUnreadSuppressed,
         currentPubkey,
@@ -300,17 +309,12 @@ export function useChannelUnreadState({
       channelFrontiers = new Map();
       threadBadgeFrontiersRef.current.set(activeChannelId, channelFrontiers);
     }
-    for (const message of timelineMessages) {
-      if (message.parentId) continue;
-      if (!isNotifiedForThread(message.id)) continue;
-      if (channelFrontiers.has(message.id)) continue;
-      // Only capture for messages that have thread replies
-      const hasReplies = timelineMessages.some(
-        (m) => m.parentId === message.id,
-      );
-      if (!hasReplies) continue;
-      channelFrontiers.set(message.id, getThreadReadAt(message.id));
-    }
+    seedThreadBadgeFrontiers(
+      channelFrontiers,
+      timelineMessages,
+      isNotifiedForThread,
+      getThreadReadAt,
+    );
   }
   // Clear the thread badge frontiers on channel leave (same cleanup as
   // openFrontierRef) so re-visiting captures fresh snapshots.
@@ -321,36 +325,29 @@ export function useChannelUnreadState({
       threadBadgeFrontiersRef.current.delete(channelId);
     };
   }, [activeChannelId]);
-  // Compute per-thread unread counts for summary rows in the main timeline.
-  // Only compute for threads the user has notification interest in — this
-  // aligns the badge display with the read-state write path. Uses the
-  // snapshotted frontier (threadBadgeFrontiersRef) so badges are stable for
-  // the session and don't flash when markChannelRead advances the channel
-  // marker.
-  const threadUnreadCounts = React.useMemo(() => {
-    const counts = new Map<string, number>();
-    const channelFrontiers = activeChannelId
-      ? threadBadgeFrontiersRef.current.get(activeChannelId)
-      : undefined;
-    for (const message of timelineMessages) {
-      if (message.parentId) continue;
-      if (!isNotifiedForThread(message.id)) continue;
-      const directReplies = timelineMessages.filter(
-        (m) => m.parentId === message.id,
-      );
-      if (directReplies.length === 0) continue;
-      const frontier = channelFrontiers?.get(message.id) ?? null;
-      const { unreadCount } = computeThreadUnreadMarker(
-        directReplies,
-        frontier,
+  // Per-thread unread counts for the main-timeline summary rows. Pure logic
+  // lives in computeThreadBadgeCounts; readStateVersion is an intentional
+  // recompute trigger so the badge re-reads the snapshot the seed block above
+  // advanced toward the live marker on mark-read.
+  // biome-ignore lint/correctness/useExhaustiveDependencies: readStateVersion is the intentional recompute trigger
+  const threadUnreadCounts = React.useMemo(
+    () =>
+      computeThreadBadgeCounts(
+        timelineMessages,
+        activeChannelId
+          ? threadBadgeFrontiersRef.current.get(activeChannelId)
+          : undefined,
+        isNotifiedForThread,
         currentPubkey,
-      );
-      if (unreadCount > 0) {
-        counts.set(message.id, unreadCount);
-      }
-    }
-    return counts;
-  }, [activeChannelId, currentPubkey, timelineMessages, isNotifiedForThread]);
+      ),
+    [
+      activeChannelId,
+      currentPubkey,
+      timelineMessages,
+      isNotifiedForThread,
+      readStateVersion,
+    ],
+  );
 
   const handleMarkUnread = React.useCallback(() => {
     if (!activeChannelId) return;