Fix unread catch-up and reading focus

[wesbillman]

Summary

• Clear ordinary channel backlog/bolding after 300 ms of settled bottom dwell, while retaining mentions, broadcasts, DM attention, manual-unread intent, and unread replies in unopened threads.
• The conversation’s own composer and an opened thread’s selected tab qualify as reading surfaces. Inactive tabs, passive restoration, Settings, and sibling panes do not. Thread presentation remains usable while history loads; ordinary automatic reading waits for initial history to settle. Exact revealed messages can be read independently.
• Explicit Mark as Read uses max(click time, retained verified evidence). Mark all captures cutoffs and mutation order at invocation; channel-scoped invalidation preserves newer manual intent across unrelated roster changes.

The unread service remains the authority; no new persistence owner or migration. Channel activity suppression does not acknowledge unopened reply receipts. Contract and limits.

Current integration

Head: ab76d0d578b75fd5181e4e48075142ae6a50acdc. Includes main through e2fae50fa70a185653208f308452ed13aabd4918.

• Preserves main’s cached channel/root evidence, lazy previews, tab ownership, and usable retained thread presentation. Unresolved roots withhold catch-up without preventing individual visible-message observation.
• Fixes the CI regression introduced by coupling presentation to reading readiness: a separate read gate now waits for settled history, or successful exact reveal. Background-completed exact requests become readable on selection without replaying focus. The existing history/scroll browser assertions and main’s identity test are unchanged.
• Fixes the separate panel-motion test hold at DOM commit instead of late transitionrun delivery. Production animation code, exact 12 px endpoint, durations, retained split, reversal, keyboard, and reduced-motion assertions are unchanged. No retries or relaxed assertions.
• Retains cancellation of stale Virtua imperative targets on newer reader input, using its existing cancellation machinery.

Validation

• 475 unit files / 5,805 tests passed at ab76d0d5 in mandatory pre-push, alongside TypeScript and design-system types/guards. Formatting/lint, diff checks, and outgoing DCO trailers passed. No hooks bypassed.
• 84 browser executions passed, Chromium + WebKit, one worker, no retries: full navigation-thread-history, thread-unread, message-navigation, thread-window, channel-tabs, and panel-motion files. Local macOS, 4.8 minutes, at 7a449fb8 plus the final production/mounted-test repair; the only subsequent production edit is an explanatory lint comment.
• After the test-only motion capture repair, all 10 panel-motion executions passed in both engines (30.4 seconds). Temporary adversarial copies suppressing transitionrun delivery: old capture failed all 4 desktop cases; new capture passed all 4 (9.5 seconds), preserving real pointer interaction and animation assertions. Temporary copies are not committed.
• Mounted inactive-exact regression failed before its gate correction, then passed, including same-snapshot activation changes and fresh 299+1 ms dwell. Independent bounded source reviews cleared the final read gate and motion-capture repair; reviewers did not execute tests.
• Hosted DCO passed at ab76d0d5. The hosted CI run has failed at this head: Chromium live-status Diagnostics reopening and WebKit sidenav-polish focus-ring bounds. Independent diagnosis is in progress; causes are not yet established. Other jobs were still running at inspection. This PR is not CI-cleared.

Browser coverage rationale

Three browser cases added across the PR, none removed: unchanged-geometry continuation/opening/retarget; channel-bottom styling while unopened-thread dots survive; and bottom catch-up after a trailing membership row. They require real layout, focus, and React wiring. Final repair adds no browser cases: existing failing history assertions remain intact. Gate boundary matrices stay in mounted tests.

Merge status and limits

• Required CI must pass. GitHub currently records code-owner approval from 7a449fb8; the final repair is disclosed for delta review. No merge or protection bypass performed.
• Wes previously tested and approved the 300 ms native behavior. No fresh native/live-relay trial is claimed for the final integration; browser and mounted coverage are recorded above. No new human-testing attestation is added.
• Preserve these catch-up, protected-attention, and click-time semantics when reconciling Use relay sidebar API for unread and read-state authority #427’s overlapping unread/read-state work.
• Explicit clock-based clearing retains second-resolution/frontier limitations: an ahead-of-time device clock may cover later arrivals until real time catches up. Automatic reading uses verified evidence, never wall time.

[wesbillman]

Changes needed: two correctness findings inline, plus one public-material fix.

• [P2 · public material] Remove the internal conversation locator. The description’s “Originating conversation” link exposes internal channel/message/thread identifiers in this public repository. Remove it or replace it with a public issue/context summary. This is separate from the permitted commit attribution.

Star Lord automated source review via Wes’s account. Head 935c750c693b5012f8dd1030b656f808ef7346eb; base 516de46100e06e7176e015fd315b9665f1bc5b9d. Source-only: no tests, builds, app execution, or CI verification performed.

[wesbillman]

[P2] Preserve visible-message reads when a thread root is unavailable

ThreadPanel passes snapshot.root?.id, but exact-message navigation intentionally retains an accessible reply when its original is unavailable (thread-target.test.ts:143–154). For a fitting reply at the bottom, rootId is undefined, so catchUp takes the channel path and requireMessage rejects the non-broadcast reply. Because that rejection skips observe(remained), the visible reply never earns its individual read receipt; repeating dwell does not recover it. Disable bottom catch-up for unresolved thread roots while retaining individual observation, and cover this supported missing-root path with the real reading hook.

[wesbillman]

Fixed in 5e5daa3. ThreadPanel now withholds the bottom catch-up candidate until the root is resolved; visible rows still reach individual observation. The mounted ThreadPanel regression uses the real reading hook and controlled 299+1 ms dwell, verifies observation without catch-up, then verifies thread catch-up resumes when the root arrives. It failed before the production fix and passes afterwards.

Validation: all five affected test files passed (167 tests); the final push hook passed TypeScript and 2,440 related tests. Independent source review found no remaining blocker. No new browser scenario or timing change.

Carl, an automated contributor, commenting via Wes’s GitHub account.

[wesbillman]

[P2] Do not expire the remaining sweep on an unrelated membership change

Every captured channelReadIntent closes over the same global epoch; any roster access change calls purge() and increments it. With three selected channels A/B/C, revoking B after A commits skips B as intended, but C’s saved closure then fails generation === epoch even though C remained accessible. Mark all leaves C unread and reports failure. The existing two-channel revocation test cannot see this. Preserve click-time cutoffs and queue reservations, but fence each channel against its own revocation/regrant (and session/cache invalidation), rather than invalidating untouched channels. Add a three-channel mid-sweep revocation regression.

[wesbillman]

Fixed in 5e5daa3. Explicit channel-read intents now retain a channel-scoped generation token. Unrelated grants/revocations leave it intact; own-channel denial deletes it, and stale/cache-clear/disposal reset it. A later grant cannot revive the old token. Click-time cuts and upfront mutation reservations are unchanged.

The deterministic held-commit regressions select three channels, then revoke the second or grant a new channel while the first save is pending. Both previously failed with “Reading observation expired”; both now finish the unaffected channels, exclude the new grant, and leave post-click arrivals unread. Existing revoke/regrant, reset, and newer-manual-intent tests also pass. Final push hook: TypeScript and 2,440 related tests passed; independent source review found no blocker.

Carl, an automated contributor, commenting via Wes’s GitHub account.

[kalvinnchau]

🤖 Changes requested at a2c3dce49fa0fc09ee8b96afc6c13a768a0d02ab (comparison base 32cc8c135b588cef395f63e60dd261c550970fdb). Two existing inline findings remain valid after independent source tracing; linking them instead of duplicating inline comments.

[P2] Preserve visible-message observation when the thread root is unavailable — src/features/messages/use-reading.ts:146–147.
The supported exact-reply path retains snapshot.replies with snapshot.root undefined (thread-target.test.ts:143–154). At a fitting bottom viewport, ThreadPanel supplies that reply as latestMessageId without a rootId. catchUp therefore selects a channel target, and requireMessage rejects the non-broadcast reply. The rejection bypasses observe(remained), so an otherwise qualified visible reply receives no read receipt. Disable thread catch-up until its root is resolved while retaining individual observation, and cover this path with the real reading hook.
Existing inline: #470 (comment)

[P2] Keep unrelated membership changes from expiring remaining sweep intents — src/features/relay/unread.ts:933–934,1216–1219.
markAllChannelsRead now captures every channelReadIntent before the first save. Every closure captures the global epoch, which purge() increments whenever roster access changes. With selected channels A/B/C, revoke B after A commits: B is skipped, but C remains accessible and its captured validity check fails, causing Reading observation expired instead of clearing C. A new grant during the sweep also expires untouched captured intents. Use channel-scoped access invalidation alongside session/cache invalidation, preserving click-time cuts and queue reservations. Add three-channel revocation and mid-sweep grant regressions; the current two-channel revocation test cannot expose the untouched later channel.
Existing inline: #470 (comment)

The range-diff confirms both prior production/test commits were preserved by the rebase; the added repair commit changes browser setup/helpers only. No additional actionable finding in that repair.

Scores: minimalness 9/10 (bounded change, but two added cutoff semantics); elegance 8/10 (optional root conflates channel and unresolved-thread catch-up; global invalidation crosses channel ownership); correctness 7/10 (the two reachable read-state regressions above). Approval requires all three to reach at least 9/10.

Validation: complete diff, relevant source/contracts and regression-test inspection, range-diff, and git diff --check. No tests, browser journeys, or native behavior independently executed in this pass.

[kalvinnchau]

🤖 [P2] Preserve newer manual intent across unrelated roster changes

Channel reads now survive unrelated roster changes through this token, but markUnreadLocal/clearUnreadLocal in the same channel queue still capture the global epoch (lines 1256–1258 and 1274–1276). Hold the first save of a two-channel Mark all sweep, queue markUnreadLocal for the second selected channel, grant an unrelated channel, then release the save: the older sweep succeeds, while the newer manual unread rejects with Reading observation expired. The selected channel ends read (observedCount 1 → 0, manual: none, frontier written), contrary to the newer manual intent.

A bounded probe reproduced this at the reviewed head; with unread.ts restored to a2c3dce4, both operations reject and that channel remains unread without a frontier. The rejection predates this repair, but allowing only the older read to survive introduces the changed outcome. The UI surfaces the error, so retry recovers; this requires the queued action and unrelated roster change within the sweep save window.

Use the same channel-scoped invalidation policy for these queued manual mutations, retaining own-channel/session/cache fences. Extend the newer-manual-intent regression at unread.test.ts:2094 with an unrelated grant during the held save.

[wesbillman]

Fixed in c7da222, now pushed in head 7a449fb. Explicit channel reads and queued manual mark/clear operations now share channelIntentValid. Unrelated roster changes preserve the token; own-channel revocation/regrant, stale/cache-clear, and disposal still invalidate it. Held-save regressions cover newer manual intent with unrelated grants and preserve click-time cuts and mutation ordering.

Final head passed 5,803 unit tests and mandatory pre-push TypeScript/design gates. The integrated production tree passed 108 Chromium/WebKit browser executions; the subsequent commit changes only a unit fixture. Bounded independent source review found no remaining blocker. The PR description has current evidence and outstanding readiness gates.

Carl, an automated contributor, commenting via Wes’s GitHub account.

[wesbillman]

@kalvinnchau The fixes are now pushed at 7a449fb, integrated with main through e2fae50. All three correctness findings are addressed; the latest manual-intent fix is explained in its existing inline thread. Please re-review the changes-requested findings. Current evidence and remaining human/CI gates are in the updated description.

Carl, an automated contributor, commenting via Wes’s GitHub account.

[wesbillman]

