Skip to content

Tab strip: stop per-render context-menu, hint-monitor and animation work - #287

Open
austinywang wants to merge 5 commits into
cmux-pin/pane-host-environmentfrom
fix/tab-chrome-render-work
Open

austinywang wants to merge 5 commits into
cmux-pin/pane-host-environmentfrom
fix/tab-chrome-render-work

Conversation

@austinywang

@austinywang austinywang commented Oct 13, 2026 •

Copy link
Copy Markdown

Summary

Sentry reports hangs of 8 s or more with the main thread rendering the pane tab strip: CMUXTERM-MACOS-41NN (new in cmux 0.65.1) and about 800 earlier events with the same TabItemView.body → tabControlShortcutHintVisibilityAnimation frame. Related groups sample the same render path: -2QV2 and -2QAN (context-menu state per tab), and -2SCZ, -2PRX and -26QQ (hint monitor initialization reading user defaults).

This change removes avoidable work from the tab strip and adds a low-volume diagnostic seam for the remaining render multiplier:

  • Tab context-menu state is built when a menu opens, instead of for every tab on every render. That stops calling host providers once per tab per render.
  • The shortcut-hint monitor no longer reads and decodes the shortcut setting from user defaults during every tab-bar rebuild.
  • The whole-tab hint .animation is removed. The hint fade keeps its own animation.
  • A host callback emits one sampled TabBarRenderDiagnostic render window when tab-bar body evaluations stay continuously active for at least 500 ms and 32 evaluations. cmux records that as a deferred Sentry breadcrumb with render, tab and pane counts.

Hint visibility and appearance are unchanged.

Testing

  • TabBarRenderWorkTests.testRenderingTabsDoesNotQueryContextMenuProviders was committed before the fix and failed with 78 host-provider calls; it passes with the lazy state change.
  • TabBarRenderWorkTests.testMenuBuildEvaluatesTabStateOnce verifies menu state is evaluated only when the menu is built.
  • TabBarRenderWorkTests.testSustainedTabBarRendersEmitOneDiagnostic verifies the continuous-render threshold and one-shot sampling behavior.
  • CI: all Bonsplit tests pass on the hosted macOS runner.

Changelog

Fixed: Pane tab bars do less work on every redraw and report sustained render storms for hang diagnosis

Proof

CI only; no visual change.

Fails today: every tab render builds TabContextMenuState, which calls the
host's fork-default and remote-disconnect providers for each tab.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 582c363c-585a-4fbd-b339-ecff26d2a643

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Every tab render built TabContextMenuState, calling the host's fork-default
and remote-disconnect providers, allTabIds and adjacentPane per tab. The
menu now builds it from a provider when it opens.

TabControlShortcutKeyMonitor's initializer read user defaults and decoded
JSON; @State evaluates it on every TabBarView init. Its initial symbol is
now a constant; start() resolves the configured one.

The whole-tab hint animation copied the tab's full view value on every
render; the crossfade keeps its own animation where it happens.

Sentry: CMUXTERM-MACOS-41NN, -2QV2, -2QAN, -2SCZ, -2PRX, -26QQ.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review follow-up: an update that queues a hint while the modifier is already
held does not reset the symbol, so start() resolves it first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
austinywang added a commit to manaflow-ai/cmux that referenced this pull request Oct 13, 2026
Pins manaflow-ai/bonsplit#287 review follow-up (9d90480).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@austinywang

Copy link
Copy Markdown
Author

Correctness review (subagent) and resolution

No correctness bugs found. Findings:

  • Risk, fixed in 9d90480. The hint monitor now starts with a constant symbol. When the tab-shortcut modifier is already held as a pane appears (for example right after a split shortcut), update(from:) queues the hint without resetting the symbol, so a multi-modifier shortcut could briefly show the default symbol. start() now resolves the configured symbol first. start() runs on appear and on enable, not on every render.
  • Risk, accepted. Before start() runs, an icon-only pinned browser tab in a focused pane reserves hint width from the default symbol, so a multi-modifier shortcut can draw one frame narrower. onAppear runs before the first display in practice.
  • Nit, intended. TabBarView no longer reads allTabIds, adjacentPane, hasCustomTitle or the host providers while rendering, so it no longer re-renders when only those change. The menu reads them fresh when it opens.
  • Tests. The render test fails on the base and passes with the fix. Changes 2 and 3 have no dedicated test (a constant initial value and a removed modifier).

Probe numbers (hosted macos-15, release, 2×2 split, 13 tabs, nil host providers)

before after
TabItemView.body, 40 hint toggles (µs, sum) 21,650 32,244
Focus change, all 13 tab bodies (µs) 1,047–1,084 1,297–1,885
Resize step (ms) 36–66 62

Within runner noise; no measurable change in this harness. The harness has no host providers, so it does not include the per-tab user-defaults reads (Sentry -2QV2) that the context-menu change removes. Removing the whole-tab animation did not measurably cut body time, so that change is cleanup rather than a measured win. The 41NN trigger remains unidentified; see the PR description.

@austinywang
austinywang force-pushed the fix/tab-chrome-render-work branch from cba6021 to 7664209 Compare October 13, 2026 22:38
@austinywang

Copy link
Copy Markdown
Author

Correctness review follow-up

The review of the render-storm diagnostic found two issues and both are resolved:

  • The first sampling window could classify ordinary animation frames as a stall. The tracker now resets after a 100 ms idle gap, requires 32 evaluations across at least 500 ms, and emits once per continuous window.
  • The first implementation called the host callback inline from TabBarView.body. cmux now defers the Sentry breadcrumb until after the body returns. Disabling the handler also resets stale counters.

The diagnostic remains low volume and reports only elapsed window duration, render count, tab count, and pane count. It does not alter tab hint behavior or layout.

austinywang added a commit to manaflow-ai/cmux that referenced this pull request Oct 14, 2026
…1NN hangs (#20238)

* Bump Bonsplit: stop per-render tab-strip work behind hang reports

Pins manaflow-ai/bonsplit#287 (663bc41): context-menu state is built when a
menu opens, the hint monitor no longer reads user defaults on every tab-bar
init, and the whole-tab hint animation is gone.

Sentry: CMUXTERM-MACOS-41NN, -2QV2, -2QAN, -2SCZ, -2PRX, -26QQ.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Bump Bonsplit: resolve the hint symbol when the monitor starts

Pins manaflow-ai/bonsplit#287 review follow-up (9d90480).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Diagnose sustained tab-bar render storms

* Tighten tab-bar render storm diagnostics

* Name tab-bar diagnostics as render windows

---------

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant