Repository navigation
fix(web): opening the terminal with Cmd+J no longer re-renders the whole sidebar - #33129
Conversation
These tests reproduce #33100: opening the route thread's terminal drawer, moving focus into and out of a terminal, and pressing a bare modifier each re-render every sidebar thread row, which makes Cmd+J slow with many threads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cmd/Ctrl+J, focusing a terminal and pressing a bare modifier each re-rendered Sidebar and every thread row, because Sidebar subscribed to the route thread's terminalOpen, terminal focus and held modifiers only to feed the thread-navigation shortcut handler and the jump hints. With a few hundred threads that took hundreds of milliseconds per keypress. The keydown handler now reads terminal state when the key is pressed, and jump-hint visibility comes from a useSyncExternalStore boolean over modifier, terminal-focus and terminal-store changes, so Sidebar re-renders only when the hints flip. Modifier tracking moved into a shared external store that useShortcutModifierState reads, keeping its editable-ignore and paste/blur reset behaviour. Fixes #33100 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the desktop thread-jump bindings, pressing Mod alone made jump hints eligible, so Sidebar rendered to arm the show delay and again when focus moved into the terminal or Mod was released: every row re-rendered twice per Cmd/Ctrl+J. The show delay now lives in a modifier-hold store beside the shortcut modifier store, and useThreadJumpHintsVisible's snapshot is the final visibility. Sidebar renders only when hints actually appear or hide. The regression test now uses mod+1/mod+2 jump bindings and the platform Mod key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Hints stayed hidden after the model picker closed while Mod was still held, because nothing told the hint snapshot the picker had closed. Model picker visibility is now observable and part of the hint inputs. Hints also never appeared for a jump binding without a modifier, because the hold delay only ran while a modifier was held. The delay now runs for every modifier combination, including none, and reads an already-passed deadline when it arms. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This refactor avoids sidebar-wide re-renders when opening the terminal by deriving shortcut hints from shared interaction-state subscriptions. The change is limited to transient client-side UI behavior, with no product-default, persistent-data, or external-system changes. Notes:
No code changes detected at You can add or adjust custom eligibility rules. Learn more. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
# Conflicts: # apps/web/src/components/Sidebar.tsx
Dismissing prior approval to re-evaluate 86f8eb3
…eady Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Dismissing prior approval to re-evaluate ed711fe
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Dismissing prior approval to re-evaluate 65e501b
EvidenceEnvironment on both sides: Linux Chromium web client, 1280×800, dark theme; separate worktrees and fresh databases with identical synthetic data. Recordings include a React commit counter; terminal output and runtime selectors are hidden for privacy. Steps: initialize terminals with one identical driver pass; then select Demo task 30, quickly toggle Ctrl+J six times (70 ms modifier hold), and repeat on Demo task 29. Separately hold Control for 1.6 seconds, release it, then hold it while opening/focusing the terminal. Explicit web numbered-jump bindings use
Both recordings show hints appearing during the long hold and disappearing on release or terminal focus. All recordings were opened and visually reviewed before upload. Scope: the initial head pass showed one Sidebar update and 31 row-wrapper renders during terminal initialization on task 30; task 29 had zero. The proof establishes elimination of repeated toggle-driven sidebar renders after initialization. Initial base pass. Reproduction materials: driver, seed, bindings, instrumentation installer, counter overlay, instructions, measurements. AI-assisted; verified with the tests and tools listed above. Not shown:
|
|
Thanks! |
…r hold stores upstream pingdotgg#33129 removed hooks/useTerminalFocus and turned useThreadJumpHintsVisible into a sidebar-specific store, so read terminal focus from lib/terminalFocus and gate the hints on the shared modifier hold. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
Opening the terminal with Cmd/Ctrl+J got slower the more threads the sidebar held. Each press re-rendered the whole sidebar and every thread row several times: when Cmd/Ctrl went down, when the drawer opened, when the terminal took focus, and when it closed again. The sidebar subscribed to the held modifier keys, terminal focus and the current thread's
terminalOpen, but only needed them for the thread-navigation shortcut context and the thread jump hints. Its row list isn't memoized, so each of those changes re-ran every row.Change
The sidebar now re-renders only when the thread jump hints actually appear or disappear.
whencontext when the key is pressed (threadShortcutContext). It reads terminal focus, the route thread'sterminalOpen, preview focus,previewOpenand model-picker visibility directly, so customwhenclauses see current values.useThreadJumpHintsVisible(keybindings, routeThreadRef)is auseSyncExternalStorewith a boolean snapshot. The snapshot is true when the current modifier combination has been held unchanged for the existing hint delay and the hint conditions still match.deadlineStorehelpers.mod+1..9jump bindings.ignoreEditablemode, with the same editable, paste, blur and dictation resets.useShortcutModifierStatekeeps its API for its other users: the composer and the pull request panels.useTerminalFocushad a single caller and is removed. Its focus subscription moves tolib/terminalFocus.ts.What else changed, visible to a user:
!modelPickerOpen && isDesktop) were inactive.Scope and approval
Fixes #33100
Verification
Tests (
vp test run):Sidebar.browser.test.tsx› "does not re-render thread rows when a quick Mod shortcut opens and focuses the terminal". Desktop-stylemod+1/mod+2jump bindings, platform Mod key. Fails on main with every row re-rendering at each step; 0 renders now. Holding Mod past the delay still shows the labels and releasing hides them.Sidebar.logic.browser.test.tsx: "follows terminal focus and the route thread's terminal while the modifier is held", "returns when the model picker closes while the modifier is still held", "shows after the delay for a jump binding without a modifier", plus the second-hold delay case.shortcutModifierState.browser.test.ts,keybindings.test.ts,ProviderModelPicker*.browser.test.tsxpass unchanged.vp run check:changedon the merged head: format, lint, knip, typecheck (2 packages), tests (9 files), React compiler (5 files) pass.Profile at this head vs. its merge-base, React development build, Linux Chromium 148, 1280×800, DPR 1, 201 synthetic threads in a fresh dev database, trusted CDP Ctrl+J (6 ms hold), three samples each (transcript):
mod+1..9bindings, beforemod+1..9bindings, afterEvidence (captured at 12ac6ac vs. merge-base fa2f4ff; later heads only merge main, with the same PR diff): before/after captures, zero-threshold and thresholded diffs and narrated recordings for the fixture matrix (hint hold/release, quick Ctrl+J, drawer and right-panel terminal focus, composer and Settings model pickers, preview conditions,
!terminalOpenbindings, unmodified bindings, paste/blur resets and subscription remount, navigation guards, empty and draft states, search, 390-wide drawer, composer queue/steer, pull request Shift actions), plus Linux Electron with the desktop default bindings:Opening the composer model picker during a held Mod: before (left) keeps the labels, after (right) hides them.
Reviews: three rounds by two independent reviewers found no code bugs; the earlier rounds asked only for the current-head evidence above. The proof audit of that evidence is sufficient.
Not checked:
cannot-verify macos: the reporter's macOS desktop app. The changed code is shared web code with no platform branches; Linux Electron with the desktop default bindings was recorded instead.AI-assisted; verified with the tests and tools listed above.
🤖 Generated with Claude Code