Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions desktop/src/features/channels/unreadChannelCounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
109 changes: 109 additions & 0 deletions desktop/src/features/channels/unreadReadMarker.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
countUnreadHighPriorityObservedEvents,
countUnreadObservedEvents,
observedUnreadEventReadAt,
pruneCoveredObservedRefs,
recordObservedUnreadEvent,
} from "./unreadChannelCounts.ts";
import {
Expand Down Expand Up @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions desktop/src/features/channels/useUnreadChannels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
makeObservedUnreadEvent,
mapsEqual,
observedUnreadEventReadAt,
pruneCoveredObservedRefs,
recordObservedUnreadEvent,
type ObservedUnreadEvent,
} from "@/features/channels/unreadChannelCounts";
Expand Down Expand Up @@ -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);
Expand Down
Loading