feat(web): highlight resolved mentions - #137
Conversation
|
Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 5:48 PM ET / 21:48 UTC. ClawSweeper reviewWhat this changesAdds post-Markdown highlighting for workspace-resolvable Merge readinessKeep this PR open: current Priority: P2 Review scores
Verification
How this fits togetherClickClack’s web client renders channel messages, thread replies, embedded views, and agent preambles from API data. Workspace member handles feed a post-Markdown decorator, which marks resolvable mentions before the Go API serves the compiled web bundle to browsers. flowchart LR
A[Workspace members] --> B[Mention target list]
C[Channel and thread content] --> D[Markdown renderer]
B --> E[Mention decorator]
D --> E
E --> F[Main and embedded views]
E --> G[Agent preambles]
F --> H[Go-served web assets]
Before merge
Agent review detailsSecurityNone. Review metricsNone. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the focused renderer-side decorator only after the exact head remains clean against current Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds a rendering capability rather than repairing a reproducible pre-existing failure. The PR body supplies after-fix Chromium output for all three intended rendering surfaces. Is this the best way to solve the issue? Yes, with moderate confidence: a post-Markdown decorator that excludes links, code, email-like text, and URL paths is a narrow way to add visual mention affordances without changing message storage, delivery, or Markdown parsing. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ca3c86ff6a8b. LabelsLabel changes:
Label 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 (26 earlier review cycles; latest 8 shown)
|
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.
Avoid blocking route loading on every member page
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.
Refresh mention targets after bot membership changes
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.
Re-run highlighting when rendered message HTML changes
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.
Exclude bare URL paths from mention matches
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.
Resolve mentions against all members in embedded views
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.
Highlight mentions in agent commentary preambles
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 👍 / 👎.
Expose the existing topic model in the web composer and timeline with resilient filtering, draft recovery, and paginated API support. Co-authored-by: PollyBot13 <pollybot13@gmail.com>
Add one persisted per-channel notification preference across browser, desktop, Pushover, API, and SDK surfaces, with resolved mention metadata and authorized thread-message hydration. Co-authored-by: Jacqueline Henriksen <jjjhenriksen@gmail.com>
Resolve workspace handles neutrally everywhere, promote current-user mentions only when the canonical channel notification preference allows attention, and cover normal, embedded, thread, and agent-preamble surfaces.\n\nCo-authored-by: Jacqueline Henriksen <jjjhenriksen@gmail.com>
Regenerate the complete Go-served asset graph from the reviewed mention sources.\n\nCo-authored-by: Jacqueline Henriksen <jjjhenriksen@gmail.com>
|
Exact-head
The previously orphaned asset graph is replaced by a full canonical regeneration. Current-user emphasis uses #116's server-backed channel preference; there is no second highlight preference or member cache. |
|
@clawsweeper re-review The complete Go-served asset graph and exact-head normal/embed/thread/preamble proof are now attached for |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
@handlementions from one complete, background-loaded member cache;allormentions, returning to neutral inmutedmode;Product coherence
This builds directly on #116. The server-backed channel preference remains the single attention policy: it controls alert delivery and whether a resolved current-user mention receives stronger emphasis. Semantic resolution itself remains visible in muted channels, so muting attention never changes message meaning.
Exact-head behavior proof
The focused Playwright scenarios run against the real Go server, embedded production web app, and fresh SQLite data. On exact head
f42712511105cf5144bf8b9c5a2b1f7b06d25747:The proof exercised:
muted→all→mentions→mutedwithout reload;The source-blind behavior contract and report are local verification artifacts, not committed to this branch.
Validation
pnpm check— passed, including web unit tests, Go tests, desktop tests, typechecks, lint, formatting, docs build, and embedded asset consistency.The generated bundle was rebuilt twice through the canonical web/SDK/embed path with identical output; both
index.htmland200.htmlreference the committedapp.zPNZ94rT.jsgraph.Related to #116.