Skip to content

perf(messages): host one context menu per message list - #3465

Merged
kalvinnchau merged 3 commits into
mainfrom
peon/perf-message-context-menu
Nov 13, 2026
Merged

kalvinnchau merged 3 commits into
mainfrom
peon/perf-message-context-menu

Conversation

@kalvinnchau

Copy link
Copy Markdown
Contributor

Problem

#3316 added right-click message actions by wrapping every toolbar message row in its own Base UI ContextMenuRoot and ContextMenuTrigger. Each of those mounts costs root state, floating-ui setup, and trigger listeners. With a full timeline that adds up to about a tenth of list render script time, and a quarter or more of long-animation-frame blocking while scrolling.

Bisect against v1.0.1 isolated this commit as the per-row cost. Box A/Bs then split the cost into parts:

  • A row with no menu at all recovers almost the whole regression.
  • A Base UI root and trigger per row with no props costs nearly as much as main.
  • The cost is therefore the per-row Base UI menu, not the action props.

Change

MessageContextMenus in MessageActionBar.tsx is one real Base UI context menu per message list. It wraps the channel timeline feed and the thread panel's rows.

  • Rows inside a host. Each toolbar row registers its actions by message ID, in an effect, and renders only its content.
  • Choosing the menu. A right-click, or the ContextMenu / Shift+F10 key, over a registered row selects that row's actions. Presses outside a registered row keep the browser's native menu, as do the existing native targets (links, media, fields, selection).
  • Gestures. Base UI still owns every gesture: mouseup guards, outside-press dismissal, and switching to another row while a menu is open.
  • Focus. The popup renders inside the list's React tree, so the timeline's focus bookkeeping still sees focus enter a row's menu, as it did with a row's own menu.
  • Updates and removal. The open menu follows its row's latest actions and closes if the row unmounts. The host releases the row when the menu closes.
  • Rows outside a host keep their own menu: standalone uses, media review, activity and huddle. Coordination rows keep their own trigger too, and Base UI stops their event before it reaches the host.

ThreadPanel.tsx only wraps the rows element. Its diff is indentation; git diff -w shows 3 lines.

Evidence

Interleaved A/B on 9 Linux and 9 macOS boxes:

  • 6 rounds × 2 samples per variant on each box, so n=108 per variant per OS.
  • 4× CPU throttle.
  • Values are the mean of per-box medians.

The baseline is main (7c786bb4) with #3464 applied (90580d91). This change was measured stacked on top of it (3caa47b0). The final commit differs from the measured one only in menu open, close and focus handling, which these scenarios do not exercise.

case metric Linux base Linux PR Δ macOS base macOS PR Δ
fast scroll paging (markdown) script ms 6148.8 5600.0 −8.9% 3859.6 3601.6 −6.7%
task ms 11384.7 10831.0 −4.9% 7206.1 6907.9 −4.1%
LoAF blocking ms 1849.6 1475.8 −20.2% 138.3 82.4 −40.4%
channel open with hover script ms 273.1 246.2 −9.9% 162.9 150.2 −7.8%
task ms 465.7 438.7 −5.8% 276.0 262.2 −5.0%
LoAF blocking ms 260.3 242.4 −6.9% 126.0 118.9 −5.6%
cold channel open script ms 292.7 267.6 −8.5% 184.8 172.2 −6.8%
task ms 515.5 492.4 −4.5% 332.9 320.4 −3.7%
LoAF blocking ms 245.1 230.1 −6.1% 106.9 98.6 −7.7%
rich thread open script ms 2417.1 2275.4 −5.9% 1138.8 1102.8 −3.2%
task ms 5020.0 4842.7 −3.5% 2385.8 2356.3 −1.2%
LoAF blocking ms 2335.3 2175.1 −6.9% 895.8 852.3 −4.8%
warm channel switch script ms 133.1 129.3 −2.9% 91.0 89.8 −1.3%
LoAF blocking ms 116.2 112.3 −3.4% 49.0 48.3 −1.3%

All 9 boxes improved on both OSes for every row except warm channel switch, which is small and mixed (Linux script 9/9, the rest 6–7/9). Style time is unchanged (±1.3%).

