Repository navigation
Fix sidebar scroll layout livelock - #8211
Conversation
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe sidebar now constructs immutable workspace snapshots and injected row actions before ChangesSidebar lazy-layout architecture
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (21 passed)
✨ Finishing Touches🧪 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 SummaryThis PR correctly fixes a SwiftUI layout feedback loop (issue #6707) where
Confidence Score: 5/5Safe to merge. The feedback-edge removal is mechanically correct and the snapshot boundary eliminates the class of re-entrant layout invalidation described in the field capture. The root-cause fix (GeometryReader synchronous layout input replacing the onGeometryChange @State write) is structurally sound and directly breaks the NSHostingView transaction re-entry cycle. The new snapshot boundary is enforced at the type level — SidebarWorkspaceRenderItem now carries only UUIDs, rows receive immutable values plus closures, and the coalescer ensures at most one @State publication per run-loop turn. All three validation runs (macOS 15 UI stress, macOS 15 app-host stress, macOS 26 app-host stress) passed on the fixed HEAD. The one open concern — O(4N) task teardown/restart per workspace-list change in the centralised observation modifiers — is a future-scale issue at the PR's benchmarked 120-workspace ceiling, not a correctness defect in the current change. Sources/WorkspaceSidebarObservation.swift — the three task(id: ids) modifiers recreate all N–2N child tasks on every workspace-list change; worth revisiting if the workspace ceiling grows beyond the benchmarked 120. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
WS["Workspace / Store (live @Observable)"] -->|sidebarObservationPublisher| OBS["task(id: ids) observation modifiers\nabove LazyVStack boundary"]
OBS -->|onChange per workspace| COAL["SidebarWorkspaceSnapshotRefreshCoalescer\nRunLoop.main.perform — one flush per turn"]
COAL -->|refreshWorkspaceSnapshots| SNAP["@State workspaceSnapshotsById\nUUID to Snapshot dict"]
SNAP --> ROWS["workspaceScrollRows\nbuild workspaceRowInputsById O(N)\nbuild notificationIndex O(K log K)\nbuild listSnapshot O(selected)"]
ROWS -->|immutable SidebarWorkspaceRowInput| LVS["LazyVStack — realizes visible rows only"]
LVS -->|rowSnapshot + actionFactory| RV["SidebarWorkspaceRowView\n+ SidebarWorkspaceRowActions closures"]
GR["GeometryReader viewport.size"] -->|contentMinHeight — layout input only| SC["workspaceScrollContent ScrollView + LazyVStack"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
WS["Workspace / Store (live @Observable)"] -->|sidebarObservationPublisher| OBS["task(id: ids) observation modifiers\nabove LazyVStack boundary"]
OBS -->|onChange per workspace| COAL["SidebarWorkspaceSnapshotRefreshCoalescer\nRunLoop.main.perform — one flush per turn"]
COAL -->|refreshWorkspaceSnapshots| SNAP["@State workspaceSnapshotsById\nUUID to Snapshot dict"]
SNAP --> ROWS["workspaceScrollRows\nbuild workspaceRowInputsById O(N)\nbuild notificationIndex O(K log K)\nbuild listSnapshot O(selected)"]
ROWS -->|immutable SidebarWorkspaceRowInput| LVS["LazyVStack — realizes visible rows only"]
LVS -->|rowSnapshot + actionFactory| RV["SidebarWorkspaceRowView\n+ SidebarWorkspaceRowActions closures"]
GR["GeometryReader viewport.size"] -->|contentMinHeight — layout input only| SC["workspaceScrollContent ScrollView + LazyVStack"]
Reviews (6): Last reviewed commit: "perf: index sidebar notification project..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/SidebarLazyLayoutScaleTests.swift`:
- Around line 280-282: Rename
testRowBodyEvaluationBuildsWorkspaceSnapshotAtMostOnce to clearly state that row
body evaluation never builds a workspace snapshot, matching the strict worstBody
== 0 assertion and updated documentation.
In `@cmuxTests/SidebarPointerInteractionScaleTests.swift`:
- Around line 246-251: Replace the fixed 50 ms Task.sleep in the scrolling test
with a real snapshot-refresh completion or generation signal, or a
deadline-bounded poll of an observable refresh predicate. Ensure each cycle
waits until the parent sidebar observation and snapshot refresh have completed
before draining the run loop, while preserving the existing test flow.
In `@Sources/ContentView.swift`:
- Line 9961: Remove the DEBUG-only sidebarLazyContractProbe environment
dependency and all related production observation seams from ContentView.swift,
including the additional referenced locations. Move the lazy-contract
instrumentation into test-target observation or an isolated debug facility,
leaving production Sources code free of test-only probes while preserving the
existing test coverage.
- Around line 12742-12744: Update the X-button’s closeWorkspace action to route
through the shared close-gesture path and pass
CloseTabConfirmationTrigger.tabCloseButton explicitly, rather than calling
closeWorkspaceWithConfirmation directly. Apply the same trigger-specific change
to the corresponding close action at the other referenced location, preserving
explicit triggers for every close flow.
- Around line 12397-12438: Update moveWorkspaceRows and
moveWorkspaceRowsToNewWindow to record only workspace IDs whose
moveWorkspaceToWindow or moveWorkspaceToNewWindow operation succeeds, and
subtract only those IDs from selectedTabIds. In moveWorkspaceRowsToNewWindow,
ensure each workspace is moved to the new window exactly once while preserving
focus on the final successfully moved workspace, without re-moving
orderedIds.last.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 053d8a99-50c1-4dfa-8e5b-bbc58899188c
📒 Files selected for processing (24)
Sources/ContentView.swiftSources/Sidebar/SidebarBonsplitTabDropDelegate.swiftSources/Sidebar/SidebarBonsplitWorkspaceRowDropModifier.swiftSources/SidebarWorkspaceContextMenuSnapshot.swiftSources/SidebarWorkspaceGroupRowView.swiftSources/SidebarWorkspaceRenderItem.swiftSources/SidebarWorkspaceRowActions.swiftSources/SidebarWorkspaceRowSnapshot.swiftSources/SidebarWorkspaceRowView.swiftSources/SidebarWorkspaceSnapshotFactory.swiftSources/SidebarWorkspaceWindowMoveTarget.swiftSources/TabItemView+WorkspaceContextMenu.swiftSources/TabItemView+WorkspaceGroups.swiftSources/TabItemView+WorkspaceNotificationsMenu.swiftSources/TabItemView+WorkspaceTodo.swiftSources/VerticalTabsSidebar+WorkspaceGroups.swiftSources/WorkspaceSidebarObservation.swiftSources/WorkspaceSidebarProcessTitleObservationModel.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SidebarLazyLayoutScaleTests.swiftcmuxTests/SidebarPointerInteractionScaleTests.swiftcmuxTests/WorkspaceGroupTests.swiftcmuxUITests/WorkspaceSidebarScrollUITests.swiftscripts/check-sidebar-lazy-layout.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ContentView.swift (1)
12795-12803: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRevalidate read/unread eligibility immediately before every mutation.
Snapshot eligibility can be heterogeneous or stale, so action handlers must filter targets using the live notification store.
Sources/ContentView.swift#L12795-L12803: apply each operation only when the corresponding singleton eligibility check succeeds.Sources/VerticalTabsSidebar+WorkspaceGroups.swift#L194-L198: add the same checks for the group anchor.Proposed fix
markRead: { workspaceIds in - for workspaceId in workspaceIds { + for workspaceId in workspaceIds + where notificationStore.canMarkWorkspaceRead(forTabIds: [workspaceId]) { notificationStore.markRead(forTabId: workspaceId) } }, markUnread: { workspaceIds in - for workspaceId in workspaceIds { + for workspaceId in workspaceIds + where notificationStore.canMarkWorkspaceUnread(forTabIds: [workspaceId]) { notificationStore.markUnread(forTabId: workspaceId) } },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/ContentView.swift` around lines 12795 - 12803, Revalidate each target immediately before mutation: in Sources/ContentView.swift lines 12795-12803, update the markRead and markUnread handlers to call the corresponding singleton eligibility check on notificationStore and mutate only eligible workspace IDs; apply the same live checks to the group anchor in Sources/VerticalTabsSidebar+WorkspaceGroups.swift lines 194-198. Preserve the existing markRead(forTabId:) and markUnread(forTabId:) operations for eligible targets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/SidebarWorkspaceRowsSnapshot.swift`:
- Around line 29-44: Build a parent-owned notification index in
SidebarWorkspaceRowsSnapshot instead of rescanning notifications: replace
hasNotification and contextMenuNotifications with indexed membership checks and
bounded merging of per-workspace buckets. In
Sources/SidebarWorkspaceRowsSnapshot.swift lines 29-44, create and reuse the
index; in Sources/SidebarWorkspaceRowInput.swift lines 122-125, consume it
without rebuilding menu data per realized row; and in
Sources/VerticalTabsSidebar+WorkspaceGroups.swift line 43, use a precomputed
workspace-ID set for group notification presence.
---
Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 12795-12803: Revalidate each target immediately before mutation:
in Sources/ContentView.swift lines 12795-12803, update the markRead and
markUnread handlers to call the corresponding singleton eligibility check on
notificationStore and mutate only eligible workspace IDs; apply the same live
checks to the group anchor in Sources/VerticalTabsSidebar+WorkspaceGroups.swift
lines 194-198. Preserve the existing markRead(forTabId:) and
markUnread(forTabId:) operations for eligible targets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3ec5b223-1547-4f5c-94cb-17a87cca0474
📒 Files selected for processing (10)
Sources/ContentView.swiftSources/SidebarWorkspaceGroupRowSnapshot.swiftSources/SidebarWorkspaceRowActions.swiftSources/SidebarWorkspaceRowInput.swiftSources/SidebarWorkspaceRowsSnapshot.swiftSources/VerticalTabsSidebar+WorkspaceGroups.swiftSources/WorkspaceSidebarObservation.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SidebarLazyLayoutScaleTests.swiftcmuxTests/SidebarPointerInteractionScaleTests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/SidebarWorkspaceRowInput.swift (1)
49-126: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftPrecompute selection-wide context-menu aggregates outside row realization.
rowSnapshotruns per realized row but repeatedly scanstargetWorkspaceIdsfor remote, grouping, notification, and read-state projections. This is O(V×S) work for V visible rows and S selected workspaces—potentially around 1,000 records—and risks restoring sidebar scroll churn.Compute these shared aggregates once in
SidebarWorkspaceRowsSnapshot; keep only row-specific todo/pin assembly here.As per path instructions, snapshot projection loops must run “once per snapshot generation, not repeatedly per row.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SidebarWorkspaceRowInput.swift` around lines 49 - 126, Move the selection-wide context-menu aggregate calculations currently performed in rowSnapshot into SidebarWorkspaceRowsSnapshot, computing remote, grouping, notification, and read-state projections once per snapshot generation. Expose the precomputed values from SidebarWorkspaceRowsSnapshot and have rowSnapshot reuse them when constructing SidebarWorkspaceContextMenuSnapshot, retaining only row-specific todo and pin assembly and preserving single-selection behavior.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/SidebarWorkspaceRowInput.swift`:
- Around line 49-126: Move the selection-wide context-menu aggregate
calculations currently performed in rowSnapshot into
SidebarWorkspaceRowsSnapshot, computing remote, grouping, notification, and
read-state projections once per snapshot generation. Expose the precomputed
values from SidebarWorkspaceRowsSnapshot and have rowSnapshot reuse them when
constructing SidebarWorkspaceContextMenuSnapshot, retaining only row-specific
todo and pin assembly and preserving single-selection behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8d4492c-6a4f-4447-ada9-b585f47d532b
📒 Files selected for processing (6)
Sources/ContentView.swiftSources/Debug/SidebarLazyContractProbe.swiftSources/SidebarWorkspaceRowInput.swiftSources/SidebarWorkspaceSnapshotRefreshCoalescer.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SidebarLazyLayoutScaleTests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/SidebarWorkspaceRowInput.swift (1)
48-129: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftDefer expensive context-menu state evaluation to prevent scroll stutter.
rowSnapshot(list:)is evaluated eagerly for every row realized by theLazyVStack. By populating thecontextMenuhere, it invokeslist.contextMenuNotifications(workspaceIds:)(and other group-eligibility list helpers), which re-filters and re-sorts the globalnotificationscollection on each row render.Because
targetWorkspaceIdsalways contains at least the row's own ID, this causes repeated O(N log N) rescans of the notifications array (expected scale: potentially 1000+ items) on the main thread during scrolling.As per path instructions, avoid per-item rescans (
filter/sorted/...) inside loops over scalable collections, and ensure iteration is bounded or uses cached snapshots so row rendering doesn't repeatedly sort unbounded collections.Proposed Fix:
To maintain the lazy-layout contract, compute the context menu data lazily (e.g., passing a closure likecontextMenuSnapshot:@escaping() -> SidebarWorkspaceContextMenuSnapshotthat evaluates only when the menu is presented, similar toactions.currentWindowMoveTargets()), or cache the resulting notification arrays at thelistsnapshot level rather than re-evaluating them for each row.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SidebarWorkspaceRowInput.swift` around lines 48 - 129, Defer construction of the context-menu snapshot in rowSnapshot(list:) instead of eagerly evaluating it for every realized row. Update the SidebarWorkspaceRowSnapshot/context-menu flow to store an escaping lazy snapshot closure, or reuse a list-level cached context-menu snapshot, and invoke it only when the context menu is presented; preserve the existing target, group, remote-state, and notification behavior without per-row rescans.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/SidebarWorkspaceRowInput.swift`:
- Around line 48-129: Defer construction of the context-menu snapshot in
rowSnapshot(list:) instead of eagerly evaluating it for every realized row.
Update the SidebarWorkspaceRowSnapshot/context-menu flow to store an escaping
lazy snapshot closure, or reuse a list-level cached context-menu snapshot, and
invoke it only when the context menu is presented; preserve the existing target,
group, remote-state, and notification behavior without per-row rescans.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 368e6662-4d1a-42c2-a380-5920dd1d3e78
📒 Files selected for processing (8)
Sources/ContentView.swiftSources/SidebarWorkspaceContextMenuSnapshot.swiftSources/SidebarWorkspaceRowActions.swiftSources/SidebarWorkspaceRowInput.swiftSources/SidebarWorkspaceRowsSnapshot.swiftSources/TabItemView+WorkspaceContextMenu.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SidebarWorkspaceContextMenuWindowTargetsTests.swift
💤 Files with no reviewable changes (2)
- Sources/SidebarWorkspaceContextMenuSnapshot.swift
- Sources/SidebarWorkspaceRowsSnapshot.swift
|
@codex review |
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 16 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedReview finished.
|
|
To use Codex here, create a Codex account and connect to github. |
|
Review triage for the non-actionable policy claims:
No code changes were made for those rejected findings. |
* Add failing demand-contract regression test for coalesceLatest DemandTrackingSubscriber records values received while downstream demand is zero instead of trapping. Without the fix, coalesceLatest forwards the @published replay and every leading-edge emission regardless of demand, which is the contract violation that crashes AsyncPublisher (.values) consumers in WorkspaceSidebarObservation. * Honor downstream demand in coalesceLatest coalesceLatest ignored downstream demand by design (sink-style only), but WorkspaceSidebarObservation consumes it through .values (Combine.AsyncPublisher), which requests one value at a time and traps with "received an unexpected value" when a value arrives at zero demand. The trap lives in the system Combine binary, so it crashes release builds too. The @published replay is forwarded synchronously in receive(subscription:) before the async iterator has requested anything, so the app can crash at startup (two crash reports on 2026-07-15, introduced by #8211). Track outstanding demand; a value reaching an emission point at zero demand is conflated into a latest-value slot and delivered from request(_:) when demand arrives. Sink-style subscribers (unlimited demand) keep the synchronous leading edge unchanged. --------- Co-authored-by: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
Merges origin/main (a5a70ff) including the sidebar row-layer refactor (SidebarWorkspaceRowInput projections, container-owned snapshots and observation, selectWorkspaceRow) plus #8211 and the #8240 terminal-resize coalescing. The AppKit table now consumes the same projection as the SwiftUI list: workspaceTableRowConfiguration builds its row model from SidebarWorkspaceRowInput and the shared context-menu aggregates, container observation replaces the per-cell pump and snapshot memo, and the AppKit actions bundle is renamed SidebarAppKitRowActions to coexist with upstream's type. Verified rendering and interaction on both flag states.
Ported from upstream manaflow-ai#8211 (0f40759): "Fix sidebar scroll layout livelock (manaflow-ai#8211)". Removes the onGeometryChange -> @State workspaceScrollContentMinHeight feedback edge (viewport height is now a downward-only GeometryReader input) and moves every live Workspace/store observation above the LazyVStack: rows receive immutable SidebarWorkspaceRowSnapshot values plus a SidebarWorkspaceRowActions closure bundle, and workspace publisher bursts are coalesced once per run-loop batch. Adapted to this fork: stripped upstream's workspace todo/checklist/task-status fields, actions and UI (the feature does not exist here); kept the fork's sidebar footer views and SidebarWorkspaceSnapshotBuilder; moveWorkspaceRow uses the fork's reorderWorkspace(tabId:toIndex:); the §6.2 per-row geometry trace now lives on SidebarWorkspaceRowView (the minHeight trace point is gone with the state it traced).
Ported from upstream manaflow-ai#8236 (14effdc): "Fix coalesceLatest demand-contract crash introduced by manaflow-ai#8211 (manaflow-ai#8236)".
2 participants
Fixes #6707.
What changed
ScrollView.onGeometryChange -> @State workspaceScrollContentMinHeightfeedback edge. Viewport geometry is now a downward-only input to the scroll content layout.LazyVStack:VerticalTabsSidebarowns all liveWorkspace/store observation and builds immutable value snapshots; workspace and group rows receive values plus action closures only.TabItemViewownership of live workspace/store references, bindings, row snapshot@State, the body-mutated snapshot scratch box, per-row Combine subscriptions, and per-row async observation tasks.Root cause
This is a SwiftUI update/layout feedback loop, not a Ghostty render deadlock.
The field capture shows 6,710/6,734 main-thread samples in
NSRunLoop.flushObservers/NSHostingView.beginTransaction, 6,701 inGraphHost.flushTransactions, and 5,072 inAG::Subgraph::update. The same capture reports 1,532 reentrantNSHostingViewlayout faults in three hours, 97% CPU over 92 seconds, and +503.84 MB physical footprint in 87.5 seconds while Ghostty renderer threads slept.The remaining cycle after #7117 was visible in code:
workspaceScrollContentMinHeightduring the hosting view layout transaction, invalidating the same scroll/lazy graph being measured.TabItemViewinstances that each retained live models/bindings and started publishers/tasks. Their initial and CLI-driven emissions wrote row@Statesnapshots whileLazyVStackwas placing/retiring rows. The snapshot getter also filled a reference scratch box from a body computation.set_status/clear_statusmutations change the row projection and sometimes its height while scrolling crosses realization boundaries. Those invalidations re-entered placement, recreated row observation, and scheduled another graph transaction. On the bad interleaving the graph never converged and allocated as it spun.PR #7117 correctly removed the row
GeometryReader, per-row AppKit hover hosts, and always-mounted drop machinery, which made each iteration cheaper. It did not remove the viewport state write or the live per-row observation/state lifecycle, so it reduced the frequency/cost without eliminating the cycle.The pointer-frame path was also audited: its frame registries are already
@ObservationIgnoredand observable hover reconciliation is deferred beyond the geometry callback, so it is not another synchronous layout-state edge.Reproduction and evidence boundary
I did not manufacture a red test result.
CGEventscroll phases, paired status churn, exact-timestamp reentrant-layout log filtering, and a convergence check passed on the pre-fix macOS 26 commit: https://github.com/manaflow-ai/cmux/actions/runs/29462786830set_status/clear_statuscommands on the same row, and a main-actor watchdog after every gesture also passed on the pre-fix macOS 15 commit: https://github.com/manaflow-ai/cmux/actions/runs/29463880365The first commit is therefore deliberately named a guard, not a reproducer. It adds the two stress/liveness workloads and the exact layout-fault/convergence assertions, but the field sample and logs remain the definitive reproduction evidence.
Validation
Fixed-HEAD focused runs:
Static checks completed locally without invoking
xcodebuild:swiftc -frontend -parsepython3 scripts/check-sidebar-lazy-layout.py./scripts/check-pbxproj.sh./scripts/lint-pbxproj-test-wiring.sh(509 test files)python3 scripts/check-workspace-package-groups.py --checkgit diff --check origin/main...HEADResources/Localizable.xcstringswithjqThe requested Swift file-length script/TSV do not exist on current
main; no budget file was generated or edited. Every new Swift file is below 500 lines (largest: 339), the existing UI test file remains below 500 lines (468), andContentView.swiftshrank..github/swift-warning-budget.tsvis untouched.python3 scripts/check-package-resolved-policy.pycurrently fails on unchangedorigin/mainbecauseios/cmux.xcworkspace/xcshareddata/swiftpm/Package.resolvedis classified as an unexpected location; this branch changes no lockfile or ignore policy.Localization audit
No user-facing strings or localization catalogs changed. Relocated UI code continues to use existing
String(localized:)keys,Resources/Localizable.xcstringsparses successfully, and the diff introduces no new runtime-facing English copy.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes a sidebar scroll livelock that froze the app while scrolling (Fixes #6707). Keeps the layout responsive under status churn, preserves
LazyVStackscaling, coalesces snapshot updates, and serves context menus from immutable snapshots with fresh window targets and indexed notifications.Bug Fixes
Equatablerows, and guards/tests to keep bounded realization.Refactors
VerticalTabsSidebar; rows mount viaSidebarWorkspaceRowView/SidebarWorkspaceGroupRowViewfrom immutable values with action closures.SidebarWorkspaceSnapshotFactory, row/group/input/context-menu snapshots, and row actions; menus read from snapshots and actions.sidebarWorkspaceObservations(...); removed unused observation conformance.SidebarWorkspaceNotificationIndex) for constant-time presence checks and bounded context-menu lists.Written for commit cb24937. Summary will update on new commits.
Summary by CodeRabbit