fix(desktop): correct thread-unread badge flicker, stale clear, phantom count, mention gate, and nested count#1080
Merged
Conversation
ee02ab8 to
5c80700
Compare
b66c1d1 to
697f3a4
Compare
…el counts Three live defects in the shipped thread-unread badge (#1069): - Flaky badge: participatedRootIds/authoredRootIds were bare in-place-mutated ref Sets, so isNotifiedForThread's useCallback never re-created and a badge whose participation was discovered async stayed suppressed. A membershipVersion reducer now re-derives identity-stable snapshots for the gate while live consumers keep reading the mutable refs. - Badge wouldn't clear on read: the threadUnreadCounts memo read a frozen open-time frontier snapshot and omitted readStateVersion from its deps. The snapshot now advances monotonically toward the live thread marker (never the latest reply, so collapsed-branch unreads survive) keyed on readStateVersion. - Phantom channel-pill counts: the marker counted relay-signed system rows and job-lifecycle events. A call-site kind predicate excludes them; unreadMarker stays kind-agnostic. Undefined kinds are treated as conversational so pending rows are never dropped. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…lookups Folds the second-review-round items into the badge-fix PR. The self-author skip in unreadMarker.ts was case-sensitive and crashed on an undefined pubkey; normalize currentPubkey once and compare with optional-chained toLowerCase so a self-reply is never miscounted as unread. computeThreadBadgeCounts and seedThreadBadgeFrontiers each ran a filter/some over the whole timeline inside their per-thread loop; they now share one parentId->replies index for O(1) lookup. The membershipVersion bump on a self-post is size-guarded so an already- tracked root no longer re-allocates the gate snapshots. Adds a clear-on-read E2E asserting the depth-0 badge clears in place after reading a thread, with no channel re-entry. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…pation isNotifiedForThread required followed/participated/authored, so a pure mention recipient who never engaged with a thread got the notification toast (shouldNotifyForEvent honors mentions) but never the unread badge — at any nesting depth. Track external-mention thread roots in useUnreadChannels (a mentionedRootIdsRef fed from the live message scan and the catch-up Pass 1, both already computing mention-ness) and OR an identity-stable mentionedRootIds snapshot into the gate. Consolidates the four duplicated localStorage helpers into one makeRootIdStore factory so the new mentioned set reuses the same capped read/write shape. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… replies computeThreadBadgeCounts tallied only a root's direct children, so a reply-to-a-reply was filed under its parent's key and never counted toward the root's badge — a nested mention reply undercounted (or, with no direct children, vanished). Walk the root's full descendant subtree through the direct-replies adjacency map instead. Adds a depth-2 mention-only E2E (viewer never participates; nested reply @-mentions them) that fails pre-fix on both the gate and the count, plus unit coverage for nested, deep-chain, branching, frontier, and self-authored cases. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The subtree walk has no visited-set; its termination on a malformed parent cycle relies on a cross-file invariant (each node filed under one parent key, only true roots seed the walk) that was stated nowhere at the walk. Document it so a future non-root caller or two-key builder change can't silently reintroduce an unbounded loop. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
697f3a4 to
af59dfb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five live defects in the shipped thread-unread badge (#1069).
Bug 1 — flaky thread badge
participatedRootIds/authoredRootIdswere bare in-place-mutated ref Sets, soisNotifiedForThread'suseCallbacknever re-created and a badge whose participation was discovered async stayed suppressed (~50% flake). AmembershipVersionreducer now re-derives identity-stable snapshot Sets for the gate while the live-notify path keeps reading the mutable refs fresh. Lives inuseUnreadChannels.ts.Bug 2 — badge wouldn't clear on read
The
threadUnreadCountsmemo read a frozen open-time frontier snapshot and omittedreadStateVersionfrom its deps, so reading a thread advanced the live marker but never the badge. The snapshot now advances monotonically toward the live thread read marker —Math.max(stored, getThreadReadAt(rootId)), the live marker only, never latest-reply — so a collapsed-branch unread survives and a newer reply re-raises the badge.readStateVersionis added to the memo deps as the intentional recompute trigger. The advance is folded into the render-phase seed block (no second racing effect), preserving the "captured during render before the mark-read effect" invariant.Bug 3 — phantom channel-pill counts
computeChannelUnreadMarkercounted relay-signed system rows (channel_created,member_joined) and job-lifecycle events as unread, so a freshly-created channel read "4 unread, 1 message." A call-siteisConversationalUnreadKindpredicate now excludesKIND_SYSTEM_MESSAGE(40099) and the job kinds (43001-43006);unreadMarker.tsstays kind-agnostic. Undefined kinds are treated as conversational so pending/optimistic rows are never dropped.KIND_STREAM_MESSAGE_V2(40002) andKIND_STREAM_MESSAGE_DIFF(40008) stay counted.Bug 4 — mention-only threads never badged
isNotifiedForThreadgated onfollowed || participated || authoredwith no mention term, whileshouldNotifyForEventhonors@-mentions. A pure mention recipient who never engaged with a thread therefore got the notification toast but never the unread badge — at any nesting depth (the nesting in the original repro was incidental).useUnreadChannelsnow tracks the thread roots of external messages that@-mention the user (amentionedRootIdsReffed from both the live message scan and the catch-up Pass 1, both of which already compute mention-ness), exposes an identity-stablementionedRootIdssnapshot via the samemembershipVersionmechanism as Bug 1, and ORs it into the gate. The four duplicatedlocalStoragehelpers (participation, authored, muted, and the new mentioned set) collapse into onemakeRootIdStorefactory.Bug 5 — nested replies undercounted the badge
computeThreadBadgeCountstallied only a root's direct children, so a reply-to-a-reply was filed under its parent's key and never counted toward the root's badge. The count now walks the root's full descendant subtree through the direct-replies adjacency map, so a nested mention reply tallies correctly. Reading still follows the established depth-aware model: opening a thread consumes the visible direct replies, and a collapsed deeper branch clears only when drilled into.Scope
useChannelUnreadState.ts(the hook that holds the unread-marker logic);ChannelScreen.tsxadds only thereadStateVersionplumbing.useUnreadChannels.ts; the Bug 4 gate term is ORed inAppShell.tsx.kinds.ts,threadBadgeCounts.ts, andthreadBadgeFrontier.ts, with unit coverage inkinds.test.mjs,threadBadgeCounts.test.mjs, andthreadBadgeFrontier.test.mjs.thread-unread-screenshots.spec.tstest 14) gates Bug 4 and Bug 5: a viewer who never participates is mentioned on a nested reply, the badge appears at the root counting the whole subtree, and clears in place once the branch is drilled into.