Checks

  • vitest run src/features/messages src/features/conversation: 3286 pass. New unit tests cover:
    • which row's actions open
    • the native menu outside rows
    • closing when the row is removed
    • keyboard open
    • focus staying in the list
    • The removal and focus tests were each confirmed to fail without their fix.
  • Browser specs pass on chromium and webkit: message-context-menu, menu-dismiss, message-actions, message-actions-floating, buzz-links, external-links, thread-composer-focus, find-on-screen.
  • tsc --noEmit and biome check are clean.

🤖 Generated with Claude Code

@kalvinnchau
kalvinnchau force-pushed the peon/perf-message-context-menu branch 2 times, most recently from 714a7ef to 1925c36 Compare November 13, 2026 08:02
@kalvinnchau
kalvinnchau marked this pull request as ready for review November 13, 2026 15:29
@kalvinnchau
kalvinnchau requested a review from a team as a code owner November 13, 2026 15:29

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 one blocking finding, inline: plugin menu contributions in the right-click menu lose the row's TargetOpenerProvider, so they can't open targets anymore. the rest of the hosted menu looks right to me: a row unmounting closes its open menu, remounts are guarded by the identity check in the cleanup, and Forward still returns focus to the row's ⋯.

kalvinnchau added a commit that referenced this pull request Nov 13, 2026
…iews (#3466)

## Problem

Link previews (#3219, #3225) find a message's preview URLs with
`previewLinks`. For any body with an `https://` link, that function
called `prepareMarkdown`, then scanned the prepared content again with
`scanMarkdown`, and kept no result. The renderer already prepares the
same body through its own cache.

A link-bearing row therefore parsed its Markdown two or three more times
than a row without links, on every mount. It did this again on each
remount while scrolling or switching channels.

## Change

- `markdown-preparation.ts`:
- Owns the cache of recent prepared bodies. `recentResults` and
`preparedBodies` move here from `MessageMarkdown.tsx`, unchanged.
  - Exposes it as `preparedMarkdown(content, scanned?)`.
- `prepareMarkdown` takes an optional `scanned` callback that receives
the scan it made. It stays pure and keeps no state.
- `MessageMarkdown.tsx` reads through `preparedMarkdown`, with the same
cache and budget as before.
- `preview-links.ts`:
- Uses `preparedMarkdown`. On a fresh preparation it takes the scan
through the callback instead of scanning again.
- Keeps recent preview link lists in a `recentResults` cache. The key is
the content and exclusions; the weight counts both the key and the URLs.
Cached lists are frozen.
- `preview-links.test.ts` asserts that the renderer and previews
together scan a body once. This fails on the previous code.

## Why behaviour is unchanged

- `prepareMarkdown` is pure, so a cached prepared body is identical to a
fresh one.
- The scan passed to the callback is the scan of the returned `content`,
the same input previews scanned before.
- The link list depends only on content and exclusions, which are both
in the cache key.

## Evidence

Interleaved A/B on 9 Linux and 9 macOS boxes:
- 6 rounds × 2 samples per variant on each box, so n=108 per variant per
OS.
- 4× CPU throttle.
- Values are the mean of per-box medians.

The baseline is `main` (`7bb45482`) with #3464 and #3465 applied. This
change was measured on top of that. The measured revision handed the
scan over through module state instead of a callback; the parse count is
the same. A variant with preview scanning disabled entirely is the
ceiling.

| case | metric | Linux base | Linux PR | Δ | macOS base | macOS PR | Δ
| ceiling Δ (Linux / macOS) |
|---|---|---|---|---|---|---|---|---|
| thread scroll open | script ms | 636.4 | 618.3 | −2.8% | 381.4 | 368.2
| −3.5% | −2.9% / −3.7% |
| | task ms | 997.3 | 976.1 | −2.1% | 600.6 | 588.1 | −2.1% | −1.4% /
−2.0% |
| | LoAF blocking ms | 551.7 | 534.3 | −3.1% | 271.2 | 258.4 | −4.7% |
−2.5% / −5.4% |
| | first rows ms | 937.1 | 914.1 | −2.5% | 561.5 | 546.2 | −2.7% |
−1.8% / −2.7% |
| cold markdown channel open | script ms | 495.6 | 487.8 | −1.6% | 320.6
| 316.8 | −1.2% | −2.0% / −1.7% |
| fast scroll paging (markdown) | LoAF blocking ms | 1489.8 | 1444.2 |
−3.1% | 83.7 | 83.5 | −0.3% | −2.3% / −5.3% |

- Thread scroll open improved on every box (9/9) for script and LoAF on
both OSes.
- The change recovers all the time that link-preview scanning costs in
these scenarios.
- An earlier A/B of desktop-v0.7.18 against the same baseline put thread
scroll open at +2.6% script on both OSes, which this removes.

## Checks

- `vitest run src/features/messages`: 3119 pass.
- `link-previews.spec` and `markdown-documents.spec` pass on chromium
and webkit.
- `bin/pnpm check` is clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Kalvin Chau <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>

@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.

🤖 Star Lord’s automated source review via Wes’s account, at head a85df436477872cabb4a44ffe6d7716c7fe15b96 against base 7bb45482dea10338f92640b603a9ee5bdf780924.

The previous target-opener finding is repaired in source: hosted menu contributions now receive the row’s opener, and the new hosted MessageRow test checks availability and thread-origin dispatch. One new P3 is attached inline: a deferred Forward action can survive removal of its source row during menu close.

Source-only review; I did not run tests, builds, the app or browser scenarios, and have not independently verified the performance claims. The current-head CI snapshot has failed JavaScript (6/6) and CI required checks. The failed job reports one failure in src/bundled/claude-code/setup.test.ts, “says why a new release quit during its test”: expected status ready, received outdated. Causality with this PR is unestablished. Windows native validation was skipped.

@kalvinnchau
kalvinnchau force-pushed the peon/perf-message-context-menu branch from a85df43 to 41a45bd Compare November 13, 2026 16:29
kalvinnchau and others added 3 commits November 13, 2026 08:48
every toolbar message row mounted its own base ui context menu root and
trigger. with many rows that cost about a tenth of list render script time
and a quarter or more of long-animation-frame blocking while scrolling.

the channel timeline and thread panel now host one context menu around
their rows. each toolbar row registers its actions by message id and renders
only its content; a right-click or context-menu key over a registered row
selects that row's actions, and presses outside rows keep the native menu.
base ui still owns every gesture, including its mouseup and dismissal
guards. the popup renders inside the list's react tree, so the list still
sees focus enter a row's menu. the open menu follows its row's latest
actions, closes when the row unmounts, and releases the row on close. rows
outside a host keep their own menu.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Kalvin Chau <[email protected]>
a list's shared context menu renders a row's plugin menu actions outside the row's target opener provider, so they read the closed default and cannot open targets. wrap each menu action in a provider with the row's link callbacks and thread root. the test opens a target from a right-click menu action and fails without the fix.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Kalvin Chau <[email protected]>
a list's shared menu outlives its rows, so a row removed while its menu closes could still run the item it queued, such as forward. clear the queued action when the hosted row unregisters. the test removes the row between forward and close completion and fails without the fix.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Kalvin Chau <[email protected]>
@kalvinnchau
kalvinnchau force-pushed the peon/perf-message-context-menu branch from 41a45bd to 62328dc Compare November 13, 2026 16:48
@kalvinnchau
kalvinnchau merged commit 5846d2a into main Nov 13, 2026
66 checks passed
@kalvinnchau
kalvinnchau deleted the peon/perf-message-context-menu branch November 13, 2026 17:17
joshfriend added a commit that referenced this pull request Nov 13, 2026
* origin/main: (153 commits)
  feat(agents): preview where interrupted agents were working before resume (#3494)
  perf(messages): close the editor and delete dialog on submit (#3493)
  feat(messages): quote selected message text into the composer (#2392)
  perf(toast): scope docked clearance to the toast host (#3489)
  fix(unread): undo exact captured ordinary reads conditionally (#3473)
  fix(solo): keep navigation responsive during chat deletion (#3488)
  fix(channels): keep custom thread tab names until the tab closes (#3458)
  perf(relay): verify cached history in the native host (#3426)
  fix(settings): refine claude and typesafe key setup dialogs (#3391)
  Update bundled help for Bestie and agent settings (#3194)
  feat(profiles): share and show a local time zone (#3332)
  Revert "feat(link-previews): unfurl web links below messages by default" (#3480)
  Show disabled plugin chats in Solo (#3456)
  perf(messages): host one context menu per message list (#3465)
  test(claude): keep the quit-release test copy above the minimum version (#3477)
  fix(profiles): restore verified names from the device cache (#3382)
  fix(mentions): show addressed agents first in Solo audience picker (#2856)
  perf: count member access once and scope find highlights to painting (#3464)
  perf(messages): prepare a message once for its renderer and link previews (#3466)
  fix(composer): speed up long pastes and paste office text as text (#3441)
  ...

# Conflicts:
#	docs/plugin-authoring-guide.md
joshfriend added a commit that referenced this pull request Nov 13, 2026
…when-ready

* origin/main: (21 commits)
  fix(agents): report a conversation wake that fails before its turn starts (#2192)
  fix(usage): read Claude transcripts over 50 MB a page at a time (#3498)
  feat(agents): preview where interrupted agents were working before resume (#3494)
  perf(messages): close the editor and delete dialog on submit (#3493)
  feat(messages): quote selected message text into the composer (#2392)
  perf(toast): scope docked clearance to the toast host (#3489)
  fix(unread): undo exact captured ordinary reads conditionally (#3473)
  fix(solo): keep navigation responsive during chat deletion (#3488)
  fix(channels): keep custom thread tab names until the tab closes (#3458)
  perf(relay): verify cached history in the native host (#3426)
  fix(settings): refine claude and typesafe key setup dialogs (#3391)
  Update bundled help for Bestie and agent settings (#3194)
  feat(profiles): share and show a local time zone (#3332)
  Revert "feat(link-previews): unfurl web links below messages by default" (#3480)
  Show disabled plugin chats in Solo (#3456)
  perf(messages): host one context menu per message list (#3465)
  test(claude): keep the quit-release test copy above the minimum version (#3477)
  fix(profiles): restore verified names from the device cache (#3382)
  fix(mentions): show addressed agents first in Solo audience picker (#2856)
  perf: count member access once and scope find highlights to painting (#3464)
  ...
joshfriend added a commit that referenced this pull request Nov 13, 2026
…n-on-github

* origin/main: (21 commits)
  fix(agents): report a conversation wake that fails before its turn starts (#2192)
  fix(usage): read Claude transcripts over 50 MB a page at a time (#3498)
  feat(agents): preview where interrupted agents were working before resume (#3494)
  perf(messages): close the editor and delete dialog on submit (#3493)
  feat(messages): quote selected message text into the composer (#2392)
  perf(toast): scope docked clearance to the toast host (#3489)
  fix(unread): undo exact captured ordinary reads conditionally (#3473)
  fix(solo): keep navigation responsive during chat deletion (#3488)
  fix(channels): keep custom thread tab names until the tab closes (#3458)
  perf(relay): verify cached history in the native host (#3426)
  fix(settings): refine claude and typesafe key setup dialogs (#3391)
  Update bundled help for Bestie and agent settings (#3194)
  feat(profiles): share and show a local time zone (#3332)
  Revert "feat(link-previews): unfurl web links below messages by default" (#3480)
  Show disabled plugin chats in Solo (#3456)
  perf(messages): host one context menu per message list (#3465)
  test(claude): keep the quit-release test copy above the minimum version (#3477)
  fix(profiles): restore verified names from the device cache (#3382)
  fix(mentions): show addressed agents first in Solo audience picker (#2856)
  perf: count member access once and scope find highlights to painting (#3464)
  ...
joshfriend added a commit that referenced this pull request Nov 13, 2026
* origin/main: (31 commits)
  Let plugin panels name their tab with panels.useTabTitle (#3251)
  fix(agents): report a conversation wake that fails before its turn starts (#2192)
  fix(usage): read Claude transcripts over 50 MB a page at a time (#3498)
  feat(agents): preview where interrupted agents were working before resume (#3494)
  perf(messages): close the editor and delete dialog on submit (#3493)
  feat(messages): quote selected message text into the composer (#2392)
  perf(toast): scope docked clearance to the toast host (#3489)
  fix(unread): undo exact captured ordinary reads conditionally (#3473)
  fix(solo): keep navigation responsive during chat deletion (#3488)
  fix(channels): keep custom thread tab names until the tab closes (#3458)
  perf(relay): verify cached history in the native host (#3426)
  fix(settings): refine claude and typesafe key setup dialogs (#3391)
  Update bundled help for Bestie and agent settings (#3194)
  feat(profiles): share and show a local time zone (#3332)
  Revert "feat(link-previews): unfurl web links below messages by default" (#3480)
  Show disabled plugin chats in Solo (#3456)
  perf(messages): host one context menu per message list (#3465)
  test(claude): keep the quit-release test copy above the minimum version (#3477)
  fix(profiles): restore verified names from the device cache (#3382)
  fix(mentions): show addressed agents first in Solo audience picker (#2856)
  ...
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.

3 participants