Skip to content

Show selected mentions as inline identity chips - #147

Merged
wesbillman merged 2 commits into
mainfrom
morganm/mention-chips
Sep 23, 2026
Merged

wesbillman merged 2 commits into
mainfrom
morganm/mention-chips

Conversation

@morgmart

@morgmart morgmart commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

What this does

Selected people and agents now appear as shared inline chips directly in the composer, replacing the separate recipient-avatar row. When two selected identities share a name, both chips show a short public-key ending, such as @Honey · npub…4hu, so the person writing can tell them apart.

The earlier chip gently reveals its qualifier when ambiguity first appears. Reduced motion keeps the information without animation. The playground includes two Alice people and two Honey agents to try it.

Why it matters

The mention and its notification intent are visible in one place. Deleting or editing the mention removes that intent through the existing editor behavior, rather than a separate recipient control.

The qualifier is display-only: copied text, saved drafts, source positions, and published recipient keys stay unchanged. Chips inside the editor have no extra buttons, focus stops, hover previews, or click actions.

How it works

The composer supplies its own identity labels to the existing design-system chip instead of consulting a global identity registry. A shared public-key formatter lengthens suffixes when necessary to distinguish a namesake group. The composer owns ambiguity and reveal history; the shared chip owns presentation and motion tokens.

This builds on #119 and preserves #137’s exact-recipient, remembered-agent, channel/reply, and restored-draft behavior. It does not redesign the picker or prevent selecting the same agent twice; duplicate prevention remains BOT-2033. Coordinate with #142 before merging both: its recipient presentation overlaps this replacement, while its agent-directory changes are intentionally outside this PR.

Visual evidence

Synthetic playground, one selected agent:

One selected Honey shown as a name-only chip

Adding a different Honey qualifies both:

Two Honey chips distinguished by public-key endings

Narrow layout, dark mode:

Namesake chips wrap inside the narrow dark composer

Verification

At 9d7c63c84ee81c9d8f443eb9a9b9ba7536d54ea5, after integrating current main:

  • Complete Vitest suite: 2,139 tests passed, using bin/pnpm exec vitest run --maxWorkers=4.
  • Mention and playground browser files: 8 passed across Chromium and WebKit. Exercises actual publication keys, copy serialization, delete/undo, source offsets, qualifier motion, reduced motion, inertness, both themes, and narrow/intermediate/wide layouts.
  • Design-system suite: 76 passed; app and design-system builds passed.
  • Mandatory commit/push hooks passed, including type checks, 163 related tests, design-system types and guards. No hooks bypassed.
  • Independent test-honesty and product/design review: clear. Fixed a reproduced animation replay when removing and re-adding the first namesake; the regression failed before the fix and now covers both removal orders.

Browser coverage adds one playground case per engine and extends the existing namesake journey. Existing recipient-control assertions now exercise inline chips and authored-span deletion while retaining exact identity assertions. Browser coverage is necessary for native selection/copy, noneditable token geometry, wrapping, and animation; group/collision and lifecycle matrices remain in unit tests.

Earlier validation on the pre-rebase tree: seven changed browser files passed 63/64, with an intermittent WebKit media-review handoff failure; the entire affected file then passed 8/8 on rerun. The default-worker unit run hit two timeouts in page activation and read-state growth; both files passed unchanged together, and the complete four-worker run passed. These results do not establish that the original failures were pre-existing or fixed.

CI and native desktop validation remain pending/not performed for this frontend-only change. Non-blocking review follow-ups: synthetic composition-event remounts can replay a reveal (not reproduced through realistic IME cancellation), and qualifier changes have correct accessible names but no separate live announcement. Neither expands the approved behavior in this PR.

Review follow-up: visible recipients inside Markdown

At dfc5a1f3f3a12b8249acfed6a0bfe95c6eb267a3, selected mentions stay visibly identifiable inside code, images, HTML, link labels, and Markdown depth fallback. Explicit recipient chips take precedence over inferred decoration; typed prose remains non-notifying.

Nine new unit regressions failed before the fix. The complete unit suite now passes 2,148 tests with four workers; both affected browser files pass 6/6 across Chromium/WebKit. One added journey per engine verifies real picker insertion into code, restored channel/reply drafts, copy text, and exact publication keys. Mandatory hooks passed; new-head CI and reviewer recheck are pending. Earlier results above remain historical, not new-head claims.

Use shared inert chips in the composer and distinguish selected namesakes without changing authored text or notification identity. Preserve draft editing and add clipboard, motion, and responsive coverage.

Co-authored-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com>
@morgmart
morgmart marked this pull request as ready for review September 22, 2026 23:08
@morgmart
morgmart requested review from a team, comp615 and wesbillman as code owners September 22, 2026 23:08

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Reviewed head 9d7c63c84ee81c9d8f443eb9a9b9ba7536d54ea5 against base c5762f47830cac9b29a6bd5eaf3d28e02312a209.

