iOS: miniature-first hub — redesign pane/tab navigation with live previews - #8087
Closed
azooz2003-bit wants to merge 27 commits into
Closed
azooz2003-bit wants to merge 27 commits into
azooz2003-bit wants to merge 27 commits into
Conversation
Locked design (owner interview 2026-07-13): miniature-first hub with live previews, selectable panes, auto-hide tab strip, attention shelf toggle, no pinning. Acceptance doc defines behavior, edge cases, evidence plan, and performance thresholds for the integrated program. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…esign # Conflicts: # Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePushCoordinator.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift # Sources/Mobile/MobileHostService.swift # cmux.xcodeproj/project.pbxproj
MobileRouteResolverTests and the render-grid demand tests move to their own files; the mobile host authorization policies and Stack verifier move to MobileHostAuthorization.swift. Budget ratcheted down to current sizes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Too many files changed for review. ( Bypass the limit by tagging |
|
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:
📝 WalkthroughWalkthroughAdds a miniature-first iOS workspace hub with authoritative layouts, demand-scoped terminal and browser previews, pane tab strips, agent-chat cards, capability-gated events, route-based navigation, Mac RPC producers, and expanded validation. ChangesPane/tab redesign
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant MacWorkspace
participant MobileHostService
participant MobileShellComposite
participant WorkspaceHubView
MacWorkspace->>MobileHostService: Emit layout or preview event
MobileHostService->>MobileShellComposite: Deliver capability-gated event
MobileShellComposite->>WorkspaceHubView: Publish layout or preview snapshot
WorkspaceHubView->>MobileShellComposite: Open pane or preview stream
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (8 errors, 2 warnings)
✅ Passed checks (15 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Found 1 test failure on Blacksmith runners: Failure
|
The merge's git add -A staged the stale checked-out ghostty and bonsplit pointers, reverting main's bumps and breaking the scrollbar API build. 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 36
🤖 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 `@ios/cmux/Resources/Localizable.xcstrings`:
- Around line 7594-7599: Add singular and plural localization keys for
mobile.workspaceHub.tabCountFormat following the existing count-key convention,
then update the caller displaying this count to choose the singular key when the
value is 1 and the plural key otherwise. Preserve the existing localized wording
and formatting for plural counts.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 248-250: Update the authoritative secondary-Mac removal path that
deletes entries from secondaryMacSubscriptions and workspacesByMac to also
remove workspaceLayoutsByMacDeviceID[macID]. Ensure re-adding the same Mac
cannot reuse its stale layout snapshots before fresh topology data arrives.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+BrowserPreview.swift:
- Around line 25-30: Update routeIncomingBrowserPreview(_:) to perform payload
extraction and MobileBrowserPreviewFrame decoding on a concurrent boundary
rather than synchronously on the MainActor. After decoding, hop back to the
MainActor only for previewGridSessionState.browserPreview.store.receive(frame),
preserving the order of frames delivered to each surface.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+WorkspaceLayout.swift:
- Around line 125-133: Replace the O(N) workspaces.first(where:) scans in
workspaceRowID, workspaceLayoutIdentity, and supportsWorkspaceLayout with O(1)
lookups backed by maintained workspace indexes, such as workspacesByID and/or
workspacesByRemoteID. Update the indexes wherever workspaces are created,
removed, or changed so all three socket-event and layout paths preserve current
matching behavior. Affected sites:
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceLayout.swift
lines 125-133 (workspaceRowID), 115-123 (workspaceLayoutIdentity), and 40-44
(supportsWorkspaceLayout).
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridAccumulator.swift`:
- Around line 34-48: Update the frame accumulation logic around current.lines to
copy the existing lines array and mutate only rows in replacedRows, using the
corresponding spansByRow values. Preserve the existing PreviewGridSnapshot
construction and empty-line fallback while eliminating the per-update linesByRow
dictionary allocation and full row reconstruction.
In `@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridStore.swift`:
- Around line 11-21: Inject a controllable clock into PreviewGridStore, using it
for all now and sleep operations instead of the concrete ContinuousClock;
preserve the production default clock. In
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridStore.swift
lines 11-21, update the store initializer and clock property accordingly. In
Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/PreviewGridStoreTests.swift
lines 66-86, provide a virtual clock and advance it beyond the 250-ms throttle
deadline instead of sleeping for a fixed duration.
In `@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridStyle.swift`:
- Around line 4-24: Declare the PreviewGridStyle type as nonisolated so its
immutable preview snapshot can be used from background layout and rendering
code. Apply the same nonisolated declaration to TerminalGridThumbnailRun in
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalGridThumbnailRun.swift
lines 5-11; both sites require direct changes.
In
`@Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/PreviewGridStoreTests.swift`:
- Around line 57-63: Update the assertions in the preview snapshot test around
snapshotA and snapshotB to require that each snapshot contains a first line
before accessing lines[0]. Preserve the existing span-text expectations, but
make an empty snapshot produce a test failure rather than an indexing crash.
In
`@Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneChatCardSnapshot.swift`:
- Line 4: Mark the pure value-model structs as nonisolated by updating
PaneChatCardSnapshot in
Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneChatCardSnapshot.swift:4,
PreviewGridSnapshot in
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridSnapshot.swift:4,
TerminalGridThumbnailLayout in
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalGridThumbnailLayout.swift:6,
and PreviewGridAccumulator in
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridAccumulator.swift:4;
make no other changes.
In
`@Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabAttentionPredicate.swift`:
- Around line 2-15: The attention predicate currently lives in the static-only
PaneTabAttentionPredicate namespace; move it onto PaneTabStripCardSnapshot as a
nonisolated computed property named needsAttention, preserving the existing
agentStatus and hasUnread logic. Remove the namespace and update all call sites
to use card.needsAttention.
In
`@Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripProjection.swift`:
- Around line 43-48: Replace the two-filter implementation in the attentionFirst
branch of the PaneTabStripProjection projection with a single traversal of
macOrderedCards, appending each card to either attention or regular buffers
based on PaneTabAttentionPredicate.needsAttention. Concatenate the buffers in
attention-first order, while preserving the existing macOrderedCards result when
attentionFirst is false.
- Around line 31-38: The pane projection currently conflates a missing layout
with a missing pane via layout.flatMap. Update the logic around macOrderedCards
so fallbackTerminals are used only when layout is nil; when a layout exists but
pane(id:in:) returns nil, produce an empty/invalid projection and keep
isDegraded set appropriately. Add a regression test covering a stale paneID in a
present layout and verifying no fallback terminal cards are surfaced.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/BrowserPreviewImageDecoder.swift`:
- Around line 7-20: Update BrowserPreviewImageDecoder.decode so cancellation of
the caller’s task propagates to the detached decode task and cancels it before
awaiting its result. Preserve the existing Task.isCancelled guard and
image-decoding behavior, while ensuring the spawned task is explicitly
caller-owned and cannot continue after decode is cancelled.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PaneTabStripCardView.swift`:
- Around line 199-201: Update the previewTaskID property in PaneTabStripCardView
to include supportsBrowserPreview alongside the existing card, visibility, and
connection-state inputs, so preview consumption restarts when browser-preview
capability changes.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift`:
- Around line 50-60: Mark the view-internal `@State` properties in
WorkspaceDetailView, including isRenamePresented, renameText,
paneTabStripVisibility, selectedBrowserSurface, and
initialSurfaceSelectionApplied, as private while preserving their existing state
and behavior.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView`+PaneTabStrip.swift:
- Around line 37-40: Update the nil case in the selected-terminal resolution
logic to use the fallback behavior provided by selectedTerminal, returning the
first available terminal when selectedTerminalID is nil or stale instead of
returning selectedTerminalID directly. Keep tab-strip selection aligned with the
visible terminal surface.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceHubContainer.swift`:
- Around line 18-24: Update the workspace computed property to use the store’s
authoritative ID-indexed workspace lookup instead of scanning store.workspaces
with first(where:). Preserve the existing routeWorkspaceSnapshot fallback for
matching workspace IDs and selectedWorkspace behavior when workspaceID is
absent; do not introduce a view-local cache.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceHubPaneView.swift`:
- Around line 242-275: Update WorkspaceHubPaneView.swift:242-275 in
previewTaskID and preview consumption to include pane kind and browser
capability in the identity, using one reliable structured surface source and
failing closed when unavailable; clear browserImage whenever that identity
changes. Update MirroredBrowserSurfaceView.swift:26-38 to include preview
capability in its task ID and clear image, title, and URL before subscribing or
when required identity/capability is unavailable.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swift`:
- Around line 285-300: Update the deeplink handling in WorkspaceShellView to
resolve the terminal’s authoritative pane from workspaceLayout and use that pane
ID when constructing the .pane route, rather than fabricating "deeplink:" plus
the terminal ID. If the layout cannot resolve the terminal’s pane, fail closed
by routing only to the hub until the layout becomes available, while preserving
the existing compact and split navigation behavior.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView`+HubNavigation.swift:
- Around line 76-82: Update openPane and its WorkspaceHubContainer callback
wiring to use the workspaceID resolved by workspaceHubDestination rather than
store.selectedWorkspaceID. Carry that authoritative workspace ID through pane
selection, and fail closed without opening a route when it is unavailable. Keep
workspace and pane routing based on the same structured hub destination source.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceTitleMenuContent.swift`:
- Around line 86-94: Remove the inline DEBUG-only copy-logs button and its
copyDebugLogs dependency from WorkspaceTitleMenuContent, and move that menu
composition into a dedicated Debug/ source file under the module’s Sources
directory. Preserve the existing label, action, accessibility identifier, and
DEBUG-only availability while keeping the production view free of debug-only
behavior.
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileBrowserPreviewDemand.swift`:
- Around line 11-15: Make previewSurfaceIDs and fullSurfaceIDs in
MobileBrowserPreviewDemand immutable or private(set), and prevent empty strings
from being introduced through mutation or synthesized Codable decoding. Add a
validating init(from:) that decodes both sets and routes them through the same
filtering used during initialization, preserving only non-empty surface IDs.
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileRenderGridDemandSummary.swift`:
- Around line 53-54: Update contains(surfaceID:) to check focusedSurfaceIDs and
previewSurfaceIDs directly, while preserving the includesLegacyAll behavior. Do
not use the surfaceIDs computed union, so membership checks avoid allocating and
scanning a combined collection on every call.
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileWorkspacePaneFrame.swift`:
- Around line 18-23: Update MobileWorkspacePaneFrame’s public initializer and
Codable decoding to enforce unit-frame geometry: require finite, non-negative
x/y/width/height values and ensure x + width and y + height remain within the
unit bounds. Reject invalid input during initialization and decoding so no
malformed instance can be created.
In `@Sources/Mobile/MobileBrowserPreviewObserver.swift`:
- Around line 153-158: Remove the DEBUG-only accessors debugDemandForTesting and
debugAwaitWorkForTesting from MobileBrowserPreviewObserver. Update tests to
observe emitted completion or use an existing internal declaration through
`@testable` import, without adding new debug/test-named seams to production code.
- Around line 164-174: Update MobileBrowserPreviewCoordinator so
configureMobileHost does not replace a process-wide observer tied to one
window’s tabManager. Use a single explicitly owned observer with an
authoritative cross-window surface resolver, or maintain separate observers
keyed by window/connection; remove the mutable singleton-slot behavior that
stops the previous observer during configure.
- Around line 224-231: Update
MobileBrowserPreviewObserver.publish(connectionID:) to preserve event ordering
by making it async and awaiting the `@MainActor` coordinator update from the
connection lifecycle, or by routing all demand updates through a single
serialized owner. Remove the unstructured Task scheduling so publish and
removeAll cannot reorder or reapply stale demand.
In `@Sources/Mobile/MobileHostService.swift`:
- Around line 1639-1642: Serialize render-demand publication with the connection
actor by making the publication method async and awaiting its MainActor update
rather than launching unstored fire-and-forget Tasks. Update every close,
subscribe, and unsubscribe transition—including the paths around
renderGridDemandScopes, browserPreviewDemand, and the referenced locations—to
await publication so snapshots cannot apply out of order or retain demand for
closed connections.
- Around line 1865-1874: Update the demand parsing before subscribe in
MobileHostService so a present but undecodable render_grid_demand or
browser_preview_demand is rejected rather than converted to nil. Preserve nil
only for absent parameters, and ensure malformed demand requests return a
failure without invoking subscribe, preventing the downstream .legacyAll
fallback.
In `@Sources/Mobile/MobileTerminalByteTee.swift`:
- Around line 48-52: Remove the renderGridDemandMirror from
MobileTerminalByteTee and eliminate the duplicated demand state. Move
render-grid demand aggregation into a single synchronized owner shared by
MobileTerminalRenderObserver and the synchronous Ghostty C callback, updating
both paths to query that owner directly while preserving bounded, nonisolated
callback access and teardown correctness.
In `@Sources/Mobile/MobileTerminalRenderObserver.swift`:
- Around line 333-335: Remove the production-only debug accessor
debugRenderGridDemandForTesting from MobileTerminalRenderObserver. Make the
underlying effectiveRenderGridDemand snapshot internal as needed, then access it
from tests via `@testable` import without adding test-named API under Sources/.
- Around line 254-279: Replace the sleeper-based logic in
enqueuePreviewEmission(surfaceID:) with event-driven coalescing through the
existing tick/frame scheduler: mark the surface dirty, register or reuse a
single scheduler callback, and emit eligible dirty surfaces on the next
scheduled tick. Remove per-surface pending tasks, previewClock.sleep(until:),
and deadline-based coordination while preserving demand checks and coalescing
repeated emissions.
In `@Sources/Mobile/MobileWorkspaceListObserver.swift`:
- Around line 39-44: Replace the sleep-based debounce in
MobileWorkspaceListObserver with an event-driven layout-committed signal. Remove
layoutDebounceDuration and any Task.sleep or delayed dispatch used for
coordination; publish snapshots only when the authoritative layout commit event
occurs, then equality-deduplicate consecutive snapshots before notifying
observers.
In `@Sources/Panels/BrowserPanel`+MobilePreview.swift:
- Around line 42-61: The mobile preview JPEG encoding loop in mobileBrowserJPEG
currently runs on the main actor and can block UI. Keep snapshot acquisition on
the main actor, then dispatch mobileBrowserJPEG and its resize/quality loop to a
nonisolated off-main helper, preserving the existing output and maximumByteCount
behavior.
In `@Sources/TerminalController.swift`:
- Around line 14257-14258: Update the pane selection logic around
requestedPaneID and v2UUID so it distinguishes an absent pane_id from an
explicitly provided malformed or empty value. Return the existing not_found
error for any invalid explicit pane_id, while preserving focused/first-pane
fallback only when pane_id is omitted and normal selection for a valid UUID.
In `@Sources/TerminalController`+MobileWorkspaceLayout.swift:
- Around line 14-21: Update the workspace resolution guard around
v2ResolveTabManager(params:) so that if the initially resolved tab manager does
not contain workspaceID, it falls back to the global workspace/tab-manager
lookup used by panel_id resolution. Preserve the existing not_found response
when no manager owns the requested workspace, and continue using the resolved
manager for the layout operation.
🪄 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: 7200a57e-395c-4a42-be2d-2be1dde0f2ab
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (135)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileBrowserPreviewDemand.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileBrowserPreviewDemandSummary.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileBrowserPreviewFrame.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileBrowserPreviewResolution.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileRenderGridDemand.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileRenderGridDemandScope.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileRenderGridDemandSummary.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileTerminalRenderGridFrame+CellWidth.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileWorkspaceAgentStatus.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileWorkspaceLayout.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileWorkspaceLayoutNode.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileWorkspacePane.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileWorkspacePaneFrame.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileWorkspaceSplit.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileWorkspaceSplitOrientation.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileWorkspaceTab.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileWorkspaceTabKind.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/MobileBrowserPreviewDemandTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/MobileRenderGridDemandTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/MobileWorkspaceLayoutCodingTests.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileCoreRPCClient+WorkspaceLayout.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileCoreRPCClient.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/BrowserPreviewSessionState.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/BrowserPreviewStore.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/BrowserPreviewSurfaceState.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+BrowserPreview.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+DeeplinkNavigation.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ForgottenMacs.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+PreviewGrid.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ReconnectRoutes.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+SecondaryPromotion.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceLayout.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridAccumulator.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridLine.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridSessionState.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridSnapshot.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridSpan.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridStore.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridStyle.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridSurfaceState.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileShellWorkspaceCapabilityTests.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileShellWorkspaceLayoutTests.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/PreviewGridStoreTests.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MacWorkspaceState.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspaceAggregation.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/MobileWorkspacePreview.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneChatCardSnapshot.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneLocalBrowserCardSnapshot.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabAttentionPredicate.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabCardKind.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripCardSnapshot.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripPreviewDemand.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripProjection.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripVisibilityState.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/WorkspaceHubFocusState.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/WorkspaceHubPaneFrame.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/WorkspaceHubPaneSnapshot.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/WorkspaceHubPreviewDemand.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/WorkspaceHubProjection.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/PaneTabStripProjectionTests.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/WorkspaceHubProjectionTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/BrowserPreviewImageDecoder.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ChatSessionDescriptor+PaneCard.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceCoordinator+Artifacts.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MirroredBrowserSurfaceView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileDisplaySettings.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePushCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PaneTabRedesignPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PaneTabStripCardView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PaneTabStripHandle.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PaneTabStripView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalGridThumbnailLayout.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalGridThumbnailRun.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalGridThumbnailView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenu.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuActions.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuDiagnostics.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuValue.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailContainer.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+AgentChat.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+MenuState.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+PaneTabStrip.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+Surfaces.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+TerminalArtifacts.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceHubContainer.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceHubPaneView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceHubView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceSelectedBrowser.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellRoute.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView+HubNavigation.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView+WorkspaceActions.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceTitleMenuContent.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileDisplaySettingsTests.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalGridThumbnailLayoutTests.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/UITestConfig.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceViewDelegate.swiftPackages/macOS/CmuxPanes/Package.swiftPackages/macOS/CmuxPanes/Sources/CmuxPanes/MobileWorkspaceLayoutMapper.swiftPackages/macOS/CmuxPanes/Tests/CmuxPanesTests/MobileWorkspaceLayoutMapperTests.swiftSources/AppDelegate.swiftSources/Mobile/MobileBrowserPreviewObserver.swiftSources/Mobile/MobileHostAuthorization.swiftSources/Mobile/MobileHostPolicy.swiftSources/Mobile/MobileHostService+Capabilities.swiftSources/Mobile/MobileHostService+TicketAuthorization.swiftSources/Mobile/MobileHostService.swiftSources/Mobile/MobileTerminalByteTee.swiftSources/Mobile/MobileTerminalRenderObserver.swiftSources/Mobile/MobileWorkspaceListObserver.swiftSources/Mobile/Workspace+MobileLayout.swiftSources/Panels/BrowserPanel+MobilePreview.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserScreenshotSnapshotter.swiftSources/TerminalController+MobileWorkspaceLayout.swiftSources/TerminalController.swiftSources/Workspace+AgentLifecycle.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/MobileBrowserPreviewObserverTests.swiftcmuxTests/MobileHostAuthorizationTests.swiftcmuxTests/MobileRouteResolverTests.swiftdocs/ios-pane-tab-redesign-acceptance.mddocs/ios-pane-tab-redesign.mdghosttyios/cmux/Resources/Localizable.xcstringsios/cmuxPackage/Tests/cmuxFeatureTests/cmuxFeatureTests.swiftios/cmuxUITests/cmuxUITests.swiftvendor/bonsplit
💤 Files with no reviewable changes (6)
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuValue.swift
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuDiagnostics.swift
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuActions.swift
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenu.swift
- Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swift
- Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+MenuState.swift
Comment on lines
+7594
to
+7599
| "mobile.workspaceHub.tabCountFormat": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "%d tabs" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "%d 個のタブ" } } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a singular tab-count localization.
%d tabs renders “1 tabs” for a single tab. Add singular/plural variants, consistent with the existing count keys, and update the caller to select the appropriate key.
🤖 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 `@ios/cmux/Resources/Localizable.xcstrings` around lines 7594 - 7599, Add
singular and plural localization keys for mobile.workspaceHub.tabCountFormat
following the existing count-key convention, then update the caller displaying
this count to choose the singular key when the value is 1 and the plural key
otherwise. Preserve the existing localized wording and formatting for plural
counts.
Comment on lines
+248
to
+250
| /// Mac-authored pane topology snapshots, keyed first by owning Mac and then | ||
| /// by Mac-local workspace id so identical ids on two Macs cannot collide. | ||
| var workspaceLayoutsByMacDeviceID: [String: [String: MobileWorkspaceLayout]] = [:] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Invalidate layout snapshots when a secondary Mac leaves aggregation.
The new cache outlives the removal path at Lines 3160-3164, which deletes secondaryMacSubscriptions[macID] and workspacesByMac[macID] but not workspaceLayoutsByMacDeviceID[macID]. Re-adding the same Mac can therefore expose stale pane topology before a fresh layout arrives.
Clear this cache in the same authoritative secondary-removal path.
As per path instructions, stale caches in correctness-sensitive snapshot paths require lifecycle-driven invalidation.
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`
around lines 248 - 250, Update the authoritative secondary-Mac removal path that
deletes entries from secondaryMacSubscriptions and workspacesByMac to also
remove workspaceLayoutsByMacDeviceID[macID]. Ensure re-adding the same Mac
cannot reuse its stale layout snapshots before fresh topology data arrives.
Source: Path instructions
Comment on lines
+25
to
+30
| func routeIncomingBrowserPreview(_ event: MobileEventEnvelope) { | ||
| guard let payload = event.payloadJSON, | ||
| let frame = try? JSONDecoder().decode(MobileBrowserPreviewFrame.self, from: payload) else { | ||
| return | ||
| } | ||
| previewGridSessionState.browserPreview.store.receive(frame) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline \
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+BrowserPreview.swift \
--items all
rg -n -C3 'routeIncomingBrowserPreview\s*\(|MobileBrowserPreviewFrame.*decode|`@concurrent`' \
--type swift PackagesRepository: manaflow-ai/cmux
Length of output: 8617
🏁 Script executed:
#!/bin/bash
ast-grep outline Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+BrowserPreview.swift --items all
sed -n '1,120p' Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+BrowserPreview.swift
printf '\n---\n'
sed -n '1,120p' Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileBrowserPreviewFrame.swift
printf '\n---\n'
sed -n '1,120p' Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftRepository: manaflow-ai/cmux
Length of output: 12607
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'MobileBrowserPreview*.swift' Packages
printf '\n---\n'
rg -n -C3 'struct MobileBrowserPreview|class MobileBrowserPreview|browserPreview.*store|func receive\(' Packages/Shared Packages/iOS -g '*.swift'
printf '\n---\n'
sed -n '1,220p' Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileBrowserPreview*.swiftRepository: manaflow-ai/cmux
Length of output: 32846
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '6180,6210p' Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift
printf '\n---\n'
sed -n '1,220p' Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/BrowserPreviewStore.swift
printf '\n---\n'
rg -n 'payloadJSON|JSONDecoder\(\)\.decode\(MobileBrowserPreviewFrame|browser.preview' Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell -g '*.swift'Repository: manaflow-ai/cmux
Length of output: 8031
Decode browser preview frames off the MainActor.
This handler runs inside the browser.preview branch of the main-actor event loop, so each synchronous JSON/base64 decode can block delivery of other events. Move decoding to a concurrent boundary, then hop back only to call BrowserPreviewStore.receive(_:), preserving per-surface ordering.
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+BrowserPreview.swift
around lines 25 - 30, Update routeIncomingBrowserPreview(_:) to perform payload
extraction and MobileBrowserPreviewFrame decoding on a concurrent boundary
rather than synchronously on the MainActor. After decoding, hop back to the
MainActor only for previewGridSessionState.browserPreview.store.receive(frame),
preserving the order of frames delivered to each surface.
Source: Coding guidelines
Comment on lines
+125
to
+133
| private func workspaceRowID( | ||
| remoteWorkspaceID: String, | ||
| macDeviceID: String | ||
| ) -> MobileWorkspacePreview.ID? { | ||
| workspaces.first(where: { workspace in | ||
| workspace.rpcWorkspaceID.rawValue == remoteWorkspaceID | ||
| && (workspace.macDeviceID ?? foregroundMacKey) == macDeviceID | ||
| })?.id | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Algorithmic complexity: O(N) rescans on workspaces during socket events.
These functions perform full-collection O(N) rescans over workspaces (a user-scalable collection) using .first(where:). Since workspaceRowID is called by handleWorkspaceLayoutUpdatedEvent on a socket event path, this repeated O(N) scan violates the algorithmic complexity guidelines. As per path instructions, avoid full rescans in hot event paths over scalable collections. Consider maintaining a workspacesByID and/or workspacesByRemoteID dictionary for O(1) lookups.
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceLayout.swift#L125-L133: replace.first(where:)inworkspaceRowIDwith an O(1) index lookup.Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceLayout.swift#L115-L123: replace.first(where:)inworkspaceLayoutIdentitywith an O(1) index lookup.Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceLayout.swift#L40-L44: replace.first(where:)insupportsWorkspaceLayoutwith an O(1) index lookup.
📍 Affects 1 file
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceLayout.swift#L125-L133(this comment)Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceLayout.swift#L115-L123Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceLayout.swift#L40-L44
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+WorkspaceLayout.swift
around lines 125 - 133, Replace the O(N) workspaces.first(where:) scans in
workspaceRowID, workspaceLayoutIdentity, and supportsWorkspaceLayout with O(1)
lookups backed by maintained workspace indexes, such as workspacesByID and/or
workspacesByRemoteID. Update the indexes wherever workspaces are created,
removed, or changed so all three socket-event and layout paths preserve current
matching behavior. Affected sites:
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+WorkspaceLayout.swift
lines 125-133 (workspaceRowID), 115-123 (workspaceLayoutIdentity), and 40-44
(supportsWorkspaceLayout).
Source: Path instructions
Comment on lines
+34
to
+48
| var linesByRow = Dictionary(uniqueKeysWithValues: current.lines.map { ($0.row, $0) }) | ||
| let replacedRows = Set(frame.clearedRows).union(frame.rowSpans.map(\.row)) | ||
| let spansByRow = resolvedSpansByRow(frame) | ||
| for row in replacedRows { | ||
| linesByRow[row] = PreviewGridLine(row: row, spans: spansByRow[row] ?? []) | ||
| } | ||
| let next = PreviewGridSnapshot( | ||
| surfaceID: frame.surfaceID, | ||
| stateSeq: frame.stateSeq, | ||
| columns: frame.columns, | ||
| rows: frame.rows, | ||
| activeScreen: frame.activeScreen, | ||
| lines: (0..<frame.rows).map { linesByRow[$0] ?? PreviewGridLine(row: $0, spans: []) }, | ||
| hasBaseline: true | ||
| ) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Avoid unnecessary dictionary allocations when accumulating grid frames.
Because current.lines is strictly indexed by row and has exactly frame.rows elements, rebuilding a Dictionary for every delta update introduces O(N) allocation overhead on an event-driven rendering path. You can safely copy the array and mutate the changed rows in-place. As per path instructions, avoid rebuilding/sorting/filtering unbounded collections on every event.
⚡ Proposed performance refactor
- var linesByRow = Dictionary(uniqueKeysWithValues: current.lines.map { ($0.row, $0) })
- let replacedRows = Set(frame.clearedRows).union(frame.rowSpans.map(\.row))
- let spansByRow = resolvedSpansByRow(frame)
- for row in replacedRows {
- linesByRow[row] = PreviewGridLine(row: row, spans: spansByRow[row] ?? [])
- }
- let next = PreviewGridSnapshot(
- surfaceID: frame.surfaceID,
- stateSeq: frame.stateSeq,
- columns: frame.columns,
- rows: frame.rows,
- activeScreen: frame.activeScreen,
- lines: (0..<frame.rows).map { linesByRow[$0] ?? PreviewGridLine(row: $0, spans: []) },
- hasBaseline: true
- )
+ var nextLines = current.lines
+ let replacedRows = Set(frame.clearedRows).union(frame.rowSpans.map(\.row))
+ let spansByRow = resolvedSpansByRow(frame)
+ for row in replacedRows where row >= 0 && row < nextLines.count {
+ nextLines[row] = PreviewGridLine(row: row, spans: spansByRow[row] ?? [])
+ }
+ let next = PreviewGridSnapshot(
+ surfaceID: frame.surfaceID,
+ stateSeq: frame.stateSeq,
+ columns: frame.columns,
+ rows: frame.rows,
+ activeScreen: frame.activeScreen,
+ lines: nextLines,
+ hasBaseline: true
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var linesByRow = Dictionary(uniqueKeysWithValues: current.lines.map { ($0.row, $0) }) | |
| let replacedRows = Set(frame.clearedRows).union(frame.rowSpans.map(\.row)) | |
| let spansByRow = resolvedSpansByRow(frame) | |
| for row in replacedRows { | |
| linesByRow[row] = PreviewGridLine(row: row, spans: spansByRow[row] ?? []) | |
| } | |
| let next = PreviewGridSnapshot( | |
| surfaceID: frame.surfaceID, | |
| stateSeq: frame.stateSeq, | |
| columns: frame.columns, | |
| rows: frame.rows, | |
| activeScreen: frame.activeScreen, | |
| lines: (0..<frame.rows).map { linesByRow[$0] ?? PreviewGridLine(row: $0, spans: []) }, | |
| hasBaseline: true | |
| ) | |
| var nextLines = current.lines | |
| let replacedRows = Set(frame.clearedRows).union(frame.rowSpans.map(\.row)) | |
| let spansByRow = resolvedSpansByRow(frame) | |
| for row in replacedRows where row >= 0 && row < nextLines.count { | |
| nextLines[row] = PreviewGridLine(row: row, spans: spansByRow[row] ?? []) | |
| } | |
| let next = PreviewGridSnapshot( | |
| surfaceID: frame.surfaceID, | |
| stateSeq: frame.stateSeq, | |
| columns: frame.columns, | |
| rows: frame.rows, | |
| activeScreen: frame.activeScreen, | |
| lines: nextLines, | |
| hasBaseline: true | |
| ) |
🤖 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/CmuxMobileShell/Sources/CmuxMobileShell/PreviewGridAccumulator.swift`
around lines 34 - 48, Update the frame accumulation logic around current.lines
to copy the existing lines array and mutate only rows in replacedRows, using the
corresponding spansByRow values. Preserve the existing PreviewGridSnapshot
construction and empty-line fallback while eliminating the per-update linesByRow
dictionary allocation and full row reconstruction.
Source: Path instructions
Comment on lines
+333
to
+335
| var debugRenderGridDemandForTesting: MobileRenderGridDemandSummary { | ||
| effectiveRenderGridDemand | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not expose production state through a DEBUG test accessor.
Make the underlying demand snapshot internal where necessary and inspect it with @testable import, rather than adding debugRenderGridDemandForTesting under production Sources/.
As per path instructions, production Swift under Sources/ must not add debug/test-named accessors solely for tests.
🤖 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/Mobile/MobileTerminalRenderObserver.swift` around lines 333 - 335,
Remove the production-only debug accessor debugRenderGridDemandForTesting from
MobileTerminalRenderObserver. Make the underlying effectiveRenderGridDemand
snapshot internal as needed, then access it from tests via `@testable` import
without adding test-named API under Sources/.
Source: Path instructions
Comment on lines
+39
to
+44
| private let layoutDebounceDuration: Duration | ||
|
|
||
| init(tabManager: TabManager, notificationStore: TerminalNotificationStore? = nil) { | ||
| init( | ||
| tabManager: TabManager, | ||
| notificationStore: TerminalNotificationStore? = nil, | ||
| layoutDebounceDuration: Duration = .milliseconds(80) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Replace the sleep-based layout debounce with an event-driven commit signal.
This introduces production Task.sleep coordination and makes layout freshness depend on an arbitrary delay. Emit from one authoritative “layout committed” signal, then equality-deduplicate the resulting snapshot instead.
As per coding guidelines, production Swift must not introduce sleeps or delayed dispatch for synchronization or delayed coordination.
Also applies to: 243-252
🤖 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/Mobile/MobileWorkspaceListObserver.swift` around lines 39 - 44,
Replace the sleep-based debounce in MobileWorkspaceListObserver with an
event-driven layout-committed signal. Remove layoutDebounceDuration and any
Task.sleep or delayed dispatch used for coordination; publish snapshots only
when the authoritative layout commit event occurs, then equality-deduplicate
consecutive snapshots before notifying observers.
Source: Coding guidelines
Comment on lines
+42
to
+61
| private static func mobileBrowserJPEG( | ||
| from image: NSImage, | ||
| maximumByteCount: Int | ||
| ) -> (data: Data, width: Int, height: Int)? { | ||
| guard let tiff = image.tiffRepresentation, | ||
| var bitmap = NSBitmapImageRep(data: tiff) else { return nil } | ||
| let qualities: [CGFloat] = [0.56, 0.46, 0.36, 0.28, 0.20] | ||
| for _ in 0..<7 { | ||
| for quality in qualities { | ||
| guard let data = bitmap.representation( | ||
| using: .jpeg, | ||
| properties: [.compressionFactor: quality] | ||
| ) else { continue } | ||
| if data.count <= maximumByteCount { | ||
| return (data, bitmap.pixelsWide, bitmap.pixelsHigh) | ||
| } | ||
| } | ||
| guard let resized = mobileBrowserResizedBitmap(bitmap, scale: 0.78) else { break } | ||
| bitmap = resized | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline Sources/Panels/BrowserPanel.swift --match BrowserPanel --view expanded
rg -n -C3 'mobileBrowserJPEG|mobileBrowserPreviewFrame|defaultIsolation|MainActor' \
Sources Packages -g 'Package.swift' -g '*.swift'Repository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file and related helpers with concise ranges.
ast-grep outline Sources/Panels/BrowserPanel+MobilePreview.swift --view expanded || true
# Read the relevant slice with line numbers.
sed -n '1,220p' Sources/Panels/BrowserPanel+MobilePreview.swift | cat -n
# Find where the helper is called and whether any off-main hop already exists.
rg -n -C 4 'mobileBrowserJPEG|mobileBrowserResizedBitmap|Task\.detached|withCheckedContinuation|jpeg' Sources/Panels Sources -g '*.swift'Repository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrowly inspect the file size and the exact lines around the helper.
wc -l Sources/Panels/BrowserPanel+MobilePreview.swift
sed -n '35,95p' Sources/Panels/BrowserPanel+MobilePreview.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 3035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target helper and its direct caller with minimal output.
rg -n -C 8 'mobileBrowserJPEG|mobileBrowserResizedBitmap|func .*MobilePreview|NSImage|representation\(using: \.jpeg' \
Sources/Panels/BrowserPanel+MobilePreview.swift -g '*.swift'
# Show the exact file content near the helper if present.
nl -ba Sources/Panels/BrowserPanel+MobilePreview.swift | sed -n '1,140p'Repository: manaflow-ai/cmux
Length of output: 3087
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'mobileBrowserJPEG|mobileBrowserResizedBitmap|func .*MobilePreview|NSImage|representation\(using: \.jpeg' Sources/Panels/BrowserPanel+MobilePreview.swift -g '*.swift'
nl -ba Sources/Panels/BrowserPanel+MobilePreview.swift | sed -n '1,140p'Repository: manaflow-ai/cmux
Length of output: 3087
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the isolation of BrowserPanel and the snapshotter that feeds the preview.
ast-grep outline Sources/Panels/BrowserPanel.swift --view expanded || true
ast-grep outline Sources/Panels/BrowserScreenshotWebViewSnapshotter.swift --view expanded || true
rg -n -C 4 '`@MainActor`|captureVisibleViewport|mobileBrowserPreviewFrame|BrowserScreenshotWebViewSnapshotter' \
Sources/Panels/BrowserPanel.swift Sources/Panels/BrowserScreenshotWebViewSnapshotter.swift Sources/Panels/BrowserPanel+MobilePreview.swift -g '*.swift'Repository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' Sources/Panels/BrowserScreenshotWebViewSnapshotter.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' Sources/Panels/BrowserPanel.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 4462
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline Sources/Panels/BrowserPanel.swift --view expanded || true
ast-grep outline Sources/Panels/BrowserScreenshotWebViewSnapshotter.swift --view expanded || true
rg -n -C 4 '`@MainActor`|captureVisibleViewport|mobileBrowserPreviewFrame|BrowserScreenshotWebViewSnapshotter' \
Sources/Panels/BrowserPanel.swift Sources/Panels/BrowserScreenshotWebViewSnapshotter.swift Sources/Panels/BrowserPanel+MobilePreview.swift -g '*.swift'Repository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' Sources/Panels/BrowserPanel.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 5112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,160p' Sources/Panels/BrowserScreenshotWebViewSnapshotter.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 254
Offload the mobile preview encode path Sources/Panels/BrowserPanel+MobilePreview.swift:23-61 keeps the full JPEG encode/rescale loop on the main actor; the 35 encode attempts plus resizing can stall browser UI. Capture the snapshot on main, then move the encoding work to an off-main helper.
🤖 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/Panels/BrowserPanel`+MobilePreview.swift around lines 42 - 61, The
mobile preview JPEG encoding loop in mobileBrowserJPEG currently runs on the
main actor and can block UI. Keep snapshot acquisition on the main actor, then
dispatch mobileBrowserJPEG and its resize/quality loop to a nonisolated off-main
helper, preserving the existing output and maximumByteCount behavior.
Source: Coding guidelines
Comment on lines
+14257
to
+14258
| let requestedPaneID = v2UUID(params, "pane_id") | ||
| guard requestedPaneID == nil || paneIds.contains(where: { $0.id == requestedPaneID }) else { return .err(code: "not_found", message: "Pane not found", data: nil) } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject an explicitly invalid pane_id instead of selecting another pane.
v2UUID returns nil for both an absent and malformed value, so pane_id: "" or a non-UUID string silently falls back to the focused/first pane.
Proposed fix
let requestedPaneID = v2UUID(params, "pane_id")
+guard !v2HasNonNullParam(params, "pane_id") || requestedPaneID != nil else {
+ return .err(code: "invalid_params", message: "Missing or invalid pane_id", data: nil)
+}
guard requestedPaneID == nil || paneIds.contains(where: { $0.id == requestedPaneID }) else {
return .err(code: "not_found", message: "Pane not found", data: nil)
}Based on learnings, explicitly provided empty targeting values must error rather than fall back. As per coding guidelines, correctness-critical routing must fail closed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let requestedPaneID = v2UUID(params, "pane_id") | |
| guard requestedPaneID == nil || paneIds.contains(where: { $0.id == requestedPaneID }) else { return .err(code: "not_found", message: "Pane not found", data: nil) } | |
| let requestedPaneID = v2UUID(params, "pane_id") | |
| guard !v2HasNonNullParam(params, "pane_id") || requestedPaneID != nil else { | |
| return .err(code: "invalid_params", message: "Missing or invalid pane_id", data: nil) | |
| } | |
| guard requestedPaneID == nil || paneIds.contains(where: { $0.id == requestedPaneID }) else { return .err(code: "not_found", message: "Pane not found", data: nil) } |
🤖 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/TerminalController.swift` around lines 14257 - 14258, Update the pane
selection logic around requestedPaneID and v2UUID so it distinguishes an absent
pane_id from an explicitly provided malformed or empty value. Return the
existing not_found error for any invalid explicit pane_id, while preserving
focused/first-pane fallback only when pane_id is omitted and normal selection
for a valid UUID.
Sources: Coding guidelines, Learnings
Comment on lines
+14
to
+21
| guard let tabManager = v2ResolveTabManager(params: params), | ||
| let workspace = tabManager.tabs.first(where: { $0.id == workspaceID }) else { | ||
| return .err( | ||
| code: "not_found", | ||
| message: "Workspace not found", | ||
| data: ["workspace_id": workspaceID.uuidString] | ||
| ) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Avoid active-window bias when resolving the workspace.
If the client requests the layout for a workspace in a background window (and doesn't provide a window_id parameter), v2ResolveTabManager(params:) will return the foreground window's tab manager, causing the lookup to fail with not_found.
To ensure reliable resolution across all windows, fall back to a global lookup if the initial manager doesn't own the workspace. Based on learnings, this mirrors the pattern used for panel_id resolution to prevent active-window bias.
💡 Proposed fix
- guard let tabManager = v2ResolveTabManager(params: params),
- let workspace = tabManager.tabs.first(where: { $0.id == workspaceID }) else {
+ let resolvedWorkspace = v2ResolveTabManager(params: params)?.tabs.first(where: { $0.id == workspaceID })
+ ?? AppDelegate.shared?.tabManagerFor(tabId: workspaceID)?.tabs.first(where: { $0.id == workspaceID })
+
+ guard let workspace = resolvedWorkspace else {
return .err(
code: "not_found",
message: "Workspace not found",
data: ["workspace_id": workspaceID.uuidString]
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| guard let tabManager = v2ResolveTabManager(params: params), | |
| let workspace = tabManager.tabs.first(where: { $0.id == workspaceID }) else { | |
| return .err( | |
| code: "not_found", | |
| message: "Workspace not found", | |
| data: ["workspace_id": workspaceID.uuidString] | |
| ) | |
| } | |
| let resolvedWorkspace = v2ResolveTabManager(params: params)?.tabs.first(where: { $0.id == workspaceID }) | |
| ?? AppDelegate.shared?.tabManagerFor(tabId: workspaceID)?.tabs.first(where: { $0.id == workspaceID }) | |
| guard let workspace = resolvedWorkspace else { | |
| return .err( | |
| code: "not_found", | |
| message: "Workspace not found", | |
| data: ["workspace_id": workspaceID.uuidString] | |
| ) | |
| } |
🤖 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/TerminalController`+MobileWorkspaceLayout.swift around lines 14 - 21,
Update the workspace resolution guard around v2ResolveTabManager(params:) so
that if the initially resolved tab manager does not contain workspaceID, it
falls back to the global workspace/tab-manager lookup used by panel_id
resolution. Preserve the existing not_found response when no manager owns the
requested workspace, and continue using the resolved manager for the layout
operation.
Source: Learnings
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripProjection.swift (1)
31-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFail closed when the pane lookup misses.
layout.flatMaptreats "layout missing" and "pane missing from a present layout" the same, so a stalepaneIDfalls back tofallbackTerminalsand can surface the wrong cards. Keep the terminal fallback only whenlayout == nil; if the pane lookup fails, leave the projection empty/invalid instead, and add a regression test.🛠 Proposed fix
- var macOrderedCards: [PaneTabStripCardSnapshot] - if let pane = layout.flatMap({ Self.pane(id: paneID, in: $0.root) }) { - macOrderedCards = Self.cards(tabs: pane.tabs, chatCards: chatCards) - isDegraded = false - } else { - macOrderedCards = Self.cards(terminals: fallbackTerminals, chatCards: chatCards) - isDegraded = true - } + let macOrderedCards: [PaneTabStripCardSnapshot] + if let layout { + if let pane = Self.pane(id: paneID, in: layout.root) { + macOrderedCards = Self.cards(tabs: pane.tabs, chatCards: chatCards) + } else { + macOrderedCards = [] + } + isDegraded = false + } else { + macOrderedCards = Self.cards(terminals: fallbackTerminals, chatCards: chatCards) + isDegraded = true + }🤖 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/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripProjection.swift` around lines 31 - 38, The pane projection currently conflates a missing layout with a missing pane because of the layout.flatMap lookup. Update the branching around pane lookup so fallbackTerminals is used only when layout is nil; when a layout exists but pane(id:in:) returns nil, leave macOrderedCards empty/invalid and preserve degraded state. Add a regression test covering a stale paneID in a present layout.Source: Path instructions
Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/PreviewGridStoreTests.swift (1)
57-63: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRequire the first line before accessing it.
A regression producing an empty snapshot crashes at
[0]instead of reporting a test failure. Based on learnings, indexed access is safe only after an assertion guarantees the required bounds.🛡️ Proposed fix to ensure test safety
let snapshotA = try `#require`(await previewA.next()) let snapshotB = try `#require`(await previewB.next()) + let lineA = try `#require`(snapshotA.lines.first) + let lineB = try `#require`(snapshotB.lines.first) `#expect`(!firstMountedChunk.data.isEmpty) `#expect`(!coalescedMountedChunk.data.isEmpty) `#expect`(shell.previewGridSessionState.store.publicationCount(surfaceID: "surface-a") == 1) - `#expect`(snapshotA.lines[0].spans.map(\.text) == ["alpha"]) - `#expect`(snapshotB.lines[0].spans.map(\.text) == ["beta"]) + `#expect`(lineA.spans.map(\.text) == ["alpha"]) + `#expect`(lineB.spans.map(\.text) == ["beta"])🤖 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/CmuxMobileShell/Tests/CmuxMobileShellTests/PreviewGridStoreTests.swift` around lines 57 - 63, Update the assertions for snapshotA.lines and snapshotB.lines to require that the first line exists before accessing index 0, preserving the existing expected span-text checks while converting empty snapshots into test failures rather than indexing crashes.Source: Learnings
🤖 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/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripProjection.swift`:
- Around line 47-48: Update the card partitioning in PaneTabStripProjection to
traverse macOrderedCards once, appending needsAttention cards to an attention
buffer and all others to a regular buffer, then combine those buffers in the
existing attention-first order. Remove the two filter calls and preserve stable
ordering within both groups.
---
Duplicate comments:
In
`@Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/PreviewGridStoreTests.swift`:
- Around line 57-63: Update the assertions for snapshotA.lines and
snapshotB.lines to require that the first line exists before accessing index 0,
preserving the existing expected span-text checks while converting empty
snapshots into test failures rather than indexing crashes.
In
`@Packages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripProjection.swift`:
- Around line 31-38: The pane projection currently conflates a missing layout
with a missing pane because of the layout.flatMap lookup. Update the branching
around pane lookup so fallbackTerminals is used only when layout is nil; when a
layout exists but pane(id:in:) returns nil, leave macOrderedCards empty/invalid
and preserve degraded state. Add a regression test covering a stale paneID in a
present layout.
🪄 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: 94ab57c5-cbdb-4b08-bcfb-13b518ff983a
📒 Files selected for processing (5)
Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/PreviewGridStoreTests.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabAttentionPredicate.swiftPackages/iOS/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripProjection.swiftPackages/iOS/CmuxMobileShellModel/Tests/CmuxMobileShellModelTests/PaneTabStripProjectionTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/PaneTabRedesignPreviewView.swift
Comment on lines
+47
to
+48
| cards = macOrderedCards.filter(\.needsAttention) | ||
| + macOrderedCards.filter { !$0.needsAttention } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Build the stable attention partition in one pass.
The two filter calls evaluate every card twice and allocate two full intermediate arrays in a UI projection path. Append cards into attention and regular buffers during one traversal. As per path instructions, hot UI paths must avoid repeated full-collection filtering over scalable collections.
⚡ Proposed fix for single-pass partitioning
- cards = macOrderedCards.filter(\.needsAttention)
- + macOrderedCards.filter { !$0.needsAttention }
+ var attention = [PaneTabStripCardSnapshot]()
+ var regular = [PaneTabStripCardSnapshot]()
+ for card in macOrderedCards {
+ if card.needsAttention {
+ attention.append(card)
+ } else {
+ regular.append(card)
+ }
+ }
+ cards = attention + regular🤖 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/CmuxMobileShellModel/Sources/CmuxMobileShellModel/PaneTabStripProjection.swift`
around lines 47 - 48, Update the card partitioning in PaneTabStripProjection to
traverse macOrderedCards once, appending needsAttention cards to an attention
buffer and all others to a regular buffer, then combine those buffers in the
existing attention-first order. Remove the two filter calls and preserve stable
ordering within both groups.
Source: Path instructions
Splits re-derive their axis from their cell's rendered aspect so the Mac's landscape tree fills a portrait phone (left pane becomes top band), ratios and order preserved; canvas grows vertically past a 150pt minimum pane height. Cards get continuous 18pt corners, softer caption gradient with a kind glyph, and a tinted focus glow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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/WorkspaceHubPaneView.swift (1)
204-206: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse plural-aware localization for the tab count.
A single
%d tabsformat string cannot express locale-specific plural rules. Add explicit.one/.othercatalog entries and select through the project’s plural-aware localization API.Based on learnings, pluralized Swift strings should use explicit
.oneand.otherlocalization keys.🤖 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/WorkspaceHubPaneView.swift` around lines 204 - 206, Update the tab-count formatting in WorkspaceHubPaneView to use the project’s plural-aware localization API instead of the single mobile.workspaceHub.tabCountFormat key. Add or reference explicit .one and .other catalog entries, selecting .one for a count of 1 and .other for all remaining counts while preserving the existing displayed count.Sources: Coding guidelines, Learnings
🤖 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/WorkspaceHubPaneView.swift`:
- Around line 204-206: Update the tab-count formatting in WorkspaceHubPaneView
to use the project’s plural-aware localization API instead of the single
mobile.workspaceHub.tabCountFormat key. Add or reference explicit .one and
.other catalog entries, selecting .one for a count of 1 and .other for all
remaining counts while preserving the existing displayed count.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ed6ccc48-cb3a-4842-a88c-9efae9f494b4
📒 Files selected for processing (1)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceHubPaneView.swift
added 2 commits
July 15, 2026 13:14
…esign # Conflicts: # .github/swift-file-length-budget.tsv # Sources/Mobile/MobileHostService.swift # Sources/Mobile/MobileTerminalByteTee.swift # cmux.xcodeproj/project.pbxproj # cmuxTests/MobileHostAuthorizationTests.swift # ios/cmux.xcworkspace/xcshareddata/swiftpm/Package.resolved
…eHostAuthorizationSupport
This was referenced Jul 15, 2026
lawrencecchen
added
the
stale-revisit
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
3 participants
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.
Redesigns the iOS pane/tab experience from the bottom up: the toolbar terminal-picker menu is replaced by a three-level, live-preview navigation model designed in a product-owner interview (closes the intent of #6347).
Navigation. Workspace list (unchanged) → live miniature hub → pane full screen. The hub renders the Mac's real split-pane layout in miniature, each pane showing a live preview of its active tab plus title, tab count, and agent status; the focused pane is indicated. Tap a pane to enter it; back-swipe walks pane → hub → list. Panes are selectable, never swipeable. Deep links now carry the terminal ID and reconstruct the hub-then-pane stack.
Tab strip. Inside a pane, a bottom strip shows live thumbnails of the pane's tabs in the Mac's tab order. Tap to switch. It auto-hides when typing or scrolling starts, leaving a thin handle; the handle reveals it without dismissing the keyboard. An attention toggle at the strip's left edge sorts attention-needing surfaces (agent waiting for input, unread bell, waiting chats) to the front; off restores exact Mac order. There is no per-tab pinning.
Previews. New wire surface:
mobile.workspace.layoutRPC +workspace.layout.updatedpush (debounced, capabilityworkspace.topology.v1-style gating per Mac) mirror the pane tree; scoped render-grid demand means the Mac only computes frames for surfaces a phone actually shows; the client renders cell grids with a lightweight CoreText thumbnail view at strip/card/miniature sizes (text_vt fidelity in v1). Mac browser panes mirror as visual cards viabrowser.preview.v1(demand-gated WKWebView snapshots, 1 s cadence floor, card/full resolutions); tapping opens a read-only "Viewing Mac browser" screen. The phone-local browser stays, badged "On this iPhone". Agent chats appear as their own cards beside their bound terminal.Older Macs without the new capabilities degrade to a flat single-pane hub; nothing breaks cross-version. All new strings localized (en + ja). Regression surface:
TerminalPickerMenuis deleted and its actions rehomed (title menu + strip "+").Design spec:
docs/ios-pane-tab-redesign.md. Acceptance criteria:docs/ios-pane-tab-redesign-acceptance.md.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Redesigned iOS pane/tab navigation to a miniature-first hub with live terminal previews and mirrored Mac browser snapshots, plus an auto‑hiding in‑pane tab strip. Adds demand‑gated render‑grid and browser preview streams and Mac‑provided workspace layouts; merges main and consolidates
MobileHostAuthorizationSupportto resolve build conflicts.New Features
terminal.render_grid.demand.v1; mirrored Mac browser snapshots viabrowser.preview.v1(preview/full); full‑screen read‑only mirrored browser; local iPhone browser stays.mobile.workspace.layout+workspace.layout.updatedgated byworkspace.layout.v1; deep links carry terminal ID and rebuild hub→pane; previews pause/resume with app lifecycle.Migration
TerminalPickerMenuremoved; actions moved to the workspace title menu and the strip “+”.workspace.layout.v1,terminal.render_grid.demand.v1, andbrowser.preview.v1.Written for commit bd6ddc6. Summary will update on new commits.
Summary by CodeRabbit