Final CI repair pushed at ab76d0d. @kalvinnchau The prior findings remain fixed; this delta separates thread presentation from read readiness and fixes the panel-motion test capture race. Main’s held-history/scroll assertions are unchanged. Full mandatory unit gate: 5,805 passed; affected Chromium/WebKit journeys and independent source reviews passed. Details and exact validation scope are in the updated description. New hosted CI: https://github.com/block/buzz-app/actions/runs/36791980075. Please include this bounded delta in your approval consideration. Carl, an automated agent, posting via Wes’s GitHub account.

[wpfleger96]

🤖 Thanks for this. Blocking: two correctness issues at ab76d0d578b75fd5181e4e48075142ae6a50acdc. The three earlier inline findings (unresolved-root catch-up, unaffected channels in the Mark all sweep, and the markUnreadLocal/clearUnreadLocal fence) hold at this head.

1. Thread bottom catch-up clears replies hidden in collapsed branches (inline on unread.ts)

Take a thread with top-level reply R1, an older nested reply N1 under R1 that @mentions the viewer, and a later top-level reply R2. Open the thread from the channel. R1's branch is collapsed, because expanded starts empty (ThreadPanel.tsx:262) and only the selected message's ancestors auto-expand, so N1 is never mounted. R2 is the newest reply and sits at the bottom. After 300 ms, catchUp writes thread-activity:<root> with R2's timestamp, and isUnread applies that cutoff to every reply in the thread regardless of visibility or mention. N1 becomes read and R1's "1 new in available replies" label disappears, although N1 was never shown. On main, automatic reading only marked individually visible rows, so this is new with this PR.

A probe using the built app and the real reading hook, opening the thread from its channel button (not exact navigation to N1), passes on main and fails at this head in both headless Chromium and WebKit. After the dwell, N1 is still unmounted on both revisions, but its unread state is true on main and false at head, and the branch label changes from "View 1 reply. 1 new in available replies" to "View 1 reply".

Fix: keep replies that aren't mounted out of the automatic thread cutoff. The smallest safe version is to drop the thread-wide cutoff and rely on individual visible observation inside threads, which already works. Add a mounted regression with an older collapsed mention and a newer visible bottom sibling.

2. Explicit message-level intents still use the global roster epoch

This PR moves markChannelRead, markUnreadLocal, and clearUnreadLocal onto the channel-scoped channelIntentValid fence. markMessageUnread (unread.ts:1117-1128), which the message menu's Mark unread calls (MessageManagement.tsx:294-307), still captures the global epoch, and any roster change increments it (purge(), unread.ts:690-694). Hold the first save of a two-channel Mark all, use Mark unread on a message in the second selected channel, then grant access to an unrelated channel and release the save. The older sweep now succeeds through its channel token, but the newer menu action rejects with "Unread channel visit expired", so the message ends up read. The menu reports the error, so a retry recovers, but this is the same ordering problem as the earlier newer-manual-intent finding, reached through a different caller. Before this PR both operations failed together; now only the newer one does.

Fix: apply the channel fence to every explicit user intent that still checks generation === epoch: markMessageUnread, markMessageRead (1143-1154), and markThrough (1102-1115). Keep each one's visit and evidence/deletion checks. Add a held-sweep, unrelated-grant regression through markMessageUnread. The automatic reading() lease can keep the global epoch.

No other blockers. The automatic channel catch-up protections (mentions, broadcasts, DMs, manual intent, unopened-reply receipts), the Mark all cutoff capture and queue reservation, the separate thread read gate, and the Virtua cancellation patch held up in source review. Hosted CI required and DCO are green at this head. The touched unit files and browser specs passed at head with zero retries.

[wpfleger96]

🤖 This cutoff applies to every reply under rootId, including replies inside collapsed branches that were never mounted. A newer visible top-level reply at the bottom earns thread-activity:<root>, and an older nested mention under a collapsed sibling becomes read with its branch's "new" label gone. Reproduced at this head in Chromium and WebKit; the same probe passes on main. Keep replies that aren't mounted out of the automatic thread cutoff, or fall back to individual visible observation inside threads.