feat(web): highlight resolved mentions - #137
Conversation
clawsweeper
Bot
added
proof: 📸 screenshot
|
Codex review: needs real behavior proof before merge. Reviewed August 1, 2026, 1:33 PM ET / 17:33 UTC. ClawSweeper reviewWhat this changesAdds presentation-only highlighting for resolvable workspace-member Merge readiness⛔ Blocked until stronger real behavior proof is added - 5 items remain Keep open. Current Priority: P2 Review scores
Verification
How this fits togetherClickClack renders stored chat and agent text as Markdown in the main application and shareable embedded views. This change derives known workspace members, decorates matching rendered text nodes, and leaves message storage and delivery unchanged. flowchart LR
A[Workspace member API] --> B[Mention target list]
C[Rendered Markdown] --> D[Resolved mention decorator]
B --> D
D --> E[Main chat and thread views]
D --> F[Embedded channel and thread views]
D --> G[Agent commentary views]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the decorator presentation-only, resolve the failing TypeScript check, and add redacted runtime proof that shows the same resolved handle highlighted in an embedded view and an agent-commentary preamble before merging. Do we have a high-confidence way to reproduce the issue? Not applicable as a feature request. The PR body provides a high-confidence live ordinary-message result, but it does not provide equivalent after-fix evidence for the embedded and agent-commentary paths. Is this the best way to solve the issue? Yes in principle: decorating post-render Markdown against resolvable workspace members preserves storage and delivery behavior. The remaining requirement is proof that the shared approach works in each claimed renderer and a passing TypeScript check. AGENTS.md: found, but no applicable review policy affected this item. Codex review notes: model internal, reasoning high; reviewed against 4843afe2e8eb. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (14 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
removed
the
proof: 📸 screenshot
clawsweeper
Bot
added
proof: 📸 screenshot
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1acfbb95ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| do { | ||
| const page = await listWorkspaceMembersPage({ | ||
| workspaceID, | ||
| cursor, | ||
| limit: 100, | ||
| }); | ||
| members.push(...page.members.map((member) => member.user)); | ||
| cursor = page.has_more ? page.next_cursor : undefined; | ||
| } while (cursor); |
There was a problem hiding this comment.
When a workspace has more than 100 members, this loop issues one request per page serially, and applyRoute awaits the entire loadWorkspaceMembers call before resolving the route target and loading its messages. Opening or switching a large workspace can therefore leave the conversation blocked behind dozens or hundreds of member-list requests; load these targets in the background or resolve only the handles needed for visible messages.
Useful? React with 👍 / 👎.
| if (serial !== workspaceMembersLoadSerial || workspaceID !== selectedWorkspaceID) return; | ||
| workspaceMemberUsers = members; |
There was a problem hiding this comment.
This assignment creates a snapshot that is only refreshed when the workspace changes. The existing bot.deleted and bot.membership_removed realtime handlers reload direct conversations, moderation data, and commands but never this list, so a removed bot's old handle remains highlighted as resolvable—and a newly added or replacement bot is absent—until the user reloads or switches workspaces.
Useful? React with 👍 / 👎.
| return { | ||
| update(nextPeople: User[] = []) { | ||
| targets = mentionTargets(nextPeople); | ||
| renderMentionHighlights(node, targets); | ||
| }, |
There was a problem hiding this comment.
The action updates only when mentionPeople changes, not when the element's {@html markdown(message.body)} content changes. For example, a realtime edit of a reply in the currently open thread replaces the reply HTML while the workspace people array remains unchanged, leaving mentions in the edited body unhighlighted until some unrelated people-list update or remount; observe child mutations or pass the body/version into the action.
Useful? React with 👍 / 👎.
| function mentionIsInsideURL(text: string, start: number): boolean { | ||
| const tokenStart = | ||
| Math.max(text.lastIndexOf(" ", start - 1), text.lastIndexOf("\n", start - 1)) + 1; | ||
| const prefix = text.slice(tokenStart, start); | ||
| return /^(?:www\.[^\s/]+|[A-Za-z][A-Za-z0-9+.-]*:\/\/[^\s/]+)\//u.test(prefix); |
There was a problem hiding this comment.
The URL check only recognizes prefixes beginning with www. or a URI scheme. Plain URL-like paths such as /users/@alice and docs/path/@alice?tab=1 therefore highlight @alice, despite the feature's intended exclusion of URL paths; detect slash-delimited path tokens as well as fully qualified URLs.
Useful? React with 👍 / 👎.
| <MessageList | ||
| {messages} | ||
| selectedChannel={channel} | ||
| {mentionPeople} |
There was a problem hiding this comment.
The embedded channel passes a mentionPeople list derived only from the authors in the currently loaded message window plus the current user, capped at 24 entries; EmbedThreadView similarly uses only conversation members and loaded authors. Consequently, a valid workspace mention remains plain whenever its target has not authored one of those loaded messages, so embedded views do not consistently provide the resolved-mention treatment added here; fetch or otherwise resolve workspace targets for embeds as well.
Useful? React with 👍 / 👎.
| {:else} | ||
| <QuoteBlock {message} onJump={onJumpToQuote} /> | ||
| <div class="markdown" use:enhanceMarkdown>{@html markdown(message.body)}</div> | ||
| <div class="markdown" use:enhanceMarkdown use:enhanceMentions={mentionPeople}>{@html markdown(message.body)}</div> |
There was a problem hiding this comment.
The new action is attached only in the ordinary-message branch, while coalesced agent_commentary rows take the earlier PreambleBlock branch and render their commentary Markdown with only enhanceMarkdown. A bot message such as Handing this to @alice therefore remains unhighlighted when it is displayed as a preamble, which omits the semantic treatment from the agent-handoff content this feature is intended to make scannable; pass the mention targets into PreambleBlock and enhance its commentary bodies too.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
rating: 🦐 gold shrimp
1 participant
What changed
@handlementions in normal channel/thread messages, embedded channel/thread views, and agent commentary preambles.Why
ClickClack already offers workspace people and bots in the
@composer menu, and #116 adds durable mention resolution for notification delivery. Rendered messages still showed those handles as ordinary text, making agent handoffs difficult to scan.The decorator runs after Markdown rendering and changes only visible presentation. Message storage, delivery, and Markdown parsing are unchanged.
Codex review revisions
Addressed the six actionable findings from the Codex connector review on
1acfbb95ed:/users/@aliceare excluded;Browser proof
Chromium loaded the branch's actual Vite-served mention resolver and renderer modules. With 30 workspace members, a message mentioning
@member-29—past the old 24-item cutoff—produced a live DOM mark:The browser snapshot visibly showed
@member-29highlighted. The same renderer excludes URL paths and inline code. Embedded and preamble wiring is now in the production build and covered by the source/type/build checks below; no private workspace data is included in this proof.Validation
node --test apps/web/src/lib/*.test.ts— 20 passed.node_modules/.bin/svelte-kit syncand TypeScript typecheck — passed.git diff --check— passed.b5be20d.Related to #116.
AI-assisted implementation and review.