Skip to content

Pace sidebar spinner animations and stop them while hidden - #14832

Merged
teamleaderleo merged 4 commits into
mainfrom
perf/sidebar-spinner-frame-rate
Sep 26, 2026
Merged

teamleaderleo merged 4 commits into
mainfrom
perf/sidebar-spinner-frame-rate

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

The sidebar's agent spinner is an endless Core Animation rotation. WindowServer runs it without involving cmux, so a sampled cmux can look idle while WindowServer keeps recompositing every spinning row, potentially at the display's full refresh rate (160 Hz on the reporter's external display). Behind window glass or a translucent background, each of those frames costs more. The spokes only step ten times a second, so most of those frames change nothing.

  • The spokes animation now sets preferredFrameRateRange to 10-20 Hz (its step rate, with 2x headroom so no step is skipped). The arc, used only by the spinner gallery, asks for at most 60 Hz.
  • Sidebar rows hide an idle spinner rather than removing it, and the hidden view kept its infinite animation installed. viewDidHide now removes the animation when the spinner or any ancestor hides, and viewDidUnhide reinstalls it through the existing shouldAnimate gate.

The look is unchanged: same spokes, same cadence, same phase lock across rows.

Evidence: regression tests first (e4c26ced41d), then the fix (e7a3a10d036, with 5b5b17cc025 moving the hide hook to viewDidHide/viewDidUnhide so an ancestor unhide also restarts the spinner; a standalone AppKit harness confirmed both hooks fire without a window, for the view itself and for an ancestor). SidebarHiddenPresentationTests gains hiddenSpinnerRemovesItsInstalledAnimation and spinnerAnimationsRequestTheirStepRateFromTheCompositor; they have not run locally (no app test build on this machine), so CI's changed-suites batch is the first execution. The animation builder was typechecked standalone against the macOS 14 SDK target and printed a 10/20/20 Hz range. scripts/verify-local.py passed (swift-syntax, test-wiring, feature-flags). I could not measure WindowServer directly here (no sudo, no screen-recording grant), so the size of the win is unmeasured.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Paces sidebar spinner animations to match their visible update rate and stops hidden spinners from consuming compositor work.

  • Spokes request 10–20 Hz, while the gallery arc is capped at 60 Hz.
  • Hiding a spinner or any ancestor removes its animation; showing it reinstalls the animation only when presentation conditions allow.
  • Adds tests for hidden animation removal and compositor frame-rate requests.
  • Drops the known-failure entry for visibilityToggleKeepsAppKitTableContainerMounted, which now passes.

Written for commit d9256dc. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Spinners now stop animating while their view or a parent view is hidden, and resume when shown again.
    • Spinner animation frame rates now better match their style: spoke animations scale with their speed, while arc animations target smoother motion with a maximum of 60 frames per second. This helps avoid unnecessary animation work when a spinner is hidden or running at a lower speed.

teamleaderleo and others added 2 commits September 26, 2026 10:08
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The sidebar row spinner is an endless Core Animation rotation that
WindowServer runs on its own, so it may recomposite each row at the
display's full refresh rate even though the spokes only step ten times a
second. Ask Core Animation for 10-20 Hz on the spokes and at most 60 Hz on
the arc. Rows also hide an idle spinner instead of removing it, and the
hidden view kept its endless animation; hiding now removes it and showing
reinstalls it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 22bc6886-b834-420d-8baa-d5bf3cf60519

📥 Commits

Reviewing files that changed from the base of the PR and between 9bae42b and d9256dc.

📒 Files selected for processing (3)
  • Sources/Sidebar/GPUSpinnerNSView.swift
  • cmuxTests/SidebarHiddenPresentationTests.swift
  • scripts/ci/app-host-known-failures.json
 ___________________________________________________
< This is not a microservice. This is a macro-mess. >
 ---------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

teamleaderleo and others added 2 commits September 26, 2026 10:36
Use viewDidHide/viewDidUnhide instead of an isHidden observer so hiding
or showing any ancestor also stops or reinstalls the spinner animation.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…sses

The changed-suites run for this branch passed
visibilityToggleKeepsAppKitTableContainerMounted, and the ratchet requires
removing entries that pass.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

The first changed-suites run passed all 10 SidebarHiddenPresentationTests, including the two new tests, and failed only on the known-failure ratchet: visibilityToggleKeepsAppKitTableContainerMounted now passes (its fix, #13931, is merged). d9256dc removes that entry from scripts/ci/app-host-known-failures.json.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 26, 2026 15:19
@teamleaderleo
teamleaderleo merged commit 7930dec into main Sep 26, 2026
60 of 61 checks passed
@teamleaderleo
teamleaderleo deleted the perf/sidebar-spinner-frame-rate branch September 26, 2026 15:23
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for d9256dce89: every check was green at merge (16 verified; 14 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 26, 2026
273d9e0 Stop shell integration spawning for disabled features before the first prompt (manaflow-ai#14847)
bb2db23 perf(startup): add millisecond and process-uptime fields to startup breadcrumbs (manaflow-ai#14846)
7930dec Pace sidebar spinner animations and stop them while hidden (manaflow-ai#14832)
2fd3c40 Add resize-window CLI and socket command, plus a scrollback resize guard script (manaflow-ai#9826)
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