Changes requested

P2: Selected mentions inside Markdown literals lose all notification disclosure

Removing RecipientAvatars makes the inline decoration the only visible record of an explicit recipient. However, RichComposerInput still suppresses that decoration when it overlaps an inline-code/code/image/HTML literal (src/features/messages/RichComposerInput.tsx:91-137). Inserting a selected mention in such a range is still accepted, retained in the draft, and sent as an exact recipient.

Verified reproduction with the actual MessageComposer, MentionPicker, and createMessages implementations in both Chromium and WebKit:

  1. Enter a backtick, a space, and a closing backtick; place the caret immediately after the opening backtick.
  2. Open Mention a member and select Honey.
  3. The draft becomes `@Honey `. There is no inline chip and no Notification recipients disclosure.
  4. Send. The emitted kind-9 event still contains ["p", "aaaaaaaa…"] for the selected identity.

The same focused comparison against the base retains the separate recipient disclosure for this draft. This is therefore a disclosure regression introduced by its removal, not a new claim that the underlying Markdown suppression is itself new. Namesakes are particularly problematic: the composer can no longer show which exact identity will be notified.

Required outcome: every retained explicit recipient remains visibly identifiable before send, including recipients whose normal decoration is suppressed. Preserve the inline-only design if desired, but reconcile the literal/decoration filter with explicit recipient intent rather than allowing invisible recipients. Add regression coverage for insertion inside a literal, restoration of that draft, and the resulting channel/reply recipient keys. Do not silently change existing notification semantics merely to hide the inconsistency.

Other reviewed lanes found no additional material blocker: scoped identity presentation, suffix collision handling, shared chip accessibility/motion, editor source serialization, and the migrated tests. Hosted CI is green. Focused probes used synthetic session/outbox data and existing dependencies; no live notifications or native packaged-app validation, and no broad CI-equivalent suites rerun.

Give explicit recipients precedence over inferred decorations without changing source or notification semantics. Cover literal and depth fallback disclosure, restored drafts, copy, and exact channel/reply publication.

Co-authored-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com>
@morgmart

Copy link
Copy Markdown
Contributor Author

Carl, an automated reviewer, commenting via Morgan’s GitHub account.

Fixed in dfc5a1f. Explicit selected recipients now take precedence over inferred decorations and remain visible inside Markdown literals or parser-depth fallback. Ordinary typed mentions still do not become notification recipients. No source, picker, or recipient-publication semantics changed.

Nine unit regressions failed on the prior code and pass with the fix: inline/fenced/indented code, images, image references, definitions, HTML, link labels, and deep Markdown. They check both namesake identities before/after draft restoration, unchanged source, plain typed mentions, and exact recipients.

At the new commit, the complete unit suite passed (2,148 tests, four workers). Both full affected browser files passed in Chromium and WebKit (6 cases), including real picker insertion inside code, restored channel/reply drafts, exact copy text, and published channel/reply recipient keys. Mandatory hooks passed. Independent scoped review found no remaining blocker. New-head CI and reviewer recheck remain pending.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Re-review: prior blocker resolved; no remaining material findings

Reviewed head dfc5a1f3f3a12b8249acfed6a0bfe95c6eb267a3 against base c5762f47830cac9b29a6bd5eaf3d28e02312a209, concentrating on the three-file follow-up since reviewed head 9d7c63c84ee81c9d8f443eb9a9b9ba7536d54ea5.

The previous P2 recipient-disclosure blocker is resolved. RichComposerInput now gives explicit recipient spans precedence over inferred decorations and does not suppress them inside Markdown literals or the depth fallback. It leaves authored text, recipient metadata, persistence and send routing under their existing owners. Typed names still do not acquire notification intent.

Validation:

  • Traced decoration precedence through the editor's source-backed tokens, copied text, draft restoration and channel/reply submission. Reviewed the nine literal/depth unit regressions; an independent coverage audit found no material gap.
  • On the clean exact head, ran both complete affected browser files, mentions.spec.mjs and composer-links.spec.mjs, in Chromium and WebKit: 6/6 passed. The new real-picker journey verifies both visible namesake chips inside code, channel/reply draft restoration, exact copied/sent source without visual qualifiers, exact selected p keys, channel h, and reply-only e routing. Existing link-editing and recipient journeys passed alongside it.
  • Hosted automatic checks are green at this head, including all four browser shards, JavaScript, Rust/tool integration and CI required; DCO and security checks are green. Full unit/broad validation relies on hosted CI, not a duplicate local run.

No additional material defect found in this bounded re-review. Synthetic browser fixtures only: no live relay notifications, native packaged-app, physical IME or screen-reader acceptance. This is a COMMENT review, not approval or permission to merge.

@wesbillman
wesbillman merged commit 877ae2a into main Sep 23, 2026
12 checks passed
@wesbillman
wesbillman deleted the morganm/mention-chips branch September 23, 2026 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants