Repository navigation
iOS: rebuild workspace list on UITableView with exact row heights (kills scrollbar stutter) - #8186
Merged
Merged
Conversation
The workspace list's scroll indicator stutters while scrolling rows it has not visited yet. To root-cause it, extend the DEBUG layout preview (CMUX_UITEST_WORKSPACE_LIST_PREVIEW=1) with deterministic long-list seeding (CMUX_UITEST_WORKSPACE_LIST_PREVIEW_COUNT, ..._GROUPS) and a scroll-metrics probe (CMUX_UITEST_SCROLL_METRICS=1) that locates the List's backing UICollectionView, records every contentSize correction, optionally drives a top-to-bottom sweep (CMUX_UITEST_SCROLL_SWEEP=1), and writes Documents/scroll-metrics.json for the host to pull. Grouped rendering is gated on a foreground Mac, which the store-free fixture never has, so canRenderGroupsForSelection gets a DEBUG-only escape scoped to the fixture (store == nil + preview env). Measured baseline, 100 uniform 92.5pt rows, one sweep: 91 contentSize corrections of exactly +38.3pt each (content grows 5745 -> 9233pt), one correction per row materialization. SwiftUI's List estimates unrealized rows at a hardwired ~54pt: setting defaultMinListRowHeight to 16 or 92.5, removing it, and fixed .frame(height:) on rows all leave the churn byte-identical, so no public knob feeds the estimator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ar stutter SwiftUI's List sizes unrealized rows with an internal ~54pt estimate no public API influences, so scrolling a ~92pt-per-row workspace list grows contentSize by +38pt on every row materialization and visibly jerks the scroll indicator (one jump per row; Apple bug, still present on iOS 26). The workspace list is uniform per row kind by design (single-line titles, reserved preview lines, fixed header/footer rows), so the true content height is exactly computable. WorkspaceListScrollIndicatorStabilizer mounts behind the List, learns per-kind heights from realized cells (nothing hardcoded, Dynamic Type safe), and rewrites contentSize.height to the exact value whenever the layout writes an estimate-based one. The kind sequence comes from the same snapshot the body renders (scrollPinModel), so the model cannot drift from the mounted rows. Fail-open guards: pinning pauses when the model is incomplete, when the item count disagrees with the collection view (mid-update), during reorder drags, and when wrap-workspace-titles makes rows non-uniform. Pausing means untouched system behavior. Fixture sweep, 100 rows: draw-time content heights per sweep go from 92 distinct values (5745 -> 9233pt drift) to 1, in flat and grouped modes; group footers still realize at 16pt and headers at 44pt. Frame analysis of interactive flicks shows constant indicator length (189px) and zero adjacent-frame anomalies across 273 indicator-visible frames. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughiOS workspace lists now use a UIKit-backed table with diffable rows, cached sizing, workspace actions, and modal presentation wiring. Debug previews generate deterministic grouped data and can mount scroll instrumentation that records layout metrics. Package policy validation accepts workspace-level SwiftPM lockfiles. ChangesUIKit workspace list
Package lockfile policy
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Preview
participant WorkspaceListView
participant WorkspaceListTable
participant Coordinator
participant UITableView
Preview->>WorkspaceListView: provide seeded workspaces and groups
WorkspaceListView->>WorkspaceListTable: construct table configuration
WorkspaceListTable->>Coordinator: attach and update configuration
Coordinator->>UITableView: apply diffable snapshot and host rows
UITableView->>Coordinator: deliver selection, swipe, menu, and refresh actions
Coordinator->>WorkspaceListView: invoke configured callbacks
Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (22 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 |
Contributor
Greptile SummaryThis PR replaces the SwiftUI
Confidence Score: 4/5Safe to merge with one open regression: the workspaceTable computed property rebuilds an O(n) workspace dictionary and traverses the grouped-item list twice on every SwiftUI body render, which will cause visible main-thread churn on large lists — working against the very goal this PR achieves. The core UITableView implementation is solid: estimation is fully disabled, the height cache keying is thorough (one gap already filed), the diffable-source reconfiguration path correctly detects all payload-changing fields, and swipe/context-menu/drag scaffolding maps cleanly to the existing model layer. The one outstanding correctness concern is WorkspaceListView+Table.swift: workspaceTable is a hot computed property in body that allocates an O(n) workspacesByID dictionary and iterates displayedGroupedListItems twice per render. For the ~1000-workspace scale the algorithmic-complexity rule targets, this introduces a measurable per-frame allocation budget that directly undermines scroll smoothness. WorkspaceListView+Table.swift — the hot-body allocations; WorkspaceListView+MacSelection.swift and WorkspaceListScrollMetricsProbe.swift — open debug-seam issues from the previous review thread. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant SV as WorkspaceListView (SwiftUI body)
participant T as WorkspaceListTable (UIViewRepresentable)
participant C as WorkspaceListTableCoordinator
participant TV as WorkspaceListUITableView (UITableView)
SV->>T: body evaluates workspaceTable
Note over SV,T: builds workspacesByID O(n), traverses displayedGroupedListItems x2
T->>C: updateUIView → update(configuration:in:)
C->>C: diff items vs previousConfiguration
C->>TV: dataSource.apply(snapshot)
TV->>C: heightForRowAt (per uncached row)
C->>C: configure(sizingCell, for: item)
C->>C: systemLayoutSizeFitting → exact height
C->>C: "heightCache[key] = exact"
TV-->>C: cached on repeat renders
TV->>C: didSelectRowAt
C->>SV: selectWorkspace(id)
TV->>C: trailingSwipeAction / contextMenu
C->>SV: requestWorkspaceClose / renameRequest
SV->>SV: workspacePendingCloseID / RenameID set
SV-->>SV: .confirmationDialog / .sheet presented
%%{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"}}}%%
sequenceDiagram
participant SV as WorkspaceListView (SwiftUI body)
participant T as WorkspaceListTable (UIViewRepresentable)
participant C as WorkspaceListTableCoordinator
participant TV as WorkspaceListUITableView (UITableView)
SV->>T: body evaluates workspaceTable
Note over SV,T: builds workspacesByID O(n), traverses displayedGroupedListItems x2
T->>C: updateUIView → update(configuration:in:)
C->>C: diff items vs previousConfiguration
C->>TV: dataSource.apply(snapshot)
TV->>C: heightForRowAt (per uncached row)
C->>C: configure(sizingCell, for: item)
C->>C: systemLayoutSizeFitting → exact height
C->>C: "heightCache[key] = exact"
TV-->>C: cached on repeat renders
TV->>C: didSelectRowAt
C->>SV: selectWorkspace(id)
TV->>C: trailingSwipeAction / contextMenu
C->>SV: requestWorkspaceClose / renameRequest
SV->>SV: workspacePendingCloseID / RenameID set
SV-->>SV: .confirmationDialog / .sheet presented
Reviews (6): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
103
to
113
| var canRenderGroupsForSelection: Bool { | ||
| macSelectionScope.canRenderGroupsForSelection | ||
| #if DEBUG | ||
| // The store-free layout fixture has no foreground Mac, so the | ||
| // foreground-scope gate can never pass there; render its seeded groups | ||
| // so grouped rows and end-of-group slots are exercised in previews. | ||
| if store == nil, UITestConfig.workspaceListLayoutPreviewEnabled { | ||
| return true | ||
| } | ||
| #endif | ||
| return macSelectionScope.canRenderGroupsForSelection | ||
| } |
Contributor
There was a problem hiding this comment.
canRenderGroupsForSelection gains a #if DEBUG branch that forces true whenever the store-free fixture is active. This is exactly the pattern the no-test-debug-seam-in-production-source rule prohibits: a debug-guarded member in production Sources/ that has no production caller and whose sole purpose is to make a test fixture behave differently. WorkspaceListScrollMetricsProbe.swift (also under Sources/) compounds this — the entire 270-line file is #if canImport(UIKit) && DEBUG with no production callers at all; it belongs in a dedicated debug folder or a test-support module.
The canonical fix for canRenderGroupsForSelection is to supply a fixture store (or a stub macSelectionScope) that already satisfies canRenderGroupsForSelection from its own logic, so no #if DEBUG branch is needed in production code. The metrics probe should be relocated out of Sources/.
Rule Used: Do not add new test/debug seams (ForTesting-styl... (source)
Comment on lines
+129
to
+152
| override func didMoveToWindow() { | ||
| super.didMoveToWindow() | ||
| guard window != nil else { | ||
| stopAttaching() | ||
| return | ||
| } | ||
| guard listCollectionView == nil, attachLink == nil else { return } | ||
| // The backing UICollectionView does not exist until SwiftUI hosts the | ||
| // List, so attachment retries per frame briefly instead of assuming | ||
| // hierarchy timing. The link is torn down as soon as it resolves. | ||
| let link = CADisplayLink(target: self, selector: #selector(attachTick)) | ||
| link.add(to: .main, forMode: .common) | ||
| attachLink = link | ||
| } | ||
|
|
||
| @objc private func attachTick() { | ||
| attachFramesLeft -= 1 | ||
| if let collectionView = nearestListCollectionView() { | ||
| stopAttaching() | ||
| attach(to: collectionView) | ||
| } else if attachFramesLeft <= 0 { | ||
| stopAttaching() | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
A CADisplayLink running on .common mode fires every frame for up to 600 frames (~10 s at 60 Hz) to discover when the SwiftUI-managed UICollectionView appears in the hierarchy. The cmux-swift-blocking-runtime rule flags timers and polling used for synchronization in production code. The workaround is justified — SwiftUI exposes no signal when the List's backing view materializes — but it is worth calling out: the poll runs on the main run loop, and nearestListCollectionView() traverses the view hierarchy on every tick until the collection view is found. In practice this resolves in one or two frames, but on a slow or deeply nested hierarchy it could take more. If the view moves out of the window before attaching (window becomes nil and stopAttaching() fires), attachFramesLeft is not reset, so a subsequent didMoveToWindow will restart the link with a depleted counter and may stop after a single tick rather than a full retry window.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+178
to
+198
| private func nearestListCollectionView() -> UICollectionView? { | ||
| var ancestor = superview | ||
| while let current = ancestor { | ||
| if let collectionView = Self.firstCollectionView(under: current) { | ||
| return collectionView | ||
| } | ||
| ancestor = current.superview | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| private static func firstCollectionView(under root: UIView) -> UICollectionView? { | ||
| var queue: [UIView] = [root] | ||
| while !queue.isEmpty { | ||
| let view = queue.removeFirst() | ||
| if let collectionView = view as? UICollectionView { | ||
| return collectionView | ||
| } | ||
| queue.append(contentsOf: view.subviews) | ||
| } | ||
| return nil |
Contributor
There was a problem hiding this comment.
UICollectionView
nearestListCollectionView() walks up the ancestor chain and, for each ancestor, BFS-searches its entire subtree for the first UICollectionView. Because the search is breadth-first starting from each ancestor (not constrained to siblings of the stabilizer's own branch), it can resolve to any UICollectionView that happens to appear earlier in the traversal — for example a picker inside a toolbar, a date-selection view, or a sheet presented over the list. Once attached, the stabilizer would mutate the wrong collection view's contentSize. The issue is latent today (workspace list view is simple), but becomes a risk if the view is ever embedded in a richer layout or if the sheet that presents workspace detail contains another scroll view.
![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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollIndicatorStabilizer.swift`:
- Around line 118-171: Replace the CADisplayLink-based attachment flow in
didMoveToWindow, attachTick, stopAttaching, and nearestListCollectionView with
an explicitly owned `@MainActor` coordinator that receives the actual list
UICollectionView bridge and snapshot through lifecycle callbacks. Remove frame
polling, the 600-frame timeout, and hierarchy scanning; update coordinator
ownership and teardown so attachment follows the bridge lifecycle and no longer
discovers or selects collections implicitly.
- Around line 111-125: Version WorkspaceListScrollPinModel with a measurement
generation or content identity covering all height-affecting render state,
including recovery-banner content and Dynamic Type. Update model equality and
the uniformHeights and variableHeights cache ownership used by learnHeights so
measurements are associated with that generation and stale entries cannot be
reused. Ensure repinIfNeeded and related pinning logic consume only measurements
matching the current render snapshot, invalidating or replacing older
measurements when the generation changes.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swift`:
- Around line 190-201: The recordCorrection path must not call writeReport
synchronously from the KVO callback; retain corrections in memory and trigger a
single report write when the sweep completes, including the related correction
handling around the indicated writeReport call. Preserve the existing phase
filtering and correction data collection while removing repeated full-array
serialization during layout.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView`+MacSelection.swift:
- Around line 103-112: Remove the DEBUG/UITestConfig special case from
canRenderGroupsForSelection and leave this production property governed solely
by macSelectionScope.canRenderGroupsForSelection. Update the store-free preview
fixture or its dedicated debug owner to provide normal foreground-Mac state so
seeded groups render without changing production selection logic.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView`+ScrollPin.swift:
- Around line 7-40: Materialize one immutable ordered workspace-list item
snapshot during each WorkspaceListView body evaluation and reuse it for both
rendering and scrollPinModel. Update scrollPinModel to consume that snapshot
instead of independently accessing showsFilterEmptyRow,
displayedGroupedListItems, or displayedFlatWorkspaces, ensuring rendered rows
and pin kinds derive from the same ordered data without repeated filtering,
sorting, or collection scans.
🪄 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: f16d61aa-31d4-4d64-9bb9-8ddba06190f9
📒 Files selected for processing (7)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollIndicatorStabilizer.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+MacSelection.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+ScrollPin.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollPinModelTests.swift
Comment on lines
+111
to
+125
| var model = WorkspaceListScrollPinModel(kinds: [], rowHeightsAreUniform: false) { | ||
| didSet { | ||
| guard model != oldValue else { return } | ||
| repinIfNeeded() | ||
| } | ||
| } | ||
|
|
||
| private weak var listCollectionView: UICollectionView? | ||
| private var contentSizeObservation: NSKeyValueObservation? | ||
| private var attachLink: CADisplayLink? | ||
| private var attachFramesLeft = 600 | ||
| /// Heights learned from realized cells, per uniform kind. | ||
| private var uniformHeights: [WorkspaceListScrollPinKind: CGFloat] = [:] | ||
| /// Realized heights of variable singletons, keyed by their model id. | ||
| private var variableHeights: [String: CGFloat] = [:] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Version cached measurements with all height-affecting state.
The model equality and cache keys contain only row kinds/static IDs. If recovery-banner content changes while offscreen, or Dynamic Type changes while a kind is unrealized, learnHeights cannot refresh that entry and repinning uses its stale height. This produces an incorrect scroll range instead of failing open.
Carry a measurement generation/content identity in the render snapshot and pin only from measurements belonging to that generation.
As per coding guidelines, preserve clear ownership and invariants rather than leaving stale cached state representable.
Also applies to: 201-243
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollIndicatorStabilizer.swift`
around lines 111 - 125, Version WorkspaceListScrollPinModel with a measurement
generation or content identity covering all height-affecting render state,
including recovery-banner content and Dynamic Type. Update model equality and
the uniformHeights and variableHeights cache ownership used by learnHeights so
measurements are associated with that generation and stale entries cannot be
reused. Ensure repinIfNeeded and related pinning logic consume only measurements
matching the current render snapshot, invalidating or replacing older
measurements when the generation changes.
Source: Coding guidelines
Comment on lines
+118
to
+171
| private weak var listCollectionView: UICollectionView? | ||
| private var contentSizeObservation: NSKeyValueObservation? | ||
| private var attachLink: CADisplayLink? | ||
| private var attachFramesLeft = 600 | ||
| /// Heights learned from realized cells, per uniform kind. | ||
| private var uniformHeights: [WorkspaceListScrollPinKind: CGFloat] = [:] | ||
| /// Realized heights of variable singletons, keyed by their model id. | ||
| private var variableHeights: [String: CGFloat] = [:] | ||
| /// Guards the KVO handler against reacting to this view's own write. | ||
| private var isRepinning = false | ||
|
|
||
| override func didMoveToWindow() { | ||
| super.didMoveToWindow() | ||
| guard window != nil else { | ||
| stopAttaching() | ||
| return | ||
| } | ||
| guard listCollectionView == nil, attachLink == nil else { return } | ||
| // The backing UICollectionView does not exist until SwiftUI hosts the | ||
| // List, so attachment retries per frame briefly instead of assuming | ||
| // hierarchy timing. The link is torn down as soon as it resolves. | ||
| let link = CADisplayLink(target: self, selector: #selector(attachTick)) | ||
| link.add(to: .main, forMode: .common) | ||
| attachLink = link | ||
| } | ||
|
|
||
| @objc private func attachTick() { | ||
| attachFramesLeft -= 1 | ||
| if let collectionView = nearestListCollectionView() { | ||
| stopAttaching() | ||
| attach(to: collectionView) | ||
| } else if attachFramesLeft <= 0 { | ||
| stopAttaching() | ||
| } | ||
| } | ||
|
|
||
| private func stopAttaching() { | ||
| attachLink?.invalidate() | ||
| attachLink = nil | ||
| } | ||
|
|
||
| private func attach(to collectionView: UICollectionView) { | ||
| listCollectionView = collectionView | ||
| contentSizeObservation = collectionView.observe( | ||
| \.contentSize, options: [.old, .new] | ||
| ) { [weak self] _, change in | ||
| guard change.oldValue?.height != change.newValue?.height else { return } | ||
| // UIKit publishes contentSize changes from main-thread layout. | ||
| MainActor.assumeIsolated { | ||
| guard let self, !self.isRepinning else { return } | ||
| self.repinIfNeeded() | ||
| } | ||
| } | ||
| repinIfNeeded() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Replace the display-link polling and post-layout repair loop with an explicitly owned bridge.
This scans the hierarchy every frame for up to 600 frames, attaches to the first discovered collection view, then repeatedly overwrites layout-owned contentSize. Attachment can silently time out or select the wrong collection view as the hierarchy changes. Have one @MainActor coordinator receive the actual list bridge and snapshot through explicit lifecycle callbacks instead.
As per coding guidelines, do not introduce CADisplayLink, polling, or similar timing repair paths to paper over lifecycle or rendering races.
Also applies to: 201-215
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollIndicatorStabilizer.swift`
around lines 118 - 171, Replace the CADisplayLink-based attachment flow in
didMoveToWindow, attachTick, stopAttaching, and nearestListCollectionView with
an explicitly owned `@MainActor` coordinator that receives the actual list
UICollectionView bridge and snapshot through lifecycle callbacks. Remove frame
polling, the 600-frame timeout, and hierarchy scanning; update coordinator
ownership and teardown so attachment follows the bridge lifecycle and no longer
discovers or selects collections implicitly.
Source: Coding guidelines
Comment on lines
103
to
+112
| var canRenderGroupsForSelection: Bool { | ||
| macSelectionScope.canRenderGroupsForSelection | ||
| #if DEBUG | ||
| // The store-free layout fixture has no foreground Mac, so the | ||
| // foreground-scope gate can never pass there; render its seeded groups | ||
| // so grouped rows and end-of-group slots are exercised in previews. | ||
| if store == nil, UITestConfig.workspaceListLayoutPreviewEnabled { | ||
| return true | ||
| } | ||
| #endif | ||
| return macSelectionScope.canRenderGroupsForSelection |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Keep preview-only grouping out of production selection logic.
This DEBUG branch makes an ambient UI-test flag a second authority for whether groups may render. Seed the preview with normal foreground-Mac state that satisfies macSelectionScope, or isolate fixture-specific rendering in a dedicated debug owner without changing this production property.
As per path instructions, production files under Sources must not add test-only or debug-only behavior seams.
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView`+MacSelection.swift
around lines 103 - 112, Remove the DEBUG/UITestConfig special case from
canRenderGroupsForSelection and leave this production property governed solely
by macSelectionScope.canRenderGroupsForSelection. Update the store-free preview
fixture or its dedicated debug owner to provide normal foreground-Mac state so
seeded groups render without changing production selection logic.
Sources: Coding guidelines, Path instructions
Comment on lines
+7
to
+40
| var scrollPinModel: WorkspaceListScrollPinModel { | ||
| var kinds: [WorkspaceListScrollPinKind] = [] | ||
| switch connectionChrome { | ||
| case .recoveryBanner: | ||
| if store != nil { | ||
| kinds.append(.variable(id: "chrome.recoveryBanner")) | ||
| } | ||
| case .macStatusRow: | ||
| kinds.append(.variable(id: "chrome.macStatusRow")) | ||
| case .none: | ||
| break | ||
| } | ||
| if rendersGroupedSections { | ||
| for item in displayedGroupedListItems { | ||
| switch item { | ||
| case .groupHeader: | ||
| kinds.append(.groupHeader) | ||
| case .groupFooter: | ||
| kinds.append(.groupFooter) | ||
| case .workspace: | ||
| kinds.append(.workspaceRow) | ||
| } | ||
| } | ||
| } else if showsFilterEmptyRow { | ||
| kinds.append(.variable(id: "filterEmpty")) | ||
| } else { | ||
| kinds.append( | ||
| contentsOf: repeatElement(.workspaceRow, count: displayedFlatWorkspaces.count) | ||
| ) | ||
| } | ||
| return WorkspaceListScrollPinModel( | ||
| kinds: kinds, | ||
| rowHeightsAreUniform: !wrapWorkspaceTitles | ||
| ) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Build one render snapshot instead of independently reconstructing the list.
At WorkspaceListView.swift Line 303, this model evaluates showsFilterEmptyRow after the body already evaluated it at Line 285. Each access can run filteredWorkspaces’s O(n log n) filter/sort over workspaces; grouped and flat collections are also walked again. For roughly 1,000 workspaces, this adds repeated body-path scans and allows rendered rows and pin kinds to drift.
Materialize one immutable ordered item snapshot per body evaluation and consume it for both rendering and pinning.
As per path instructions, avoid repeated full scans and sorting over scalable workspace collections in hot UI paths.
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView`+ScrollPin.swift
around lines 7 - 40, Materialize one immutable ordered workspace-list item
snapshot during each WorkspaceListView body evaluation and reuse it for both
rendering and scrollPinModel. Update scrollPinModel to consume that snapshot
instead of independently accessing showsFilterEmptyRow,
displayedGroupedListItems, or displayedFlatWorkspaces, ensuring rendered rows
and pin kinds derive from the same ordered data without repeated filtering,
sorting, or collection scans.
Sources: Coding guidelines, Path instructions
…heck #8180 committed ios/cmux.xcworkspace/xcshareddata/swiftpm/Package.resolved, but check-package-resolved-policy.py only recognized the macOS project's workspace lockfile, so workflow-guard-tests fails on main and on every branch cut from it ("Unexpected cmux Package.resolved location"). It went unnoticed because PR CI is currently disabled for the advisory experiment. The iOS workspace is a cmux-owned Xcode workspace whose resolution should be tracked at exactly that fixed location, matching the policy's intent that resolution changes stay visible in PR diffs. Generalize the expected-location check to a tuple of workspace lockfiles and add the iOS one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…at-ios-wslist-scrollbar
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.
Actionable comments posted: 1
🤖 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 `@scripts/check-package-resolved-policy.py`:
- Line 262: Update the lockfile validation in main() to allow diffs in
WORKSPACE_PACKAGE_RESOLVED without requiring XCODE_PACKAGE_RESOLVED, while
preserving project-reference validation. Add a regression test covering a change
only to ios/cmux.xcworkspace/xcshareddata/swiftpm/Package.resolved.
🪄 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: fcc659f9-0119-43b6-9c8a-9a2c842255df
📒 Files selected for processing (1)
scripts/check-package-resolved-policy.py
|
|
||
| def is_expected_lockfile_path(lockfile: str, roots: set[str]) -> bool: | ||
| if lockfile == XCODE_PACKAGE_RESOLVED: | ||
| if lockfile in WORKSPACE_PACKAGE_RESOLVED: |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
rg -n -A25 -B5 \
'def xcode_package_reference_changed|XCODE_PACKAGE_RESOLVED|WORKSPACE_PACKAGE_RESOLVED|is_expected_lockfile_path' \
scripts/check-package-resolved-policy.py
rg -n \
'Package\.resolved|xcode_package_reference_changed|cmux\.xcworkspace' \
.Repository: manaflow-ai/cmux
Length of output: 10267
Allow workspace lockfile diffs in the Xcode reference check. main() still requires XCODE_PACKAGE_RESOLVED at line 340, so a change that only updates ios/cmux.xcworkspace/xcshareddata/swiftpm/Package.resolved fails CI. Use WORKSPACE_PACKAGE_RESOLVED here, or branch on project vs. workspace references, and add a regression test.
🤖 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 `@scripts/check-package-resolved-policy.py` at line 262, Update the lockfile
validation in main() to allow diffs in WORKSPACE_PACKAGE_RESOLVED without
requiring XCODE_PACKAGE_RESOLVED, while preserving project-reference validation.
Add a regression test covering a change only to
ios/cmux.xcworkspace/xcshareddata/swiftpm/Package.resolved.
Collaborator
Author
|
CI state while the advisory experiment has PR checks off:
🤖 Generated with Claude Code |
Collaborator
Author
|
Extended validation matrix while CI lanes are blocked (all via the seeded fixture + scroll-metrics probe; "draw heights" = distinct contentSize values the scroll indicator renders from during one full top-to-bottom sweep):
The Dynamic Type row is the self-calibration proof: rows realize 43% taller at that size and the stabilizer learns the height from realized cells and pins the exact total — no hardcoded numbers involved. Also: interactive flick recording shows constant indicator length (189px) and zero adjacent-frame anomalies across 273 indicator-visible frames. Evidence artifacts preserved at 🤖 Generated with Claude Code |
… scrollbar stutter" This reverts commit 139243d.
SwiftUI List sizes unrealized rows with a hardwired ~54pt estimate no public API influences, so scrolling the ~92pt-per-row workspace list corrected contentSize by +38pt per row materialization and visibly jerked the scroll indicator (one jump per row; Apple bug, present through iOS 26). A contentSize-pinning mitigation was rejected as too hacky; this is the class-eliminating fix, mirroring the macOS sidebar's AppKit-table decision: own the list in UIKit where exact heights are expressible. WorkspaceListTable (UIViewRepresentable) drives a UITableView with a diffable data source over identity-only items. Row visuals reuse the existing SwiftUI views (WorkspaceRow, group header/footer, connection chrome, filter-empty) via UIHostingConfiguration, so the look is unchanged. heightForRowAt returns exact measured heights from an offscreen sizing cell, memoized per kind/width/content-size-category (per-row when wrap-titles makes heights content-dependent); with estimation gone, UIKit computes the true content size up front and never rewrites it. Swipe actions, context menus, selection, and pull-to-refresh map 1:1 onto UIKit with the same localization keys and accessibility identifiers; the rename sheet and close confirmation move to list scope. The macOS path keeps the SwiftUI List. Reorder drag is intentionally inert in this commit and returns on top of MobileWorkspaceMovePolicy via drop delegates next. The hosting configuration's default minimum content size clamped the compact 32pt group header to ~42pt; zeroing minSize on header/footer cells restores exact parity with the List (44pt header rows). Fixture sweeps, 100 rows: contentSize corrections per full scroll go from 91 (List) to 0, flat and grouped; grouped totals byte-match the List (8910pt: 90x92.5 + 10x44 + 10x16), and accessibility-medium Dynamic Type measures identically (100x132 = 13200) with 0 corrections. The DEBUG scroll-metrics probe now attaches to UITableView as well. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Codex <codex@openai.com>
azooz2003-bit
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView`+Table.swift:
- Around line 6-12: Extract the shared filter-empty boolean condition from the
iOS showsWorkspaceTableFilterEmptyRow property and the macOS WorkspaceListView
List branch into one computed property on the base WorkspaceListView type.
Update both platform branches to use that shared property, preserving the
existing behavior and eliminating duplicated condition logic.
🪄 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: 2bd8905f-0f4a-4b97-8d41-39960f4c2945
⛔ Files ignored due to path filters (1)
ios/cmux.xcworkspace/xcshareddata/swiftpm/Package.resolvedis excluded by!**/Package.resolved
📒 Files selected for processing (10)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTable.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+Actions.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableItem.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListUITableView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+Actions.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+Table.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceNavigationStyle.swift
Comment on lines
+6
to
+12
| extension WorkspaceListView { | ||
| var showsWorkspaceTableFilterEmptyRow: Bool { | ||
| activeFilter.isActive | ||
| && trimmedQuery.isEmpty | ||
| && filteredWorkspaces.isEmpty | ||
| && !workspaces.isEmpty | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Filter-empty condition duplicated from the macOS branch.
showsWorkspaceTableFilterEmptyRow re-implements the exact same boolean condition already used inline in the macOS List branch (WorkspaceListView.swift, unchanged condition at its else if clause). Extracting a single shared computed property (e.g., on the base WorkspaceListView type) used by both platform branches would remove the duplication and prevent the two conditions from silently diverging.
Also applies to: 36-38
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView`+Table.swift
around lines 6 - 12, Extract the shared filter-empty boolean condition from the
iOS showsWorkspaceTableFilterEmptyRow property and the macOS WorkspaceListView
List branch into one computed property on the base WorkspaceListView type.
Update both platform branches to use that shared property, preserving the
existing behavior and eliminating duplicated condition logic.
UIKit reports a drop below the last row as an insertion slot at row == rowCount. The drop guard rejected that index, so dragging a workspace to the very bottom of the list silently snapped back. Map it to an end-of-list move (SwiftUI onMove semantics allow destination == count) and clamp the drop animation target to the last valid row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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. |
Collaborator
Author
|
Reorder drag restored (2ad5d36 + 32a316d): 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift`:
- Line 38: Replace the one-shot dropJustCompleted flag with state identifying
the reordered item or generation, and update the reorder handling and
update(configuration:) flow to record and consume that identity. Suppress
animation only when the incoming snapshot matches the reorder that triggered the
suppression; preserve normal animation for intervening or unrelated updates.
🪄 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: 3593a94b-7601-4302-902f-bbafe9c3e2b1
📒 Files selected for processing (4)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTable.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+Table.swift
| private var dataSource: UITableViewDiffableDataSource<Int, WorkspaceListTableItem>? | ||
| private let sizingCell = UITableViewCell(style: .default, reuseIdentifier: nil) | ||
| private var heightCache: [HeightCacheKey: CGFloat] = [:] | ||
| private var dropJustCompleted = false |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== WorkspaceListTableCoordinator.swift ==\n'
sed -n '1,260p' Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift
printf '\n== Search for dropJustCompleted / update(configuration:) ==\n'
rg -n "dropJustCompleted|update\\(configuration:|moveRows|animatesSnapshot|dragDelegate|dropDelegate" Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI -S
printf '\n== WorkspaceListView+DragDrop.swift ==\n'
sed -n '1,260p' Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+DragDrop.swiftRepository: manaflow-ai/cmux
Length of output: 21973
🏁 Script executed:
set -euo pipefail
sed -n '1,180p' Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+Table.swiftRepository: manaflow-ai/cmux
Length of output: 5327
🏁 Script executed:
set -euo pipefail
sed -n '1,180p' Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTable.swiftRepository: manaflow-ai/cmux
Length of output: 3564
Tie the snapshot suppression to the reorder that triggered it.
dropJustCompleted is a one-shot boolean, so any intervening update(configuration:) can clear it before the reorder snapshot arrives. That can make the drop-driven update animate anyway, or suppress an unrelated refresh instead. Track the drop by item/generation and only skip animation when the incoming snapshot matches that reorder.
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift`
at line 38, Replace the one-shot dropJustCompleted flag with state identifying
the reordered item or generation, and update the reorder handling and
update(configuration:) flow to record and consume that identity. Suppress
animation only when the incoming snapshot matches the reorder that triggered the
suppression; preserve normal animation for intervening or unrelated updates.
Source: Coding guidelines
Dogfood found the dropped row ghosting at its old position for a beat: UIKit animated the drop against the pre-move layout because the moved snapshot only arrived a runloop later via SwiftUI state. Apply the moved order to the data source synchronously inside performDrop, then animate the drop into that final layout; the authoritative SwiftUI snapshot that follows has the same order and settles as a no-op. The other two dogfood findings were fixture stubs, not product bugs: the DEBUG fixture passed nil for select-navigation, rename, pin, read-state, and close, so taps navigated nowhere and context menus were correctly empty. With CMUX_UITEST_WORKSPACE_LIST_PREVIEW_REORDER=1 the fixture now wires all of those to local seeded state (plus a trivial pushed detail screen), making tap, swipe, menu, rename, delete, and drag dogfoodable without a paired Mac. Verified on the simulator: tap pushes the detail, long-press shows Pin/Rename/Mark as Unread/Delete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines
+506
to
+517
| case .workspace(let id, _): | ||
| if configuration.wrapWorkspaceTitles, | ||
| let workspace = configuration.workspacesByID[id] { | ||
| kind = .workspaceWrapped( | ||
| id: id, | ||
| name: workspace.name, | ||
| isSelected: configuration.navigationStyle == .sidebar | ||
| && configuration.selectedWorkspaceID == id | ||
| ) | ||
| } else { | ||
| kind = .workspaceUniform | ||
| } |
Contributor
There was a problem hiding this comment.
isIndentedWorkspace for wrapped rows
When wrapWorkspaceTitles is true, the cache key is workspaceWrapped(id:name:isSelected:), which ignores the indented flag. But indented workspace rows have a 20 pt wider leading margin (32 pt vs 12 pt, set in configure), which reduces the effective content width and can cause the title to wrap to an extra line. When a workspace transitions from ungrouped (non-indented) to grouped (indented) — or vice versa — the cache returns the height measured under the old margin, while the cell is rendered under the new margin. The mismatch leaves the row too short, clipping the wrapped title, until a layout-metrics change (screen rotation or text-size change) clears the cache.
The fix is to include item.isIndentedWorkspace in the workspaceWrapped case — either as an extra label on the enum case or as an extra field on HeightCacheKey.
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)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift (1)
567-567: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winOnly reconfigure the affected rows when selection changes.
Currently, if the global
selectedWorkspaceIDchanges, this condition evaluates totruefor every single.workspaceand.groupHeaderitem, forcing a full-list cell reconfiguration on every selection tap. To avoid unnecessary layout and diffing overhead on large lists, restrict this check to only the rows whose selection state actually flipped.
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift#L567-L567: constrain the workspace check, e.g.,(previous.selectedWorkspaceID != next.selectedWorkspaceID && (id == previous.selectedWorkspaceID || id == next.selectedWorkspaceID)).Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift#L582-L582: constrain the group header check, e.g.,(previous.selectedWorkspaceID != next.selectedWorkspaceID && (anchorID == previous.selectedWorkspaceID || anchorID == next.selectedWorkspaceID)).🤖 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 `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift` at line 567, Restrict selection-change reconfiguration in WorkspaceListTableCoordinator: at Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift#L567-L567, require the workspace ID to match either the previous or next selectedWorkspaceID; at Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift#L582-L582, apply the equivalent anchorID check for group headers. Keep unchanged behavior for rows whose selection state did flip.
🤖 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
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift`:
- Line 567: Restrict selection-change reconfiguration in
WorkspaceListTableCoordinator: at
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift#L567-L567,
require the workspace ID to match either the previous or next
selectedWorkspaceID; at
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift#L582-L582,
apply the equivalent anchorID check for group headers. Keep unchanged behavior
for rows whose selection state did flip.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 473d7260-809b-4d28-aa14-b9220eab67d6
📒 Files selected for processing (2)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift
…lbar # Conflicts: # scripts/check-package-resolved-policy.py
azooz2003-bit
added a commit
that referenced
this pull request
Jul 17, 2026
…shows it Main's UITableView workspace list (#8186) hosts WorkspaceRow directly, bypassing WorkspaceNavigationRow where the +adds −dels chip lived, so chips vanished after merging main. The chip (and its localized accessibility label) moves into WorkspaceRow itself, both pipelines pass it through, and the table coordinator reconfigures exactly the cells whose chip value changed. Also keeps concise changes.summary debug-log lines that made this diagnosable from the container log. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 18, 2026
azooz2003-bit
added a commit
that referenced
this pull request
Jul 24, 2026
…8221) * Mac side: mobile.workspace.changes.* RPCs backed by CmuxGit WorkspaceChangesService Adds a subprocess-backed workspace-changes service (summary/files/file_diff vs merge-base of the default branch, untracked included, 15s summary TTL cache, path containment validation, 400KiB/6000-line hunk-aligned diff truncation) and exposes it to the phone as three mobile data-plane RPCs with capability workspace.changes.v1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS data layer: changes DTOs, CmuxMobileChanges parser package, composite integration Lenient DTOs for the three mobile.workspace.changes RPCs, a Foundation-only CmuxMobileChanges package (unified-diff parser with line numbering and CRLF preservation, grapheme-safe intra-line emphasis, clamped diff font preference), and MobileShellComposite integration: workspace.changes.v1 capability gate, 64-id batched summary fetches with a 15s reuse window, rpcWorkspaceID-keyed chip snapshots, and a cancellable 250ms debounce off list refreshes and workspace.updated events. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS UI: Changes sheet — file list, swipe-paged diff viewer, preview route Value-driven Changes screens in CmuxMobileChanges (GitHub-calibrated adaptive theme, summary header, status-glyph file rows with mini add/delete bars, dual-gutter soft-wrapped unified diff with intra-line emphasis, page TabView with position pill, pinch font sizing, copy line/hunk), the ShellUI sheet mount with parsed-document cache, and the deterministic CMUX_UITEST_CHANGES_PREVIEW fixture route (populated/diff/empty/states). All strings localized en+ja. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS entry points: Changes toolbar button, workspace-list chips, one-time hint One shared openWorkspaceChanges() action presents the Changes sheet from the capability/connection-gated toolbar button (badge capped at 99+) and the dismissible first-time hint banner; workspace rows get an ambient +A −D chip fed by value snapshots keyed by rpcWorkspaceID. Also renders 'No newline at end of file' markers as dimmed gutter-less rows (parser emits them; Copy Hunk excludes them). All new strings localized en+ja. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Keep wire DTO types out of ShellUI: map change status in the shell layer CmuxMobileShellUI never imports CmuxMobileRPC; the status→FileChangeKind mapping moves into CmuxMobileShell (which gains a CmuxMobileChanges dependency) so the sheet consumes model values by member access only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Render an honest not-a-repository state in the Changes sheet A workspace whose directory is outside any Git repository previously fell into the generic connection-error state. The composite now maps the not_a_repo RPC code onto a shell-owned WorkspaceChangesFetchError and the list renders a dedicated localized state (folder.badge.questionmark, no summary header, no retry). Verified live against the tagged Mac's home workspace. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix macOS Debug build broken on main: explicit color capture in NSImage draw closure Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swift from #8034 references the slot view's color property inside the escaping NSImage draw handler without explicit capture, which fails to compile (CI is currently advisory, so it landed unnoticed). Capturing the color value keeps the view out of the image's retained draw block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Render the changes chip in the shared WorkspaceRow so the UIKit list shows it Main's UITableView workspace list (#8186) hosts WorkspaceRow directly, bypassing WorkspaceNavigationRow where the +adds −dels chip lived, so chips vanished after merging main. The chip (and its localized accessibility label) moves into WorkspaceRow itself, both pipelines pass it through, and the table coordinator reconfigures exactly the cells whose chip value changed. Also keeps concise changes.summary debug-log lines that made this diagnosable from the container log. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Calm the file list: text badges instead of a status-icon zoo Five leading pictograms in four hues made the list read as a barrage of symbols. The row now leads with the path; magnitude stays on the counts and mini-bar; and only exceptional states get a quiet capsule badge in the BIN badge's language: green 'New' (added and untracked collapse into one concept), red 'Deleted' with the whole path dimmed. Renames keep only their old → new line. Modified rows, the common case, carry no marker at all, and the palette drops to green/red (orange and blue status tokens removed). Badges are localized en+ja; VoiceOver labels keep the full status wording. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Say what the app means: 'Binary' badge and a truncation footer that explains itself 'BIN' was insider shorthand; the badge now reads Binary (ja already said バイナリ). The truncation footer stops announcing a mechanism and states the tradeoff: 'Large diff. Showing the first N lines to keep things fast. See the rest on your Mac.' The 6,000-line/400KiB per-file cap itself is unchanged; it exists so generated files and lockfiles cannot balloon the RPC payload or phone memory. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * View changed binary files with the artifact viewer, at either revision Changed images, PDFs, and other binaries stop dead-ending at a placeholder: the diff page's binary card offers View Before / View After (or a single View File for added, untracked, and deleted files) and pushes the shared ChatArtifactViewerDestination — zoomable images, PDFKit, AVKit, QuickLook — fed by two new data-plane RPCs, mobile.workspace.changes.file_stat and .file_fetch. Reads are authorized against the workspace's current changed-file set (rename old paths only for revision=base) plus the path containment check; base blobs materialize once via git show into an actor-owned 256 MiB LRU temp cache and serve 3 MiB chunks with honest EOF math. Loader cache scope keys by workspace + revision + path so before and after never collide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Log workspace-changes content failures to the container debug log One line per failed stat/fetch with method, params, and the underlying error, matching the changes.summary logging style, so preview failures are diagnosable from the device log instead of a generic viewer state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Binary previews render inline with their actions in place Paging onto a changed image or PDF now shows the content immediately: the page hosts a chrome-free ChatArtifactInlineViewer (new public component reusing the pager's per-type hosts and lifecycle), with a Before | After selector for modified and renamed files. The full-screen hop is gone; the viewer toolbar's Share / Save to Files / Copy-image actions render in place, conditional on the loaded content type, through a factored ChatArtifactActionBar the full viewer now shares. Zoom coexists with page swipes the way Photos does: a zoomed-out pan falls through to the pager. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Preview actions live in the sheet toolbar, conditionally for the current page Share / Save to Files / Copy image move from floating pills into the Changes sheet's top-trailing navigation toolbar, driven by an Equatable descriptor the inline viewer publishes via a SwiftUI preference (execution stays in the viewer through a registration-generation host, so a stale page can't clear a fresh performer). Only the selected pager page mounts a preview, so the toolbar always reflects the visible file and empties out on text pages. Includes the fix that attaches the toolbar group to the pushed pager screen, which owns its own navigation bar, instead of the sheet root. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Copy image via standard glyph and image long-press menu The copy-image action now uses the doc.on.doc copy symbol instead of the gallery-reading photo-on-rectangle glyph, everywhere the shared ChatArtifactAction metadata is consumed (Changes sheet toolbar and full viewer). Long-pressing a rendered image presents Share, Save to Files, and Copy image through a UIContextMenuInteraction on the hosted UIImageView, routed through the same performers as the toolbar; the full viewer's copy-image performer now actually copies the rendered image. Adds a changes.hint debug-log line reporting hint eligibility inputs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add Changes row to the workspace title menu A second, non-toolbar entry point: the workspace title menu now has a Changes row (shown whenever the host supports workspace changes) routing through the same openWorkspaceChanges() action as the toolbar button. The toolbar keeps the existing +/- icon button unchanged. Also factors the list chip's +N -M text into a unit-tested WorkspaceChangesChipTextPolicy that falls back to a localized file count for binary-only change sets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Tappable list chip, counts in the Changes toolbar button, drop menu row Three entry-point changes from the placement interview. The +N -M chip on workspace-list rows is now a button that opens that workspace's Changes sheet directly over the list (both the SwiftUI List and UIKit-table pipelines; row selection untouched). The workspace-detail toolbar button replaces its abstract +/- glyph with the same green/red counts whenever the tree is dirty, falling back to the glyph when clean; counts resolve their colors against the terminal theme's chrome scheme rather than the system scheme so they stay legible on dark chrome. The title-menu Changes row is removed as redundant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Stack the toolbar Changes counts vertically The workspace-detail toolbar button now stacks +N over -M so the counts cost no more horizontal space than a plain icon button. The shared chip label gains a stacksVertically variant used only by the toolbar; list rows keep the horizontal layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Failing test: oversized single hunk truncates to nothing A file whose first diff hunk alone exceeds the 6,000-line/400KiB cap comes back as a header-only diff, which the phone renders as "Showing the first 0 lines" with an empty page. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Split oversized first hunk instead of emitting an empty diff When the first hunk alone exceeds the byte/line cap, emit as much of its body as fits under a hunk header rewritten to describe the partial body (start lines preserved, old/new counts recomputed), so the phone shows the head of the change instead of "the first 0 lines". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Make diff laziness per-line so huge hunks scroll The diff body lazily rendered per HUNK, so a single multi-thousand-line hunk (e.g. a truncated 6,000-line rewrite) became one eagerly laid-out child: seconds of layout and frozen/stuttering scrolling. DiffRowSnapshot now flattens hunks into per-line rows and the LazyVStack iterates those, so only visible lines lay out regardless of hunk shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Expand hidden unchanged lines GitHub-style in the diff reader Hidden unchanged regions (above the first hunk, between hunks, after the last hunk) now show tappable expander bands: 100-line steps, full reveal when 120 or fewer remain, split up/down bands between hunks. Revealed lines come from the current working-tree file over the existing authorized chunked file_fetch path (fetched once per file, 5 MiB cap, inline retry on failure) and render as per-line context rows with both gutters mapped through the hunk offsets, preserving per-line laziness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Progressive Show more past the diff cap and unified short-gap expanders The 6,000-line/400KiB per-file cap becomes progressive: file_diff accepts an optional max_lines (clamped 6,000...1,000,000 lines / 64 MiB abuse guard, byte budget scaled proportionally) and reports diff_total_lines, and the truncated footer becomes "Showing X of Y diff lines" with a Show more button that requests 4x the current budget and replaces the document in place (stable row IDs preserve scroll and expansion state). Expander bands whose whole run reveals in one tap now render a single unified button instead of a split up/down pair whose halves did the same thing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review findings: bounded work, truthful truncation, stable routes Fixes the six accepted P1 review findings plus bot feedback: - Clamp progressive diff responses to 6 MiB so they always fit the 8 MiB RPC frame; the client stops offering Show more when a larger budget stops growing the loaded window. - Suppress the trailing context expander on truncated diffs (the region after the last included hunk is not known unchanged). - Read git diff output through a bounded incremental reader (terminate past the budget) and report the diff total as unknown when cut short. - Size base blobs with cat-file before materializing, stream git show to the temp cache incrementally, refuse blobs over the cache budget, and never pin an oversized entry through eviction. - Parse diff responses off the main actor and cache the flat row projection in state instead of rebuilding it per body evaluation. - Apply the 500-file cap before untracked-file inspection and count untracked additions with bounded in-process reads instead of one git process per file. - Decode summary identity fields strictly (lossy batch drops malformed entries), route the diff pager by stable file path with a fail-closed missing state, single-pass prefix truncation, read summary-cache entries after the suspension point, and scrub RPC parameter names from user-facing error copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bound snapshot and inspection work, fail closed on remote provenance Second review round: snapshot git commands stream through the bounded reader with a 32 MiB ceiling, 30s wall deadline, cancellation checks, and explicit truncation; untracked inspection gets a 64 MiB aggregate budget with cancellation between files; the client clamps Show more progression at 96,000 lines and builds the parsed document, row projection, and gutter width together off the main actor as one immutable presentation; intra-line emphasis is skipped for lines over 4,096 UTF-8 bytes before any Character materialization; remote-provenance workspace paths never reach local git (summary reports not a repository, content verbs return not_a_repo); both process-lifetime caches purge expired entries globally and hold at most 64 entries with LRU eviction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Move directory policy into CmuxGit and bound zero-byte cache entries The workspace-directory provenance policy is consumed by the macOS app target, so it lives in CmuxGit's changes domain (CmuxMobileRPC is an iOS-group package the Mac app cannot resolve; its tests move to CmuxGitTests). The base-content cache adds a 256-entry LRU count bound so zero-byte blobs, which are invisible to the byte budget, cannot grow the entry map and temp-file population without limit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Scope refresh fanout, decouple pinch from rows, bound caches, pin revisions Review round 4: workspace deltas schedule changes-summary refreshes only for the delta's workspace IDs (group-only deltas skip entirely); pinch no longer rebuilds the row projection (rows depend only on document, expansion, and current lines; gutter width derives cheaply at render); the sheet's parsed-presentation cache is a 7-entry LRU around the selected page; file_diff/file_stat/file_fetch carry an additive stat fingerprint so expansion fetches from a newer working tree are discarded and the diff refreshed instead of splicing mixed revisions; Show more now continues when the host cannot report a total, with loaded-only progress copy (en+ja). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Harden content reads, cache keys, fingerprints, and refresh coalescing Review round 5: expansion downloads enforce a cumulative 5 MiB cap and chunk-count ceiling inside the transport loop; the diff's content fingerprint stats the working file before and after git runs and returns a never-matching unstable token when they differ; base blobs are keyed and fetched by an immutable commit OID instead of the moving HEAD ref; content reads walk the validated path component-by-component with O_NOFOLLOW anchored at a repository-root descriptor so a post-validation symlink swap cannot escape the repository; the summary-refresh debounce accumulates a union of pending workspace IDs with a dominating all-workspaces flag instead of dropping scopes on restart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Recoverable cancellation, pinned transfers, safe inspection, guarded publishes Review round 6: connection-transition CancellationErrors publish the error state (silent stop only under real task cancellation); chunked content transfers resolve scope, authorization, base OID, and base size once and stay pinned via the authorized-path cache, which is now revision-keyed so a moved base refreshes the snapshot; artifact transfers verify every chunk's content fingerprint against the initial stat; untracked inspection reuses the O_NOFOLLOW component-walk opener and rejects symlinks and non-regular files with cancellation checks; diff load, Show more, and expansion publishes are generation-guarded so a superseded request can never overwrite a newer presentation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Single-flight summaries, bounded parsing, post-read fingerprint checks Review round 7: the summary debounce is separate from the fetch, which is single-flight with a trailing coalesced pass instead of being cancelled by every workspace delta; expansion line materialization runs on a nonisolated worker with a 200,000-line bound; snapshot output parses incrementally keeping at most the 500-entry cap plus running totals instead of materializing unbounded path collections; content chunks fstat the descriptor again after reading and fail closed when identity, size, mtime, or ctime moved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Literal pathspecs, fail-closed fingerprints, serialized expansion Review round 8: Git commands taking network-selected paths run with literal pathspec semantics; a working file that changes across the diff capture retries once then fails with the retryable error instead of publishing content with an unstable token; once a fingerprint is established every subsequent response must carry a matching token (nil observed fails closed, all-legacy hosts keep working); cached-lines expansion sets pending state and coalesces reveal intents into one cancellable rebuild; skipped-fresh summary refreshes arm one trailing fetch at expiry and an additive force param bypasses the host's TTL cache; the full image viewer regains Copy Path; the UIKit workspace table's height caching accounts for chip presence so interactive chips never clip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Deadline every git call, typed repo outcomes, leases, identity fingerprints Review round 9 accepted findings: the plain runner overload gains the same 30s deadline and cancellation termination as the bounded overloads; unborn repositories diff against the empty tree so untracked files list (git failures map to gitFailure, never notARepository); truncated changed-file snapshots render a bounded-result footer; untracked files past the scan budget are prefix-probed and classified binary when unknown; chunked base transfers hold eviction leases on their cache entries; fingerprints carry device, inode, and ctime so same-size same-mtime replacement is detectable. Two round-9 findings rejected by design and documented in place: the default-branch HEAD comparison fallback, and delta+TTL-driven summary refresh (repo-watching is a follow-up). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Blob-identity fingerprints, hard git deadlines, budget-honest leases Review round 10: base-revision fingerprints derive from the immutable commit and blob OIDs so cache eviction cannot change a transfer's identity; the git deadline terminates the process group, unblocks pipe readers, and escalates to SIGKILL after a grace period; the base cache reserves projected bytes before materializing, rejects when no unleased victim can satisfy the budget, and leases every returned URL through its use; missing fingerprints fail closed (this protocol always emits them); Show more tasks are retained and cancelled with generation invalidation when a page disappears; read-capped untracked counts are marked partial and flip the snapshot's truncated flag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Scope legacy refreshes, pin diff base to OID, prompt exits, self-expiry Review round 11: legacy workspace.updated reloads schedule TTL-respecting summary refreshes instead of forced app-wide sweeps; the verified base commit OID is the diff base for every operation (symbolic name kept only for display); the git deadline path stops waiting out the SIGKILL grace once the process group is gone; successful summary fetches arm the trailing expiry so chips self-refresh after the TTL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Group-liveness deadlines, honest TTLs, pruned state, bounded page memory Review round 12: git deadline termination and escalation track process-group liveness so descendants holding stdout are reaped; summary TTLs stamp at batch completion with a floored trailing delay so slow hosts cannot loop at zero delay; summary state prunes against the current workspace set and consumes state-sync removals before rearming; diff pages hold heavy state only in a selected-neighborhood window, with other tabs mounting on selection; the workspace table's height cache keys by digit-count buckets and chip mode instead of exact live totals. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Restore submodule pointers, cache snapshots, gate polling, unpin executors Review round 14 (Claude engine): the origin/main merge had resolved the ghostty and vendor/bonsplit gitlinks to older branch-side commits; both are restored to main's pointers. File diffs and changed-file lists now serve a 15-second LRU loaded-snapshot cache so pager mounts reuse one repository walk (force bypasses it); the summary trailing refresh only re-arms while workspace events are recent, so an idle connected phone cannot hold the Mac in a perpetual 15-second git poll; blocking git spawn/poll/reap loops run on a dedicated GCD queue bridged with continuations instead of pinning the cooperative executor; the developer-scratch live-repo probe test is removed; SwiftUI list rows gate the chip tap closure like the UIKit path so chip-less rows keep combined VoiceOver navigation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Move base-revision git and large decodes off blocking executors Review round 15: the base-content cache takes an async materializer so the actor suspends instead of blocking on git show (post-await collision adopts the winning entry); the rev-parse and cat-file probes and the materializer run through the dedicated blocking queue per the service's own executor contract; multi-megabyte file-diff and content-chunk JSON payloads decode in nonisolated async helpers so Show more and binary previews never run their decode pass on the main thread. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Live cancellation on GCD git work, bounded hint keys, injected clock Review round 16 nits: a thread-bound cancellation signal bridges Swift task cancellation into the GCD-hosted git loops (Task.isCancelled reads false there), so unmounted pages and dropped connections stop subprocess reads and untracked scans early instead of riding out the wall deadline; the hint-dismissal store keeps seen workspace IDs in one 256-entry FIFO array key instead of unbounded per-workspace defaults keys; the summary debounce and trailing-expiry sleeps use an injected Clock so scheduling is test-drivable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Assert renamed-file diffs are paired, not add-only The rename test accepted any non-empty diff, so a full-file addition (the current behavior) passed. It now requires rename headers and no content lines as additions for a pure git mv. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Pair renamed-file diffs by including the old path in the pathspec git applies pathspec filtering before rename detection, so fileDiff's new-path-only pathspec made -M unable to pair renames: the diff page showed the whole file as added while the file list's paired numstat showed the true +/- counts. Tracked diffs now pass both the validated old and new paths after --, restoring similarity headers and an empty hunk body for a pure git mv. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Assert CRLF diffs split, count, and truncate per line Red test: the truncator must count each CRLF-terminated content line and break at hunk boundaries inside CRLF diffs. Character-based splitting treats \r\n as one grapheme, so today the whole CRLF hunk body is one mega-line and these assertions fail. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Split diff lines on literal newlines so CRLF hunks survive truncation Character-based split treats \r\n as one grapheme, so CRLF diff bodies collapsed into a single mega-line: totals undercounted, interior hunk headers went undetected, and an over-cap CRLF diff truncated to metadata-only text that the phone rendered as an empty diff. The truncator now splits with components(separatedBy: "\n"), matching the iOS UnifiedDiffParser's handling of the same pitfall. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Keep content reads and diff truncation off the cooperative pool fileStat's open+fstat, fileFetch's chunk read (up to 3 MiB), and fileDiff's decode+hunk-split of up to ~13 MiB of git output ran on Swift-concurrency cooperative threads; a repo on a network or external volume could pin one for seconds per call. All three now route through the same offCooperativePool seam as the service's git subprocess work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Retire the trailing expander once a fetched file proves it empty On added files and EOF-touching diffs the trailing "Expand hidden lines" band was a permanently dead control: the tapped gap resolved to nothing against the fetched file, and that path cleared pending state without publishing the fetched lines, so the projection never learned the line count, the band never disappeared, and every tap re-downloaded the whole file. The nil-gap path now recomputes the presentation with the fetched lines, which removes the band and caches the lines. Covered at the projection level (band present without a line count, gone with one); a true page-level red/green is not practical because the page is @State-bound SwiftUI rather than an observable model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Decode changed-file paths strictly so malformed entries are omitted File.init decoded every field leniently, so an entry missing its path became path "" instead of throwing: the batch loop's omission filter never fired, the list showed a nonsense row whose diff request the host rejects, and two such entries collided on the path-keyed SwiftUI identity. The path now decodes strictly and rejects empty strings, matching the sibling summaries decoder (strict identity, lenient counts), so identity-less objects are dropped like non-object entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The iOS workspace list's scrollbar stutters while scrolling rows it has not visited: SwiftUI
Listsizes unrealized rows with a hardwired ~54pt estimate no public API influences (measured:defaultMinListRowHeight16/removed/92.5 and fixed.frame(height:)all leave the churn byte-identical), so every ~92.5pt row materialization corrects contentSize by +38.3pt and jerks the indicator — one jump per row, 91 per full scroll of 100 rows. Known Apple bug through iOS 26 (https://developer.apple.com/forums/thread/716998).Fix (this PR's final form)
Rebuild the list container on UIKit, mirroring the macOS sidebar's AppKit-table decision:
WorkspaceListTabledrives aUITableViewwith a diffable data source;heightForRowAtreturns exact measured heights (offscreen sizing cell, memoized per kind/width/type-size; per-row when wrap-titles makes heights content-dependent). With estimation gone, UIKit computes the true content size up front and never rewrites it — native indicator, zero churn, no framework fighting. An earlier contentSize-pinning mitigation was rejected as too hacky and is reverted in-branch.Row visuals reuse the existing SwiftUI views via
UIHostingConfiguration(one gotcha: its defaultminSizeclamped the compact 32pt group header to ~42pt; zeroed for header/footer cells). Swipe actions, context menus, selection, and pull-to-refresh map 1:1 with the same localization keys and accessibility identifiers. The macOS path keeps the SwiftUIList. Reorder drag is inert in the current head and returns viaUITableViewDragDelegate/DropDelegateon top of the untouchedMobileWorkspaceMovePolicymachinery (in progress).Evidence (fixture sweeps, DEBUG probe from the first commit)
Grouped screenshot and default 3-row fixture render identically to the List version.
CI context: PR checks are off this week (advisory experiment);
workflow-guard-testspasses with #8191 merged in; remaining red jobs reproduce on plain main.🤖 Generated with Claude Code
Summary by CodeRabbit