From 30702135c7f1f0ec771ebec607e3d75c5f1bec71 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Sat, 20 Jun 2026 15:42:40 -0400 Subject: [PATCH] test(channels): demonstrate thread-unread badge Case 1 + Case 3 defects Investigation guards for the LP4 thread-unread badge flakiness. Each new test passes against current behavior to pin the exact trigger, labeled DEFECT (documents the bug) or DESIRED/control. Case 1 (badge at level N, not root): a deep reply whose intermediate ancestor is absent from the loaded timeline is keyed under that absent parent, so collectSubtreeReplies never reaches it from the root bucket. The root undercounts or shows no badge while the reply is genuinely unread. Case 3 (no badge anywhere): seedThreadBadgeFrontiers seeds via the effective thread marker max(thread_own, channel_marker); once channel-open mark-read folds the channel marker past the unread reply, the seed lands past it and computeThreadBadgeCounts yields zero. The pre-mark-read marker control keeps the badge. These are demonstration tests only; no fix code changes. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../channels/lib/threadBadgeCounts.test.mjs | 57 +++++++++++++ .../channels/lib/threadBadgeFrontier.test.mjs | 83 +++++++++++++++++++ 2 files changed, 140 insertions(+) diff --git a/desktop/src/features/channels/lib/threadBadgeCounts.test.mjs b/desktop/src/features/channels/lib/threadBadgeCounts.test.mjs index 5214ab781b..cfd09e20f1 100644 --- a/desktop/src/features/channels/lib/threadBadgeCounts.test.mjs +++ b/desktop/src/features/channels/lib/threadBadgeCounts.test.mjs @@ -113,3 +113,60 @@ test("computeThreadBadgeCounts_multipleRoots_eachCountsOwnSubtree", () => { assert.equal(result.get("root1"), 2); assert.equal(result.get("root2"), 1); }); + +// --- LP4 Case 1 demonstration: orphaned subtree from a broken parent chain --- +// +// The roll-up keys each reply under its immediate `parentId` and walks the +// adjacency map down from true roots (collectSubtreeReplies). A descendant is +// only reached if its FULL parent chain is present in the loaded timeline. +// Pagination / load windows can drop an intermediate ancestor, which severs the +// chain: the deep reply still sits under its (absent) parent's key, but that key +// is never visited from any root's bucket, so it is never tallied at the root. +// +// These two tests pin the exact trigger — a missing middle ancestor — and pass +// against TODAY'S behavior. The first DOCUMENTS THE DEFECT (root undercounts / +// shows no badge); the second is the contrasting full-chain control. + +test("computeThreadBadgeCounts_brokenParentChain_orphanedReplyMissesRoot_DEFECT", () => { + // Full thread is root -> a -> b -> c, but intermediate ancestor `b` is NOT in + // the loaded array (unloaded by the timeline window). `c` is genuinely unread. + // DEFECT: `c` is keyed under "b", and "b" is never reached from root's bucket + // (root -> [a], a -> [] because b is absent), so `c` is orphaned. The root + // badge counts only `a` (1), NOT the 2 it would show with the chain intact. + // With the bug, opening the thread shows `c` unread at its level while the + // channel-root summary undercounts it. + const loaded = [ + msg("root", null), + msg("a", "root"), + // msg("b", "a") — intentionally absent: unloaded intermediate ancestor. + msg("c", "b"), + ]; + assert.equal(counts(loaded, undefined).get("root"), 1); +}); + +test("computeThreadBadgeCounts_brokenParentChain_orphanedSoleReply_noBadge_DEFECT", () => { + // Sharper form: root's ONLY unread content is the deep reply `c`, and its + // intermediate ancestor `b` is unloaded. DEFECT: root shows NO badge at all + // (count absent) even though `c` is genuinely unread, because the orphaned + // `c` is unreachable from root and root then has zero tallied replies. + const loaded = [ + msg("root", null), + // msg("b", "root") — intentionally absent: unloaded intermediate ancestor. + msg("c", "b"), + ]; + assert.equal(counts(loaded, undefined).has("root"), false); +}); + +test("computeThreadBadgeCounts_fullParentChain_orphanRollsUp_DESIRED", () => { + // Control: the SAME thread with the intermediate ancestor `b` present. The + // chain root -> a -> b -> c is intact, so every descendant rolls up and the + // root badge correctly counts 3. This is the behavior the broken-chain cases + // above SHOULD produce once the deep reply's ancestors are guaranteed loaded. + const loaded = [ + msg("root", null), + msg("a", "root"), + msg("b", "a"), + msg("c", "b"), + ]; + assert.equal(counts(loaded, undefined).get("root"), 3); +}); diff --git a/desktop/src/features/channels/lib/threadBadgeFrontier.test.mjs b/desktop/src/features/channels/lib/threadBadgeFrontier.test.mjs index 89817f2c88..3cf0733877 100644 --- a/desktop/src/features/channels/lib/threadBadgeFrontier.test.mjs +++ b/desktop/src/features/channels/lib/threadBadgeFrontier.test.mjs @@ -4,6 +4,7 @@ import test from "node:test"; import { nextThreadBadgeFrontier } from "./threadBadgeFrontier.ts"; import { seedThreadBadgeFrontiers } from "./threadBadgeFrontier.ts"; import { buildDirectRepliesByParentId } from "./subtreeCreatedAt.ts"; +import { computeThreadBadgeCounts } from "./threadBadgeCounts.ts"; const msg = (id, parentId) => ({ id, parentId }); const seedAll = () => true; @@ -96,3 +97,85 @@ test("seedThreadBadgeFrontiers_reseed_advancesMonotonically", () => { seed(frontiers, messages, seedAll, () => 100); assert.equal(frontiers.get("root"), 250); }); + +// --- LP4 Case 3 demonstration: seed-vs-mark-read race poisons the frontier --- +// +// seedThreadBadgeFrontiers seeds each root via getReadAt(root), which resolves +// to the EFFECTIVE thread marker = max(thread_own_marker, channel_marker). +// On channel open, markChannelRead advances the channel marker to the newest +// top-level message. If that fold lands before (or in) the render where a root +// is first seeded, the seed adopts a frontier PAST the unread reply, and +// computeThreadBadgeCounts then reads zero unread — the badge vanishes +// everywhere. What the seed READS (folded vs. pre-mark-read marker) is the sole +// determinant; the seed/compute mechanics are otherwise identical. +// +// These tests drive seed -> compute end-to-end and pass against TODAY's code. +// The first DOCUMENTS THE DEFECT (folded marker -> no badge); the second is the +// pre-mark-read control (own marker -> badge survives). + +// Richer message shape than the file-level `msg`: computeThreadBadgeCounts reads +// createdAt and pubkey, which the frontier-only helper omits. +const reply = (id, parentId, createdAt) => ({ + id, + parentId, + createdAt, + pubkey: "author", +}); + +test("seedThreadBadgeFrontiers_channelMarkerFoldedIntoSeed_badgeVanishes_DEFECT", () => { + // Thread "root" has one unread reply at createdAt 200. The thread's OWN read + // marker is 100 (reply is genuinely unread). But channel-open mark-read has + // already advanced the channel marker to 250, so the EFFECTIVE marker + // getReadAt returns is max(100, 250) = 250. + const messages = [reply("root", null, 50), reply("r1", "root", 200)]; + const directReplies = buildDirectRepliesByParentId(messages); + const foldedEffectiveMarker = Math.max(100, 250); // thread_own vs channel + + const frontiers = new Map(); + seedThreadBadgeFrontiers( + frontiers, + messages, + directReplies, + seedAll, + () => foldedEffectiveMarker, + ); + // DEFECT: frontier seeded to 250, past the unread reply at 200. + assert.equal(frontiers.get("root"), 250); + + const result = computeThreadBadgeCounts( + messages, + directReplies, + frontiers, + seedAll, + ); + // DEFECT: badge vanishes — no count anywhere despite a genuinely unread reply. + assert.equal(result.has("root"), false); +}); + +test("seedThreadBadgeFrontiers_preMarkReadMarkerSeeded_badgeSurvives_DESIRED", () => { + // Identical thread, but the seed reads the PRE-mark-read marker: the thread's + // own marker (100), captured before the channel-open fold advanced it to 250. + const messages = [reply("root", null, 50), reply("r1", "root", 200)]; + const directReplies = buildDirectRepliesByParentId(messages); + const preMarkReadMarker = 100; // thread_own only, channel fold not applied + + const frontiers = new Map(); + seedThreadBadgeFrontiers( + frontiers, + messages, + directReplies, + seedAll, + () => preMarkReadMarker, + ); + // Frontier seeded to 100, behind the unread reply at 200. + assert.equal(frontiers.get("root"), 100); + + const result = computeThreadBadgeCounts( + messages, + directReplies, + frontiers, + seedAll, + ); + // DESIRED: badge survives — the reply at 200 is correctly counted unread. + assert.equal(result.get("root"), 1); +});