Repository navigation
perf: defer closed menus to speed up cold worktree switching - #20311
Conversation
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This is a behavior-preserving refactor that exploits Radix's existing mount-on-open behavior to skip constructing closed menus and dialogs during terminal/worktree activation. Closed DropdownMenuContent/DialogContent already do not mount children (no forceMount), so relocating translate() calls and JSX construction into components rendered under the content defers that work until a surface opens. Child-component mount semantics are unchanged — they were already inside the content subtree.
TerminalContextMenu: the outer component now renders onlyDropdownMenu/trigger/content; the full item list moved to a module-scopeTerminalContextMenuItemschild, including theuseMemoshortcut-label computation. TheonOpenChange100 ms guard and allonSelecthandlers are byte-for-byte equivalent.CloseTerminalDialog: body extracted toCloseTerminalDialogBody;useId/useStateand the render-phase reopen/subjectKeyreset stay in the always-mounted parent, so queued confirmations and reopen resets keep working.TabBarStaticCreateMenu:renderTabBarStaticCreateMenu()converted to a component;tab-bar-surface.tsxnow suspends it underDropdownMenuContentinstead of eagerly calling it on every render (which happened regardless of menu open, sinceshowStaticCreateMenuItemsonly depends on the query).- Tests: added closed-state tests asserting zero
translatework while closed and construction on open. I confirmed theTerminalContextMenutest is falsifiable — forcing eager construction makes it fail with 13translatecalls — and both changed suites pass, as doespnpm tc:web.
openrouter/deepseek/deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

ELI5
Returning to a worktree after a while briefly shows a blank terminal while Orca rebuilds its renderer. That rebuild also prepared menus and a confirmation dialog that were closed. This PR waits until those surfaces open to construct their contents, leaving less work ahead of terminal restoration.
What Changed
DropdownMenuContent.DialogContent, while keeping checkbox/reset state with its existing owner so reopening and queued confirmations retain their behavior.Why
This removes repeated work from cold activation without extending terminal retention or adding a cache. It targets the reported case of returning after a while; it does not eliminate snapshot restore latency.
In a hidden macOS Electron test app using a production-built renderer served through Vite, real sidebar clicks switched between two Git worktrees with three terminals each and 1,500 lines of scrollback. Cold samples reload onto the other worktree first and verify that the target terminal manager is absent.
The metric is pointerdown to the first animation-frame sample containing restored terminal-buffer text, not pixel-paint timing or a packaged-app benchmark. This is a small local sample, not a guaranteed improvement on every machine. A separate development-renderer folder-workspace fixture with 10,000 lines improved from 194.8 ms to 156.1 ms median.
User trade-offs and remaining opportunities
No observed loss of functionality or visible menu regression. The deliberate trade-off is that menu/dialog body construction and translation now happen when opened, rather than repeatedly while closed. Opening could absorb that small amount of work; menu-opening latency was not separately benchmarked. Existing Radix mount/exit behavior and confirmation state ownership are preserved.
There is no additional terminal memory retention, no shorter scrollback, and no change to PTY identity, replay ordering, geometry settling, repaint safeguards, SSH/remote protocols, or resource policies. A brief cold-restore blank interval remains.
No further material, trade-off-free improvement is established by this investigation. Hidden sibling admission did not delay initial content in the three-tab fixture. Additional probes saw snapshot writes at 62–74 ms and restored content at 73–97 ms; the startup-command grid-settle gate does not apply to that fixture. Keeping more renderers alive would spend memory, while skipping geometry/repaint safeguards risks incorrect or stale terminal rendering. Further elimination of redundant UI work remains a profiling candidate, not a claimed benefit.
Linked Issue
Fixes #20308
Visual Proof
These before/after screenshots verify equivalent restored terminal content and layout; the timing samples above provide the performance evidence. They do not capture the duration of the blank interval.
Testing
ORCA_BACKGROUND_LAUNCH=1, through Playwright CDP.23 tests passed across
TerminalContextMenu,CloseTerminalDialog, and the TabBar Windows shell launch / SSH host / remote runtime suites. New closed-copy tests were verified failing against the original source and passing with the change.pnpm tc:web,pnpm run check:code-quality:changed, formatting checks, andgit diff --checkpassed. Production renderer builds succeeded for both benchmark variants.Rendered checks covered the terminal context menu, New Tab menu, creating a terminal, and opening/cancelling the running-command close confirmation. A live alternate-screen redraw fixture survived sidebar switches and one renderer reload with the same PTY ID, continued advancing frames, and restored its normal buffer on completion.
Actual runtime testing was on macOS, with local Git and folder workspaces. Windows/SSH routing was covered by automated tests; Linux, Windows, and live SSH performance were not benchmarked. Full repository checks remain for CI.
Review
Self-reviewed component boundaries, event callbacks, confirmation state resets, and unchanged platform/remote routing. No new effect, cache, timer, IPC, or lifecycle policy.
Agent skill upstream boundary
Checklist