Repository navigation
feat(channels): list unread top-level mentions in sidebar Activity, and count only unread mentions on the Inbox card - #2722
Conversation
6e747b0 to
a3e796e
Compare
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket, Mantis and Groot coverage.
Reviewed head a3e796e19b2f10692eefb60bd49f3cead07e01ab against base fb9cd7cf45c6d80368cc79a7c34535f8f3b2fd20. One P2 finding is attached inline: the new mention preview needs to honor the existing incomplete edit/deletion state. The review covered activity snapshots, Channels/Solo navigation, read-mark isolation, visibility tolerance, accessible labels, Inbox counts and regression-test source, preserving the documented non-goals.
Source-only: no tests, builds or app flows were executed, and author-reported results were not independently reproduced. The PR description and sampled frames across its 87.68-second demo were inspected; the recording identifies an earlier branch revision and is not current-head validation. Native behavior, assistive technology and the full theme/size matrix remain unverified.
a3e796e to
883090f
Compare
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 One browser-test regression blocks required CI.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord automated source review via Wes’s account, integrating independent Rocket, Mantis and Groot review lanes.
Focused follow-up at a699b471922f51bef6ac44ff102b76410bd9ac42 against base/merge-base 3bcf1c4bb1111d924a84d9b710c1da2db80ccf0b: no new actionable findings.
The prior top-level mention preview finding is addressed: mention rows subscribe to the existing Inbox feed and share its incomplete-preview rule for both visible text and accessible names. The new mounted regression covers a held overlay read, failure, retry and recovery. The repaired browser selectors match the current Activity page and list labels, addressing the other review’s selector concern. The rebased reveal/navigation context preserves ordinary sidebar mention opening.
This is a source-only repair review, retaining prior coverage of the unchanged feature patches. Tests, builds, current CI and app flows were not run or independently verified. The unchanged demo was inspected through historical sampled frames, not as validation of this head. No approval or merge recommendation is implied.
kazemek21
left a comment
There was a problem hiding this comment.
🤖 Kyle reviewed this PR and the Nexus review-tracker notes and approved with no comments.
Posted by Codex (gpt-6.1-sol) on behalf of kylez
a699b47 to
b6d0800
Compare
|
🤖 Design review (Design Reviewer agent, head b6d0800) Looks good from a design standpoint. A top-level mention now shows in the sidebar Activity popover, and the Inbox card counts only unread mentions, so both match what people actually need to look at. No must-fix issues. Optional: with mentions and unread threads in one popover, check that the two read as clearly different groups, and that a channel with many mentions doesn't push threads out of view. Not checked: I didn't run the app, so I didn't check dark mode, narrow windows, or busy channels with many agents mentioning you. |
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord automated source review via Wes’s account, integrating independent Rocket, Mantis and Groot review lanes.
Integration follow-up at b6d0800d63ac7f1dca77254adce8b7c88516b251 against base/merge-base cae3922574c5c1f0ce5d32231810229e94b1bb40: no new actionable findings.
The six previously reviewed feature patches are retained. The Solo callback and shared message-row adaptations preserve exact-message navigation and safe visible/accessibility previews. The earlier incomplete-preview P2 repair and Activity selector repair remain intact. We checked their interaction with the updated reading-observer lifetime, DM thread read boundaries, copied catch-up evidence and profile-owner cache invalidation, along with the relevant regression-test source. Documented non-goals remain unchanged.
Source-only: no tests, builds or app flows were run; performance and current CI were not independently verified. The current PR description and sampled demo frames were inspected; the attachment is unchanged and records earlier revisions, so it does not validate this head. Native behavior, assistive technology and the theme/size matrix remain unverified.
b6d0800 to
d1c91d1
Compare
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord automated source review via Wes’s account, integrating independent Rocket, Mantis and Groot coverage.
Integration follow-up at d1c91d1e5a536e868094923bc261e090d5ccdc20 against base/merge-base 5d9d1ef33c9fb3127497c2ecfba9819c0ebf9826: one low-priority P3 is attached inline. The new mention label uses retained unread totals while the rebased sidebar uses unseen row counts.
The six feature patches and prior incomplete-preview and Activity-selector repairs remain intact. The review checked their interaction with channel visits, foreground/background seen receipts, explicit unread intent, snapshot invalidation, retained Activity/Inbox counts and the mention browser journeys. Documented non-goals are preserved.
Source-only: no tests, builds or app flows were run; current CI and performance were not verified. The current description and historical sampled demo frames were inspected; the recording shows earlier revisions and does not validate this head. Native behavior, assistive technology and the theme/size matrix remain unverified.
| ? `Marked unread${snapshot.manual === "local-only" ? " on this device only" : ""}` | ||
| : `${count} observed unread messages${snapshot.freshness === "stale" ? "; may be out of date" : ""}. Not an exact total.`; | ||
| const threadLabel = `${threadCount} unread ${threadCount === 1 ? "thread" : "threads"}${activity.freshness === "stale" ? "; may be out of date" : ""}`; | ||
| const mentionCount = activity.mentions?.length ?? 0; |
There was a problem hiding this comment.
🤖 [P3] Derive the mention label from the row’s unseen evidence
This count bypasses the new snapshot.row filtering used by count, attentionCount and threadCount. After an active channel visit quiets two off-screen mentions without reading them, a later ordinary message makes the badge render again: it can announce “1 observed unread messages” followed by “2 unread mentions”, despite having no priority dot. A new unseen thread likewise restores the already-seen mentions in the combined label; label-bearing Solo rows can retain the mention text even while otherwise quiet. This conflicts with the sidebar-following-row contract in docs/unread.md:93–100; the popover retaining those unread mentions is correct.
Count top-level mentions that pass the existing rowCut check in the unread owner (alongside row.threadCount), include that field in snapshot equality, and use it here with the full-activity fallback when row is absent for explicit unread intent. Cover seen mention → ordinary arrival and seen mention → new thread in the mounted label tests, while preserving the mention in the popover. This is a source-inferred accessibility/count inconsistency, not a runtime reproduction.
… total
The sidebar Inbox card's header counted any conversation that had ever
mentioned you as an unread mention, so a read mention followed by a plain
reply read "1 mention unread" while Mentions with Unread only was empty.
It also gave a Solo chat's flat row no kind, so two unread rows could read
"1 mention unread".
The header now leads with the number of unread conversations, then counts
each once by its most specific kind ("2 unread · 1 mention"). A mention
counts only while its explicit-mention projection is unread, matching the
page's Mentions filter with Unread only; a Solo row alone reads "1 unread".
The summary stays non-interactive.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Sidebar Activity listed only unread thread replies, so a top-level
@mention never appeared in it. A row whose only unread item was a
mention opened no popover, and next to an unread thread a mention left
no trace: the dot still read "1 unread thread".
The unread engine now adds each channel's unread top-level mentions of
the viewer to its Activity snapshot, newest first, under a new optional
`mentions` field. A reply's mention stays under its thread; DMs and
broadcasts add none. The popover lists a Mentions section above Unread
threads. A mention opens at its message with no thread root, through
the existing ChannelInventory and Solo open paths, and Mark read writes
a `message` mark ("Could not mark this mention as read. Try again." on
failure). Revealing the popover still reads nothing.
There is still one dot. Its label and title name mentions too
("1 unread mention, 1 unread thread"), and a mention-only row keeps
its priority dot. Channel rows get mentions through a new optional
`onOpenMessage` prop on ChannelSidebarItem, wired to
openActivityMessage, so this rebases mechanically onto #2555.
docs/unread.md and docs/buzz-help.md describe the Mentions section.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Opening an off-screen top-level message exactly, such as a mention
from sidebar Activity, revealed and focused it but never read it.
Scroll offsets snap to whole pixels while rows sit at fractional
offsets, so scrollIntoView({ block: "start" }) left the row 0.047px
above the scroller (row top 96.953 vs 97 in Chromium and WebKit).
use-reading's shown() required the row to be strictly inside, so the
row stayed unread, and stayed in Activity, until the reader scrolled.
shown() now allows one pixel at each edge, like the 1px slack the
bottom check already uses. It covers any fractional scroll position,
not just this reveal. It can't count a visibly clipped row: the
timeline has no sticky header or scroll padding over rows, and
shown() measures against the scroller box, so overlays were never
counted anyway. A cached-window blip that opens the message as a
lone tab is a separate gap and is unchanged.
A new use-reading case pins the tolerance at the top and bottom
edges. A new browser journey opens a mention above the bottom from
Activity: it is revealed, focused and read, leaves Activity, and its
Inbox row loses Unread. Both fail without the fix. docs/unread.md
notes the tolerance.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
On a row whose only Activity is top-level mentions, nothing named them
to a screen reader. The priority dot is aria-hidden, and the activity
label only rendered with a thread, so the row read "Beta 1 observed
unread messages. Not an exact total." A muted row hides the priority
dot too, so the label was the only place to name them.
The badge now adds a screen-reader-only "N unread mention(s)", with the
same "; may be out of date" suffix, whenever there are mentions and no
thread, muted or not. With a thread, the dot's label already names
both, so nothing is announced twice. The visible dot, its CSS and
[data-channel-priority] are unchanged. In Chromium's accessibility tree
and Playwright's name in Chromium and WebKit, the row now reads "Beta 1
observed unread messages. Not an exact total. 1 unread mention", so
name selectors like /^Beta\b/ still match.
ChannelSidebarItem's onOpenMessage is now required, so a row can't list
a mention that opens nothing. ChannelInventory, its only consumer,
already passes it. Test renders and two browser fixtures pass it too.
The ThreadActivitySnapshot.mentions doc now says a reply whose thread
root is unresolved is in neither section. Behaviour is unchanged.
New ChannelSidebarMenuItem cases cover the mention-only label, muted
and unmuted, and that it goes away once a thread arrives. They fail
without the badge change. unread.test.ts now pins three cases with no
production change: a quiet audience=agents mention is never listed, a
top-level mention with an unmarked or quote e tag still is, and
activity("room") keeps its identity through an unrelated publish. Each
fails against a mutation that breaks it. docs/unread.md now covers the
mention-only label and the muted-row sentence.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Opening Activity can admit an unread top-level mention before its stored edit and deletion reads finish. Until they settle, or after they fail, the message ID stays in the Inbox feed's incomplete list and the retained body may be superseded. The Activity page hides it behind a placeholder, but sidebar Activity's mention rows showed it as current, both visibly and in the row's accessible name. Mention rows now subscribe to the existing feed snapshot and apply the same rule: "Preview updating…" while the reads are pending, "Preview unavailable. Retry activity." after they fail, and the body once they settle. The rule moves out of InboxRowButton into an exported incompletePreview helper that both surfaces use, so the strings live in one place. It adds no fetch or cache. Thread rows are unchanged. A new mounted popover test drives a real Inbox feed through a held edit/deletion read, its failure and a retry. It checks the placeholder in the visible text and the accessible name, that the body appears nowhere while the ID is incomplete, and that the body returns after recovery. It fails without the fix. Popover tests that render rows now give their fake session a settled feed. docs/unread.md describes the placeholder. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
#2543 renamed the Inbox page to Activity, so the two top-level mention journeys this branch adds could no longer find it. They opened the page as "Inbox" and looked for the "Inbox" region and the "Inbox conversations" list. They now open "Activity" and use the region and list named "Activity", matching what the page exposes on main and the other Activity journeys. Nothing else changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
d1c91d1 to
773f0b1
Compare
…uctions * origin/main: refactor(settings): move external identity authorization to security (#3032) test(typeahead): serve the fixture pages as a build so cold loads fit (#2793) feat(channels): list unread top-level mentions in sidebar Activity, and count only unread mentions on the Inbox card (#2722) fix(solo): delete a removed workspace's settings after the groups write (#2819) fix(solo): ask before dismissing chat and workspace settings drops edits (#2616) fix(messages): open the forwarded message from Forward's confirmation (#2802) fix(pairing): tell the tool why its relay was refused, and refuse at once when Buzz can't ask (#2823)
Picks up c202642 (block#2837), which fixes the ThreadPanel catch-up test that fails on every run of a merge with the older main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * origin/main: (155 commits) deps: accept RUSTSEC-2026-0459 for libsqlite3-sys 0.38, move rmcp to 3.9.0 (block#3077) fix(communities): accept invite links and bare hosts in Join a community (block#2412) fix(builderlab): open a remote agent's Activity on the conversation it was opened from (block#3051) refactor(builderlab): keep capabilities fresh in the session (block#3003) feat(moderation): show invite links as a QR code (block#3029) Keep Create agent's Model row readable and give Goose a Thinking row (block#2673) Join a community right after creating it in Settings (block#2991) Let new Solo chats start temporary by default (block#2874) fix(messages): fill a screen that a head of folded agent activity leaves empty (block#3037) fix(search): search saved conversations while the community is offline or connecting (block#2724) test(agents): make version-history date assertion time-zone independent (block#3010) feat(relay): gate artifact writers on NIP-AR support (block#2735) fix(channels): open the dock's community switcher from Personal space (block#2806) fix(members): keep the list's place after a member change (block#2818) fix(channels): keep a launcher's side panel open on Close all tabs (block#2803) fix(activity): show a remote agent's Buzz send as its reply (block#3025) refactor(settings): move external identity authorization to security (block#3032) test(typeahead): serve the fixture pages as a build so cold loads fit (block#2793) feat(channels): list unread top-level mentions in sidebar Activity, and count only unread mentions on the Inbox card (block#2722) fix(solo): delete a removed workspace's settings after the groups write (block#2819) ... Signed-off-by: Andrew Harvard <aharvard@squareup.com>
…pover-card-layout-7eeda1e0 * origin/main: (128 commits) deps: accept RUSTSEC-2026-0459 for libsqlite3-sys 0.38, move rmcp to 3.9.0 (#3077) fix(communities): accept invite links and bare hosts in Join a community (#2412) fix(builderlab): open a remote agent's Activity on the conversation it was opened from (#3051) refactor(builderlab): keep capabilities fresh in the session (#3003) feat(moderation): show invite links as a QR code (#3029) Keep Create agent's Model row readable and give Goose a Thinking row (#2673) Join a community right after creating it in Settings (#2991) Let new Solo chats start temporary by default (#2874) fix(messages): fill a screen that a head of folded agent activity leaves empty (#3037) fix(search): search saved conversations while the community is offline or connecting (#2724) test(agents): make version-history date assertion time-zone independent (#3010) feat(relay): gate artifact writers on NIP-AR support (#2735) fix(channels): open the dock's community switcher from Personal space (#2806) fix(members): keep the list's place after a member change (#2818) fix(channels): keep a launcher's side panel open on Close all tabs (#2803) fix(activity): show a remote agent's Buzz send as its reply (#3025) refactor(settings): move external identity authorization to security (#3032) test(typeahead): serve the fixture pages as a build so cold loads fit (#2793) feat(channels): list unread top-level mentions in sidebar Activity, and count only unread mentions on the Inbox card (#2722) fix(solo): delete a removed workspace's settings after the groups write (#2819) ...
Two summaries of what needs your attention got mentions wrong. The sidebar Activity popover listed only unread thread replies, so a top-level @mention never appeared in it: a channel with only a mention opened no popover, and a mention next to an unread thread left no trace. The sidebar Inbox card's header counted any conversation that had ever mentioned you as an unread mention, and gave Solo rows no kind, so its numbers disagreed with the Inbox page.
Activity now lists a channel's unread top-level mentions under Mentions, above Unread threads, and the single dot's label names both. The Inbox card leads with the total and counts a mention only while it is unread.
Before / after
attention-sbs.mp4
Left is
main@ b621738, right is this branch @ f404497. Both halves run the same flow with the same fixture data, and each step is padded to a fixed time so they stay aligned. Every claim on screen is checked by an assertion in that half.Recorded in Chromium at 1280×800 with synthetic fixture data.
What changed
Four commits, each landable on its own review.
fix(inbox): the card counts unread mentions and leads with the total (InboxPreview.tsx, +5/−3)item.mentionedkeeps meaning "ever mentioned", because the page uses it to list read mentions too.feat(channels): Activity lists unread top-level mentionsunread.ts:ThreadActivitySnapshotgains an optionalmentionslist (MentionActivityItem), newest first. IncomputeActivity, top-level entries (no thread reference at all, not just no root) take a branch before the reply skip: listed whencategory()is "mention" and the message is unread. DMs are "direct", so they never have one; broadcasts alone and mentions inside replies are left out.equalActivitycompares mentions, so an unrelated publish keeps the snapshot's identity.ChannelActivityPopover.tsx:ActivityRowtakes a noun, an open handler and a read function. A Mentions section reuses the section heading.hasActivity, the stale check, the hover gate and the profile prefetch include mentions, so a mention-only row opens on hover, focus + Enter and click. Mark read writes amessagemark; on failure the row shows "Could not mark this mention as read. Try again." Revealing the popover reads nothing.UnreadBadge.tsx: still one dot. Its label and title read "1 unread mention, 1 unread thread". A row with only mentions keeps its priority dot, which already looks the same.ChannelSidebarItemgetsonOpenMessage(channelId, messageId);ChannelInventorypassesopenActivityMessage, andSoloInventoryuses itsopenMessage. A mention opens with no thread root. Once fix(channels): open the unread reply from sidebar Activity #2555 lands, this folds into its widened(channelId, messageId, rootId?)callback.fix(messages): read a revealed row that lands a sub-pixel past the edge (use-reading.ts)scrollIntoView({ block: "start" })left the row 0.047px above the scroller (top 96.953 against 97, in Chromium and WebKit), andshown()required the row to be strictly inside. It stayed unread, and stayed in Activity, until the reader scrolled.shown()now allows one pixel at each edge, like the 1px slack the bottom check already uses. Bottom catch-up is unchanged. This also fixes message links to an off-screen message.shown()has never allowed for them; the extra pixel makes no difference to that.fix(channels): announce a mention-only row's mentions to assistive tech (UnreadBadge.tsx)aria-hiddenand the activity label rendered only with a thread, so a mention-only row read "Beta 1 observed unread messages. Not an exact total." It now adds a screen-reader-only "N unread mention(s)", muted or not; with a thread, the dot's label already names both.onOpenMessagebecomes required.Docs:
docs/inbox.md(the header rule),docs/unread.md(Mentions in the popover, one dot, the pixel tolerance, muted rows),docs/buzz-help.md(a "Sidebar activity" bullet).Not in this PR
docs/unread.md, bottom-edge catch-up). Landing at the "New messages" divider instead needs a product call and stacks after fix(channels): keep the reader's place when Back returns to a link-opened channel #2500 and fix(channels): open a thread's channel at its root message from the #channel link #2377.@channel/@heremessages are not listed under Mentions, matching the Inbox's Mentions filter.Tests
Every new test fails without its production change.
InboxPage.test.tsx: a new case walks the header through "3 unread · 1 mention · 1 thread" → "1 unread" (Solo alone) → "2 unread · 1 mention" → "Nothing unread" → "1 unread · 1 thread" after a plain reply. On the old code it reads "1 mention · 1 thread unread"; a wording-only change that still countsitem.mentionedfails at the last step with "1 unread · 1 mention". Three existing strings updated.unread.test.ts: mentions in the snapshot, newest first, with a broadcast and a reply's mention excluded and a DM's list empty; Mark read; a quietaudience=agentsmention never listed; a mention with a quote or unmarkedetag still listed;activity("room")keeps its identity through an unrelated publish.ChannelActivityPopover.test.tsx,ChannelSidebarMenuItem.test.tsx(dot label; mention-only label muted and unmuted, and gone once a thread arrives),ChannelInventory.test.tsxandSoloInventory.test.tsx(a mention opens with no thread root),use-reading.test.ts(rows 0.047px above the top or 0.5px below the bottom count as seen; 1.5px past either edge don't).tests/browser/thread-unread.spec.mjs, "top-level mention activity": a mention-only row opens Activity, and opening its mention reads it; opening a mention above the bottom from Activity reveals and focuses it without scrolling to the bottom, reads it, removes it from the popover and clears its Inbox row. Both fail onmainin Chromium and WebKit; 20/20 repeats with the fix.tsc --noEmit, lint and format clean; full vitest 17,917 passed, 29 skipped; 224/224 across 18 reading and reveal browser specs in both engines.Landing
computeActivitycondition; the new top-level branch sits above it.InboxPreviewimports and strings, not the count loop.🤖 Generated with Claude Code