fix(channels): re-key thread-unread badge roll-up, seed, and ceiling on rootId

Thread-unread badges relied on the direct-parent adjacency map to roll up
replies under their root. When the loaded timeline window dropped a middle
ancestor, a deep reply keyed under its absent parent and was unreachable
from the root, so the channel-root badge under-counted (Case 1) and, when a
root's only reply was such an orphan, the root was never seed-eligible and
got no frontier at all (Case 3, seed face).

Group replies by their resolved rootId instead. The root e-tag travels with
every event (getThreadReference), so a reply reaches its true root even with
the intermediate ancestor unloaded. For an intact chain every descendant
carries the root's rootId, so counts are identical to the old walk. The
thread-open mark-read ceiling gets a root-scoped variant that folds in
rootId-matched orphans; the expand caller keeps its branch-scoped walk, since
a branch node owns no rootId bucket and must advance only its own branch.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
2026-06-22 15:17:17 -04:00
parent 59e584e076
commit 87b79aff9b
9 changed files with 205 additions and 164 deletions

View File

@@ -9,6 +9,7 @@ export function subtreeMaxCreatedAt(
messageId: string,
directReplyIdsByParentId: ReadonlyMap<string, string[]>,
createdAtByMessageId: ReadonlyMap<string, number>,
repliesByRootId?: ReadonlyMap<string, readonly ReplyGraphMessage[]>,
): number | null {
const ownCreatedAt = createdAtByMessageId.get(messageId);
if (ownCreatedAt === undefined) return null;
@@ -24,6 +25,18 @@ export function subtreeMaxCreatedAt(
}
pendingIds.push(...(directReplyIdsByParentId.get(currentId) ?? []));
}
// Orphan-immune ceiling: also fold in replies that resolve to this id by
// rootId. When the timeline window drops a middle ancestor, a deep reply
// keys under its absent parent and the adjacency walk above can't reach it,
// so the root-started ceiling stops short and the channel-root badge can
// never clear. rootId travels with the event (getThreadReference), so a root
// reaches its severed orphans here. A BRANCH node is no reply's rootId, so
// its rootId-bucket is empty and the branch-scoped ceiling is unchanged.
for (const reply of repliesByRootId?.get(messageId) ?? []) {
if (reply.createdAt > maxCreatedAt) {
maxCreatedAt = reply.createdAt;
}
}
return maxCreatedAt;
}
@@ -31,6 +44,7 @@ export function subtreeMaxCreatedAt(
interface ReplyGraphMessage {
id: string;
parentId?: string | null;
rootId?: string | null;
createdAt: number;
}
@@ -66,6 +80,29 @@ export function buildDirectRepliesByParentId<T extends ReplyGraphMessage>(
return map;
}
/**
* Maps each thread root id to every reply that resolves to it by `rootId`,
* in timeline order. Unlike the parent-keyed maps above, this groups by the
* reply's own `rootId` (getThreadReference: the `root` e-tag that travels with
* the event), so a deep reply lands under its true root even when an
* intermediate ancestor is absent from the loaded window. Root-keyed badge
* consumers use this to roll up severed orphans the parent-chain walk misses.
* Top-level messages (no rootId) and self-referential roots are excluded.
*/
export function buildRepliesByRootId<T extends ReplyGraphMessage>(
messages: readonly T[],
): Map<string, T[]> {
const map = new Map<string, T[]>();
for (const message of messages) {
const rootId = message.rootId;
if (!rootId || rootId === message.id) continue;
const currentReplies = map.get(rootId) ?? [];
currentReplies.push(message);
map.set(rootId, currentReplies);
}
return map;
}
/** Maps each message id to its `createdAt`. */
export function buildCreatedAtByMessageId(
messages: readonly ReplyGraphMessage[],

View File

@@ -5,8 +5,8 @@ import { computeThreadBadgeCounts } from "./threadBadgeCounts.ts";
import { nextThreadBadgeFrontier } from "./threadBadgeFrontier.ts";
import {
buildCreatedAtByMessageId,
buildDirectRepliesByParentId,
buildDirectReplyIdsByParentId,
buildRepliesByRootId,
subtreeMaxCreatedAt,
} from "./subtreeCreatedAt.ts";
@@ -21,9 +21,13 @@ import {
// (subtreeMaxCreatedAt); these tests pin that the badge collapses to 0 on open
// whether or not the OWN marker actually advances.
// rootId is "root" on every reply: these threads are all rooted at "root", and
// the badge roll-up groups by rootId (getThreadReference's `root` e-tag), so the
// nested b under a still tallies at root. Top-level "root" carries its own id.
const msg = (id, parentId, createdAt, pubkey = "author") => ({
id,
parentId,
rootId: parentId === null ? id : "root",
createdAt,
pubkey,
});
@@ -53,7 +57,7 @@ const badgeAfterOpen = (rootId, messages, priorOwnMarker, currentPubkey) => {
const frontier = nextThreadBadgeFrontier(undefined, liveMarker);
return computeThreadBadgeCounts(
messages,
buildDirectRepliesByParentId(messages),
buildRepliesByRootId(messages),
new Map([[rootId, frontier]]),
() => true,
currentPubkey,
@@ -102,7 +106,7 @@ test("openThreadWithUnreadNestedReply_oldDirectCeilingLeftBadgeLit", () => {
const frontier = nextThreadBadgeFrontier(undefined, oldDirectCeiling);
const count = computeThreadBadgeCounts(
messages,
buildDirectRepliesByParentId(messages),
buildRepliesByRootId(messages),
new Map([["root", frontier]]),
() => true,
).get("root");
@@ -135,7 +139,7 @@ test("openThreadWhereOnlyUnreadIsOwnReply_neverShowsBadge", () => {
// Frontier below every reply (never read) — only "other"'s reply a counts.
const beforeOpen = computeThreadBadgeCounts(
messages,
buildDirectRepliesByParentId(messages),
buildRepliesByRootId(messages),
new Map([["root", null]]),
() => true,
"me",
@@ -154,7 +158,7 @@ test("openThreadWhereEveryUnreadIsOwnReply_inertNoBadgeEver", () => {
];
const before = computeThreadBadgeCounts(
messages,
buildDirectRepliesByParentId(messages),
buildRepliesByRootId(messages),
new Map([["root", null]]),
() => true,
"me",

View File

@@ -2,15 +2,17 @@ import assert from "node:assert/strict";
import test from "node:test";
import { computeThreadBadgeCounts } from "./threadBadgeCounts.ts";
import { buildDirectRepliesByParentId } from "./subtreeCreatedAt.ts";
import { buildRepliesByRootId } from "./subtreeCreatedAt.ts";
// Minimal TimelineMessage shape the badge counter reads: id, parentId, rootId,
// createdAt, pubkey. createdAt defaults high so replies count unread against a
// null frontier unless a test sets it lower. `rootId` defaults to the parent
// (mirroring getThreadReference's `rootTag?.[1] ?? parentId` fallback) so the
// broken-chain fixtures can pin the orphan-immune root explicitly: the orphan
// carries its true rootId even when the middle ancestor is unloaded, which is
// what keeps these tests falsifiable once the roll-up re-keys on rootId.
// (mirroring getThreadReference's `rootTag?.[1] ?? parentId` fallback), which is
// correct for a DIRECT reply (parent IS the root); nested replies must pass
// their true thread root explicitly, exactly as getThreadReference resolves the
// `root` e-tag that travels with every event regardless of which ancestors are
// loaded. The roll-up groups by that rootId, so a severed orphan still tallies
// at its true root.
const msg = (id, parentId, createdAt = 100, pubkey = "author", rootId) => ({
id,
parentId,
@@ -23,7 +25,7 @@ const countAll = () => true;
const counts = (messages, frontiers, isNotified = countAll, currentPubkey) =>
computeThreadBadgeCounts(
messages,
buildDirectRepliesByParentId(messages),
buildRepliesByRootId(messages),
frontiers,
isNotified,
currentPubkey,
@@ -35,31 +37,37 @@ test("computeThreadBadgeCounts_directRepliesOnly_countsEach", () => {
});
test("computeThreadBadgeCounts_nestedReply_countsTowardRoot", () => {
// root -> a -> b: b is a reply-to-a-reply. Pre-fix it lived under a's key
// and was never tallied toward root; the subtree walk must count it.
const messages = [msg("root", null), msg("a", "root"), msg("b", "a")];
// root -> a -> b: b is a reply-to-a-reply. It carries the thread root in its
// rootId, so the root-keyed roll-up tallies it toward root, not toward a.
const messages = [
msg("root", null),
msg("a", "root"),
msg("b", "a", 100, "author", "root"),
];
assert.equal(counts(messages, undefined).get("root"), 2);
});
test("computeThreadBadgeCounts_deepChain_countsWholeSubtree", () => {
// root -> a -> b -> c -> d: every descendant tallies toward the root.
// root -> a -> b -> c -> d: every descendant carries rootId "root" and tallies
// toward the root.
const messages = [
msg("root", null),
msg("a", "root"),
msg("b", "a"),
msg("c", "b"),
msg("d", "c"),
msg("b", "a", 100, "author", "root"),
msg("c", "b", 100, "author", "root"),
msg("d", "c", 100, "author", "root"),
];
assert.equal(counts(messages, undefined).get("root"), 4);
});
test("computeThreadBadgeCounts_branchingSubtree_countsAllBranches", () => {
// root -> a -> {b, c}; root -> d. Four descendants across two branches.
// root -> a -> {b, c}; root -> d. Four descendants across two branches, all
// carrying rootId "root".
const messages = [
msg("root", null),
msg("a", "root"),
msg("b", "a"),
msg("c", "a"),
msg("b", "a", 100, "author", "root"),
msg("c", "a", 100, "author", "root"),
msg("d", "root"),
];
assert.equal(counts(messages, undefined).get("root"), 4);
@@ -71,7 +79,11 @@ test("computeThreadBadgeCounts_rootWithNoReplies_omitted", () => {
});
test("computeThreadBadgeCounts_notNotified_omitted", () => {
const messages = [msg("root", null), msg("a", "root"), msg("b", "a")];
const messages = [
msg("root", null),
msg("a", "root"),
msg("b", "a", 100, "author", "root"),
];
assert.equal(counts(messages, undefined, () => false).size, 0);
});
@@ -80,7 +92,7 @@ test("computeThreadBadgeCounts_frontierCoversNestedReplies_excludesRead", () =>
const messages = [
msg("root", null),
msg("a", "root", 100),
msg("b", "a", 200),
msg("b", "a", 200, "author", "root"),
];
const frontiers = new Map([["root", 150]]);
assert.equal(counts(messages, frontiers).get("root"), 1);
@@ -90,7 +102,7 @@ test("computeThreadBadgeCounts_frontierCoversWholeSubtree_omitsRoot", () => {
const messages = [
msg("root", null),
msg("a", "root", 100),
msg("b", "a", 120),
msg("b", "a", 120, "author", "root"),
];
const frontiers = new Map([["root", 150]]);
assert.equal(counts(messages, frontiers).has("root"), false);
@@ -101,7 +113,7 @@ test("computeThreadBadgeCounts_selfAuthoredNestedReply_notCounted", () => {
const messages = [
msg("root", null),
msg("a", "root", 100, "other"),
msg("b", "a", 200, "ME"),
msg("b", "a", 200, "ME", "root"),
];
assert.equal(counts(messages, undefined, countAll, "me").get("root"), 1);
});
@@ -110,7 +122,7 @@ test("computeThreadBadgeCounts_multipleRoots_eachCountsOwnSubtree", () => {
const messages = [
msg("root1", null),
msg("a", "root1"),
msg("b", "a"),
msg("b", "a", 100, "author", "root1"),
msg("root2", null),
msg("c", "root2"),
];
@@ -119,67 +131,55 @@ test("computeThreadBadgeCounts_multipleRoots_eachCountsOwnSubtree", () => {
assert.equal(result.get("root2"), 1);
});
// --- LP4 Case 1 demonstration: orphaned subtree from a broken parent chain ---
// --- LP4 Case 1: orphaned subtree from a broken parent chain rolls up ---
//
// 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.
// The roll-up groups each reply under its `rootId` (buildRepliesByRootId).
// Pagination / load windows can drop an intermediate ancestor, severing the
// parent chain — but every reply still carries its true rootId (the `root`
// e-tag travels with the event, getThreadReference), so a deep reply tallies at
// its real root even when the middle ancestor is absent from the loaded array.
//
// 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.
// These two tests pin the exact trigger — a missing middle ancestor — and the
// orphan-immune roll-up that counts it anyway. The third is the full-chain
// control, identical to the broken-chain result by construction.
test("computeThreadBadgeCounts_brokenParentChain_orphanedReplyMissesRoot_DEFECT", () => {
test("computeThreadBadgeCounts_brokenParentChain_orphanedReplyRollsUpToRoot", () => {
// 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. `c.rootId === "root"` is set
// explicitly: today's parentId-keyed walk ignores it and still orphans `c`
// (this assertion holds), but once the roll-up re-keys on rootId this fixture
// flips — `c` will roll up and the expected count becomes 2.
// the loaded array (unloaded by the timeline window). `c` is genuinely unread
// and carries rootId "root", so the root-keyed roll-up tallies both `a` and
// `c`: count 2, the same as if the chain were intact. The old parentId-walk
// orphaned `c` (keyed under absent "b") and undercounted to 1.
const loaded = [
msg("root", null),
msg("a", "root"),
// msg("b", "a") — intentionally absent: unloaded intermediate ancestor.
msg("c", "b", 100, "author", "root"),
];
assert.equal(counts(loaded, undefined).get("root"), 1);
assert.equal(counts(loaded, undefined).get("root"), 2);
});
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.
// `c.rootId === "root"` is set explicitly so the fixture stays falsifiable:
// today's parentId walk still orphans it (root absent from the count map),
// but once re-keyed on rootId the badge appears with count 1.
test("computeThreadBadgeCounts_brokenParentChain_orphanedSoleReply_showsBadge", () => {
// Sharper form: root's ONLY unread content is the deep reply `c`, whose
// intermediate ancestor `b` is unloaded. `c` carries rootId "root", so the
// roll-up still groups it under root and the badge shows count 1. The old
// parentId-walk produced NO badge at all (root unreachable to its sole reply).
const loaded = [
msg("root", null),
// msg("b", "root") — intentionally absent: unloaded intermediate ancestor.
msg("c", "b", 100, "author", "root"),
];
assert.equal(counts(loaded, undefined).has("root"), false);
assert.equal(counts(loaded, undefined).get("root"), 1);
});
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.
// rootId is set on `c` for parity with the broken-chain fixtures; with the
// chain intact the parentId walk already reaches it, so this stays green
// whether the roll-up keys on parentId or rootId.
// chain root -> a -> b -> c is intact and every descendant carries rootId
// "root", so the root badge counts 3 — the baseline the broken-chain cases
// above match by rolling severed orphans up by rootId.
const loaded = [
msg("root", null),
msg("a", "root"),
msg("b", "a"),
msg("b", "a", 100, "author", "root"),
msg("c", "b", 100, "author", "root"),
];
assert.equal(counts(loaded, undefined).get("root"), 3);

View File

@@ -1,34 +1,6 @@
import { computeThreadUnreadMarker } from "@/features/messages/lib/unreadMarker";
import type { TimelineMessage } from "@/features/messages/types";
/**
* All reply messages in a root's subtree — direct children plus every deeper
* descendant, walked through the direct-replies adjacency map. A reply-to-a-
* reply must count toward the root's badge, so the badge tally needs the whole
* subtree rather than the root's direct children alone.
*
* Terminates without a visited-set: buildDirectRepliesByParentId places each
* message under exactly one parent key, and the only caller seeds the walk from
* true roots (parentId === null), so a node in a malformed parent cycle — whose
* members all key off each other, never off a root — is unreachable from any
* root's bucket. Seeding from a non-root id, or a builder that filed one node
* under two keys, would break that invariant.
*/
function collectSubtreeReplies(
rootId: string,
directRepliesByParentId: ReadonlyMap<string, TimelineMessage[]>,
): TimelineMessage[] {
const replies: TimelineMessage[] = [];
const pending = [...(directRepliesByParentId.get(rootId) ?? [])];
while (pending.length > 0) {
const reply = pending.pop();
if (!reply) continue;
replies.push(reply);
pending.push(...(directRepliesByParentId.get(reply.id) ?? []));
}
return replies;
}
/**
* Per-thread unread reply counts for the summary rows in the main timeline.
*
@@ -39,9 +11,16 @@ function collectSubtreeReplies(
* count spans the root's WHOLE subtree, so a reply nested under another reply
* still tallies toward the root's badge.
*
* Subtree membership is keyed on each reply's `rootId` rather than walked
* through the parent chain: a reply whose intermediate ancestor is absent from
* the loaded window still carries its true rootId (getThreadReference), so it
* rolls up to the root the parent-chain walk could never reach. For an intact
* chain every descendant carries the root's rootId, so the tally is identical
* to the old adjacency walk. Each reply has exactly one rootId, so it is
* counted once and a malformed parent cycle keys off no root.
*
* @param messages Top-level timeline entries in chronological order.
* @param directRepliesByParentId Direct replies keyed by parent id, walked to
* collect each root's full descendant subtree.
* @param repliesByRootId Replies grouped by their resolved thread root id.
* @param frontiers Per-root read frontier in unix seconds, or null/undefined
* when the thread was never read (every reply counts unread).
* @param isNotified Whether a thread root is one the user is notified for.
@@ -49,7 +28,7 @@ function collectSubtreeReplies(
*/
export function computeThreadBadgeCounts(
messages: TimelineMessage[],
directRepliesByParentId: ReadonlyMap<string, TimelineMessage[]>,
repliesByRootId: ReadonlyMap<string, TimelineMessage[]>,
frontiers: ReadonlyMap<string, number | null> | undefined,
isNotified: (rootId: string) => boolean,
currentPubkey?: string,
@@ -58,11 +37,8 @@ export function computeThreadBadgeCounts(
for (const message of messages) {
if (message.parentId) continue;
if (!isNotified(message.id)) continue;
const subtreeReplies = collectSubtreeReplies(
message.id,
directRepliesByParentId,
);
if (subtreeReplies.length === 0) continue;
const subtreeReplies = repliesByRootId.get(message.id);
if (!subtreeReplies || subtreeReplies.length === 0) continue;
const { unreadCount } = computeThreadUnreadMarker(
subtreeReplies,
frontiers?.get(message.id) ?? null,

View File

@@ -3,16 +3,16 @@ import test from "node:test";
import { nextThreadBadgeFrontier } from "./threadBadgeFrontier.ts";
import { seedThreadBadgeFrontiers } from "./threadBadgeFrontier.ts";
import { buildDirectRepliesByParentId } from "./subtreeCreatedAt.ts";
import { buildRepliesByRootId } from "./subtreeCreatedAt.ts";
import { computeThreadBadgeCounts } from "./threadBadgeCounts.ts";
const msg = (id, parentId) => ({ id, parentId });
const msg = (id, parentId) => ({ id, parentId, rootId: parentId ?? id });
const seedAll = () => true;
const seed = (frontiers, messages, isNotified, getReadAt) =>
seedThreadBadgeFrontiers(
frontiers,
messages,
buildDirectRepliesByParentId(messages),
buildRepliesByRootId(messages),
isNotified,
getReadAt,
);
@@ -131,14 +131,14 @@ test("seedThreadBadgeFrontiers_channelMarkerFoldedIntoSeed_badgeVanishes_DEFECT"
// 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 repliesByRootId = buildRepliesByRootId(messages);
const foldedEffectiveMarker = Math.max(100, 250); // thread_own vs channel
const frontiers = new Map();
seedThreadBadgeFrontiers(
frontiers,
messages,
directReplies,
repliesByRootId,
seedAll,
() => foldedEffectiveMarker,
);
@@ -147,7 +147,7 @@ test("seedThreadBadgeFrontiers_channelMarkerFoldedIntoSeed_badgeVanishes_DEFECT"
const result = computeThreadBadgeCounts(
messages,
directReplies,
repliesByRootId,
frontiers,
seedAll,
);
@@ -159,14 +159,14 @@ 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 repliesByRootId = buildRepliesByRootId(messages);
const preMarkReadMarker = 100; // thread_own only, channel fold not applied
const frontiers = new Map();
seedThreadBadgeFrontiers(
frontiers,
messages,
directReplies,
repliesByRootId,
seedAll,
() => preMarkReadMarker,
);
@@ -175,7 +175,7 @@ test("seedThreadBadgeFrontiers_preMarkReadMarkerSeeded_badgeSurvives_DESIRED", (
const result = computeThreadBadgeCounts(
messages,
directReplies,
repliesByRootId,
frontiers,
seedAll,
);
@@ -183,31 +183,23 @@ test("seedThreadBadgeFrontiers_preMarkReadMarkerSeeded_badgeSurvives_DESIRED", (
assert.equal(result.get("root"), 1);
});
// --- LP4 Case 3, second face: orphan-only root is never seeded at all ---
// --- LP4 Case 3, second face: orphan-only root IS seed-eligible ---
//
// seedThreadBadgeFrontiers gates seed-eligibility on a DIRECT reply existing:
// `if (!directRepliesByParentId.has(message.id)) continue;` (threadBadgeFrontier
// .ts). A root whose ONLY reply is a deep orphan — the middle ancestor unloaded,
// so the orphan keys under its absent parent, not the root — has NO direct-reply
// entry. So the root is skipped: its frontier is never created. This is a
// distinct failure from a wrong COUNT (the count tests above) — here the
// frontier snapshot itself never exists, so even a corrected count path has
// nothing to measure against.
//
// The orphan carries rootId === "root" (getThreadReference resolves it from the
// event's own `root` e-tag regardless of ancestor load state), which is exactly
// the key the redesign will use to make the root seed-eligible. This test is
// expected-RED on current code (root is skipped, so it asserts the DESIRED
// seeded state and fails today) and flips green once seeding keys eligibility
// on rootId-reachability rather than a direct-reply entry.
// seedThreadBadgeFrontiers gates seed-eligibility on a reply existing under the
// root by rootId: `if (!repliesByRootId.has(message.id)) continue;`. A root
// whose ONLY reply is a deep orphan — the middle ancestor unloaded, so the
// orphan keys under its absent parent in the direct-reply map — still owns that
// reply by rootId (getThreadReference resolves rootId from the event's own
// `root` e-tag regardless of ancestor load state). The old direct-reply gate
// skipped such a root entirely, so its frontier never existed and its badge
// could never clear; keying on rootId makes it seed-eligible. This is a face of
// Case 3 distinct from a wrong COUNT — here the frontier snapshot itself was
// missing, so even a corrected count path had nothing to measure against.
test("seedThreadBadgeFrontiers_orphanOnlyRoot_seedEligible_DEFECT", {
todo: "Case 3 second face: direct-reply seed gate skips orphan-only roots; P2 rootId re-key fixes it",
}, () => {
test("seedThreadBadgeFrontiers_orphanOnlyRoot_seedEligible", () => {
// root's only reply is `c`, whose middle ancestor `b` is unloaded. `c` keys
// under "b" (absent), so directRepliesByParentId has NO entry for "root" and
// today's seed skips it. DESIRED: the root IS seed-eligible (orphan is part of
// its thread by rootId) and seeds to the read marker 100.
// under "b" (absent) in the direct-reply map but carries rootId === "root",
// so repliesByRootId has an entry for "root" and the root seeds to marker 100.
const messages = [
reply("root", null, 50),
// reply("b", "root", ...) — intentionally absent: unloaded ancestor.
@@ -215,14 +207,12 @@ test("seedThreadBadgeFrontiers_orphanOnlyRoot_seedEligible_DEFECT", {
];
const frontiers = new Map();
seed(frontiers, messages, seedAll, () => 100);
// EXPECTED-RED on current code: root is skipped (frontier absent). Post-fix
// the root is seeded to 100 and this passes.
assert.equal(frontiers.get("root"), 100);
});
test("seedThreadBadgeFrontiers_directReplyRoot_seedEligible_DESIRED", () => {
// Control: the SAME root with a DIRECT reply present. directRepliesByParentId
// has an entry for "root", so it is seed-eligible today and after the fix.
// Control: the SAME root with a DIRECT reply present. repliesByRootId has an
// entry for "root", so it is seed-eligible — the intact-chain baseline.
const messages = [reply("root", null, 50), reply("r1", "root", 200)];
const frontiers = new Map();
seed(frontiers, messages, seedAll, () => 100);

View File

@@ -35,17 +35,24 @@ export function nextThreadBadgeFrontier(
// seeded once at open then advanced toward the live marker on subsequent reads
// (see nextThreadBadgeFrontier). Called during render so snapshots reflect
// "what was unread on open," matching the openFrontierRef pattern.
//
// Reply-presence is keyed on `repliesByRootId`, not the direct-parent map: a
// root whose only reply is a deep orphan (intermediate ancestor absent from the
// loaded window) has no direct child but still owns that reply by rootId. Gating
// on direct children alone skipped such a root entirely, so its frontier never
// existed and its badge could never clear — the seed-side face of the orphan
// defect that the count rollup fixes on the tally side.
export function seedThreadBadgeFrontiers(
channelFrontiers: Map<string, number | null>,
messages: TimelineMessage[],
directRepliesByParentId: ReadonlyMap<string, TimelineMessage[]>,
repliesByRootId: ReadonlyMap<string, TimelineMessage[]>,
isNotified: (rootId: string) => boolean,
getReadAt: (rootId: string) => number | null,
): void {
for (const message of messages) {
if (message.parentId) continue;
if (!isNotified(message.id)) continue;
if (!directRepliesByParentId.has(message.id)) continue;
if (!repliesByRootId.has(message.id)) continue;
channelFrontiers.set(
message.id,
nextThreadBadgeFrontier(

View File

@@ -6,7 +6,7 @@ import {
nextThreadBadgeFrontier,
seedThreadBadgeFrontiers,
} from "./threadBadgeFrontier.ts";
import { buildDirectRepliesByParentId } from "./subtreeCreatedAt.ts";
import { buildRepliesByRootId } from "./subtreeCreatedAt.ts";
// LP4 characterization invariants for thread-unread badges.
//
@@ -35,7 +35,7 @@ const notifiedAll = () => true;
const counts = (messages, frontiers, isNotified = notifiedAll, currentPubkey) =>
computeThreadBadgeCounts(
messages,
buildDirectRepliesByParentId(messages),
buildRepliesByRootId(messages),
frontiers,
isNotified,
currentPubkey,
@@ -46,7 +46,7 @@ const seed = (frontiers, messages, getReadAt, isNotified = seedAll) =>
seedThreadBadgeFrontiers(
frontiers,
messages,
buildDirectRepliesByParentId(messages),
buildRepliesByRootId(messages),
isNotified,
getReadAt,
);

View File

@@ -5,14 +5,16 @@ import {
subtreeMaxCreatedAt,
buildDirectReplyIdsByParentId,
buildCreatedAtByMessageId,
buildRepliesByRootId,
} from "./subtreeCreatedAt.ts";
// The thread-open read ceiling: opening a thread advances its read frontier to
// subtreeMaxCreatedAt(headId), the newest createdAt anywhere in the head's
// subtree. The thread-open caller starts the walk at the ROOT (consume the
// whole thread); the expand caller starts at a BRANCH node (consume only that
// branch). These tests pin both — Case 1's orphan-misses-ceiling defect and the
// call-site split that must NOT let expand cross into a sibling branch.
// whole thread) and folds in rootId-matched orphans; the expand caller starts
// at a BRANCH node (consume only that branch) with no rootId folding. These
// tests pin both — Case 1's orphan-misses-ceiling defect and the call-site
// split that must NOT let expand cross into a sibling branch.
const msg = (id, parentId, createdAt, rootId) => ({
id,
@@ -21,6 +23,7 @@ const msg = (id, parentId, createdAt, rootId) => ({
createdAt,
});
// Branch-scoped ceiling — the expand caller. Parent-chain walk only.
const ceiling = (headId, messages) =>
subtreeMaxCreatedAt(
headId,
@@ -28,28 +31,29 @@ const ceiling = (headId, messages) =>
buildCreatedAtByMessageId(messages),
);
// Root-scoped ceiling — the thread-open caller. Folds in replies that resolve
// to the head by rootId, so a severed orphan still raises the ceiling.
const rootCeiling = (headId, messages) =>
subtreeMaxCreatedAt(
headId,
buildDirectReplyIdsByParentId(messages),
buildCreatedAtByMessageId(messages),
buildRepliesByRootId(messages),
);
// (3) Case 1 — orphan misses the thread-open ceiling. The deep reply `c` is the
// newest content (300), but its middle ancestor `b` is unloaded, so `c` keys
// under absent "b" and the root-started walk never reaches it. The ceiling
// stops at the newest REACHABLE node (`a` at 200), leaving `c` permanently
// above the frontier — the channel-root badge can never clear via thread-open.
//
// EXPECTED-RED on current code: the parentId-only walk yields 200, not 300.
// `c.rootId === "root"` is set explicitly — the redesign keys the ceiling walk
// on rootId-reachability so the orphan is included and the assertion flips to
// 300. Asserting the DESIRED value (300) makes this a failing characterization
// of the live defect, not a pin of the bug.
test("openThreadCeiling_deepOrphanMissingAncestor_includedInCeiling_DEFECT", {
todo: "Case 1: parentId walk can't reach the orphan; P2 rootId re-key fixes it",
}, () => {
// under absent "b" and the parentId-only walk never reaches it. The root-scoped
// ceiling folds in `c` by its rootId ("root", which travels with the event via
// getThreadReference), so it reaches 300 and the channel-root badge can clear.
test("openThreadCeiling_deepOrphanMissingAncestor_includedInCeiling", () => {
const loaded = [
msg("root", null, 50, "root"),
msg("a", "root", 200, "root"),
// msg("b", "a", ...) — intentionally absent: unloaded middle ancestor.
msg("c", "b", 300, "root"),
];
// DESIRED: ceiling reaches the orphan's 300. Current code returns 200 (RED).
assert.equal(ceiling("root", loaded), 300);
assert.equal(rootCeiling("root", loaded), 300);
});
// (3) control — with the middle ancestor present the chain is intact and the

View File

@@ -2,8 +2,8 @@ import * as React from "react";
import {
buildCreatedAtByMessageId,
buildDirectRepliesByParentId,
buildDirectReplyIdsByParentId,
buildRepliesByRootId,
collectReplyDescendantIds,
subtreeMaxCreatedAt,
} from "@/features/channels/lib/subtreeCreatedAt";
@@ -118,8 +118,8 @@ export function useChannelUnreadState({
() => buildDirectReplyIdsByParentId(timelineMessages),
[timelineMessages],
);
const directRepliesByParentId = React.useMemo(
() => buildDirectRepliesByParentId(timelineMessages),
const repliesByRootId = React.useMemo(
() => buildRepliesByRootId(timelineMessages),
[timelineMessages],
);
const getFirstReplyIdForMessage = React.useCallback(
@@ -148,6 +148,24 @@ export function useChannelUnreadState({
),
[createdAtByMessageId, directReplyIdsByParentId],
);
// Root-scoped variant of the ceiling, used only by the thread-open mark-read
// effect below. Folds in replies that resolve to the root by rootId, so a
// severed orphan (intermediate ancestor outside the loaded window) still
// raises the ceiling and the channel-root badge can clear on open. The
// branch-scoped getSubtreeMaxCreatedAt above stays as-is for the expand
// caller, which must advance only its own branch — a branch node owns no
// rootId bucket, so passing repliesByRootId there would be a no-op anyway,
// but keeping the two callbacks distinct makes the scope intent explicit.
const getRootSubtreeMaxCreatedAt = React.useCallback(
(rootId: string) =>
subtreeMaxCreatedAt(
rootId,
directReplyIdsByParentId,
createdAtByMessageId,
repliesByRootId,
),
[createdAtByMessageId, directReplyIdsByParentId, repliesByRootId],
);
const threadPanelIndex = React.useMemo(
() => buildThreadPanelIndex(timelineMessages),
[timelineMessages],
@@ -232,10 +250,15 @@ export function useChannelUnreadState({
React.useEffect(() => {
if (!openThreadHeadId) return;
if (isThreadMuted(openThreadHeadId)) return;
const openReadCeiling = getSubtreeMaxCreatedAt(openThreadHeadId);
const openReadCeiling = getRootSubtreeMaxCreatedAt(openThreadHeadId);
if (openReadCeiling === null) return;
markThreadRead(openThreadHeadId, openReadCeiling);
}, [openThreadHeadId, getSubtreeMaxCreatedAt, markThreadRead, isThreadMuted]);
}, [
openThreadHeadId,
getRootSubtreeMaxCreatedAt,
markThreadRead,
isThreadMuted,
]);
// Compute the in-thread "New" divider position from the open-time frontier.
const { firstUnreadReplyId: threadFirstUnreadReplyId } = React.useMemo(() => {
if (!openThreadHeadId || threadMessages.length === 0) {
@@ -307,7 +330,7 @@ export function useChannelUnreadState({
seedThreadBadgeFrontiers(
channelFrontiers,
timelineMessages,
directRepliesByParentId,
repliesByRootId,
(rootId) => !isThreadMuted(rootId),
(rootId) => getThreadReadAt(rootId, activeChannelId),
);
@@ -330,7 +353,7 @@ export function useChannelUnreadState({
() =>
computeThreadBadgeCounts(
timelineMessages,
directRepliesByParentId,
repliesByRootId,
activeChannelId
? threadBadgeFrontiersRef.current.get(activeChannelId)
: undefined,
@@ -341,7 +364,7 @@ export function useChannelUnreadState({
activeChannelId,
currentPubkey,
timelineMessages,
directRepliesByParentId,
repliesByRootId,
isThreadMuted,
readStateVersion,
],