diff --git a/desktop/src/features/channels/unreadChannelCounts.ts b/desktop/src/features/channels/unreadChannelCounts.ts index cb33c6297..ff7bf2203 100644 --- a/desktop/src/features/channels/unreadChannelCounts.ts +++ b/desktop/src/features/channels/unreadChannelCounts.ts @@ -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:`/ +// `msg:` 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, + observedByChannel: Map>, + channelIds: Iterable, + unreadChannelIds: ReadonlySet, + forcedChannelIds: ReadonlySet, + 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 | undefined, getReadAt: (event: ObservedUnreadEvent) => number | null, diff --git a/desktop/src/features/channels/unreadReadMarker.test.mjs b/desktop/src/features/channels/unreadReadMarker.test.mjs index 266bca2df..1cf6b90fe 100644 --- a/desktop/src/features/channels/unreadReadMarker.test.mjs +++ b/desktop/src/features/channels/unreadReadMarker.test.mjs @@ -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: so +// the badge counts 0, but markChannelRead("thread:") 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; diff --git a/desktop/src/features/channels/useUnreadChannels.ts b/desktop/src/features/channels/useUnreadChannels.ts index f65f4bd82..f80e363be 100644 --- a/desktop/src/features/channels/useUnreadChannels.ts +++ b/desktop/src/features/channels/useUnreadChannels.ts @@ -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);