Repository navigation
Fix hidden right-sidebar FileExplorer mount loop on launch - #4937
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces ChangesRight-Sidebar Content Mount Policy
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (16 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryGates right-sidebar panel content (Files, Find, Sessions, Feed, Dock) behind a new
Confidence Score: 5/5Safe to merge; the change narrows the mount boundary to a single, well-tested guard and leaves all existing visible-sidebar paths unchanged. The fix is narrowly scoped to the cold-hidden launch path. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[App Launch] --> B{fileExplorerState.isVisible?}
B -- false --> C[Cold hidden path\nhasMountedRightSidebarContent = false]
B -- true --> D[Visible path\nhasMountedRightSidebarContent = true]
C --> E[contentForMode:\nRightSidebarContentMountPolicy.shouldMountContent]
D --> E
E --> F{isVisible OR\nhasMountedContent?}
F -- false --> G[Color.clear\nNo FileExplorer/Find/Sessions/Feed/Dock mounted]
F -- true --> H[Switch on mode\nMount FileExplorerPanelView / SessionIndexView etc.]
I[User opens sidebar] --> J[onChange isVisible = true]
J --> K[hasMountedRightSidebarContent = true]
K --> E
H --> L[User hides sidebar]
L --> M[isVisible = false\nhasMountedContent = true]
M --> E
E --> F
F -- true --> H
Reviews (6): Last reviewed commit: "fix: preserve mounted sidebar content af..." | Re-trigger Greptile |
…top macOS 26 hang
The right-sidebar FileExplorer pinned the main thread at 100% CPU on macOS 26
(Tahoe), beachballing the app until force-quit. A captured .hang shows every
main-thread sample in one SwiftUI transaction loop:
FileExplorerPanelView.updateNSView
-> FileExplorerContainerView.updateSearchLayout
-> -[NSView _setHidden:] -> KVO didChangeValueForKey: -> re-enter graph
updateNSView mutated AppKit state on every SwiftUI pass: updateSearchLayout,
updateVisibility, and the header reassigned NSView.isHidden, a constraint
constant, an NSImage, and label strings unconditionally. Assigning isHidden to
its current value still emits KVO and setNeedsDisplay, which on macOS 26
re-enters the graph through updateNSView and never settles.
Guard every write so the pass mutates (and requests layout) only when a value
actually changes. Reproduces via Cmd+Shift+F (Find in Directory) with the
sidebar visible, and on launch with a persisted Find sidebar.
Complements manaflow-ai#4937, which stops the hidden panel mounting at launch, by removing
the loop at its source.
…top macOS 26 hang
The right-sidebar FileExplorer pinned the main thread at 100% CPU on macOS 26
(Tahoe), beachballing the app until force-quit. A captured .hang shows every
main-thread sample in one SwiftUI transaction loop:
FileExplorerPanelView.updateNSView
-> FileExplorerContainerView.updateSearchLayout
-> -[NSView _setHidden:] -> KVO didChangeValueForKey: -> re-enter graph
updateNSView mutated AppKit state on every SwiftUI pass: updateSearchLayout,
updateVisibility, and the header reassigned NSView.isHidden, a constraint
constant, an NSImage, and label strings unconditionally. Assigning isHidden to
its current value still emits KVO and setNeedsDisplay, which on macOS 26
re-enters the graph through updateNSView and never settles.
Guard every write so the pass mutates (and requests layout) only when a value
actually changes. Reproduces via Cmd+Shift+F (Find in Directory) with the
sidebar visible, and on launch with a persisted Find sidebar.
Complements manaflow-ai#4937, which stops the hidden panel mounting at launch, by removing
the loop at its source.
|
Merged current |
) (#5786) * Add failing test: FileExplorer update passes must stay idempotent Drive the container to a steady Find state and assert that redundant updateVisibility and updatePresentation(.find) passes leave needsLayout unchanged, with a positive control that a genuine change still invalidates. Fails today because updateSearchLayout sets needsLayout on every call. * Fix #4931: make FileExplorer panel updates idempotent to stop macOS 26 hang The right-sidebar FileExplorer pinned the main thread at 100% CPU on macOS 26 (Tahoe), beachballing the app until force-quit. A captured .hang shows every main-thread sample in one SwiftUI transaction loop: FileExplorerPanelView.updateNSView -> FileExplorerContainerView.updateSearchLayout -> -[NSView _setHidden:] -> KVO didChangeValueForKey: -> re-enter graph updateNSView mutated AppKit state on every SwiftUI pass: updateSearchLayout, updateVisibility, and the header reassigned NSView.isHidden, a constraint constant, an NSImage, and label strings unconditionally. Assigning isHidden to its current value still emits KVO and setNeedsDisplay, which on macOS 26 re-enters the graph through updateNSView and never settles. Guard every write so the pass mutates (and requests layout) only when a value actually changes. Reproduces via Cmd+Shift+F (Find in Directory) with the sidebar visible, and on launch with a persisted Find sidebar. Complements #4937, which stops the hidden panel mounting at launch, by removing the loop at its source. * Bump Swift file length budget for FileExplorer idempotency fix The #4931 fix adds idempotency guards to FileExplorerView.swift (+26 lines) and a regression test to FileExplorerStoreTests.swift (+50 lines), pushing both past their tracked budgets. Refresh the two entries to accept this debt. * Fix FileExplorer tests to use Swift Testing * Fix FileExplorer Swift Testing discovery --------- Co-authored-by: austinpower1258 <austinwang115@gmail.com>
…naflow-ai#4931) (manaflow-ai#5786) * Add failing test: FileExplorer update passes must stay idempotent Drive the container to a steady Find state and assert that redundant updateVisibility and updatePresentation(.find) passes leave needsLayout unchanged, with a positive control that a genuine change still invalidates. Fails today because updateSearchLayout sets needsLayout on every call. * Fix manaflow-ai#4931: make FileExplorer panel updates idempotent to stop macOS 26 hang The right-sidebar FileExplorer pinned the main thread at 100% CPU on macOS 26 (Tahoe), beachballing the app until force-quit. A captured .hang shows every main-thread sample in one SwiftUI transaction loop: FileExplorerPanelView.updateNSView -> FileExplorerContainerView.updateSearchLayout -> -[NSView _setHidden:] -> KVO didChangeValueForKey: -> re-enter graph updateNSView mutated AppKit state on every SwiftUI pass: updateSearchLayout, updateVisibility, and the header reassigned NSView.isHidden, a constraint constant, an NSImage, and label strings unconditionally. Assigning isHidden to its current value still emits KVO and setNeedsDisplay, which on macOS 26 re-enters the graph through updateNSView and never settles. Guard every write so the pass mutates (and requests layout) only when a value actually changes. Reproduces via Cmd+Shift+F (Find in Directory) with the sidebar visible, and on launch with a persisted Find sidebar. Complements manaflow-ai#4937, which stops the hidden panel mounting at launch, by removing the loop at its source. * Bump Swift file length budget for FileExplorer idempotency fix The manaflow-ai#4931 fix adds idempotency guards to FileExplorerView.swift (+26 lines) and a regression test to FileExplorerStoreTests.swift (+50 lines), pushing both past their tracked budgets. Refresh the two entries to accept this debt. * Fix FileExplorer tests to use Swift Testing * Fix FileExplorer Swift Testing discovery --------- Co-authored-by: austinpower1258 <austinwang115@gmail.com>
…ai#4937) * test: cover hidden find sidebar launch * fix: skip hidden right-sidebar content mounts * fix: update right sidebar merge follow-ups * fix: migrate right sidebar regression to Swift Testing * fix: move hidden sidebar regression test * fix: preserve mounted sidebar content after open --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com> Co-authored-by: austinpower1258 <austinwang115@gmail.com>
Summary
Issue triage
Proof
cmuxTests/WindowDragHandleHitTests/testHiddenRightSidebarDoesNotMountFileExplorerPanelContentfailed on AWS withXCTAssertNil failedbecauseFileExplorerContainerViewwas mounted while hidden.BonsplitTabDragUITests/testLaunchCompletesWithHiddenRightSidebarRestoringFindMode.Notes
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes SwiftUI mounting for the main right sidebar on launch and toggle paths; behavior is policy-gated and covered by unit/UI tests, but affects File Explorer/Find lifecycle.
Overview
Fixes a launch-time issue where persisted Find/Files mode still mounted heavy right-sidebar panels (
FileExplorerPanelView, etc.) while the sidebar was hidden, which could trigger a SwiftUI update loop.RightSidebarContentMountPolicygatescontentForMode: on a cold hidden launch it rendersColor.clearinstead of mode panels; once the sidebar has been shown, content stays mounted when hidden again so width animation and mode state stay smooth. The outer sidebar shell inContentViewremains in the tree unchanged.Adds
HiddenRightSidebarContentMountingTests(policy + noFileExplorerContainerViewwhen hidden) and a Bonsplit UI test that launch succeeds with hidden sidebar restoring Find mode (search field absent).Reviewed by Cursor Bugbot for commit 92ee8c6. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes #4931 by stopping right-sidebar panel content from mounting while the sidebar is hidden to prevent a launch-time SwiftUI update loop. The restored mode and shell remain, and after the first show the content stays mounted when hidden to avoid churn.
RightSidebarContentMountPolicy(requiresfileExplorerState.isVisibleor a prior mount); renderColor.clearotherwise while keeping the shell for width animation.cmuxTests/HiddenRightSidebarContentMountingTests.swift(policy + hidden-mount regression) and a UI test that launch completes with a hidden sidebar restoring Find mode.Written for commit 92ee8c6. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests