Repository navigation
perf(messages): host one context menu per message list - #3465
Conversation
714a7ef to
1925c36
Compare
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 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 ⋯.
…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
left a comment
There was a problem hiding this comment.
🤖 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.
a85df43 to
41a45bd
Compare
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]>
41a45bd to
62328dc
Compare
* 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
…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) ...
…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) ...
* 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) ...
Problem
#3316 added right-click message actions by wrapping every toolbar message row in its own Base UI
ContextMenuRootandContextMenuTrigger. 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:
Change
MessageContextMenusinMessageActionBar.tsxis one real Base UI context menu per message list. It wraps the channel timeline feed and the thread panel's rows.ThreadPanel.tsxonly wraps the rows element. Its diff is indentation;git diff -wshows 3 lines.Evidence
Interleaved A/B on 9 Linux and 9 macOS boxes:
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.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:message-context-menu,menu-dismiss,message-actions,message-actions-floating,buzz-links,external-links,thread-composer-focus,find-on-screen.tsc --noEmitandbiome checkare clean.🤖 Generated with Claude Code