fix(desktop): clear channel unread badges from thread reads (#1148)

Signed-off-by: npub1v9a0cp05uazvvx3ph34jcfqr4ts3j7wwcve0tvgh4nuhphdkfe2sg7jw4q <617afc05f4e744c61a21bc6b2c2403aae11979cec332f5b117acf970ddb64e55@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: npub1v9a0cp05uazvvx3ph34jcfqr4ts3j7wwcve0tvgh4nuhphdkfe2sg7jw4q <617afc05f4e744c61a21bc6b2c2403aae11979cec332f5b117acf970ddb64e55@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
Wes
2026-06-19 16:58:21 -07:00
committed by GitHub
co-authored by npub1v9a0cp05uazvvx3ph34jcfqr4ts3j7wwcve0tvgh4nuhphdkfe2sg7jw4q Pinky
parent c488d8452c
commit 7c3e411fa8
3 changed files with 282 additions and 229 deletions
@@ -1,6 +1,8 @@
export type ObservedUnreadEvent = {
id: string;
createdAt: number;
rootId: string | null;
highPriority: boolean;
};
export function mapsEqual(
@@ -15,72 +17,68 @@ export function mapsEqual(
}
export function recordObservedUnreadEvent(
eventsByChannel: Map<string, Map<string, number>>,
eventsByChannel: Map<string, Map<string, ObservedUnreadEvent>>,
channelId: string,
event: ObservedUnreadEvent,
limit: number,
): void {
): boolean {
let eventsById = eventsByChannel.get(channelId);
if (!eventsById) {
eventsById = new Map<string, number>();
eventsById = new Map<string, ObservedUnreadEvent>();
eventsByChannel.set(channelId, eventsById);
}
if (eventsById.has(event.id)) return;
if (eventsById.has(event.id)) return false;
eventsById.set(event.id, event.createdAt);
if (eventsById.size <= limit) return;
eventsById.set(event.id, event);
if (eventsById.size <= limit) return true;
const oldest = [...eventsById.entries()].sort((a, b) => a[1] - b[1])[0]?.[0];
const oldest = [...eventsById.values()].sort(
(a, b) => a.createdAt - b.createdAt,
)[0]?.id;
if (oldest) {
eventsById.delete(oldest);
}
return true;
}
export function countUnreadObservedEvents(
eventsById: ReadonlyMap<string, number> | undefined,
readAt: number | null,
eventsById: ReadonlyMap<string, ObservedUnreadEvent> | undefined,
getReadAt: (event: ObservedUnreadEvent) => number | null,
): number {
if (!eventsById) return 0;
let count = 0;
for (const createdAt of eventsById.values()) {
if (readAt === null || createdAt > readAt) count += 1;
for (const event of eventsById.values()) {
const readAt = getReadAt(event);
if (readAt === null || event.createdAt > readAt) count += 1;
}
return count;
}
export function buildChannelThreadRoots<
T extends { channelId: string; tags: string[][] },
>(
items: readonly T[],
getRootId: (tags: string[][]) => string | null,
): Map<string, Set<string>> {
const byChannel = new Map<string, Set<string>>();
for (const item of items) {
const rootId = getRootId(item.tags);
if (rootId === null) continue;
let roots = byChannel.get(item.channelId);
if (!roots) {
roots = new Set<string>();
byChannel.set(item.channelId, roots);
}
roots.add(rootId);
export function countUnreadHighPriorityObservedEvents(
eventsById: ReadonlyMap<string, ObservedUnreadEvent> | undefined,
getReadAt: (event: ObservedUnreadEvent) => number | null,
): number {
if (!eventsById) return 0;
let count = 0;
for (const event of eventsById.values()) {
if (!event.highPriority) continue;
const readAt = getReadAt(event);
if (readAt === null || event.createdAt > readAt) count += 1;
}
return byChannel;
return count;
}
export function channelUnreadFrontier(
channelMarker: number | null,
threadRoots: ReadonlySet<string> | undefined,
export function observedUnreadEventReadAt(
event: ObservedUnreadEvent,
channelReadAt: number | null,
getThreadOwnMarker: (rootId: string) => number | null,
): number | null {
let frontier = channelMarker;
if (threadRoots) {
for (const rootId of threadRoots) {
const own = getThreadOwnMarker(rootId);
if (own !== null && (frontier === null || own > frontier)) {
frontier = own;
}
}
if (event.rootId === null) return channelReadAt;
const threadReadAt = getThreadOwnMarker(event.rootId);
if (threadReadAt === null) return channelReadAt;
if (channelReadAt === null || threadReadAt > channelReadAt) {
return threadReadAt;
}
return frontier;
return channelReadAt;
}
@@ -3,10 +3,15 @@ import test from "node:test";
import { computeChannelUnreadMarker } from "../messages/lib/unreadMarker.ts";
import {
buildChannelThreadRoots,
channelUnreadFrontier,
countUnreadHighPriorityObservedEvents,
countUnreadObservedEvents,
observedUnreadEventReadAt,
recordObservedUnreadEvent,
} from "./unreadChannelCounts.ts";
import { resolveChannelReadMarker } from "./useUnreadChannels.ts";
import {
resolveChannelReadMarker,
resolveObservedUnreadRootId,
} from "./useUnreadChannels.ts";
function topLevel(id, createdAt) {
return { id, createdAt, author: "a", time: "", body: "", depth: 0 };
@@ -78,104 +83,190 @@ test("resolveChannelReadMarker_noCallerNoObserved_returnsNull", () => {
assert.equal(result.clearObserved, false);
});
// --- Fix 2: sidebar dot folds per-thread read markers into the channel frontier ---
// --- Fix 2: sidebar badge evaluates each observed event against its own read context ---
function replyItem(channelId, rootId) {
return {
id: `${channelId}:${rootId}:${Math.random()}`,
channelId,
tags: [["e", rootId, "", "root"]],
};
}
// rootId for these fixtures is the "root"-marked e-tag.
const getRootId = (tags) =>
tags.find((t) => t[0] === "e" && t[3] === "root")?.[1] ?? null;
test("buildChannelThreadRoots_groupsRootsByChannel", () => {
const items = [
replyItem("chan-a", "root-1"),
replyItem("chan-a", "root-1"), // dedup within a channel
replyItem("chan-a", "root-2"),
replyItem("chan-b", "root-3"),
];
const map = buildChannelThreadRoots(items, getRootId);
assert.deepEqual([...(map.get("chan-a") ?? [])].sort(), ["root-1", "root-2"]);
assert.deepEqual([...(map.get("chan-b") ?? [])], ["root-3"]);
assert.equal(map.has("chan-c"), false);
});
test("buildChannelThreadRoots_skipsItemsWithNoRoot", () => {
const items = [{ id: "x", channelId: "chan-a", tags: [["p", "someone"]] }];
const map = buildChannelThreadRoots(items, getRootId);
assert.equal(map.size, 0);
});
test("channelUnreadFrontier_unopenedThreadReply_dotPersists", () => {
// Channel's only unread is a thread reply at t=500. The channel marker sits
// at the newest TOP-LEVEL message (t=300, Option-1) and the thread has never
// been opened (own marker null). Folded frontier stays at 300 < 500 → unread.
const channelMarker = 300;
const threadRoots = new Set(["root-1"]);
const getThreadOwnMarker = () => null; // never opened
const frontier = channelUnreadFrontier(
channelMarker,
threadRoots,
getThreadOwnMarker,
);
const latest = 500; // the thread reply timestamp
assert.equal(frontier, 300);
assert.equal(latest > frontier, true); // dot present (Will-accepted)
});
test("channelUnreadFrontier_openedThreadReply_dotClears", () => {
// Same channel, but the user opened the thread: markThreadRead advanced
// thread:root-1's OWN marker to 500 (the reply). Folded frontier rises to
// 500 ≥ latest → dot clears, even though the channel marker is still 300.
const channelMarker = 300;
const threadRoots = new Set(["root-1"]);
const getThreadOwnMarker = (rootId) => (rootId === "root-1" ? 500 : null);
const frontier = channelUnreadFrontier(
channelMarker,
threadRoots,
getThreadOwnMarker,
);
const latest = 500;
assert.equal(frontier, 500);
assert.equal(latest > frontier, false); // dot cleared
});
test("channelUnreadFrontier_noThreadRoots_usesChannelMarker", () => {
// No thread roots observed (or evicted) → channel marker governs unchanged.
test("resolveObservedUnreadRootId_treatsBroadcastReplyAsTopLevelUnread", () => {
assert.equal(
channelUnreadFrontier(300, undefined, () => null),
300,
);
assert.equal(
channelUnreadFrontier(null, undefined, () => null),
resolveObservedUnreadRootId([
["e", "root-1", "", "reply"],
["broadcast", "1"],
]),
null,
);
});
test("channelUnreadFrontier_nullChannelMarker_threadMarkerGoverns", () => {
// Channel never marked read but a thread was opened: the thread's own marker
// becomes the frontier rather than crashing on the null channel marker.
const frontier = channelUnreadFrontier(null, new Set(["root-1"]), () => 500);
assert.equal(frontier, 500);
test("observedUnreadEventReadAt_unopenedThreadReplyUsesChannelMarker", () => {
const event = observed("reply", 500, "root-1");
const readAt = observedUnreadEventReadAt(event, 300, () => null);
assert.equal(readAt, 300);
assert.equal(event.createdAt > readAt, true);
});
test("channelUnreadFrontier_takesMaxAcrossMultipleThreads", () => {
// Two opened threads with different markers → the highest wins.
const frontier = channelUnreadFrontier(
100,
new Set(["root-1", "root-2"]),
(rootId) => (rootId === "root-1" ? 400 : 700),
test("observedUnreadEventReadAt_openedThreadReplyUsesThreadMarker", () => {
const event = observed("reply", 500, "root-1");
const readAt = observedUnreadEventReadAt(event, 300, (rootId) =>
rootId === "root-1" ? 500 : null,
);
assert.equal(frontier, 700);
assert.equal(readAt, 500);
assert.equal(event.createdAt > readAt, false);
});
test("observedUnreadEventReadAt_topLevelUsesChannelMarker", () => {
assert.equal(
observedUnreadEventReadAt(observed("top", 500), 300, () => 900),
300,
);
});
test("observedUnreadEventReadAt_nullChannelMarkerThreadMarkerCanClear", () => {
assert.equal(
observedUnreadEventReadAt(
observed("reply", 500, "root-1"),
null,
() => 500,
),
500,
);
});
// --- Fix 2b: sidebar badge evaluates all observed events, not a single aggregate frontier ---
function observed(id, createdAt, rootId = null, highPriority = false) {
return { id, createdAt, rootId, highPriority };
}
function readAtFor(channelMarker, threadMarkers) {
return (event) =>
observedUnreadEventReadAt(
event,
channelMarker,
(rootId) => threadMarkers.get(rootId) ?? null,
);
}
test("countUnreadObservedEvents_clearsOpenedThreadButKeepsOtherUnreadThread", () => {
// Channel marker is the newest top-level message (300). Two thread replies
// arrived at 400 and 500. Opening root-newer writes thread:root-newer=500,
// but root-older was never opened. The sidebar must stay unread because
// root-older still has a reply newer than its own effective frontier.
const events = new Map([
["older", observed("older", 400, "root-older")],
["newer", observed("newer", 500, "root-newer")],
]);
const getReadAt = readAtFor(300, new Map([["root-newer", 500]]));
assert.equal(countUnreadObservedEvents(events, getReadAt), 1);
});
test("sidebarPipeline_openThreadClearsOnlyUnreadThreadContribution", () => {
const channelId = "chan";
const rootId = "root-1";
const reply = observed("reply", 500, rootId);
const observedByChannel = new Map();
recordObservedUnreadEvent(observedByChannel, channelId, reply, 20);
// Channel-open advances only to the newest top-level message. Before the
// thread is opened, the reply remains newer than the channel frontier, so the
// sidebar badge is present.
const beforeOpenReadAt = readAtFor(300, new Map());
assert.equal(
countUnreadObservedEvents(
observedByChannel.get(channelId),
beforeOpenReadAt,
),
1,
);
// Thread-open writes the thread OWN marker. The sidebar recompute must check
// the observed reply against that thread marker (not just the channel marker),
// which clears the channel count for the reported scenario.
const afterOpenReadAt = readAtFor(300, new Map([[rootId, 500]]));
assert.equal(
countUnreadObservedEvents(
observedByChannel.get(channelId),
afterOpenReadAt,
),
0,
);
});
test("latestObservedEvent_latestThreadReadDoesNotImplyChannelClear", () => {
const events = new Map([
["older", observed("older", 400, "root-older")],
["newer", observed("newer", 500, "root-newer")],
]);
const getReadAt = readAtFor(300, new Map([["root-newer", 500]]));
// This reproduces the bug in the rejected aggregate-frontier model:
// checking only the latest event would clear the whole channel after reading
// root-newer, even though root-older remains unread.
const latestOnly = new Map([["newer", events.get("newer")]]);
assert.equal(countUnreadObservedEvents(latestOnly, getReadAt), 0);
assert.equal(countUnreadObservedEvents(events, getReadAt), 1);
});
test("countUnreadObservedEvents_topLevelUsesChannelMarker", () => {
const events = new Map([
["top-old", observed("top-old", 250)],
["top-new", observed("top-new", 350)],
]);
assert.equal(countUnreadObservedEvents(events, readAtFor(300, new Map())), 1);
});
test("recordObservedUnreadEvent_reportsOutOfOrderInsertForInvalidation", () => {
const channelId = "chan";
const observedByChannel = new Map();
assert.equal(
recordObservedUnreadEvent(
observedByChannel,
channelId,
observed("latest", 500, "root-latest"),
20,
),
true,
);
assert.equal(
recordObservedUnreadEvent(
observedByChannel,
channelId,
observed("older", 400, "root-older"),
20,
),
true,
);
assert.equal(
recordObservedUnreadEvent(
observedByChannel,
channelId,
observed("older", 400, "root-older"),
20,
),
false,
);
assert.equal(observedByChannel.get(channelId).size, 2);
});
test("highPriorityObservedEvents_countOnlyUnreadHighPriorityItems", () => {
const events = new Map([
["mention-read", observed("mention-read", 500, "root-read", true)],
["normal-unread", observed("normal-unread", 600, "root-unread", false)],
["mention-unread", observed("mention-unread", 700, "root-hot", true)],
]);
const getReadAt = readAtFor(
300,
new Map([
["root-read", 500],
["root-unread", 300],
["root-hot", 300],
]),
);
assert.equal(countUnreadObservedEvents(events, getReadAt), 2);
assert.equal(countUnreadHighPriorityObservedEvents(events, getReadAt), 1);
});
@@ -5,10 +5,10 @@ import {
type UseLiveChannelUpdatesOptions,
} from "@/features/channels/useLiveChannelUpdates";
import {
buildChannelThreadRoots,
channelUnreadFrontier,
countUnreadHighPriorityObservedEvents,
countUnreadObservedEvents,
mapsEqual,
observedUnreadEventReadAt,
recordObservedUnreadEvent,
type ObservedUnreadEvent,
} from "@/features/channels/unreadChannelCounts";
@@ -173,6 +173,10 @@ export function resolveChannelReadMarker(
};
}
export function resolveObservedUnreadRootId(tags: string[][]): string | null {
return isBroadcastReply(tags) ? null : getThreadReference(tags).rootId;
}
function setsEqual(a: ReadonlySet<string>, b: ReadonlySet<string>): boolean {
if (a.size !== b.size) return false;
for (const item of a) {
@@ -214,10 +218,7 @@ export function useUnreadChannels(
// ignored by the memo (it iterates the current channels list, not the map).
const latestByChannelRef = React.useRef(new Map<string, number>());
const observedUnreadEventsByChannelRef = React.useRef(
new Map<string, Map<string, number>>(),
);
const latestHighPriorityByChannelRef = React.useRef(
new Map<string, number>(),
new Map<string, Map<string, ObservedUnreadEvent>>(),
);
const channelsRef = React.useRef(channels);
@@ -295,7 +296,6 @@ export function useUnreadChannels(
React.useEffect(() => {
latestByChannelRef.current = new Map();
observedUnreadEventsByChannelRef.current = new Map();
latestHighPriorityByChannelRef.current = new Map();
forcedUnreadRef.current = new Set();
caughtUpChannelsRef.current = new Set();
participatedRootIdsRef.current = pubkey
@@ -349,7 +349,6 @@ export function useUnreadChannels(
if (clearObserved) {
latestByChannelRef.current.delete(channelId);
observedUnreadEventsByChannelRef.current.delete(channelId);
latestHighPriorityByChannelRef.current.delete(channelId);
bumpLatestVersion();
}
},
@@ -395,26 +394,34 @@ export function useUnreadChannels(
// external trigger message this client has observed."
const callerOnChannelMessage = liveUpdateOptions.onChannelMessage;
const recordUnreadEvent = React.useCallback(
(channelId: string, event: ObservedUnreadEvent) => {
(channelId: string, event: ObservedUnreadEvent) =>
recordObservedUnreadEvent(
observedUnreadEventsByChannelRef.current,
channelId,
event,
CATCH_UP_LIMIT,
);
},
),
[],
);
const handleChannelMessage = React.useCallback(
(channelId: string, event: RelayEvent) => {
recordUnreadEvent(channelId, {
const channel = channelsRef.current.find((ch) => ch.id === channelId);
const isHighPriority =
channel?.channelType === "dm" ||
(normalizedPubkey !== null &&
isHighPriorityEventForUser(event, normalizedPubkey));
const didRecordUnreadEvent = recordUnreadEvent(channelId, {
id: event.id,
createdAt: event.created_at,
rootId: resolveObservedUnreadRootId(event.tags),
highPriority: isHighPriority,
});
const current = latestByChannelRef.current.get(channelId) ?? 0;
if (event.created_at > current) {
latestByChannelRef.current.set(channelId, event.created_at);
bumpLatestVersion();
} else if (didRecordUnreadEvent) {
bumpLatestVersion();
}
// A mention on a reply makes its thread badge-eligible even when the
@@ -423,22 +430,11 @@ export function useUnreadChannels(
bumpMembershipVersion();
}
// Track high-priority events (DMs, mentions, broadcasts) separately.
const channel = channelsRef.current.find((ch) => ch.id === channelId);
if (
channel?.channelType === "dm" ||
(normalizedPubkey !== null &&
isHighPriorityEventForUser(event, normalizedPubkey))
) {
const currentHigh =
latestHighPriorityByChannelRef.current.get(channelId) ?? 0;
if (event.created_at > currentHigh) {
latestHighPriorityByChannelRef.current.set(
channelId,
event.created_at,
);
bumpLatestVersion();
}
// A high-priority event can be older than the channel's latest observed
// normal unread, so it may not advance latestByChannelRef. Still bump so
// highPriorityUnreadChannelIds re-reads the per-event priority flag.
if (isHighPriority) {
bumpLatestVersion();
}
callerOnChannelMessage?.(channelId, event);
@@ -592,7 +588,6 @@ export function useUnreadChannels(
channelId: string;
ok: true;
maxExternal: number;
maxHighPriority: number;
unreadEvents: ObservedUnreadEvent[];
threadReplies: ThreadActivityItem[];
}
@@ -644,7 +639,6 @@ export function useUnreadChannels(
// Pass 2: compute maxExternal and collect thread reply activity,
// applying the notification filter to both.
let maxExternal = 0;
let maxHighPriority = 0;
const unreadEvents: ObservedUnreadEvent[] = [];
const threadReplies: ThreadActivityItem[] = [];
const ch = channels.find((c) => c.id === channelId);
@@ -672,20 +666,20 @@ export function useUnreadChannels(
) {
continue;
}
const evtRef = getThreadReference(event.tags);
if (event.created_at > maxExternal) {
maxExternal = event.created_at;
}
unreadEvents.push({ id: event.id, createdAt: event.created_at });
if (
const isHighPriority =
chType === "dm" ||
(normalizedPubkey !== null &&
isHighPriorityEventForUser(event, normalizedPubkey))
) {
if (event.created_at > maxHighPriority) {
maxHighPriority = event.created_at;
}
}
const evtRef = getThreadReference(event.tags);
isHighPriorityEventForUser(event, normalizedPubkey));
unreadEvents.push({
id: event.id,
createdAt: event.created_at,
rootId: resolveObservedUnreadRootId(event.tags),
highPriority: isHighPriority,
});
if (evtRef.parentId !== null && !isBroadcastReply(event.tags)) {
threadReplies.push({
id: event.id,
@@ -704,7 +698,6 @@ export function useUnreadChannels(
channelId,
ok: true,
maxExternal,
maxHighPriority,
unreadEvents,
threadReplies,
};
@@ -724,13 +717,7 @@ export function useUnreadChannels(
caughtUpChannelsRef.current.delete(result.channelId);
continue;
}
const {
channelId,
maxExternal,
maxHighPriority,
unreadEvents,
threadReplies,
} = result;
const { channelId, maxExternal, unreadEvents, threadReplies } = result;
allThreadReplies.push(...threadReplies);
if (unreadEvents.length > 0) {
for (const event of unreadEvents) {
@@ -748,20 +735,6 @@ export function useUnreadChannels(
}
}
}
if (maxHighPriority > 0) {
const readAtNow = getEffectiveTimestamp(channelId) ?? 0;
if (maxHighPriority > readAtNow) {
const currentHigh =
latestHighPriorityByChannelRef.current.get(channelId) ?? 0;
if (maxHighPriority > currentHigh) {
latestHighPriorityByChannelRef.current.set(
channelId,
maxHighPriority,
);
didAdvance = true;
}
}
}
}
if (allThreadReplies.length > 0) {
const existingIds = new Set(threadActivityRef.current.map((e) => e.id));
@@ -831,16 +804,6 @@ export function useUnreadChannels(
const highPriority = new Set<string>();
const counts = new Map<string, number>();
// Map each channel to the thread roots observed in it, so a channel's
// frontier can fold in per-thread read markers (Option A): opening a
// thread advances thread:<root> and must clear the channel dot even
// though markChannelRead only advances the channel marker to the newest
// TOP-LEVEL message.
const threadRootsByChannel = buildChannelThreadRoots(
threadActivityRef.current,
(tags) => getThreadReference(tags).rootId,
);
for (const channel of channels) {
if (channel.id === activeChannelId) continue;
@@ -851,37 +814,38 @@ export function useUnreadChannels(
continue;
}
const latest = latestByChannelRef.current.get(channel.id);
if (latest === undefined) continue;
if (latestByChannelRef.current.get(channel.id) === undefined) continue;
const readAt = channelUnreadFrontier(
getEffectiveTimestamp(channel.id),
threadRootsByChannel.get(channel.id),
(rootId) => getOwnTimestamp(`thread:${rootId}`),
);
if (readAt !== null && latest <= readAt) continue;
unread.add(channel.id);
const observedEvents = observedUnreadEventsByChannelRef.current.get(
channel.id,
);
const unreadCount = countUnreadObservedEvents(observedEvents, readAt);
counts.set(channel.id, Math.max(unreadCount, 1));
const channelReadAt = getEffectiveTimestamp(channel.id);
const readAtForObservedEvent = (event: ObservedUnreadEvent) =>
observedUnreadEventReadAt(event, channelReadAt, (rootId) =>
getOwnTimestamp(`thread:${rootId}`),
);
const unreadCount = countUnreadObservedEvents(
observedEvents,
readAtForObservedEvent,
);
if (unreadCount === 0) continue;
unread.add(channel.id);
counts.set(channel.id, unreadCount);
// DM channels: any unread DM is high-priority.
if (channel.channelType === "dm") {
highPriority.add(channel.id);
} else {
// Non-DM: high-priority only if there's a mention/broadcast newer than read marker.
const latestHigh = latestHighPriorityByChannelRef.current.get(
channel.id,
);
if (
latestHigh !== undefined &&
(readAt === null || latestHigh > readAt)
) {
highPriority.add(channel.id);
}
} else if (
countUnreadHighPriorityObservedEvents(
observedEvents,
readAtForObservedEvent,
) > 0
) {
// Non-DM: high-priority only if at least one mention/broadcast
// remains unread in its own channel/thread context.
highPriority.add(channel.id);
}
}
@@ -946,7 +910,7 @@ export function useUnreadChannels(
markContextRead(channelId, unixSeconds);
}
latestByChannelRef.current.delete(channelId);
latestHighPriorityByChannelRef.current.delete(channelId);
observedUnreadEventsByChannelRef.current.delete(channelId);
}
bumpLatestVersion();
}, [getEffectiveTimestamp, markContextRead]);