Skip to content

perf: split BrowserPanelView's modifier chain so it type-checks quickly - #13105

Closed
teamleaderleo wants to merge 1 commit into
manaflow-ai:mainfrom
teamleaderleo:perf/browser-panel-typecheck
Closed

teamleaderleo wants to merge 1 commit into
manaflow-ai:mainfrom
teamleaderleo:perf/browser-panel-typecheck

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • BrowserPanelView's body ended in one chain of 20 modifiers (coordinateSpace, three onPreferenceChange, two onReceive, onAppear/onDisappear, twelve onChange). The compiler type-checks that chain as a single expression, and it was the slowest expression in the app module.
  • This splits the chain into three computed views (browserPanelLifecyclePreferencesView -> browserPanelLifecycleNotificationsView -> the onChange group). The modifiers, their closures and their order are unchanged; nothing else in the file is touched.

Testing

Measured on one machine (MacBook Air M5, Xcode 27.0) with -Xfrontend -warn-long-expression-type-checking=200 -Xfrontend -warn-long-function-bodies=200:

Sources/Panels/BrowserPanelView.swift before after
sites over 200 ms 4 0
slowest expression (:1073) 8,120 ms in a cold build under load; 6.8 s in an unloaded profile under 200 ms
  • The threshold crossing is the result to rely on, not the millisecond values: the "before" number comes from a cold parallel build.
  • Whole-build wall clock: no measurable change on a 10-core machine (the file compiles in parallel with ~3,700 others). What it removes is a multi-second floor on every incremental build that recompiles this file.
  • xcodebuild -scheme cmux -configuration Debug build on this branch: BUILD SUCCEEDED.
  • Not tested: UI behaviour by hand (launching a tagged build needs dev credentials that are not on the benchmark machine). The change is a mechanical regrouping of modifiers in the same order.
  • Next slowest site in the module, not addressed here: Sources/Surfaces/SurfaceCatalog+CloudPorts.swift:37 (2.7 s).

Demo Video

Not applicable: no behaviour change.

Checklist

  • I tested the change locally
  • I added or updated tests for behavior changes (none: no behaviour change)
  • I updated docs/changelog if needed (not needed)

🤖 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

Splits BrowserPanelView's single chain of 20 modifers into three computed views so the Swift compiler type-checks it in under 200 ms instead of ~7–8 s. Modifier order and closures are unchanged, so there is no behavior change.

Benchmarks

  • Slowest expression in Sources/Panels/BrowserPanelView.swift drops from 8,120 ms to under 200 ms.
  • No measurable change to whole-build wall clock; the win is removing a multi-second floor on incremental rebuilds of this file.
  • Build succeeds with xcodebuild -scheme cmux -configuration Debug build; UI behavior not manually tested.

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

Review in cubic

Summary by CodeRabbit

  • Refactor
    • Reorganized internal view composition without changing user-visible behavior.
    • Existing appearance, disappearance, notification, preference, and state-change handling remain unchanged.

The chain of preference, notification, appear and onChange modifiers on
BrowserPanelView's body was one expression that took about 7-8 s to
type-check. Split it into three computed views applied in the same order.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 2785dafb-520b-4716-abba-f1bd015523d4

📥 Commits

Reviewing files that changed from the base of the PR and between 498c154 and a5a8296.

📒 Files selected for processing (1)
  • Sources/Panels/BrowserPanelView.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

BrowserPanelView now composes its existing lifecycle and preference modifiers through three private computed views. The registered callbacks and their behavior remain unchanged.

Changes

Browser panel lifecycle composition

Layer / File(s) Summary
Lifecycle state-change handlers
Sources/Panels/BrowserPanelView.swift
The existing focus, screenshot, URL, rendering, appearance, theme, address-bar, omnibar, and shortcut-hint handlers are grouped in browserPanelLifecycleView.
Notification and preference composition
Sources/Panels/BrowserPanelView.swift
Notification, appearance, disappearance, coordinate-space, and preference handlers are grouped in browserPanelLifecycleNotificationsView and browserPanelLifecyclePreferencesView.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: austinywang

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: splitting BrowserPanelView’s modifier chain to reduce Swift compiler type-checking time.
Description check ✅ Passed The description explains what changed, why it changed, test results, build verification, and the reason manual UI testing was not performed. It omits the review trigger block and two review-related ch…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS: The review-scoped diff changes only Sources/Panels/BrowserPanelView.swift. It splits existing SwiftUI preference, notification, lifecycle, and onChange modifiers into computed views. No Clou…
Cmux Swift Actor Isolation ✅ Passed PASS: The PR changes only Sources/Panels/BrowserPanelView.swift. The diff moves existing SwiftUI modifiers and closures into three private some View computed properties inside `BrowserPanelView: V…
Cmux Swift Blocking Runtime ✅ Passed PASS. The PR only reorganizes existing SwiftUI modifiers into computed views in BrowserPanelView.swift. The added lines contain no semaphores, blocking waits, sleeps, delayed dispatch, polling, main…
Cmux Browser Automation Off-Main ✅ Passed PASS: The authoritative diff changes only Sources/Panels/BrowserPanelView.swift. It moves existing SwiftUI onReceive, onAppear, onDisappear, onChange, and preference modifiers into computed …
Cmux Expensive Synchronous Load ✅ Passed PASS. The diff only regroups existing SwiftUI modifiers in BrowserPanelView.swift. It adds no RestorableAgentSessionIndex, agent hook/session store, transcript, trajectory, JSONL, or broad-scan lo…
Cmux Cache Substitution Correctness ✅ Passed PASS. The authoritative PR diff changes only Sources/Panels/BrowserPanelView.swift and extracts existing SwiftUI modifiers into three computed views. It does not replace a fresh read with a cached o…
Cmux No Hacky Sleeps ✅ Passed PASS. The PR changes only Sources/Panels/BrowserPanelView.swift. The authoritative diff only splits existing SwiftUI modifiers into computed views. No added sleep, timer, polling, delayed dispatch, …
Cmux Algorithmic Complexity ✅ Passed The PR changes only Sources/Panels/BrowserPanelView.swift. The diff factors the existing modifier chain into three computed views. It adds no loops, collection scans, sorting, filtering, joins, or b…
Cmux Swift Concurrency ✅ Passed PASS. The reviewed range changes only Sources/Panels/BrowserPanelView.swift and splits an existing SwiftUI modifier chain into computed views. The base and head contain identical counts for `Dispatc…
Cmux Swift @Concurrent ✅ Passed PASS: The PR changes only Sources/Panels/BrowserPanelView.swift and only regroups existing SwiftUI modifiers into three synchronous private computed views. The authoritative diff adds no async, `n…
Cmux Swift Package Boundaries ✅ Passed PASS. The diff changes only Sources/Panels/BrowserPanelView.swift. It adds two private computed some View properties and redistributes existing SwiftUI lifecycle, preference, notification, and sta…
Cmux Swiftpm Lockfiles ✅ Passed PASS: The pull request changes only Sources/Panels/BrowserPanelView.swift. The diff contains no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project/package-reference change…
Cmux Swift Logging ✅ Passed PASS. The authoritative diff changes only Sources/Panels/BrowserPanelView.swift and only regroups existing SwiftUI modifiers into computed views. The added lines contain no print, debugPrint, `d…
Cmux User-Facing Error Privacy ✅ Passed PASS. The PR changes only Sources/Panels/BrowserPanelView.swift. The diff adds private computed-view wrappers and moves existing SwiftUI lifecycle, notification, preference, and state-change modifie…
Cmux Full Internationalization ✅ Passed PASS: The review-scoped diff changes only Sources/Panels/BrowserPanelView.swift. It moves the existing SwiftUI modifiers and closures into three private computed views. The patch adds no user-facing…
Cmux Swiftui State Layout ✅ Passed PASS. The PR changes only Sources/Panels/BrowserPanelView.swift. The diff adds three private computed view boundaries and relocates the existing modifier chain. The effective modifier sequence is id…
Cmux Architecture Rethink ✅ Passed PASS. The diff only adds three private computed-view helpers inside the existing BrowserPanelView. It adds no timing repair, mutable state, cache, singleton, lock, polling, or duplicate action path.…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The PR changes only Sources/Panels/BrowserPanelView.swift and only restructures existing SwiftUI preference, notification, lifecycle, and onChange modifiers into computed views. The authorit…
Cmux Source Artifacts ✅ Passed PASS: The review range changes only Sources/Panels/BrowserPanelView.swift, a normal tracked Swift source file. The diff only reorganizes existing SwiftUI modifiers into computed views. No artifact d…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS — The only changed file is Sources/Panels/BrowserPanelView.swift, which is production source, but the PR only splits the existing SwiftUI modifier chain into three private computed views. The a…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • 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.

@greptile-apps

greptile-apps Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge because the refactor preserves the original SwiftUI modifier structure and behavior.

Summary

This PR reduces Swift compiler type-checking time by dividing BrowserPanelView’s long modifier expression into three sequential computed views.

  • Preserves the existing preference, notification, lifecycle, and state-change modifiers.
  • Preserves modifier order and closure implementations.
  • Introduces no behavioral, dependency, localization, or security changes.

Reviews (1) · Last reviewed commit: "perf: split BrowserPanelView's modifier ..."

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Continued in #13130 (in-org branch).

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