fix(desktop): prune real-channel observed refs on thread/message read

markThreadRead/markMessageRead route through markChannelRead with a
synthetic thread:<root>/msg:<id> key, so its clearObserved prune looks up
latestByChannelRef under that key — never the real channel — and the real
channel's observed refs survive a read that covers their last badge event.
The count stays correct (markers are evaluated per event), but the channel
stays permanently eligible for recount, a latent liability. A new lazy
prune effect, driven off the unread memo's own output, drops those dead
refs when the channel is no longer counted unread.

Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
2026-06-24 19:05:39 -04:00
parent 08d375aa2f
commit 1df6744b72
3 changed files with 151 additions and 0 deletions
@@ -65,6 +65,32 @@ export function recordObservedUnreadEvent(
return true;
}
// Drop observed refs for channels whose unread events the read markers now
// fully cover. `markThreadRead`/`markMessageRead` advance a `thread:<root>`/
// `msg:<id>` marker, but `markChannelRead`'s clearObserved prune keys these maps
// by that synthetic key — never the real channel — so the real channel's refs
// linger after a thread/message read covers its last badge event. A channel
// absent from `unreadChannelIds` (and not forced or active) has no unread
// observed events left, so its refs are dead weight. Pruning them is invisible
// to the count (a covered channel and an absent one both contribute 0).
export function pruneCoveredObservedRefs(
latestByChannel: Map<string, number>,
observedByChannel: Map<string, Map<string, ObservedUnreadEvent>>,
channelIds: Iterable<string>,
unreadChannelIds: ReadonlySet<string>,
forcedChannelIds: ReadonlySet<string>,
activeChannelId: string | null,
): void {
for (const channelId of channelIds) {
if (channelId === activeChannelId) continue;
if (unreadChannelIds.has(channelId)) continue;
if (forcedChannelIds.has(channelId)) continue;
if (latestByChannel.get(channelId) === undefined) continue;
latestByChannel.delete(channelId);
observedByChannel.delete(channelId);
}
}
export function countUnreadObservedEvents(
eventsById: ReadonlyMap<string, ObservedUnreadEvent> | undefined,
getReadAt: (event: ObservedUnreadEvent) => number | null,
@@ -8,6 +8,7 @@ import {
countUnreadHighPriorityObservedEvents,
countUnreadObservedEvents,
observedUnreadEventReadAt,
pruneCoveredObservedRefs,
recordObservedUnreadEvent,
} from "./unreadChannelCounts.ts";
import {
@@ -388,6 +389,114 @@ test("highPriorityObservedEvents_countOnlyUnreadHighPriorityItems", () => {
assert.equal(countUnreadHighPriorityObservedEvents(events, getReadAt), 1);
});
// --- Clearing asymmetry: prune covered observed refs ---
// The headline regression. A channel was read top-level-only (channel-open) and
// later a thread reply arrived; reading THAT thread advances thread:<root> so
// the badge counts 0, but markChannelRead("thread:<root>") can't prune the real
// channel's refs (wrong key). pruneCoveredObservedRefs drops the dead refs once
// the memo no longer reports the channel unread, and the badge must STAY 0 even
// if a catch-up REQ re-records the same already-covered event.
test("pruneCoveredObservedRefs_threadReadCovers_prunesRefsAndBadgeStaysZero", () => {
const channelId = "chan";
const rootId = "root-1";
const reply = observed("reply", 500, rootId);
const latestByChannel = new Map([[channelId, 500]]);
const observedByChannel = new Map();
recordObservedUnreadEvent(observedByChannel, channelId, reply, 20);
// Before reading the thread: channel marker 300 < reply 500, badge counts 1.
const beforeRead = readAtFor(300, new Map());
assert.equal(
countUnreadBadgeObservedEvents(
observedByChannel.get(channelId),
beforeRead,
),
1,
);
// Reading the thread advances thread:root-1=500. The badge now counts 0, so
// the memo drops the channel from unreadChannelIds — the prune signal.
const afterRead = readAtFor(300, new Map([[rootId, 500]]));
assert.equal(
countUnreadBadgeObservedEvents(observedByChannel.get(channelId), afterRead),
0,
);
pruneCoveredObservedRefs(
latestByChannel,
observedByChannel,
[channelId],
new Set(),
new Set(),
null,
);
assert.equal(latestByChannel.has(channelId), false);
assert.equal(observedByChannel.has(channelId), false);
// A catch-up REQ re-records the same covered event. The badge must stay 0.
recordObservedUnreadEvent(observedByChannel, channelId, reply, 20);
assert.equal(
countUnreadBadgeObservedEvents(observedByChannel.get(channelId), afterRead),
0,
);
});
test("pruneCoveredObservedRefs_channelStillUnread_keepsRefs", () => {
const channelId = "chan";
const latestByChannel = new Map([[channelId, 500]]);
const observedByChannel = new Map([[channelId, new Map()]]);
pruneCoveredObservedRefs(
latestByChannel,
observedByChannel,
[channelId],
new Set([channelId]),
new Set(),
null,
);
assert.equal(latestByChannel.has(channelId), true);
assert.equal(observedByChannel.has(channelId), true);
});
test("pruneCoveredObservedRefs_activeChannel_keepsRefs", () => {
const channelId = "chan";
const latestByChannel = new Map([[channelId, 500]]);
const observedByChannel = new Map([[channelId, new Map()]]);
pruneCoveredObservedRefs(
latestByChannel,
observedByChannel,
[channelId],
new Set(),
new Set(),
channelId,
);
assert.equal(latestByChannel.has(channelId), true);
assert.equal(observedByChannel.has(channelId), true);
});
test("pruneCoveredObservedRefs_forcedUnread_keepsRefs", () => {
const channelId = "chan";
const latestByChannel = new Map([[channelId, 500]]);
const observedByChannel = new Map([[channelId, new Map()]]);
pruneCoveredObservedRefs(
latestByChannel,
observedByChannel,
[channelId],
new Set(),
new Set([channelId]),
null,
);
assert.equal(latestByChannel.has(channelId), true);
assert.equal(observedByChannel.has(channelId), true);
});
test("addThreadActivityItems keeps newest items when input is newest-first", () => {
const newestFirst = Array.from({ length: 101 }, (_, index) => {
const createdAt = 100 - index;
@@ -12,6 +12,7 @@ import {
makeObservedUnreadEvent,
mapsEqual,
observedUnreadEventReadAt,
pruneCoveredObservedRefs,
recordObservedUnreadEvent,
type ObservedUnreadEvent,
} from "@/features/channels/unreadChannelCounts";
@@ -918,6 +919,21 @@ export function useUnreadChannels(
const unreadChannelIdsRef = React.useRef(unreadChannelIds);
unreadChannelIdsRef.current = unreadChannelIds;
// Drop observed refs the read markers now fully cover (the thread/message
// read clearing asymmetry — see pruneCoveredObservedRefs). Driven off the
// memo's own output; no version bump and no loop because pruning is invisible
// to the count.
React.useEffect(() => {
pruneCoveredObservedRefs(
latestByChannelRef.current,
observedUnreadEventsByChannelRef.current,
channels.map((channel) => channel.id),
unreadChannelIds,
forcedUnreadRef.current,
activeChannelId,
);
}, [unreadChannelIds, channels, activeChannelId]);
const markAllChannelsRead = React.useCallback(() => {
for (const channelId of unreadChannelIdsRef.current) {
forcedUnreadRef.current.delete(channelId);