Skip to content

fix(web): opening the terminal with Cmd+J no longer re-renders the whole sidebar - #33129

Merged
juliusmarminge merged 11 commits into
mainfrom
fix/issue-33100
Nov 11, 2026
Merged

juliusmarminge merged 11 commits into
mainfrom
fix/issue-33100

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Nov 8, 2026 •

Copy link
Copy Markdown
Member

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.

  • The thread-navigation keydown handler builds its when context when the key is pressed (threadShortcutContext). It reads terminal focus, the route thread's terminalOpen, preview focus, previewOpen and model-picker visibility directly, so custom when clauses see current values.
  • useThreadJumpHintsVisible(keybindings, routeThreadRef) is a useSyncExternalStore with 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.
    • The delay lives in a small modifier-hold store built on the existing deadlineStore helpers.
    • A quick Cmd/Ctrl+J changes the modifier combination before the delay passes, so it causes no sidebar render, even with the desktop default mod+1..9 jump bindings.
  • Held-modifier tracking moves from per-component state into one shared store per ignoreEditable mode, with the same editable, paste, blur and dictation resets. useShortcutModifierState keeps its API for its other users: the composer and the pull request panels.
  • Model-picker visibility now notifies subscribers when the picker opens or closes.
  • useTerminalFocus had a single caller and is removed. Its focus subscription moves to lib/terminalFocus.ts.

What else changed, visible to a user:

  • Jump hints now hide while a model picker (composer or Settings) is open or a preview has focus. Before, the sidebar never re-rendered for those, so the labels stayed up while the desktop default bindings (!modelPickerOpen && isDesktop) were inactive.
  • The hint delay follows an unchanged modifier combination. After a hold past the delay, hints show immediately when they become eligible again (terminal blur, picker close, route or binding change) instead of after a fresh 200 ms; switching between two eligible combinations restarts the delay.
  • A component that mounts while a modifier is held sees it held; before it started empty.

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-style mod+1/mod+2 jump 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.tsx pass unchanged.
  • vp run check:changed on 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):

Ctrl+J Sidebar renders Row renders Sidebar render time Keydown → first frame
Open, web defaults, before 4 804 153–188 ms 225–470 ms
Open, web defaults, after 0 0 0 48–56 ms
Open, mod+1..9 bindings, before 4 804 173–180 ms 228–231 ms
Open, mod+1..9 bindings, after 0 0 0 54–75 ms
Close, both fixtures, before 4 804 154–212 ms 158–236 ms
Close, both fixtures, after 0 0 0 29–35 ms

Evidence (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, !terminalOpen bindings, 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.

pair-wide-picker-open-sheet.png

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.
  • Preview focus is shown for the browser preview's URL field only, not focus inside an embedded page or the Electron webview. The initial no-modifier delay is shown in a labelled harness of the production hook.
  • Captures use an anchored, advancing clock on both sides; a frozen clock stops the hint deadline from maturing.
  • The terminal-open path on the server (waiting on the workspace lease behind a git operation) is not changed here.

AI-assisted; verified with the tests and tools listed above.

🤖 Generated with Claude Code

juliusmarminge and others added 5 commits November 8, 2026 22:59
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>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Nov 8, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Nov 9, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 65e501b

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 objects were reviewed. Approvability was decided on eligibility alone.

No code changes detected at a3106d5. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The sidebar now derives shortcut context from the active route thread and computes jump-hint visibility from modifier, terminal, model-picker, and keybinding state. Shared external-store subscriptions track state changes. Browser tests cover hint timing, context changes, snooze-clock behavior, and sidebar row renders.

Changes: Sidebar shortcut and hint behavior

Layer / File(s) Summary
Shared modifier and visibility state
apps/web/src/shortcutModifierState.ts, apps/web/src/lib/terminalFocus.ts, apps/web/src/modelPickerVisibility.ts, apps/web/src/hooks/useTerminalFocus.ts
Modifier tracking now uses an external store. Terminal-focus and model-picker visibility changes expose subscriptions. The terminal-focus hook was removed.
Shortcut context and hint timing
apps/web/src/components/Sidebar.logic.ts, apps/web/src/components/Sidebar.logic.browser.test.tsx
Shortcut context reads current route-thread, terminal, model-picker, and platform state. Jump hints use a shared delay clock and current keybindings. Browser tests cover timing, context changes, and snooze-clock behavior.
Sidebar shortcut integration and render checks
apps/web/src/components/Sidebar.tsx, apps/web/src/components/Sidebar.browser.test.tsx
The sidebar passes route-thread context to shortcut resolution and jump-hint logic. Browser tests check row renders during terminal interactions and hint changes.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Keyboard
  participant shortcutModifierStore
  participant useThreadJumpHintsVisible
  participant Sidebar
  Keyboard->>shortcutModifierStore: Update modifier state
  shortcutModifierStore->>useThreadJumpHintsVisible: Notify modifier change
  useThreadJumpHintsVisible->>Sidebar: Update hints after delay and context checks
Loading

Suggested reviewers: t3dotgg

Merge Risk: Minimal · up to a3106

No merge-blocking issue was established in the shortcut and hint changes.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly identifies the main change: preventing sidebar re-renders when opening the terminal with Cmd+J.
Description check Passed The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the behavior and implementation, links the issue, reports focused tests and profiling …
Linked Issues check Passed [ #33100 ] The change removes Sidebar subscriptions that caused thread rows to render on modifier, terminal-focus, and terminal-open updates. useThreadJumpHintsVisible now subscribes to a boolean vi…
Out of Scope Changes check Passed All changed files support [#33100] or protect the behavior affected by the optimization. The shared modifier store preserves the existing hook API for its other callers. Model-picker and terminal-focu…
Full details: Description check

Explanation

The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the behavior and implementation, links the issue, reports focused tests and profiling results, includes UI evidence, and states what was not checked. The agent attribution does not specify a model and harness, but the description is otherwise complete.

Full details: Linked Issues check

Explanation

[ #33100 ] The change removes Sidebar subscriptions that caused thread rows to render on modifier, terminal-focus, and terminal-open updates. useThreadJumpHintsVisible now subscribes to a boolean visibility snapshot, and Sidebar.browser.test.tsx verifies that a quick Mod shortcut causes zero row renders. The PR reports first-frame times of 54–75 ms after the change, compared with 225–470 ms before it. This supports the issue’s near-instant terminal-opening objective. The separate server workspace-lease wait remains unchanged.

Full details: Out of Scope Changes check

Explanation

All changed files support [#33100] or protect the behavior affected by the optimization. The shared modifier store preserves the existing hook API for its other callers. Model-picker and terminal-focus subscriptions provide current shortcut context and hint visibility. The added tests cover these behaviors. No unrelated change is evident.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 3bdf28a and a3106d5.

Files selected for processing: 1

  • apps/web/src/components/Sidebar.tsx

Limit details: You’ve used all 10 included reviews currently available.

@juliusmarminge juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed pr:in-progress A lead is working on this PR and removed pr:gate-failed A land gate failed labels Nov 9, 2026
# Conflicts:
#	apps/web/src/components/Sidebar.tsx
@macroscopeapp
macroscopeapp Bot dismissed their stale review November 10, 2026 07:15

Dismissing prior approval to re-evaluate 86f8eb3

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Nov 10, 2026
…eady

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@macroscopeapp
macroscopeapp Bot dismissed their stale review November 10, 2026 07:20

Dismissing prior approval to re-evaluate ed711fe

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Nov 10, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@macroscopeapp
macroscopeapp Bot dismissed their stale review November 10, 2026 07:24

Dismissing prior approval to re-evaluate 65e501b

@juliusmarminge juliusmarminge added pr:gate-failed A land gate failed pr:in-progress A lead is working on this PR and removed pr:in-progress A lead is working on this PR pr:gate-failed A land gate failed labels Nov 10, 2026
@juliusmarminge juliusmarminge removed the pr:in-progress A lead is working on this PR label Nov 10, 2026
@juliusmarminge juliusmarminge added the pr:needs-julius Needs Julius: route gate, unverifiable surface, or blocked label Nov 10, 2026

Copy link
Copy Markdown
Member Author

Evidence

Environment 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 !terminalFocus && !modelPickerOpen && isWeb.

Before After
871b00907d a3106d5f52
Before recording After recording
Each six-toggle window: 24 Sidebar / 744 SortableThreadRow renders, on both threads. Each identical window: 0 Sidebar / 0 SortableThreadRow renders, on both threads.

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:

  • macOS Cmd+J and Electron shell behavior were not exercised; evidence uses the real Linux web client.
  • Elimination of every cold terminal-initialization sidebar update is not established: the initial head pass contained one Sidebar update and 31 row-wrapper renders.
  • Preview-focus conditions, model-picker transitions, and arbitrary custom shortcut expressions were not exercised.
  • Frame latency and perceived responsiveness were not quantified; the recordings prove committed component-render counts.
  • The initial origin fetch returned HTTP 503; the reported merge-base was computed from the locally available origin/main reference.

@juliusmarminge
juliusmarminge merged commit facd190 into main Nov 11, 2026
35 checks passed
@juliusmarminge
juliusmarminge deleted the fix/issue-33100 branch November 11, 2026 03:16
@cullen-b

Copy link
Copy Markdown

Thanks!

colonelpanic8 added a commit to colonelpanic8/t3code that referenced this pull request Nov 11, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop pr:needs-julius Needs Julius: route gate, unverifiable surface, or blocked size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Opening the terminal with Cmd+J is slow, sometimes very slow

2 participants