Repository navigation
Vibe-codable custom sidebars (runtime Swift interpreter), behind a beta flag - #5254
Merged
Merged
Conversation
Custom sidebars authored as a single SwiftUI-style view, interpreted at runtime (no toolchain/signing), discovered from ~/.config/cmux/sidebars/ and selectable in the sidebar button's provider picker. Renders in the real sidebar area, hot-reloads on save, binds to a live workspace data context, and runs cmux commands on tap. Packages (mirroring CmuxSettings/CmuxSettingsUI): - CmuxSwiftRender (logic): swift-syntax parser + tree-walking interpreter for a growing Swift subset -> RenderNode IR. Pure, no SwiftUI. Unit tested. Covers Text/VStack/HStack/ZStack/HSplitView/Button(both forms)/Image/ shapes/Spacer/Divider; for/if/let/ForEach/ternary; member access, subscript, array+string methods; data binding + cmux() action capture. - CmuxSwiftRenderUI (UI): RenderNode -> native SwiftUI (RenderNodeView, resizable HSplitView, JSON DSL renderer, CustomSidebarModel/View). The cmux-coupled action executor is injected via SidebarActionDispatch (Environment), so the package has no TerminalController dependency. App target: thin composition root. ContentView adds the provider, the live data context (workspaces/tabs/clock), and a TimelineView auto-refresh; makeCmuxSidebarActionDispatch() runs actions via TerminalController; a runV2CommandLine seam exposes the dispatcher in-process. Gated by betaFeatures.customSidebars (default off). docs/custom-sidebar-gap-report.md captures a multi-agent survey of opinionated sidebar ideas and the prioritized interpreter gaps driving further work. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughThis PR introduces a runtime-interpreted sidebar feature that allows authors to write custom sidebars in SwiftUI-like source or a declarative JSON DSL. It adds two new packages (CmuxSwiftRender, CmuxSwiftRenderUI), implements expression and view interpreters producing a RenderNode IR, JSON rendering and SwiftUI lowering, host action dispatch, project wiring, tests, corpus examples, and docs. ChangesCustom Sidebars Feature
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
Greptile SummaryThis PR introduces an opt-in beta feature for runtime-interpreted custom sidebars: users write a SwiftUI-style
Confidence Score: 3/5The feature is correctly gated behind a default-off beta flag and the packages are well-structured, but the interpreter has multiple silent correctness gaps that make it unreliable for the use cases it documents. Two new correctness issues compound the ones already on the thread:
Important Files Changed
Sequence DiagramsequenceDiagram
participant TV as TimelineView (1s tick)
participant CV as CustomSidebarView.body
participant SVI as SwiftViewInterpreter
participant EE as ExpressionEvaluator
participant SVM as CustomSidebarModel
participant FW as FileWatcher (kqueue)
FW-->>SVM: file change event
SVM->>SVM: reload() [main actor, blocking I/O]
SVM->>SVM: "state = .swiftSource(newSource)"
loop Every 1 second
TV->>CV: timeline.date tick
CV->>CV: customSidebarDataContext(now:)
CV->>SVI: evaluate(source, state: dataContext)
SVI->>SVI: Parser.parse(source:) [full parse]
SVI->>EE: evalItems / evalFor / evalIf
EE->>EE: sorted closure O(n²) insertion sort
SVI-->>CV: RenderNode tree
CV-->>TV: rendered SwiftUI view
end
note over CV,SVI: if let conditions silently false
note over EE: ReturnStmtSyntax in closures dropped
Reviews (10): Last reviewed commit: "Strengthen div-by-zero test to assert th..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+123
to
+124
| case "/": return bothInt ? .int(Int(l) / Int(r)) : .double(l / r) | ||
| case "%": return bothInt ? .int(Int(l) % Int(r)) : nil |
Contributor
There was a problem hiding this comment.
x / 0 or count % 0, Int(l) / Int(r) (and %) traps and crashes the host app. Guard both cases before the switch, or return nil for zero denominators, so the interpreter silently drops the result rather than taking the process down.
Suggested change
| case "/": return bothInt ? .int(Int(l) / Int(r)) : .double(l / r) | |
| case "%": return bothInt ? .int(Int(l) % Int(r)) : nil | |
| case "/": | |
| if bothInt { | |
| guard Int(r) != 0 else { return nil } | |
| return .int(Int(l) / Int(r)) | |
| } | |
| return .double(l / r) | |
| case "%": | |
| guard Int(r) != 0 else { return nil } | |
| return bothInt ? .int(Int(l) % Int(r)) : nil |
Comment on lines
+56
to
+68
| do { | ||
| state = .swiftSource(try String(contentsOf: fileURL, encoding: .utf8)) | ||
| } catch { | ||
| state = .failed(Self.describe(error)) | ||
| } | ||
| return | ||
| } | ||
| do { | ||
| let data = try Data(contentsOf: fileURL) | ||
| let document = try JSONDecoder().decode(DSLDocument.self, from: data) | ||
| state = .json(document) | ||
| } catch { | ||
| state = .failed(Self.describe(error)) |
Contributor
There was a problem hiding this comment.
Both String(contentsOf:encoding:) (line 57) and Data(contentsOf:) (line 64) are synchronous, blocking file reads. Because reload() is @MainActor (inherited from the class), every load and hot-reload blocks the main thread. A slow filesystem or large sidebar file will jank or freeze the UI. Move the file read into a Task { @concurrent in ... } and deliver the result back to main with await MainActor.run { ... }.
Rule Used: Flag new blocking or timing-based synchronization ... (source)
| case let .swiftSource(source): | ||
| // Interpret here (not in the model) so the view re-evaluates against | ||
| // `dataContext` whenever live workspace state changes. | ||
| if let node = SwiftViewInterpreter().evaluate(source, state: dataContext) { |
Contributor
There was a problem hiding this comment.
body
SwiftViewInterpreter().evaluate(source, state: dataContext) calls Parser.parse(source:) (a full swift-syntax parse) and tree-walks the result on every SwiftUI render. Combined with the enclosing 1-second TimelineView, this runs an expensive CPU pass on the main thread every tick. The interpreter result should be cached in the model keyed on source, so re-evaluation only happens when the source changes, not on every clock tick.
Rule Used: Flag SwiftUI changes that can cause stale state, b... (source)
Comment on lines
+11038
to
+11047
| // Periodic tick so the custom sidebar re-renders live (clock, | ||
| // countdowns, and refreshed workspace/data context), mirroring the | ||
| // default sidebar's TimelineView. No banned timers involved. | ||
| TimelineView(.periodic(from: .now, by: 1)) { timeline in | ||
| CustomSidebarView( | ||
| fileURL: customSidebarURL, | ||
| dataContext: customSidebarDataContext(now: timeline.date), | ||
| dispatch: makeCmuxSidebarActionDispatch() | ||
| ) | ||
| .id(customSidebarURL) |
Contributor
There was a problem hiding this comment.
SidebarActionDispatch closure created on every TimelineView tick
makeCmuxSidebarActionDispatch() is called inside the TimelineView body, allocating a new SidebarActionDispatch struct each second. SidebarActionDispatch has no Equatable conformance, so SwiftUI cannot elide the environment propagation — every button and tappable node in the entire RenderNodeView tree will be re-evaluated every second. Move the dispatch creation to a @State or stored property stable across ticks.
Rule Used: Flag SwiftUI changes that can cause stale state, b... (source)
Comment on lines
+10455
to
+10468
| // persisted selection via an observer) routes correctly on the first render | ||
| // pass and restores the user's choice if extensions are re-enabled. | ||
| private var effectiveExtensionSidebarProviderId: String { | ||
| extensionsExperimentalEnabled | ||
| ? selectedExtensionSidebarProviderId | ||
| : CmuxExtensionSidebarSelection.defaultProviderId | ||
| let selected = selectedExtensionSidebarProviderId | ||
| if selected.hasPrefix(CmuxExtensionSidebarSelection.customSidebarProviderPrefix) { | ||
| // Touch the @LiveSetting so toggling the flag in Settings still | ||
| // re-renders, but decide with the synchronous UserDefaults read: | ||
| // on a sidebar remount @LiveSetting's initial value lags one tick, | ||
| // which would otherwise flash the default sidebar for a frame | ||
| // before swapping to the custom one. | ||
| _ = customSidebarsExperimentalEnabled | ||
| return CmuxExtensionSidebarSelection.customSidebarsEnabled | ||
| ? selected | ||
| : CmuxExtensionSidebarSelection.defaultProviderId |
Contributor
There was a problem hiding this comment.
effectiveExtensionSidebarProviderId touches customSidebarsExperimentalEnabled (the @LiveSetting subscription) for change-notification but reads the actual decision from CmuxExtensionSidebarSelection.customSidebarsEnabled (a direct UserDefaults read) to avoid a one-tick flash. The comment names the problem: @LiveSetting lags on sidebar remount. Using two sources of truth for the same boolean is a timing patch that leaves the state representation ambiguous.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
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
+72
to
+88
| private static func describe(_ error: Error) -> String { | ||
| if let decoding = error as? DecodingError { | ||
| switch decoding { | ||
| case let .keyNotFound(key, ctx): | ||
| return "Missing key '\(key.stringValue)' at \(path(ctx))" | ||
| case let .typeMismatch(_, ctx): | ||
| return "Type mismatch at \(path(ctx)): \(ctx.debugDescription)" | ||
| case let .valueNotFound(_, ctx): | ||
| return "Missing value at \(path(ctx))" | ||
| case let .dataCorrupted(ctx): | ||
| return "Invalid JSON at \(path(ctx)): \(ctx.debugDescription)" | ||
| @unknown default: | ||
| return decoding.localizedDescription | ||
| } | ||
| } | ||
| return (error as NSError).localizedDescription | ||
| } |
Contributor
There was a problem hiding this comment.
debugDescription
ctx.debugDescription in the DecodingError cases is an internal diagnostic string from the Foundation decoder that can contain type names, property-wrapper internals, and key paths not meaningful to a sidebar author. The failed state surfaces this directly via errorView(message). Replace ctx.debugDescription with a human-readable summary derived only from ctx.codingPath and the already-extracted typed information.
Comment on lines
+45
to
+88
| scrollWrap( | ||
| Text(String(localized: "sidebar.custom.missing", defaultValue: "Sidebar file is empty or missing.")) | ||
| .font(.caption) | ||
| .foregroundStyle(.secondary) | ||
| ) | ||
| case let .json(document): | ||
| scrollWrap(DSLSidebarRenderer(node: document.root) { _ in }) | ||
| case let .swiftSource(source): | ||
| // Interpret here (not in the model) so the view re-evaluates against | ||
| // `dataContext` whenever live workspace state changes. | ||
| if let node = SwiftViewInterpreter().evaluate(source, state: dataContext) { | ||
| // A split root owns its own per-column scrolling and fills the | ||
| // sidebar height, so it is not wrapped in the outer ScrollView. | ||
| if node.kind == .hsplit { | ||
| RenderNodeView(node: node) | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity) | ||
| } else { | ||
| scrollWrap(RenderNodeView(node: node)) | ||
| } | ||
| } else { | ||
| scrollWrap(errorView(String(localized: "sidebar.custom.noView", defaultValue: "No supported SwiftUI view found."))) | ||
| } | ||
| case let .failed(message): | ||
| scrollWrap(errorView(message)) | ||
| } | ||
| } | ||
|
|
||
| /// Wraps non-split content in the scrolling container with host-owned | ||
| /// outer insets (authors control inner spacing). | ||
| private func scrollWrap(_ view: some View) -> some View { | ||
| ScrollView { | ||
| view | ||
| .frame(maxWidth: .infinity, alignment: .topLeading) | ||
| .padding(.horizontal, 12) | ||
| .padding(.top, 8) | ||
| .padding(.bottom, 16) | ||
| } | ||
| } | ||
|
|
||
| private func errorView(_ message: String) -> some View { | ||
| VStack(alignment: .leading, spacing: 6) { | ||
| Label( | ||
| String(localized: "sidebar.custom.error", defaultValue: "Sidebar error"), | ||
| systemImage: "exclamationmark.triangle.fill" |
Contributor
There was a problem hiding this comment.
Localizable.xcstrings
CustomSidebarView introduces sidebar.custom.missing, sidebar.custom.noView, and sidebar.custom.error; ContentView adds sidebar.provider.custom.subtitle. The existing Localizable.xcstrings ships translations for English, Japanese, and Korean; none of these keys appear in the diff. All four keys need entries for every supported locale before ship.
Rule Used: Flag production user-facing text that is not fully... (source)
Comment on lines
+10050
to
+10072
| /// One provider descriptor per `<name>.swift`/`<name>.json` file in the | ||
| /// sidebars directory (`.swift` preferred when both exist), titled by the | ||
| /// file's base name. | ||
| static var customSidebarDescriptors: [CmuxExtensionSidebarProviderDescriptor] { | ||
| guard let entries = try? FileManager.default.contentsOfDirectory( | ||
| at: customSidebarsDirectory, | ||
| includingPropertiesForKeys: nil | ||
| ) else { return [] } | ||
| var extensionByName: [String: String] = [:] | ||
| for url in entries { | ||
| let ext = url.pathExtension.lowercased() | ||
| guard ext == "swift" || ext == "json" else { continue } | ||
| let name = url.deletingPathExtension().lastPathComponent | ||
| if extensionByName[name] == "swift" { continue } | ||
| extensionByName[name] = ext | ||
| } | ||
| return extensionByName.keys.sorted().map { name in | ||
| CmuxExtensionSidebarProviderDescriptor( | ||
| id: customSidebarProviderPrefix + name, | ||
| title: CmuxExtensionLocalizedText(key: "sidebar.provider.custom.\(name)", defaultValue: name), | ||
| subtitle: CmuxExtensionLocalizedText( | ||
| key: "sidebar.provider.custom.subtitle", | ||
| defaultValue: String(localized: "sidebar.provider.custom.subtitle", defaultValue: "Custom sidebar") |
Contributor
There was a problem hiding this comment.
customSidebarDescriptors
FileManager.default.contentsOfDirectory(at:includingPropertiesForKeys:) is a blocking syscall called from the @MainActor descriptors property on every picker refresh. On a slow filesystem (remote home, encrypted volume) this stalls the sidebar toggle menu. The directory listing should be cached or fetched off-main.
Rule Used: Flag new blocking or timing-based synchronization ... (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 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 `@docs/custom-sidebar-gap-report.md`:
- Around line 50-52: Update the report text to correct inaccurate claims:
explicitly state that clock is already provided in the evaluation context and
that the host already performs periodic re-renders via TimelineView; then
clarify the remaining gaps are event-driven context subscriptions and
interpreter lifecycle hooks (e.g. .onAppear) and the larger work needed to add a
host-side provider protocol for named context objects (the evaluation
environment, typed read-only bridge, and per-source refresh wiring). Replace the
blanket “missing” statements at the affected sections (lines referencing
workspaces/workspaceCount/selectedTitle, clock, and TimelineView) with wording
that distinguishes “exists today” (clock + periodic tick via TimelineView)
versus “remaining work” (event-driven subscriptions and interpreter-level
lifecycle hooks and the host-side provider protocol).
In `@Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift`:
- Around line 119-130: The integer-division and modulo branches in
ExpressionEvaluator.swift (the switch handling op in the evaluator) currently
use Int(l) / Int(r) and Int(l) % Int(r) which will trap on a zero divisor;
update the "/" and "%" cases (when bothInt is true) to guard the divisor before
converting/operating (e.g., check Int(r) or r != 0) and return nil when the
divisor is zero so the evaluator safely fails instead of crashing; keep the
existing .double path unchanged and optionally note overflow risks for Int
conversions but do not change other operators.
- Around line 181-182: The "sorted" branch currently ignores a detected
comparator closure and always calls sortedScalars(values), producing incorrect
ascending order when a by: closure is supplied; update the case "sorted"
handling to detect the presence of a two-argument comparator closure (the same
closure detected earlier), and if present evaluate it as a two-arg predicate by
calling the closure with pairwise scalar values and using its truthiness to
drive the sort order, otherwise fallback to sortedScalars(values);
alternatively, if implementing the comparator is complex, explicitly return nil
when a by: closure is provided so the caller can skip rendering rather than
silently honoring ascending sort—update the return at case "sorted" (currently
return .array(sortedScalars(values))) accordingly.
- Around line 98-109: The current code eagerly evaluates both operands with
guard let lhs = eval(node.leftOperand, env), let rhs = eval(node.rightOperand,
env) which breaks short-circuiting for "&&" and "||"; change eval flow in
ExpressionEvaluator (the eval call handling BinaryExpression
node.leftOperand/node.rightOperand) to evaluate lhs first, then for "&&" return
.bool(false) if lhs.isFalsy (or truthiness check) without evaluating rhs, and
for "||" return .bool(true) if lhs.isTruthy without evaluating rhs; only call
eval(node.rightOperand, env) when the operator requires the right-hand side,
while preserving existing behavior for range ("..", "..."), equality ("==","!=")
and other operators (evaluate rhs and handle nils as before).
In `@Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/RenderNode.swift`:
- Around line 6-123: This file contains multiple public API types and should be
split so each public type lives in its own file: create ModifierArg.swift
(containing ModifierArg), RenderModifier.swift (RenderModifier),
ActionCommand.swift (ActionCommand), ButtonAction.swift (ButtonAction) and keep
RenderNode.swift only for RenderNode (and its nested Kind). Move the
corresponding declarations and their initializers/derived members into those
files, update imports/module visibility if needed, and ensure references to
symbols like ModifierArg, RenderModifier, ActionCommand, ButtonAction and
RenderNode.Kind continue to compile.
In `@Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift`:
- Around line 37-48: The evaluator lacks a recursion-depth guard; thread a depth
counter (e.g., currentDepth:Int) from evaluate(_:) into evalView(_:env:depth:),
then into evalItems(_:env:depth:), and decrement/propagate into
evalFor(_:env:depth:), evalIf(_:env:depth:), and evalForEach(_:env:depth:);
enforce a configurable MAX_DEPTH constant and when exceeded return nil (or a
controlled failure) so evaluation stops gracefully; update evaluate to start
depth at 0 and ensure all recursive calls pass depth+1, and add a unit test that
builds deeply nested if/else-if chains to assert it does not crash and returns
the expected graceful failure.
In
`@Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLSidebarRenderer.swift`:
- Around line 33-35: The current code always renders Button(node.title ?? "")
which produces an unlabeled, interactive control when node.title is nil/blank;
update DSLSidebarRenderer to only render the Button when the title is non-empty
(e.g., guard let title = node.title?.trimmingCharacters(in:
.whitespacesAndNewlines), !title.isEmpty { Button(title) { if let action =
node.action { onAction(action) } } }) or alternatively supply a deterministic
fallback label (e.g., "Untitled Action") instead of an empty string; ensure you
reference Button, node.title, node.action, and onAction when making the change.
In
`@Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift`:
- Around line 56-69: The cursor can remain as NSCursor.resizeLeftRight if the
drag ends outside hover bounds; update the DragGesture .onEnded handler in
ResizableHSplit (the DragGesture attached to the divider) to explicitly reset
the cursor by calling NSCursor.arrow.set() and clear dragStartFraction there
(and consider doing the same in any cancellation path), ensuring the cursor is
always restored after a drag ends; reference the DragGesture's .onChanged,
.onEnded, dragStartFraction and fraction to place the change.
- Line 13: The AppStorage key used for the shared split fraction
("cmux.customSidebar.splitFraction") makes the stored value global across all
sidebars; update the storage key in ResizableHSplit (the `@AppStorage-backed`
private var fraction: Double) to be scoped per-sidebar (for example by
incorporating a sidebar identifier like sidebarName or providerID into the key,
e.g. "cmux.customSidebar.\(sidebarID).splitFraction") so each sidebar instance
persists its own splitFraction; ensure you pass or compute the sidebarID into
ResizableHSplit (constructor/initializer or environment) and use that when
building the AppStorage key.
In
`@Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift`:
- Around line 72-88: The describe(_:) function returns hardcoded English error
messages; update each user-facing branch (cases .keyNotFound, .typeMismatch,
.valueNotFound, .dataCorrupted and the default decoding.localizedDescription
fallback) to use localized strings via String(localized:defaultValue:) (or the
app’s existing localization helper) and include clear translation keys/strings
in the string catalog for each message and supported locale so
CustomSidebarView.errorView renders localized text.
In
`@Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swift`:
- Around line 50-51: The .json branch in CustomSidebarView uses
DSLSidebarRenderer(node: document.root) { _ in } which discards DSLAction
events; change the closure to forward DSLAction events through the same
dispatcher used by the .swiftSource path by mapping DSLAction → the
corresponding cmux action and calling sidebarActionDispatch with that mapped
action (or reuse the existing mapping helper if one exists); update the
DSLSidebarRenderer initializer call in the .json case to provide a closure like
{ action in let cmuxAction = mapDSLActionToCmux(action);
sidebarActionDispatch(cmuxAction) } so JSON-authored interactive nodes trigger
the shared dispatcher, preserving DSLNode.action handling.
- Around line 52-66: CustomSidebarView is re-parsing Swift source on every body
render because it calls SwiftViewInterpreter().evaluate(source:state:) directly;
change the flow to parse once when the file is reloaded and reuse the parsed AST
during rendering. Update CustomSidebarModel.reload() to store a parsed
representation (result of Parser.parse(source:) and
OperatorTable.standardOperators.foldAll(...)) instead of only the raw String, or
add new SwiftViewInterpreter APIs parse(source:) -> ParsedTree and
evaluate(parsedTree:state:) and use those in CustomSidebarView (replace
evaluate(source:state:) with evaluate(parsedTree:state:) or pass the cached
parsed tree from CustomSidebarModel). Ensure CustomSidebarView checks the cached
parsed tree for nil and falls back to errorView when absent.
In `@Sources/ContentView.swift`:
- Around line 10478-10517: customSidebarDataContext is rebuilding a full
workspaces→panes→tabs SwiftValue snapshot every second (via TimelineView) which
will be expensive at scale; change it to return a cached snapshot and only
rebuild when workspace/tab structure changes or on a throttled interval: add a
stored cache (e.g., cachedSidebarSnapshot and cachedSnapshotVersion/timestamp)
used by customSidebarDataContext, compute and store the heavy workspaces array
(the map over tabManager.tabs and the nested bonsplitController.tabs loop) into
that cache when you observe mutations (subscribe to tabManager tabs changes,
workspace additions/removals, bonsplitController pane/tab changes or expose a
workspaceChanged() hook) or when a configurable throttle timer elapses (e.g.,
5s), while keeping the clock object rebuilt every second; locate and modify
customSidebarDataContext and the call sites interacting with
tabManager/TimelineView to read from the cache and trigger invalidation on the
identified mutation points (tabManager.tabs, Workspace.bonsplitController
tabs/panes, Workspace.customTitle/currentDirectory 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: c78bfbb5-a332-4f3a-9a4a-6abb69945c72
📒 Files selected for processing (29)
Packages/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swiftPackages/CmuxSwiftRender/Package.resolvedPackages/CmuxSwiftRender/Package.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/RenderNode.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftValue.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swiftPackages/CmuxSwiftRenderUI/Package.resolvedPackages/CmuxSwiftRenderUI/Package.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Action/SidebarActionDispatch.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLAction.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLDocument.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNode.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNodeKind.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLSidebarRenderer.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/OptionalStyleModifiers.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderStyle.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swiftSources/ContentView.swiftSources/DSLSidebarPlayground.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmux.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolveddocs/custom-sidebar-gap-report.md
Comment on lines
+50
to
+52
| - Rationale: Every single persona invented read-only context arrays beyond the documented workspaces/tabs. The interpreter only exposes workspaces/workspaceCount/selectedTitle, so almost every sidebar's primary content (alerts, PRs, services, GPUs, runs, notes, ports, git state) is fictional today. This is the universal blocker and the highest-leverage investment: most other gaps are cosmetic on top of data that does not exist. | ||
| - Example: `services = [{ id, name, port, up, healthy }]; git = { branch, dirty, ahead }; pulls = [{ number, ciState, needsMyReview }]; gpus = [{ index, utilPct, vramUsedGB, tempC }]; ports = [{ port, owner }]; clock.time` | ||
| - Effort: large — define a host-side provider protocol that populates named context objects (shelling out to git/lsof/gh/nvidia-smi/health probes) and inject them into the evaluation environment exactly like workspaces; needs a typed read-only value bridge and per-source refresh wiring. |
There was a problem hiding this comment.
Correct roadmap claims that contradict current implementation.
This report currently describes two already-shipped capabilities as missing:
- data context excludes
clock(it is already provided), and - sidebar refresh is a static snapshot (the host already re-renders periodically via
TimelineView).
Please update these sections to distinguish what exists today (periodic tick + clock in context) from the remaining gap (e.g., event-driven context subscriptions and interpreter-level lifecycle hooks like .onAppear).
Also applies to: 69-73, 151-154
🤖 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 `@docs/custom-sidebar-gap-report.md` around lines 50 - 52, Update the report
text to correct inaccurate claims: explicitly state that clock is already
provided in the evaluation context and that the host already performs periodic
re-renders via TimelineView; then clarify the remaining gaps are event-driven
context subscriptions and interpreter lifecycle hooks (e.g. .onAppear) and the
larger work needed to add a host-side provider protocol for named context
objects (the evaluation environment, typed read-only bridge, and per-source
refresh wiring). Replace the blanket “missing” statements at the affected
sections (lines referencing workspaces/workspaceCount/selectedTitle, clock, and
TimelineView) with wording that distinguishes “exists today” (clock + periodic
tick via TimelineView) versus “remaining work” (event-driven subscriptions and
interpreter-level lifecycle hooks and the host-side provider protocol).
Comment on lines
+56
to
+69
| .onHover { inside in | ||
| if inside { NSCursor.resizeLeftRight.set() } else { NSCursor.arrow.set() } | ||
| } | ||
| .gesture( | ||
| DragGesture() | ||
| .onChanged { value in | ||
| let start = dragStartFraction ?? fraction | ||
| if dragStartFraction == nil { dragStartFraction = start } | ||
| let newLeading = CGFloat(start) * total + value.translation.width | ||
| let lower = Double(minColumnWidth / total) | ||
| fraction = min(max(Double(newLeading / total), lower), 1 - lower) | ||
| } | ||
| .onEnded { _ in dragStartFraction = nil } | ||
| ) |
There was a problem hiding this comment.
Cursor may not reset if hover ends during drag.
The onHover handler sets NSCursor.resizeLeftRight on enter and NSCursor.arrow on exit, but if the user drags outside the divider bounds, onHover won't fire an exit event during the active gesture. The cursor can remain resizeLeftRight after the drag ends. Consider resetting the cursor in the .onEnded handler:
Proposed fix
.onEnded { _ in
+ NSCursor.arrow.set()
dragStartFraction = nil
}📝 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
| .onHover { inside in | |
| if inside { NSCursor.resizeLeftRight.set() } else { NSCursor.arrow.set() } | |
| } | |
| .gesture( | |
| DragGesture() | |
| .onChanged { value in | |
| let start = dragStartFraction ?? fraction | |
| if dragStartFraction == nil { dragStartFraction = start } | |
| let newLeading = CGFloat(start) * total + value.translation.width | |
| let lower = Double(minColumnWidth / total) | |
| fraction = min(max(Double(newLeading / total), lower), 1 - lower) | |
| } | |
| .onEnded { _ in dragStartFraction = nil } | |
| ) | |
| .onHover { inside in | |
| if inside { NSCursor.resizeLeftRight.set() } else { NSCursor.arrow.set() } | |
| } | |
| .gesture( | |
| DragGesture() | |
| .onChanged { value in | |
| let start = dragStartFraction ?? fraction | |
| if dragStartFraction == nil { dragStartFraction = start } | |
| let newLeading = CGFloat(start) * total + value.translation.width | |
| let lower = Double(minColumnWidth / total) | |
| fraction = min(max(Double(newLeading / total), lower), 1 - lower) | |
| } | |
| .onEnded { _ in | |
| NSCursor.arrow.set() | |
| dragStartFraction = 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
`@Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift`
around lines 56 - 69, The cursor can remain as NSCursor.resizeLeftRight if the
drag ends outside hover bounds; update the DragGesture .onEnded handler in
ResizableHSplit (the DragGesture attached to the divider) to explicitly reset
the cursor by calling NSCursor.arrow.set() and clear dragStartFraction there
(and consider doing the same in any cancellation path), ensuring the cursor is
always restored after a drag ends; reference the DragGesture's .onChanged,
.onEnded, dragStartFraction and fraction to place the change.
Comment on lines
+72
to
+88
| private static func describe(_ error: Error) -> String { | ||
| if let decoding = error as? DecodingError { | ||
| switch decoding { | ||
| case let .keyNotFound(key, ctx): | ||
| return "Missing key '\(key.stringValue)' at \(path(ctx))" | ||
| case let .typeMismatch(_, ctx): | ||
| return "Type mismatch at \(path(ctx)): \(ctx.debugDescription)" | ||
| case let .valueNotFound(_, ctx): | ||
| return "Missing value at \(path(ctx))" | ||
| case let .dataCorrupted(ctx): | ||
| return "Invalid JSON at \(path(ctx)): \(ctx.debugDescription)" | ||
| @unknown default: | ||
| return decoding.localizedDescription | ||
| } | ||
| } | ||
| return (error as NSError).localizedDescription | ||
| } |
There was a problem hiding this comment.
Localize the user-facing decoding-error messages.
describe(_:) returns hardcoded English ("Missing key …", "Type mismatch at …", "Invalid JSON at …") that flows into state.failed and is rendered verbatim by CustomSidebarView.errorView. The rest of the sidebar UI already uses String(localized:), so this is partial localization.
As per coding guidelines: "Swift text must use localized APIs with matching translated string-catalog entries."
🌐 Suggested direction
- case let .keyNotFound(key, ctx):
- return "Missing key '\(key.stringValue)' at \(path(ctx))"
+ case let .keyNotFound(key, ctx):
+ return String(
+ localized: "sidebar.custom.error.missingKey",
+ defaultValue: "Missing key '\(key.stringValue)' at \(path(ctx))"
+ )Apply the same String(localized:defaultValue:) treatment to the remaining cases and add matching string-catalog entries for every supported locale.
🤖 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/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift`
around lines 72 - 88, The describe(_:) function returns hardcoded English error
messages; update each user-facing branch (cases .keyNotFound, .typeMismatch,
.valueNotFound, .dataCorrupted and the default decoding.localizedDescription
fallback) to use localized strings via String(localized:defaultValue:) (or the
app’s existing localization helper) and include clear translation keys/strings
in the string catalog for each message and supported locale so
CustomSidebarView.errorView renders localized text.
Comment on lines
+10478
to
10517
| private func customSidebarDataContext(now: Date) -> [String: SwiftValue] { | ||
| let selectedId = tabManager.selectedTabId | ||
| let workspaces: [SwiftValue] = tabManager.tabs.map { workspace in | ||
| let focusedPanelId = workspace.focusedPanelId | ||
| var tabs: [SwiftValue] = [] | ||
| for paneId in workspace.bonsplitController.allPaneIds { | ||
| for tab in workspace.bonsplitController.tabs(inPane: paneId) { | ||
| guard let panelId = workspace.panelIdFromSurfaceId(tab.id) else { continue } | ||
| tabs.append(.object([ | ||
| "id": .string(panelId.uuidString), | ||
| "title": .string(tab.title), | ||
| "focused": .bool(panelId == focusedPanelId), | ||
| ])) | ||
| } | ||
| } | ||
| return .object([ | ||
| "id": .string(workspace.id.uuidString), | ||
| "title": .string(workspace.customTitle ?? workspace.title), | ||
| "selected": .bool(workspace.id == selectedId), | ||
| "directory": .string(workspace.currentDirectory), | ||
| "tabs": .array(tabs), | ||
| ]) | ||
| } | ||
| let selectedWorkspace = tabManager.tabs.first { $0.id == selectedId } | ||
| let c = Calendar.current.dateComponents([.hour, .minute, .second, .weekday], from: now) | ||
| let hour = c.hour ?? 0, minute = c.minute ?? 0, second = c.second ?? 0 | ||
| let clock: SwiftValue = .object([ | ||
| "time": .string(String(format: "%02d:%02d:%02d", hour, minute, second)), | ||
| "hour": .int(hour), | ||
| "minute": .int(minute), | ||
| "second": .int(second), | ||
| "epoch": .int(Int(now.timeIntervalSince1970)), | ||
| ]) | ||
| return [ | ||
| "workspaces": .array(workspaces), | ||
| "workspaceCount": .int(tabManager.tabs.count), | ||
| "selectedTitle": .string(selectedWorkspace?.customTitle ?? selectedWorkspace?.title ?? ""), | ||
| "clock": clock, | ||
| ] | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider caching or throttling the workspace-to-SwiftValue transformation.
The customSidebarDataContext performs triple-nested iteration (workspaces → panes → tabs) and runs every second via TimelineView. While this is acceptable for typical usage behind a beta flag, at scale (~1000 workspaces) this could become expensive.
For the beta phase this is fine, but if custom sidebars graduate to GA, consider:
- Caching the computed snapshot and invalidating only on workspace/tab mutations
- Throttling the data context rebuild (e.g., 5-second interval) separately from clock updates
The snapshot boundary is correctly respected—SwiftValue objects are built rather than passing store references.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/ContentView.swift` around lines 10478 - 10517,
customSidebarDataContext is rebuilding a full workspaces→panes→tabs SwiftValue
snapshot every second (via TimelineView) which will be expensive at scale;
change it to return a cached snapshot and only rebuild when workspace/tab
structure changes or on a throttled interval: add a stored cache (e.g.,
cachedSidebarSnapshot and cachedSnapshotVersion/timestamp) used by
customSidebarDataContext, compute and store the heavy workspaces array (the map
over tabManager.tabs and the nested bonsplitController.tabs loop) into that
cache when you observe mutations (subscribe to tabManager tabs changes,
workspace additions/removals, bonsplitController pane/tab changes or expose a
workspaceChanged() hook) or when a configurable throttle timer elapses (e.g.,
5s), while keeping the clock object rebuilt every second; locate and modify
customSidebarDataContext and the call sites interacting with
tabManager/TimelineView to read from the cache and trigger invalidation on the
identified mutation points (tabManager.tabs, Workspace.bonsplitController
tabs/panes, Workspace.customTitle/currentDirectory updates).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/OptionalStyleModifiers.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/OptionalStyleModifiers.swift:4">
P3: Three ViewModifier types in one file violates the repo's one-major-type-per-file convention. Split `OptionalForeground`, `OptionalPadding`, and `OptionalBackground` into separate files for consistency with the codebase convention.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNodeKind.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNodeKind.swift:4">
P1: Missing fallback for unknown raw values during JSON decoding — user-authored sidebar files with an unrecognized node kind (e.g., a typo like `"vstak"` or a future plugin authoring a new kind before this code adds it) will throw a hard `DecodingError.dataCorrupted` and fail to load the entire sidebar. Add a custom `Decodable` that maps unknown strings to a fallback `.unknown` case.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift:66">
P2: Drag gesture writes to `@AppStorage` on every `.onChanged` event (~60 writes/sec), causing unnecessary `UserDefaults` synchronization and potential UI jank. Use a local `@State` for the transient drag value and persist only on `.onEnded`.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:123">
P0: Integer division and modulo by zero crash the interpreter with a fatal runtime error. User-authored sidebar expressions like `5 / 0` or `x % 0` (where x is an integer variable resolving to 0) cause an unhandled Swift division-by-zero trap, crashing the host process.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNode.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNode.swift:9">
P2: Auto-synthesized `Equatable` includes `id` (`UUID()`) in the comparison, making equality checks always return `false` for distinct instances even when semantically identical. The doc comment explicitly says `id` is "Not decoded; a stable identity per decoded node" — it is a runtime identity marker, not part of the value. Including it in `Equatable` means `==` can never be true for two separately-initialized or decoded nodes with identical content, breaking any code that relies on value equality (SwiftUI `EquatableView`, test assertions, collection diffing).</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift:51">
P2: Uses `FileManager.default` directly instead of an injected dependency, making the model untestable without real file I/O. The repository guidance (`CLAUDE.md`) requires package APIs to be testable without launching the app or relying on `FileManager.default`. The `reload()` method also reads files via `Data(contentsOf:)` and `String(contentsOf:encoding:)` which bypass any controllable FileManager.</violation>
</file>
<file name="cmux.xcodeproj/project.pbxproj">
<violation number="1" location="cmux.xcodeproj/project.pbxproj:1932">
P2: CmuxSwiftRenderUI (a SwiftUI+AppKit UI-rendering package) is added as a dependency of the cmux-cli command-line tool target. Every source file in this package imports SwiftUI, and ResizableHSplit.swift imports AppKit. The cmux-cli target (product-type tool) has no UI runtime and cannot meaningfully use SwiftUI View types, which will unnecessarily link SwiftUI/AppKit frameworks into the CLI binary and create a maintenance burden.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swift:51">
P2: JSON sidebar action handler silently discards all button actions. The `onAction:` closure is `{ _ in }`, so any interactive element (button, tap) in a `.json` sidebar produces zero side effects — the buttons render but do nothing when tapped. Meanwhile, interpreted Swift sidebars correctly dispatch actions through the environment's `sidebarActionDispatch`.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLSidebarRenderer.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLSidebarRenderer.swift:33">
P2: Button node does not apply `resolvedFont`, unlike `.text` and `.image` cases. When a JSON DSL button specifies `font` or `size`, those properties are silently ignored, while the same properties work correctly on text and image nodes. This means the button's title text will always appear at the system default font regardless of the authored style.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:75">
P1: `Button("title", action: { … })` drops the title string because the "label form" branch (triggered by finding an `action:` labeled closure argument) returns a node with no text and an empty children array, ignoring the unlabeled first argument.</violation>
<violation number="2" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:178">
P3: `closureParameterName(_:)` is duplicated verbatim in both `SwiftViewInterpreter` and `ExpressionEvaluator`, each as a private method doing the same closure-signature parameter-name extraction.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| import Foundation | ||
|
|
||
| /// The kind of a ``DSLNode`` in the declarative JSON sidebar format. | ||
| enum DSLNodeKind: String, Codable, Sendable { |
There was a problem hiding this comment.
P1: Missing fallback for unknown raw values during JSON decoding — user-authored sidebar files with an unrecognized node kind (e.g., a typo like "vstak" or a future plugin authoring a new kind before this code adds it) will throw a hard DecodingError.dataCorrupted and fail to load the entire sidebar. Add a custom Decodable that maps unknown strings to a fallback .unknown case.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNodeKind.swift, line 4:
<comment>Missing fallback for unknown raw values during JSON decoding — user-authored sidebar files with an unrecognized node kind (e.g., a typo like `"vstak"` or a future plugin authoring a new kind before this code adds it) will throw a hard `DecodingError.dataCorrupted` and fail to load the entire sidebar. Add a custom `Decodable` that maps unknown strings to a fallback `.unknown` case.</comment>
<file context>
@@ -0,0 +1,13 @@
+import Foundation
+
+/// The kind of a ``DSLNode`` in the declarative JSON sidebar format.
+enum DSLNodeKind: String, Codable, Sendable {
+ case vstack
+ case hstack
</file context>
| switch ref.baseName.text { | ||
| case "Text": | ||
| return RenderNode(kind: .text, text: stringArgument(call.arguments, env) ?? "") | ||
| case "Button": |
There was a problem hiding this comment.
P1: Button("title", action: { … }) drops the title string because the "label form" branch (triggered by finding an action: labeled closure argument) returns a node with no text and an empty children array, ignoring the unlabeled first argument.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift, line 75:
<comment>`Button("title", action: { … })` drops the title string because the "label form" branch (triggered by finding an `action:` labeled closure argument) returns a node with no text and an empty children array, ignoring the unlabeled first argument.</comment>
<file context>
@@ -0,0 +1,308 @@
+ switch ref.baseName.text {
+ case "Text":
+ return RenderNode(kind: .text, text: stringArgument(call.arguments, env) ?? "")
+ case "Button":
+ // Label form: `Button(action: { … }) { labelView }` — the action
+ // is the `action:` closure and the trailing closure is a rich
</file context>
| if dragStartFraction == nil { dragStartFraction = start } | ||
| let newLeading = CGFloat(start) * total + value.translation.width | ||
| let lower = Double(minColumnWidth / total) | ||
| fraction = min(max(Double(newLeading / total), lower), 1 - lower) |
There was a problem hiding this comment.
P2: Drag gesture writes to @AppStorage on every .onChanged event (~60 writes/sec), causing unnecessary UserDefaults synchronization and potential UI jank. Use a local @State for the transient drag value and persist only on .onEnded.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift, line 66:
<comment>Drag gesture writes to `@AppStorage` on every `.onChanged` event (~60 writes/sec), causing unnecessary `UserDefaults` synchronization and potential UI jank. Use a local `@State` for the transient drag value and persist only on `.onEnded`.</comment>
<file context>
@@ -0,0 +1,71 @@
+ if dragStartFraction == nil { dragStartFraction = start }
+ let newLeading = CGFloat(start) * total + value.translation.width
+ let lower = Double(minColumnWidth / total)
+ fraction = min(max(Double(newLeading / total), lower), 1 - lower)
+ }
+ .onEnded { _ in dragStartFraction = nil }
</file context>
| /// JSON trivial to author and the renderer a single recursive `switch`. | ||
| struct DSLNode: Codable, Equatable, Sendable, Identifiable { | ||
| /// Not decoded; a stable identity per decoded node for `ForEach`. | ||
| let id = UUID() |
There was a problem hiding this comment.
P2: Auto-synthesized Equatable includes id (UUID()) in the comparison, making equality checks always return false for distinct instances even when semantically identical. The doc comment explicitly says id is "Not decoded; a stable identity per decoded node" — it is a runtime identity marker, not part of the value. Including it in Equatable means == can never be true for two separately-initialized or decoded nodes with identical content, breaking any code that relies on value equality (SwiftUI EquatableView, test assertions, collection diffing).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNode.swift, line 9:
<comment>Auto-synthesized `Equatable` includes `id` (`UUID()`) in the comparison, making equality checks always return `false` for distinct instances even when semantically identical. The doc comment explicitly says `id` is "Not decoded; a stable identity per decoded node" — it is a runtime identity marker, not part of the value. Including it in `Equatable` means `==` can never be true for two separately-initialized or decoded nodes with identical content, breaking any code that relies on value equality (SwiftUI `EquatableView`, test assertions, collection diffing).</comment>
<file context>
@@ -0,0 +1,33 @@
+/// JSON trivial to author and the renderer a single recursive `switch`.
+struct DSLNode: Codable, Equatable, Sendable, Identifiable {
+ /// Not decoded; a stable identity per decoded node for `ForEach`.
+ let id = UUID()
+ var type: DSLNodeKind
+ var children: [DSLNode]?
</file context>
| A5B00002A1B2C3D4E5F60718 /* CMUXAgentLaunch */, | ||
| A5354305A5354305A5354305 /* CmuxSocketControl */, | ||
| C5A1FED000000000000000C3 /* CmuxSwiftRender */, | ||
| C5A1FED100000000000000D3 /* CmuxSwiftRenderUI */, |
There was a problem hiding this comment.
P2: CmuxSwiftRenderUI (a SwiftUI+AppKit UI-rendering package) is added as a dependency of the cmux-cli command-line tool target. Every source file in this package imports SwiftUI, and ResizableHSplit.swift imports AppKit. The cmux-cli target (product-type tool) has no UI runtime and cannot meaningfully use SwiftUI View types, which will unnecessarily link SwiftUI/AppKit frameworks into the CLI binary and create a maintenance burden.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmux.xcodeproj/project.pbxproj, line 1932:
<comment>CmuxSwiftRenderUI (a SwiftUI+AppKit UI-rendering package) is added as a dependency of the cmux-cli command-line tool target. Every source file in this package imports SwiftUI, and ResizableHSplit.swift imports AppKit. The cmux-cli target (product-type tool) has no UI runtime and cannot meaningfully use SwiftUI View types, which will unnecessarily link SwiftUI/AppKit frameworks into the CLI binary and create a maintenance burden.</comment>
<file context>
@@ -1917,6 +1928,8 @@
A5B00002A1B2C3D4E5F60718 /* CMUXAgentLaunch */,
A5354305A5354305A5354305 /* CmuxSocketControl */,
+ C5A1FED000000000000000C3 /* CmuxSwiftRender */,
+ C5A1FED100000000000000D3 /* CmuxSwiftRenderUI */,
C8000302C8000302C8000302 /* CmuxProcess */,
A500D013A1B2C3D4E5F60718 /* CMUXDebugLog */,
</file context>
| .font(resolvedFont) | ||
| .fontWeight(dslFontWeight(node.weight)) | ||
| case .button: | ||
| Button(node.title ?? "") { |
There was a problem hiding this comment.
P2: Button node does not apply resolvedFont, unlike .text and .image cases. When a JSON DSL button specifies font or size, those properties are silently ignored, while the same properties work correctly on text and image nodes. This means the button's title text will always appear at the system default font regardless of the authored style.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLSidebarRenderer.swift, line 33:
<comment>Button node does not apply `resolvedFont`, unlike `.text` and `.image` cases. When a JSON DSL button specifies `font` or `size`, those properties are silently ignored, while the same properties work correctly on text and image nodes. This means the button's title text will always appear at the system default font regardless of the authored style.</comment>
<file context>
@@ -0,0 +1,64 @@
+ .font(resolvedFont)
+ .fontWeight(dslFontWeight(node.weight))
+ case .button:
+ Button(node.title ?? "") {
+ if let action = node.action { onAction(action) }
+ }
</file context>
| import SwiftUI | ||
|
|
||
| /// Applies a foreground color only when one is resolved. | ||
| struct OptionalForeground: ViewModifier { |
There was a problem hiding this comment.
P3: Three ViewModifier types in one file violates the repo's one-major-type-per-file convention. Split OptionalForeground, OptionalPadding, and OptionalBackground into separate files for consistency with the codebase convention.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/OptionalStyleModifiers.swift, line 4:
<comment>Three ViewModifier types in one file violates the repo's one-major-type-per-file convention. Split `OptionalForeground`, `OptionalPadding`, and `OptionalBackground` into separate files for consistency with the codebase convention.</comment>
<file context>
@@ -0,0 +1,29 @@
+import SwiftUI
+
+/// Applies a foreground color only when one is resolved.
+struct OptionalForeground: ViewModifier {
+ let color: Color?
+ func body(content: Content) -> some View {
</file context>
| } | ||
|
|
||
| private func closureParameterName(_ closure: ClosureExprSyntax) -> String? { | ||
| guard let parameterClause = closure.signature?.parameterClause else { return nil } |
There was a problem hiding this comment.
P3: closureParameterName(_:) is duplicated verbatim in both SwiftViewInterpreter and ExpressionEvaluator, each as a private method doing the same closure-signature parameter-name extraction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift, line 178:
<comment>`closureParameterName(_:)` is duplicated verbatim in both `SwiftViewInterpreter` and `ExpressionEvaluator`, each as a private method doing the same closure-signature parameter-name extraction.</comment>
<file context>
@@ -0,0 +1,308 @@
+ }
+
+ private func closureParameterName(_ closure: ClosureExprSyntax) -> String? {
+ guard let parameterClause = closure.signature?.parameterClause else { return nil }
+ if case let .simpleInput(list) = parameterClause {
+ return list.first?.name.text
</file context>
New `Reorderable(data, move: "method") { item in row }` interpreter primitive
lowers to a `.reorderable` node carrying a ReorderSpec (method + id/index
params + ordered item ids). The renderer makes each row `.draggable` with the
item id and a `.dropDestination`; dropping dispatches the reorder command
(e.g. `workspace.reorder` with workspace_id + index), so for workspaces cmux
both reorders and persists, and the next auto-refresh tick reflects it. Numeric
params (the index) are coerced to numbers in the dispatch. demo.swift uses it
for the workspace list.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment on lines
+59
to
+62
| case let .range(lower, upper, inclusive): | ||
| let end = inclusive ? upper + 1 : upper | ||
| guard end >= lower else { return [] } | ||
| return (lower..<end).map(SwiftValue.int) |
Contributor
There was a problem hiding this comment.
Int.max. Swift traps on Int.max + 1 before the guard end >= lower check can run. A user-authored sidebar containing a large closed range (e.g. for i in 0...2147483647) will unconditionally crash the host process on every 1-second TimelineView tick.
Suggested change
| case let .range(lower, upper, inclusive): | |
| let end = inclusive ? upper + 1 : upper | |
| guard end >= lower else { return [] } | |
| return (lower..<end).map(SwiftValue.int) | |
| case let .range(lower, upper, inclusive): | |
| let end: Int | |
| if inclusive { | |
| guard upper < Int.max else { return [] } | |
| end = upper + 1 | |
| } else { | |
| end = upper | |
| } | |
| guard end >= lower else { return [] } | |
| return (lower..<end).map(SwiftValue.int) |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/RenderNode.swift (1)
6-150: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winSplit public types into per-type files.
This file declares six public API types (
ModifierArg,RenderModifier,ActionCommand,ButtonAction,ReorderSpec,RenderNode). The repo convention requires each public type in its own file named after the type; only small closely-bound helpers may stay with a parent.As per coding guidelines: "Extract one major type per file; each
struct,class,enum,actor, orprotocolthat is part of a public API lives in its own file named after the type; small closely-bound helpers can stay with the parent".🤖 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/CmuxSwiftRender/Sources/CmuxSwiftRender/RenderNode.swift` around lines 6 - 150, The file currently declares multiple public API types; split each public type into its own file named after the type (ModifierArg, RenderModifier, ActionCommand, ButtonAction, ReorderSpec, RenderNode). For each type, create a new source file containing the type declaration and its public init/props (preserve Sendable/Equatable conformance and documentation comments), and remove the duplicate declarations from the original file so only the intended parent type remains; ensure imports and access levels remain unchanged and update any module tests or references to use the moved types (search for ModifierArg, RenderModifier, ActionCommand, ButtonAction, ReorderSpec, RenderNode to verify).Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift (1)
37-48:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd recursion-depth guard to prevent stack overflow.
The evaluator lacks a recursion-depth limit.
evalItemsrecursively callsevalFor/evalIf/evalForEach, which call back intoevalItems, andevalIfrecursively handleselse if—deeply nested user input can overflow the stack. Thread a depth counter throughevaluate→evalView→evalItems→evalFor/evalIf/evalForEachand fail gracefully past a threshold.🤖 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/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift` around lines 37 - 48, Add a recursion-depth guard by introducing a depth parameter (or using an internal counter) that is threaded from evaluate into evalView, evalItems and down into evalFor, evalIf and evalForEach, and enforce a defined MAX_DEPTH constant; on each recursive entry increment the counter and immediately fail gracefully (return nil or a distinct error RenderNode) when the counter exceeds MAX_DEPTH. Update the signatures of evaluate, evalView, evalItems, evalFor, evalIf and evalForEach to accept an optional depth:Int (defaulting to 0 in the public evaluate), increment depth on recursive calls, and perform the guard check at the start of each function so deeply nested user input cannot overflow the stack. Ensure callers in the file use the new parameter and keep the public API behavior by defaulting depth to 0.
🤖 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.
Duplicate comments:
In `@Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/RenderNode.swift`:
- Around line 6-150: The file currently declares multiple public API types;
split each public type into its own file named after the type (ModifierArg,
RenderModifier, ActionCommand, ButtonAction, ReorderSpec, RenderNode). For each
type, create a new source file containing the type declaration and its public
init/props (preserve Sendable/Equatable conformance and documentation comments),
and remove the duplicate declarations from the original file so only the
intended parent type remains; ensure imports and access levels remain unchanged
and update any module tests or references to use the moved types (search for
ModifierArg, RenderModifier, ActionCommand, ButtonAction, ReorderSpec,
RenderNode to verify).
In `@Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift`:
- Around line 37-48: Add a recursion-depth guard by introducing a depth
parameter (or using an internal counter) that is threaded from evaluate into
evalView, evalItems and down into evalFor, evalIf and evalForEach, and enforce a
defined MAX_DEPTH constant; on each recursive entry increment the counter and
immediately fail gracefully (return nil or a distinct error RenderNode) when the
counter exceeds MAX_DEPTH. Update the signatures of evaluate, evalView,
evalItems, evalFor, evalIf and evalForEach to accept an optional depth:Int
(defaulting to 0 in the public evaluate), increment depth on recursive calls,
and perform the guard check at the start of each function so deeply nested user
input cannot overflow the stack. Ensure callers in the file use the new
parameter and keep the public API behavior by defaulting depth to 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 98e3ad16-153c-4094-83c9-a98603f246f0
📒 Files selected for processing (6)
Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/RenderNode.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swiftSources/DSLSidebarPlayground.swift
There was a problem hiding this comment.
4 issues found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift:18">
P1: ForEach uses index-based identity (`id: \.offset`) in a reorderable list, even though stable per-item identifiers are available via `spec.itemIds`. When a row is dragged to a new position and `rows` updates, SwiftUI identifies each row by its index (0, 1, 2...) rather than by its stable id. This prevents SwiftUI from tracking which view corresponds to which item across the reorder, losing view state, breaking transition animations, and making the reorder feel unresponsive or glitchy.
Use stable IDs from `spec.itemIds` as the ForEach identity, paired with each row, so SwiftUI correctly preserves view identity when items move.</violation>
<violation number="2" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift:37">
P2: Validate `ReorderSpec` command/key strings (and `draggedId`) before dispatching `.cmux`; currently malformed or empty dynamic keys can produce invalid reorder commands.
(Based on your team's feedback about avoiding sentinel/missing-key patterns in cmux param construction.) [FEEDBACK_USED].</violation>
</file>
<file name="Sources/DSLSidebarPlayground.swift">
<violation number="1" location="Sources/DSLSidebarPlayground.swift:31">
P2: Unconditionally coercing all numeric-looking string params to Int can break commands that require string-typed values.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:178">
P1: Empty-string sentinel for missing item IDs in Reorderable: `item.member(idField)?.displayString ?? ""` inserts `""` into `itemIds` when the id field is absent, causing invalid reorder commands (e.g., `workspace_id: ""`) instead of a clear failure or graceful skip.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| var body: some View { | ||
| VStack(spacing: 0) { | ||
| ForEach(Array(rows.enumerated()), id: \.offset) { index, row in |
There was a problem hiding this comment.
P1: ForEach uses index-based identity (id: \.offset) in a reorderable list, even though stable per-item identifiers are available via spec.itemIds. When a row is dragged to a new position and rows updates, SwiftUI identifies each row by its index (0, 1, 2...) rather than by its stable id. This prevents SwiftUI from tracking which view corresponds to which item across the reorder, losing view state, breaking transition animations, and making the reorder feel unresponsive or glitchy.
Use stable IDs from spec.itemIds as the ForEach identity, paired with each row, so SwiftUI correctly preserves view identity when items move.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift, line 18:
<comment>ForEach uses index-based identity (`id: \.offset`) in a reorderable list, even though stable per-item identifiers are available via `spec.itemIds`. When a row is dragged to a new position and `rows` updates, SwiftUI identifies each row by its index (0, 1, 2...) rather than by its stable id. This prevents SwiftUI from tracking which view corresponds to which item across the reorder, losing view state, breaking transition animations, and making the reorder feel unresponsive or glitchy.
Use stable IDs from `spec.itemIds` as the ForEach identity, paired with each row, so SwiftUI correctly preserves view identity when items move.</comment>
<file context>
@@ -0,0 +1,41 @@
+
+ var body: some View {
+ VStack(spacing: 0) {
+ ForEach(Array(rows.enumerated()), id: \.offset) { index, row in
+ RenderNodeView(node: row)
+ .draggable(itemId(index))
</file context>
| scope.define("$0", item) | ||
| let rowNodes = evalItems(closure.statements, scope) | ||
| rows.append(rowNodes.count == 1 ? rowNodes[0] : RenderNode(kind: .vstack, children: rowNodes)) | ||
| ids.append(item.member(idField)?.displayString ?? "") |
There was a problem hiding this comment.
P1: Empty-string sentinel for missing item IDs in Reorderable: item.member(idField)?.displayString ?? "" inserts "" into itemIds when the id field is absent, causing invalid reorder commands (e.g., workspace_id: "") instead of a clear failure or graceful skip.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift, line 178:
<comment>Empty-string sentinel for missing item IDs in Reorderable: `item.member(idField)?.displayString ?? ""` inserts `""` into `itemIds` when the id field is absent, causing invalid reorder commands (e.g., `workspace_id: ""`) instead of a clear failure or graceful skip.</comment>
<file context>
@@ -152,6 +154,41 @@ public struct SwiftViewInterpreter: Sendable {
+ scope.define("$0", item)
+ let rowNodes = evalItems(closure.statements, scope)
+ rows.append(rowNodes.count == 1 ? rowNodes[0] : RenderNode(kind: .vstack, children: rowNodes))
+ ids.append(item.member(idField)?.displayString ?? "")
+ }
+ return RenderNode(
</file context>
| // like v2Int decode them. | ||
| var typed: [String: Any] = [:] | ||
| for (key, value) in params { | ||
| if let intValue = Int(value) { typed[key] = intValue } else { typed[key] = value } |
There was a problem hiding this comment.
P2: Unconditionally coercing all numeric-looking string params to Int can break commands that require string-typed values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/DSLSidebarPlayground.swift, line 31:
<comment>Unconditionally coercing all numeric-looking string params to Int can break commands that require string-typed values.</comment>
<file context>
@@ -22,7 +22,16 @@ func makeCmuxSidebarActionDispatch() -> SidebarActionDispatch {
+ // like v2Int decode them.
+ var typed: [String: Any] = [:]
+ for (key, value) in params {
+ if let intValue = Int(value) { typed[key] = intValue } else { typed[key] = value }
+ }
+ payload["params"] = typed
</file context>
Suggested change
| if let intValue = Int(value) { typed[key] = intValue } else { typed[key] = value } | |
| if key == "index", let intValue = Int(value) { | |
| typed[key] = intValue | |
| } else { | |
| typed[key] = value | |
| } |
|
|
||
| private func reorder(_ draggedId: String, to index: Int) { | ||
| guard let spec else { return } | ||
| dispatch.run(ButtonAction(commands: [ |
There was a problem hiding this comment.
P2: Validate ReorderSpec command/key strings (and draggedId) before dispatching .cmux; currently malformed or empty dynamic keys can produce invalid reorder commands.
(Based on your team's feedback about avoiding sentinel/missing-key patterns in cmux param construction.) .
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift, line 37:
<comment>Validate `ReorderSpec` command/key strings (and `draggedId`) before dispatching `.cmux`; currently malformed or empty dynamic keys can produce invalid reorder commands.
(Based on your team's feedback about avoiding sentinel/missing-key patterns in cmux param construction.) .</comment>
<file context>
@@ -0,0 +1,41 @@
+
+ private func reorder(_ draggedId: String, to index: Int) {
+ guard let spec else { return }
+ dispatch.run(ButtonAction(commands: [
+ .cmux(method: spec.method, params: [spec.idParam: draggedId, spec.indexParam: String(index)]),
+ ]))
</file context>
…ents New docs/custom-sidebars.md is the authoring contract for vibe-coding a custom sidebar: file location + picker, the full supported interpreter subset (views, modifiers, language, live data context, cmux() actions, Reorderable drag-and- drop, auto-refresh), worked examples, and an agent-facing section on building a clean, interactive, native result from a non-technical "make me a sidebar" request (default to live data, make rows tappable, prefer Reorderable for orderable lists, cap/lazy-load long lists). Surfaced via a new `cmux docs sidebars` topic so an in-pane coding agent can discover and fetch it. Co-Authored-By: Claude Opus 4.8 <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)
CLI/CMUXCLI+DocsSettings.swift (1)
154-155:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate docs command usage text to include
sidebars.
sidebarsis now a valid topic indocsReferences, but both the thrown usage string anddocsUsage()still show the old topic set. This makes help/error output incorrect for users.Proposed fix
- throw CLIError(message: "Usage: cmux docs [settings|shortcuts|api|browser|agents|dock]") + throw CLIError(message: "Usage: cmux docs [settings|shortcuts|api|browser|agents|dock|sidebars]")- Usage: cmux docs [settings|shortcuts|api|browser|agents|dock] + Usage: cmux docs [settings|shortcuts|api|browser|agents|dock|sidebars]Also applies to: 177-180
🤖 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 `@CLI/CMUXCLI`+DocsSettings.swift around lines 154 - 155, The thrown usage string and the docsUsage() output are missing the new "sidebars" topic; update the usage text in the docs command (the CLIError thrown in the docs handler) to include "sidebars" and also add "sidebars" to the list returned/printed by docsUsage() so both error/help output reflect the new valid topic; locate the throw CLIError(message: "...") in the docs command handler and the docsUsage() function and insert "sidebars" into their topic lists.
🤖 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 `@CLI/CMUXCLI`+DocsSettings.swift:
- Around line 154-155: The thrown usage string and the docsUsage() output are
missing the new "sidebars" topic; update the usage text in the docs command (the
CLIError thrown in the docs handler) to include "sidebars" and also add
"sidebars" to the list returned/printed by docsUsage() so both error/help output
reflect the new valid topic; locate the throw CLIError(message: "...") in the
docs command handler and the docsUsage() function and insert "sidebars" into
their topic lists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e1c24437-3846-4733-a1c5-890bf309429b
📒 Files selected for processing (3)
CLI/CMUXCLI+DocsSettings.swiftCLI/cmux.swiftdocs/custom-sidebars.md
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:31283">
P2: Inconsistent docs topic list: main CLI help advertises `sidebars` but the `docs` command's own usage string and error message still show the old topic list without it. Users running `cmux docs --help` or triggering the extra-args error path will see `docs [settings|shortcuts|api|browser|agents|dock]` — no mention of sidebars.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| Commands: | ||
| welcome | ||
| docs [settings|shortcuts|api|browser|agents|dock] | ||
| docs [settings|shortcuts|api|browser|agents|dock|sidebars] |
There was a problem hiding this comment.
P2: Inconsistent docs topic list: main CLI help advertises sidebars but the docs command's own usage string and error message still show the old topic list without it. Users running cmux docs --help or triggering the extra-args error path will see docs [settings|shortcuts|api|browser|agents|dock] — no mention of sidebars.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CLI/cmux.swift, line 31283:
<comment>Inconsistent docs topic list: main CLI help advertises `sidebars` but the `docs` command's own usage string and error message still show the old topic list without it. Users running `cmux docs --help` or triggering the extra-args error path will see `docs [settings|shortcuts|api|browser|agents|dock]` — no mention of sidebars.</comment>
<file context>
@@ -31280,7 +31280,7 @@ export default function cmuxPiSessionExtension(pi: ExtensionAPI) {
Commands:
welcome
- docs [settings|shortcuts|api|browser|agents|dock]
+ docs [settings|shortcuts|api|browser|agents|dock|sidebars]
settings [open [target]|path|docs|<target>]
config <doctor|check|validate|path|paths|docs|documentation|reload>
</file context>
…harness
Generated 14 opinionated sidebars (one per workflow persona) as a test corpus
and built a coverage harness that runs each through the interpreter against a
rich stub data context and statically lists unsupported symbols. Iterated the
interpreter until every corpus sidebar renders (57-255 nodes each):
- User-defined functions: `func` value helpers (let / if-else with return /
trailing return) and view helpers (`func row(x) -> some View { ... }`),
registered in the environment and callable in value and view position.
- Array higher-order: flatMap, reduce (two-arg closure).
- Number/currency/compact formatting via `.formatted(.currency/.notation/...)`.
- `Color(...)` values (hex string or red/green/blue) usable by color modifiers.
- `ScrollView { ... }` (passthrough), `.strikethrough`, and the
`.font(.system(size:design:))` / `.system(.style, design:)` forms.
- `openURL(...)` button action (opens via NSWorkspace).
Remaining flagged symbols are undefined helpers the personas referenced but
never declared (authoring gaps, not interpreter limits) plus graceful-degrade
cosmetics (contextMenu/overlay/stroke/AsyncImage). 29 tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift (1)
136-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
RenderNodeView.applyFramealignment +resolveFont.system(size:weight:design:)weight parsing
applyFramesetsalignmenton the min/max.frame(...), but then wraps with a second.frame(width:height:)that defaults to center—so the requestedframe(...alignment:)is lost wheneverwidth/heightare provided.resolveFontdocuments.system(size:weight:design:), but the implementation only parsessize:(plusmonospaced/named text styles) and never handlesweight:—so authored font weight is silently dropped; implementweight:parsing or update the doc/comment to match supported tokens.🤖 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/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift` around lines 136 - 145, In applyFrame (RenderNodeView.applyFrame), ensure the requested alignment is applied to the outermost frame so it isn't overridden by the width/height wrapper: pass alignment into the second .frame call (i.e., .frame(width: dim("width"), height: dim("height"), alignment: alignment)) or otherwise apply the same alignment to both frames instead of only the min/max frame. In resolveFont (resolveFont function), actually parse and honor the font weight token referenced in the docs by reading the weight modifier (e.g., modifier.value("weight") or corresponding token), map common names ("ultraLight","thin","light","regular","medium","semibold","bold","heavy","black") to Font.Weight, and supply that mapped weight into Font.system(size: weight: design:) when creating system fonts (while preserving existing size/monospaced/named-style handling).Sources/DSLSidebarPlayground.swift (1)
27-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSupport non-integer JSON literals in sidebar
cmuxparams.Only coercing
Intnarrows the v2 surface for custom sidebars: commands that needDoubleorBoolparams still go through as strings and fail downstream validation/dispatch. A sidebar-authoredratio: "0.5"for split sizing is one concrete breakage.Proposed fix
- var typed: [String: Any] = [:] - for (key, value) in params { - if let intValue = Int(value) { typed[key] = intValue } else { typed[key] = value } - } + var typed: [String: Any] = [:] + for (key, value) in params { + switch value.lowercased() { + case "true": + typed[key] = true + case "false": + typed[key] = false + default: + if let intValue = Int(value) { + typed[key] = intValue + } else if let doubleValue = Double(value) { + typed[key] = doubleValue + } else { + typed[key] = value + } + } + }Based on learnings: in
Sources/TerminalController.swift, v2 param parsing relies on typed JSON numerics (NSNumber/Double), and present-but-invalid numeric params such asratiomust error rather than fall back.🤖 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/DSLSidebarPlayground.swift` around lines 27 - 34, The current coercion only tries Int; update the params-to-typed conversion in DSLSidebarPlayground.swift (the loop that builds typed from params and assigns payload["params"]) to: for each value, attempt Int(value), then Double(value), then Bool(value) and set the first successful typed value; if the string clearly represents a numeric/boolean literal (e.g., contains digits, a decimal point, exponent, or "true"/"false") but all parses fail, treat that as an invalid param and return/error rather than leaving it as a string so downstream v2 validation/dispatch fails fast.
🤖 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/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift`:
- Around line 14-15: The functions dictionary currently maps names to
FunctionDeclSyntax only, causing helpers to be dynamically scoped because
call-time code binds parameters into the caller environment; update
defineFunction/Environment to capture and store the defining Environment
alongside the FunctionDeclSyntax (e.g., store a tuple or small struct {decl:
FunctionDeclSyntax, definingEnv: Environment?} in functions) and, when invoking
a function, create the call scope as a child of that captured definingEnv
instead of the call-site parent so free variables resolve from the definition
site; apply the same change to the related code paths referenced around the
existing functions and parent usage (also lines ~33-40).
In `@Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift`:
- Around line 318-323: The numberMethod(_:_:_:) implementation currently detects
"currency" by string search and returns a hardcoded "$%.2f"; change it to use a
NumberFormatter configured for .currency and set its currencyCode from the call
arguments (parse the currency code from call.arguments/trimmedDescription or
evaluate the first argument expression when name == "formatted" and arg contains
"currency(code:") so it supports non-USD codes (e.g., "EUR"); format the Double
value with that formatter and return .string of the formatted result instead of
the hardcoded USD string.
In `@Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift`:
- Around line 139-145: The view-helper branch that handles user-defined
functions (env.lookupFunction + bindParameters) calls evalItems(body.statements,
scope), but evalItems ignores ReturnStmtSyntax so helpers that return a single
view produce no nodes; modify this path to either (A) reuse the value-function
block handling used elsewhere: evaluate the function body as a value-expression
block and extract its returned expression before passing to evalItems, or (B)
extend evalItems to detect and unwrap ReturnStmtSyntax (extract the returned
expression from ReturnStmtSyntax and evaluate it into nodes) so that functions
like func row() -> some View { return HStack { ... } } yield the HStack node;
update the same logic for the similar block at the other mentioned location
(lines 153-172) and ensure you still wrap multiple nodes with RenderNode(kind:
.vstack, children: nodes).
In
`@Packages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/a-multi-repo-maintainer-juggling-the-cmu.swift`:
- Around line 27-40: The test uses hard-coded roots in notesByDir that don't
match the seeded workspace from corpusStubContext(), so the intended
note/grouping branches aren't exercised; update notesByDir (and any roots array)
to use the same paths produced by corpusStubContext()—e.g., /Users/me/proj0,
/Users/me/proj1, /Users/me/proj2, /Users/me/proj3—or derive roots directly from
corpusStubContext() when constructing notesByDir so the tests hit the
non-empty-note branches and not the OTHER fallback (adjust the variables named
notesByDir and roots accordingly).
In
`@Packages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/backend-devops-engineer-running-a-fleet-.swift`:
- Around line 84-159: corpusStubContext() currently seeds ci and builds but not
the single-item view-model fields used by the pipeline pane (build, deploys,
deployLog), so the UI under CorpusCoverageTests renders empty; update
corpusStubContext() to also populate a representative Build object (with runId,
status, sha, durationSec, ahead/behind, branch, dirty, changedFiles) and assign
it to the key/variable named build, add an array of Deploy objects (with env,
state, version, relativeTime) for deploys, and add an array of LogLine entries
for deployLog; this ensures the view code (which references build, deploys,
deployLog) has concrete data to exercise the CI/deploy interpreter paths.
In
`@Packages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/data-scientist-running-ml-training-jobs-.swift`:
- Around line 88-142: The runs and datasets returned by the shared corpus stub
are missing fields used by the UI (in the ForEach rendering of runs and datasets
in this test file), so update the corpus stub used by CorpusCoverageTests to
seed runs with status, epoch, totalEpochs, lr, etaMin, totalSteps, surfaceId and
datasets with workspaceId, surfaceId, mountCommand, mounted, sizeGB, rows (in
addition to existing id/step/loss/eta/name) so the rendering paths in the run
card (Text checks, progress bar using totalSteps, cmux surface.focus using
surfaceId) and dataset buttons (workspace.select, surface.run, mounted state,
size/rows display) exercise the intended interpreter logic.
- Around line 12-72: The test stub is missing fields used by the telemetry pane
(gpu.name, gpu.vramTotalGB, gpu.ownerSurfaceId, host.diskFreeGB), so update the
corpusStubContext() test fixture (and any GPU/Host test object builders it uses)
to populate those fields with realistic sample values; specifically add a name
string, vramTotalGB Int, and ownerSurfaceId (string/int as used by
cmux("surface.focus")) to each GPU entry and add diskFreeGB to the host object
so the ForEach GPU rendering and host Disk label/usage paths exercise the
intended UI branches. Ensure any helpers that construct GPU/Host samples (e.g.,
the GPU struct or factory used in corpusStubContext()) are adjusted accordingly
so existing tests compile and exercise the new labels/actions.
In
`@Packages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/CorpusCoverageTests.swift`:
- Around line 14-65: The test currently only prints diagnostics; change
reportCorpusCoverage() to assert failures when rendering regresses by (1)
asserting node is non-nil for each file (use XCTAssertNotNil(node, "...
\(file.lastPathComponent)")) and (2) asserting the parsed tree size is
meaningful (e.g., XCTAssertTrue(count > 1, "trivial tree for
\(file.lastPathComponent)")) using the existing nodeCount helper, and (3) fail
the test if unsupportedCalls or unsupportedMembers are non-empty (XCTFail or
XCTAssertTrue(unsupportedCalls.isEmpty, ...) and similarly for
unsupportedMembers) so CI fails on regressions; keep references to
SwiftViewInterpreter, nodeCount, collector.declaredFunctions, unsupportedCalls
and unsupportedMembers to locate where to add the assertions.
---
Outside diff comments:
In
`@Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift`:
- Around line 136-145: In applyFrame (RenderNodeView.applyFrame), ensure the
requested alignment is applied to the outermost frame so it isn't overridden by
the width/height wrapper: pass alignment into the second .frame call (i.e.,
.frame(width: dim("width"), height: dim("height"), alignment: alignment)) or
otherwise apply the same alignment to both frames instead of only the min/max
frame. In resolveFont (resolveFont function), actually parse and honor the font
weight token referenced in the docs by reading the weight modifier (e.g.,
modifier.value("weight") or corresponding token), map common names
("ultraLight","thin","light","regular","medium","semibold","bold","heavy","black")
to Font.Weight, and supply that mapped weight into Font.system(size: weight:
design:) when creating system fonts (while preserving existing
size/monospaced/named-style handling).
In `@Sources/DSLSidebarPlayground.swift`:
- Around line 27-34: The current coercion only tries Int; update the
params-to-typed conversion in DSLSidebarPlayground.swift (the loop that builds
typed from params and assigns payload["params"]) to: for each value, attempt
Int(value), then Double(value), then Bool(value) and set the first successful
typed value; if the string clearly represents a numeric/boolean literal (e.g.,
contains digits, a decimal point, exponent, or "true"/"false") but all parses
fail, treat that as an invalid param and return/error rather than leaving it as
a string so downstream v2 validation/dispatch fails fast.
🪄 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: e093001c-00c7-4c35-8e7e-149bedd6cd27
📒 Files selected for processing (23)
Packages/CmuxSwiftRender/Package.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/RenderNode.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/a-multi-repo-maintainer-juggling-the-cmu.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/backend-devops-engineer-running-a-fleet-.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/cs-undergrad-mid-semester-juggling-three.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/data-scientist-running-ml-training-jobs-.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/engineering-manager-for-cmux-my-day-is-t.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/frontend-developer-running-multiple-loca.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/ios-developer-who-lives-in-build-run-deb.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/keyboard-first-vim-purist-i-live-in-norm.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/open-source-maintainer-triaging-issues-a.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/power-user-running-6-12-coding-agents-in.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/product-designer-running-a-design-system.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/project-manager-running-a-small-eng-team.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/sre-on-call-wants-active-alerts-ranked-b.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/technical-writer-who-lives-in-markdown-d.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/CorpusCoverageTests.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swiftSources/DSLSidebarPlayground.swift
Comment on lines
+14
to
+15
| private var functions: [String: FunctionDeclSyntax] | ||
| private let parent: Environment? |
There was a problem hiding this comment.
User-defined helpers are dynamically scoped today.
defineFunction only stores the FunctionDeclSyntax. Both call paths then bind parameters into a child of the caller environment, so free variables inside the helper resolve from the call site instead of the definition site. A helper declared next to let accent = "#..." will lose that binding or pick up a shadowed accent when invoked from another nested scope. Store the defining environment alongside the declaration and create the call scope from that captured parent.
Also applies to: 33-40
🤖 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/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift` around
lines 14 - 15, The functions dictionary currently maps names to
FunctionDeclSyntax only, causing helpers to be dynamically scoped because
call-time code binds parameters into the caller environment; update
defineFunction/Environment to capture and store the defining Environment
alongside the FunctionDeclSyntax (e.g., store a tuple or small struct {decl:
FunctionDeclSyntax, definingEnv: Environment?} in functions) and, when invoking
a function, create the call scope as a child of that captured definingEnv
instead of the call-site parent so free variables resolve from the definition
site; apply the same change to the related code paths referenced around the
existing functions and parent usage (also lines ~33-40).
Comment on lines
+27
to
+40
| let notesByDir = [ | ||
| "/Users/me/cmux": [ | ||
| "next: wire pbxproj test target for new file", | ||
| "gotcha: macOS 26 CFURL normalization differs from 14/15", | ||
| "blocked: waiting on dogfood approval before merge" | ||
| ], | ||
| "/Users/me/web": [ | ||
| "waiting on Vercel preview URL before reporting done", | ||
| "Effect: map typed errors at route boundary" | ||
| ], | ||
| "/Users/me/ios": [ | ||
| "next: reload sim + best-effort iPhone, short tag (<=6 chars)" | ||
| ] | ||
| ] |
There was a problem hiding this comment.
The hard-coded roots never match the shared corpus stub.
corpusStubContext() seeds workspace directories as /Users/me/proj0...proj3, so notesByDir and roots never hit in coverage. This sidebar will only exercise the empty-notes branch and the OTHER catch-all instead of the intended note/grouping paths.
Also applies to: 137-186
🤖 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/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/a-multi-repo-maintainer-juggling-the-cmu.swift`
around lines 27 - 40, The test uses hard-coded roots in notesByDir that don't
match the seeded workspace from corpusStubContext(), so the intended
note/grouping branches aren't exercised; update notesByDir (and any roots array)
to use the same paths produced by corpusStubContext()—e.g., /Users/me/proj0,
/Users/me/proj1, /Users/me/proj2, /Users/me/proj3—or derive roots directly from
corpusStubContext() when constructing notesByDir so the tests hit the
non-empty-note branches and not the OTHER fallback (adjust the variables named
notesByDir and roots accordingly).
Comment on lines
+84
to
+159
| HStack { | ||
| Image(systemName: "arrow.triangle.branch") | ||
| Text(git.branch).fontWeight(.semibold) | ||
| if git.dirty { | ||
| Text("●").foregroundColor("#FF9F0A") | ||
| Text("\(git.changedFiles) changed").font(.caption).foregroundColor("#FF9F0A") | ||
| } else { | ||
| Text("clean").font(.caption).foregroundColor("#34C759") | ||
| } | ||
| Spacer() | ||
| Text("↑\(git.ahead) ↓\(git.behind)").font(.caption).foregroundColor("#8E8E93") | ||
| } | ||
|
|
||
| Divider() | ||
|
|
||
| // --- CI build status --- | ||
| HStack(spacing: 8) { | ||
| Text("●").foregroundColor( | ||
| build.status == "passing" ? "#34C759" : | ||
| build.status == "running" ? "#FF9F0A" : "#FF453A" | ||
| ) | ||
| VStack(alignment: .leading, spacing: 1) { | ||
| Text("CI: \(build.status)").fontWeight(.semibold) | ||
| Text("\(build.sha) · \(build.durationSec)s").font(.caption).foregroundColor("#8E8E93") | ||
| } | ||
| Spacer() | ||
| Button(action: { cmux("ci.rerun", run_id: build.runId) }) { | ||
| Image(systemName: "arrow.clockwise") | ||
| } | ||
| Button(action: { cmux("ci.open", run_id: build.runId) }) { | ||
| Image(systemName: "arrow.up.forward.square") | ||
| } | ||
| } | ||
|
|
||
| Divider() | ||
|
|
||
| // --- deploy targets --- | ||
| Text("Deploy").font(.headline).bold() | ||
| ForEach(deploys) { d in | ||
| VStack(alignment: .leading, spacing: 3) { | ||
| HStack { | ||
| Text("●").foregroundColor( | ||
| d.state == "live" ? "#34C759" : | ||
| d.state == "deploying" ? "#FF9F0A" : "#FF453A" | ||
| ) | ||
| Text(d.env).fontWeight(.semibold) | ||
| Spacer() | ||
| Text(d.relativeTime).font(.caption).foregroundColor("#8E8E93") | ||
| } | ||
| HStack { | ||
| Text(d.version).font(.caption).foregroundColor("#8E8E93") | ||
| Spacer() | ||
| Button(d.env == "prod" ? "Ship prod" : "Deploy") { | ||
| cmux("deploy.start", env: d.env, sha: build.sha) | ||
| } | ||
| .foregroundColor(d.env == "prod" ? "#FF453A" : "#0A84FF") | ||
| .bold() | ||
| } | ||
| } | ||
| .padding(6) | ||
| } | ||
|
|
||
| Divider() | ||
|
|
||
| // --- recent deploy log tail --- | ||
| Text("Recent log").font(.caption).bold().foregroundColor("#8E8E93") | ||
| ForEach(deployLog) { line in | ||
| HStack(spacing: 6) { | ||
| Text(line.level == "error" ? "✗" : line.level == "warn" ? "!" : "·") | ||
| .foregroundColor( | ||
| line.level == "error" ? "#FF453A" : | ||
| line.level == "warn" ? "#FF9F0A" : "#8E8E93" | ||
| ) | ||
| Text(line.text).font(.caption).foregroundColor(line.level == "error" ? "#FF453A" : "#C7C7CC") | ||
| } | ||
| } |
There was a problem hiding this comment.
The pipeline pane is bound to nonexistent corpus data.
corpusStubContext() seeds ci and builds, but not build, deploys, or deployLog. Under CorpusCoverageTests, this entire half will render mostly blank/empty and stop validating the interpreter paths added for CI/deploy content.
🤖 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/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/backend-devops-engineer-running-a-fleet-.swift`
around lines 84 - 159, corpusStubContext() currently seeds ci and builds but not
the single-item view-model fields used by the pipeline pane (build, deploys,
deployLog), so the UI under CorpusCoverageTests renders empty; update
corpusStubContext() to also populate a representative Build object (with runId,
status, sha, durationSec, ahead/behind, branch, dirty, changedFiles) and assign
it to the key/variable named build, add an array of Deploy objects (with env,
state, version, relativeTime) for deploys, and add an array of LogLine entries
for deployLog; this ensures the view code (which references build, deploys,
deployLog) has concrete data to exercise the CI/deploy interpreter paths.
Comment on lines
+12
to
+72
| ForEach(gpus) { gpu in | ||
| VStack(alignment: .leading, spacing: 3) { | ||
| HStack(spacing: 6) { | ||
| Text("GPU \(gpu.index)").font(.caption).bold() | ||
| Text(gpu.name).font(.caption).foregroundColor("#8b95a5") | ||
| Spacer() | ||
| Text("\(gpu.tempC)°C") | ||
| .font(.caption) | ||
| .foregroundColor(gpu.tempC >= 84 ? "#ff5c5c" : "#8b95a5") | ||
| } | ||
| // utilization bar | ||
| HStack(spacing: 2) { | ||
| for seg in 0..<20 { | ||
| Text("▉") | ||
| .font(.caption) | ||
| .foregroundColor(seg < gpu.utilPct / 5 | ||
| ? (gpu.utilPct >= 95 ? "#3ddc84" : "#4aa8ff") | ||
| : "#2a2f3a") | ||
| } | ||
| Text("\(gpu.utilPct)%").font(.caption).bold() | ||
| } | ||
| // VRAM bar with OOM warning | ||
| HStack(spacing: 2) { | ||
| for seg in 0..<20 { | ||
| Text("▉") | ||
| .font(.caption) | ||
| .foregroundColor(seg < gpu.vramUsedGB * 20 / gpu.vramTotalGB | ||
| ? (gpu.vramUsedGB * 100 / gpu.vramTotalGB >= 92 ? "#ff5c5c" : "#b072ff") | ||
| : "#2a2f3a") | ||
| } | ||
| Text("\(gpu.vramUsedGB)/\(gpu.vramTotalGB)G").font(.caption) | ||
| } | ||
| } | ||
| .padding(6) | ||
| .background(RoundedRectangle(cornerRadius: 8).fill("#161a22")) | ||
| .onTapGesture { cmux("surface.focus", surface_id: gpu.ownerSurfaceId) } | ||
| } | ||
|
|
||
| Divider() | ||
|
|
||
| // Host RAM | ||
| HStack(spacing: 6) { | ||
| Image(systemName: "memorychip") | ||
| Text("RAM").font(.caption).bold() | ||
| Spacer() | ||
| Text("\(host.ramUsedGB)/\(host.ramTotalGB)G") | ||
| .font(.caption) | ||
| .foregroundColor(host.ramUsedGB * 100 / host.ramTotalGB >= 90 ? "#ff5c5c" : .primary) | ||
| } | ||
| HStack(spacing: 2) { | ||
| for seg in 0..<24 { | ||
| Text("▉").font(.caption) | ||
| .foregroundColor(seg < host.ramUsedGB * 24 / host.ramTotalGB ? "#f0a14a" : "#2a2f3a") | ||
| } | ||
| } | ||
| HStack(spacing: 6) { | ||
| Image(systemName: "internaldrive") | ||
| Text("Disk \(host.diskFreeGB)G free").font(.caption).foregroundColor("#8b95a5") | ||
| Spacer() | ||
| Text("CPU \(host.cpuPct)%").font(.caption).foregroundColor("#8b95a5") | ||
| } |
There was a problem hiding this comment.
The telemetry pane outgrew the shared GPU/host stub.
corpusStubContext() does not provide gpu.name, gpu.vramTotalGB, gpu.ownerSurfaceId, or host.diskFreeGB. That leaves several labels/actions unresolved here, so coverage no longer exercises the intended GPU-card and host-summary behavior.
🤖 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/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/data-scientist-running-ml-training-jobs-.swift`
around lines 12 - 72, The test stub is missing fields used by the telemetry pane
(gpu.name, gpu.vramTotalGB, gpu.ownerSurfaceId, host.diskFreeGB), so update the
corpusStubContext() test fixture (and any GPU/Host test object builders it uses)
to populate those fields with realistic sample values; specifically add a name
string, vramTotalGB Int, and ownerSurfaceId (string/int as used by
cmux("surface.focus")) to each GPU entry and add diskFreeGB to the host object
so the ForEach GPU rendering and host Disk label/usage paths exercise the
intended UI branches. Ensure any helpers that construct GPU/Host samples (e.g.,
the GPU struct or factory used in corpusStubContext()) are adjusted accordingly
so existing tests compile and exercise the new labels/actions.
Comment on lines
+88
to
+142
| ForEach(runs) { run in | ||
| VStack(alignment: .leading, spacing: 3) { | ||
| HStack(spacing: 6) { | ||
| Text(run.status == "running" ? "●" : (run.status == "failed" ? "✕" : "✓")) | ||
| .foregroundColor(run.status == "running" ? "#3ddc84" | ||
| : (run.status == "failed" ? "#ff5c5c" : "#8b95a5")) | ||
| Text(run.name).font(.caption).bold() | ||
| Spacer() | ||
| Text("ep \(run.epoch)/\(run.totalEpochs)").font(.caption).foregroundColor("#8b95a5") | ||
| } | ||
| HStack(spacing: 8) { | ||
| Text("loss \(run.loss)").font(.caption).foregroundColor("#4aa8ff") | ||
| Text("lr \(run.lr)").font(.caption).foregroundColor("#8b95a5") | ||
| Spacer() | ||
| Text("ETA \(run.etaMin)m").font(.caption).foregroundColor("#f0a14a") | ||
| } | ||
| // step progress | ||
| HStack(spacing: 2) { | ||
| for seg in 0..<18 { | ||
| Text("▬").font(.caption) | ||
| .foregroundColor(seg < run.step * 18 / run.totalSteps ? "#3ddc84" : "#2a2f3a") | ||
| } | ||
| } | ||
| } | ||
| .padding(6) | ||
| .background(RoundedRectangle(cornerRadius: 8).fill("#161a22")) | ||
| .onTapGesture { cmux("surface.focus", surface_id: run.surfaceId) } | ||
| } | ||
| } | ||
|
|
||
| Divider() | ||
|
|
||
| // Dataset shortcuts | ||
| HStack(spacing: 6) { | ||
| Image(systemName: "tray.full") | ||
| Text("DATASETS").font(.caption).bold().foregroundColor("#8b95a5") | ||
| } | ||
| ForEach(datasets) { ds in | ||
| Button(action: { | ||
| cmux("workspace.select", workspace_id: ds.workspaceId) | ||
| cmux("surface.run", surface_id: ds.surfaceId, command: ds.mountCommand) | ||
| }) { | ||
| HStack(spacing: 8) { | ||
| Image(systemName: ds.mounted ? "checkmark.circle.fill" : "circle") | ||
| .foregroundColor(ds.mounted ? "#3ddc84" : "#8b95a5") | ||
| VStack(alignment: .leading, spacing: 1) { | ||
| Text(ds.name).font(.caption).bold() | ||
| Text("\(ds.sizeGB)G · \(ds.rows) rows").font(.caption).foregroundColor("#8b95a5") | ||
| } | ||
| Spacer() | ||
| Image(systemName: "arrow.right.circle").foregroundColor("#4aa8ff") | ||
| } | ||
| .padding(6) | ||
| } | ||
| } |
There was a problem hiding this comment.
The run and dataset cards depend on fields the corpus stub never seeds.
The shared stub only gives runs { id, step, loss, eta } and datasets { id, name }, but this file expects status, epoch, totalEpochs, lr, etaMin, totalSteps, surfaceId, workspaceId, mountCommand, mounted, sizeGB, and rows. Those cards therefore render with missing data/actions in CorpusCoverageTests instead of covering the intended interpreter 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/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/Corpus/data-scientist-running-ml-training-jobs-.swift`
around lines 88 - 142, The runs and datasets returned by the shared corpus stub
are missing fields used by the UI (in the ForEach rendering of runs and datasets
in this test file), so update the corpus stub used by CorpusCoverageTests to
seed runs with status, epoch, totalEpochs, lr, etaMin, totalSteps, surfaceId and
datasets with workspaceId, surfaceId, mountCommand, mounted, sizeGB, rows (in
addition to existing id/step/loss/eta/name) so the rendering paths in the run
card (Text checks, progress bar using totalSteps, cmux surface.focus using
surfaceId) and dataset buttons (workspace.select, surface.run, mounted state,
size/rows display) exercise the intended interpreter logic.
Comment on lines
+14
to
+65
| @Test func reportCorpusCoverage() { | ||
| let dir = URL(fileURLWithPath: #filePath).deletingLastPathComponent().appendingPathComponent("Corpus") | ||
| let files = ((try? FileManager.default.contentsOfDirectory(at: dir, includingPropertiesForKeys: nil)) ?? []) | ||
| .filter { $0.pathExtension == "swift" } | ||
| .sorted { $0.lastPathComponent < $1.lastPathComponent } | ||
|
|
||
| let interp = SwiftViewInterpreter() | ||
| let ctx = corpusStubContext() | ||
|
|
||
| let supportedCalls: Set<String> = [ | ||
| "Text", "VStack", "HStack", "ZStack", "HSplitView", "Button", "Image", | ||
| "Spacer", "Divider", "Rectangle", "RoundedRectangle", "Capsule", "Circle", | ||
| "ForEach", "Reorderable", "cmux", "log", "Color", "ScrollView", "openURL", | ||
| ] | ||
| let supportedMembers: Set<String> = [ | ||
| "font", "bold", "fontWeight", "foregroundColor", "foregroundStyle", "fill", | ||
| "tint", "padding", "background", "cornerRadius", "opacity", "lineLimit", | ||
| "frame", "onTapGesture", | ||
| "filter", "map", "flatMap", "reduce", "sorted", "first", "contains", "count", | ||
| "reversed", "prefix", "isEmpty", "hasPrefix", "hasSuffix", "uppercased", | ||
| "lowercased", "split", "formatted", "currency", "notation", "percent", "strikethrough", "system", | ||
| ] | ||
|
|
||
| var unsupportedCalls: [String: Int] = [:] | ||
| var unsupportedMembers: [String: Int] = [:] | ||
| var coverage: [String] = [] | ||
|
|
||
| for file in files { | ||
| let src = (try? String(contentsOf: file, encoding: .utf8)) ?? "" | ||
| let node = interp.evaluate(src, state: ctx) | ||
| let count = node.map(Self.nodeCount) ?? 0 | ||
| let collector = SymbolCollector(viewMode: .sourceAccurate) | ||
| collector.walk(Parser.parse(source: src)) | ||
| // User-defined funcs in the same file are supported via the | ||
| // interpreter's function table; don't flag them. | ||
| let userFuncs = collector.declaredFunctions | ||
| for (name, n) in collector.declCalls where !supportedCalls.contains(name) && !userFuncs.contains(name) { | ||
| unsupportedCalls[name, default: 0] += n | ||
| } | ||
| for (name, n) in collector.memberCalls where !supportedMembers.contains(name) { | ||
| unsupportedMembers[name, default: 0] += n | ||
| } | ||
| coverage.append(String(format: "%@ %4d nodes %@", node != nil ? "OK " : "NIL", count, file.lastPathComponent)) | ||
| } | ||
|
|
||
| print("\n===== CORPUS RENDER COVERAGE (\(files.count) sidebars) =====") | ||
| coverage.forEach { print($0) } | ||
| print("\n===== UNSUPPORTED CONSTRUCTORS / FUNCTIONS =====") | ||
| for (k, n) in unsupportedCalls.sorted(by: { ($0.value, $1.key) > ($1.value, $0.key) }) { print("\(n)x \(k)") } | ||
| print("\n===== UNSUPPORTED MODIFIERS / METHODS =====") | ||
| for (k, n) in unsupportedMembers.sorted(by: { ($0.value, $1.key) > ($1.value, $0.key) }) { print("\(n)x \(k)") } | ||
| print("===== END =====\n") |
There was a problem hiding this comment.
Make the coverage test fail on render regressions.
This test only prints diagnostics today. If a future change makes a corpus file return nil or collapse to a trivial tree, CI still passes, so the suite does not actually protect the interpreter behavior this PR is trying to lock in.
Suggested assertion layer
`@Test` func reportCorpusCoverage() {
+ var failedFiles: [String] = []
let dir = URL(fileURLWithPath: `#filePath`).deletingLastPathComponent().appendingPathComponent("Corpus")
let files = ((try? FileManager.default.contentsOfDirectory(at: dir, includingPropertiesForKeys: nil)) ?? [])
.filter { $0.pathExtension == "swift" }
.sorted { $0.lastPathComponent < $1.lastPathComponent }
@@
for file in files {
let src = (try? String(contentsOf: file, encoding: .utf8)) ?? ""
let node = interp.evaluate(src, state: ctx)
let count = node.map(Self.nodeCount) ?? 0
+ if node == nil || count == 0 {
+ failedFiles.append(file.lastPathComponent)
+ }
let collector = SymbolCollector(viewMode: .sourceAccurate)
collector.walk(Parser.parse(source: src))
@@
print("\n===== CORPUS RENDER COVERAGE (\(files.count) sidebars) =====")
coverage.forEach { print($0) }
@@
print("===== END =====\n")
+ `#expect`(failedFiles.isEmpty, "Corpus files failed to render: \(failedFiles.joined(separator: ", "))")
}Based on learnings: Applies to **/Test.swift : Do not add tests that only verify source code text, method signatures, AST fragments, or grep-style patterns. Tests must verify observable runtime behavior through executable paths (unit/integration/e2e/CLI), not implementation shape.
📝 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
| @Test func reportCorpusCoverage() { | |
| let dir = URL(fileURLWithPath: #filePath).deletingLastPathComponent().appendingPathComponent("Corpus") | |
| let files = ((try? FileManager.default.contentsOfDirectory(at: dir, includingPropertiesForKeys: nil)) ?? []) | |
| .filter { $0.pathExtension == "swift" } | |
| .sorted { $0.lastPathComponent < $1.lastPathComponent } | |
| let interp = SwiftViewInterpreter() | |
| let ctx = corpusStubContext() | |
| let supportedCalls: Set<String> = [ | |
| "Text", "VStack", "HStack", "ZStack", "HSplitView", "Button", "Image", | |
| "Spacer", "Divider", "Rectangle", "RoundedRectangle", "Capsule", "Circle", | |
| "ForEach", "Reorderable", "cmux", "log", "Color", "ScrollView", "openURL", | |
| ] | |
| let supportedMembers: Set<String> = [ | |
| "font", "bold", "fontWeight", "foregroundColor", "foregroundStyle", "fill", | |
| "tint", "padding", "background", "cornerRadius", "opacity", "lineLimit", | |
| "frame", "onTapGesture", | |
| "filter", "map", "flatMap", "reduce", "sorted", "first", "contains", "count", | |
| "reversed", "prefix", "isEmpty", "hasPrefix", "hasSuffix", "uppercased", | |
| "lowercased", "split", "formatted", "currency", "notation", "percent", "strikethrough", "system", | |
| ] | |
| var unsupportedCalls: [String: Int] = [:] | |
| var unsupportedMembers: [String: Int] = [:] | |
| var coverage: [String] = [] | |
| for file in files { | |
| let src = (try? String(contentsOf: file, encoding: .utf8)) ?? "" | |
| let node = interp.evaluate(src, state: ctx) | |
| let count = node.map(Self.nodeCount) ?? 0 | |
| let collector = SymbolCollector(viewMode: .sourceAccurate) | |
| collector.walk(Parser.parse(source: src)) | |
| // User-defined funcs in the same file are supported via the | |
| // interpreter's function table; don't flag them. | |
| let userFuncs = collector.declaredFunctions | |
| for (name, n) in collector.declCalls where !supportedCalls.contains(name) && !userFuncs.contains(name) { | |
| unsupportedCalls[name, default: 0] += n | |
| } | |
| for (name, n) in collector.memberCalls where !supportedMembers.contains(name) { | |
| unsupportedMembers[name, default: 0] += n | |
| } | |
| coverage.append(String(format: "%@ %4d nodes %@", node != nil ? "OK " : "NIL", count, file.lastPathComponent)) | |
| } | |
| print("\n===== CORPUS RENDER COVERAGE (\(files.count) sidebars) =====") | |
| coverage.forEach { print($0) } | |
| print("\n===== UNSUPPORTED CONSTRUCTORS / FUNCTIONS =====") | |
| for (k, n) in unsupportedCalls.sorted(by: { ($0.value, $1.key) > ($1.value, $0.key) }) { print("\(n)x \(k)") } | |
| print("\n===== UNSUPPORTED MODIFIERS / METHODS =====") | |
| for (k, n) in unsupportedMembers.sorted(by: { ($0.value, $1.key) > ($1.value, $0.key) }) { print("\(n)x \(k)") } | |
| print("===== END =====\n") | |
| `@Test` func reportCorpusCoverage() { | |
| var failedFiles: [String] = [] | |
| let dir = URL(fileURLWithPath: `#filePath`).deletingLastPathComponent().appendingPathComponent("Corpus") | |
| let files = ((try? FileManager.default.contentsOfDirectory(at: dir, includingPropertiesForKeys: nil)) ?? []) | |
| .filter { $0.pathExtension == "swift" } | |
| .sorted { $0.lastPathComponent < $1.lastPathComponent } | |
| let interp = SwiftViewInterpreter() | |
| let ctx = corpusStubContext() | |
| let supportedCalls: Set<String> = [ | |
| "Text", "VStack", "HStack", "ZStack", "HSplitView", "Button", "Image", | |
| "Spacer", "Divider", "Rectangle", "RoundedRectangle", "Capsule", "Circle", | |
| "ForEach", "Reorderable", "cmux", "log", "Color", "ScrollView", "openURL", | |
| ] | |
| let supportedMembers: Set<String> = [ | |
| "font", "bold", "fontWeight", "foregroundColor", "foregroundStyle", "fill", | |
| "tint", "padding", "background", "cornerRadius", "opacity", "lineLimit", | |
| "frame", "onTapGesture", | |
| "filter", "map", "flatMap", "reduce", "sorted", "first", "contains", "count", | |
| "reversed", "prefix", "isEmpty", "hasPrefix", "hasSuffix", "uppercased", | |
| "lowercased", "split", "formatted", "currency", "notation", "percent", "strikethrough", "system", | |
| ] | |
| var unsupportedCalls: [String: Int] = [:] | |
| var unsupportedMembers: [String: Int] = [:] | |
| var coverage: [String] = [] | |
| for file in files { | |
| let src = (try? String(contentsOf: file, encoding: .utf8)) ?? "" | |
| let node = interp.evaluate(src, state: ctx) | |
| let count = node.map(Self.nodeCount) ?? 0 | |
| if node == nil || count == 0 { | |
| failedFiles.append(file.lastPathComponent) | |
| } | |
| let collector = SymbolCollector(viewMode: .sourceAccurate) | |
| collector.walk(Parser.parse(source: src)) | |
| // User-defined funcs in the same file are supported via the | |
| // interpreter's function table; don't flag them. | |
| let userFuncs = collector.declaredFunctions | |
| for (name, n) in collector.declCalls where !supportedCalls.contains(name) && !userFuncs.contains(name) { | |
| unsupportedCalls[name, default: 0] += n | |
| } | |
| for (name, n) in collector.memberCalls where !supportedMembers.contains(name) { | |
| unsupportedMembers[name, default: 0] += n | |
| } | |
| coverage.append(String(format: "%@ %4d nodes %@", node != nil ? "OK " : "NIL", count, file.lastPathComponent)) | |
| } | |
| print("\n===== CORPUS RENDER COVERAGE (\(files.count) sidebars) =====") | |
| coverage.forEach { print($0) } | |
| print("\n===== UNSUPPORTED CONSTRUCTORS / FUNCTIONS =====") | |
| for (k, n) in unsupportedCalls.sorted(by: { ($0.value, $1.key) > ($1.value, $0.key) }) { print("\(n)x \(k)") } | |
| print("\n===== UNSUPPORTED MODIFIERS / METHODS =====") | |
| for (k, n) in unsupportedMembers.sorted(by: { ($0.value, $1.key) > ($1.value, $0.key) }) { print("\(n)x \(k)") } | |
| print("===== END =====\n") | |
| `#expect`(failedFiles.isEmpty, "Corpus files failed to render: \(failedFiles.joined(separator: ", "))") | |
| } |
🤖 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/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/CorpusCoverageTests.swift`
around lines 14 - 65, The test currently only prints diagnostics; change
reportCorpusCoverage() to assert failures when rendering regresses by (1)
asserting node is non-nil for each file (use XCTAssertNotNil(node, "...
\(file.lastPathComponent)")) and (2) asserting the parsed tree size is
meaningful (e.g., XCTAssertTrue(count > 1, "trivial tree for
\(file.lastPathComponent)")) using the existing nodeCount helper, and (3) fail
the test if unsupportedCalls or unsupportedMembers are non-empty (XCTFail or
XCTAssertTrue(unsupportedCalls.isEmpty, ...) and similarly for
unsupportedMembers) so CI fails on regressions; keep references to
SwiftViewInterpreter, nodeCount, collector.declaredFunctions, unsupportedCalls
and unsupportedMembers to locate where to add the assertions.
There was a problem hiding this comment.
10 issues found across 23 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift:172">
P2: `resolveFont` ignores `weight:` for `.system(size:weight:design:)`, so authored font weights are silently dropped.</violation>
<violation number="2" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift:179">
P1: Font-style lookup matches "headline" instead of "subheadline" due to substring collision. `token.contains("headline")` returns true for the string `"subheadline"`, so `.system(.subheadline)` is resolved to `.headline`.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:143">
P2: View helper functions with an explicit `return` are not interpreted, so `func ... -> some View { return ... }` can render empty output.</violation>
<violation number="2" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:154">
P2: `registerFunctions(items, env)` inside `evalItems` writes function declarations into whichever `Environment` is passed — but for VStack, HStack, ZStack, and HSplitView, `evalCall` passes the **parent** `env` directly (not a child scope). This means any `func` defined inside a stack's trailing closure body leaks to the outer lexical scope and silently overwrites any function with the same name declared outside the stack, creating a scoping bug.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift:14">
P2: Concurrency model gap widened: adding `private var functions` and mutating methods to a non-Sendable `final class` under Swift 6 strict mode. The class holds mutable state (`values`, `functions`) without `@MainActor`, actor isolation, or any concurrency primitive, violating the project's Swift 6 concurrency requirement.</violation>
<violation number="2" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift:35">
P1: Capture and reuse the function’s defining environment when registering user functions; binding from the caller scope here gives dynamic scoping and can resolve shadowed/free variables incorrectly.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:288">
P3: Dead code: unreachable `IfExprSyntax` check inside `ExprSyntax` branch of `evalBlockValue`. The preceding check (`node.as(ExpressionStmtSyntax.self)?.expression.as(IfExprSyntax.self) ?? node.as(IfExprSyntax.self)`) already catches all `IfExprSyntax` nodes and either returns or continues past this point.</violation>
<violation number="2" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:300">
P2: `evalIfValue` treats non-expression conditions (e.g., `if let x = optional`, `if #available(...)`) as always false. Optional-binding conditions silently skip the if-body, which will confound authors who use them in sidebar functions.</violation>
<violation number="3" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:322">
P2: This hardcodes USD formatting for every `.currency(code:)` call. Parse and honor the requested currency code (or use a formatter) so non-USD values render correctly.</violation>
<violation number="4" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:366">
P2: `evalClosure2` only handles single-expression closures. Multi-statement closures with `let` declarations, `return` statements, or `if` expressions silently return `nil`. This is inconsistent with `evalBlockValue` which supports full multi-statement function bodies, and could silently corrupt `reduce` results.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| ("headline", .headline), ("subheadline", .subheadline), ("body", .body), ("callout", .callout), | ||
| ("footnote", .footnote), ("caption2", .caption2), ("caption", .caption), | ||
| ] | ||
| for (name, style) in styles where token.contains(name) { |
There was a problem hiding this comment.
P1: Font-style lookup matches "headline" instead of "subheadline" due to substring collision. token.contains("headline") returns true for the string "subheadline", so .system(.subheadline) is resolved to .headline.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift, line 179:
<comment>Font-style lookup matches "headline" instead of "subheadline" due to substring collision. `token.contains("headline")` returns true for the string `"subheadline"`, so `.system(.subheadline)` is resolved to `.headline`.</comment>
<file context>
@@ -158,6 +160,28 @@ struct RenderNodeView: View {
+ ("headline", .headline), ("subheadline", .subheadline), ("body", .body), ("callout", .callout),
+ ("footnote", .footnote), ("caption2", .caption2), ("caption", .caption),
+ ]
+ for (name, style) in styles where token.contains(name) {
+ return .system(style, design: design)
+ }
</file context>
|
|
||
| /// Registers a user-defined function in this scope. | ||
| func defineFunction(_ name: String, _ decl: FunctionDeclSyntax) { | ||
| functions[name] = decl |
There was a problem hiding this comment.
P1: Capture and reuse the function’s defining environment when registering user functions; binding from the caller scope here gives dynamic scoping and can resolve shadowed/free variables incorrectly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift, line 35:
<comment>Capture and reuse the function’s defining environment when registering user functions; binding from the caller scope here gives dynamic scoping and can resolve shadowed/free variables incorrectly.</comment>
<file context>
@@ -24,6 +30,16 @@ final class Environment {
+ /// Registers a user-defined function in this scope.
+ func defineFunction(_ name: String, _ decl: FunctionDeclSyntax) {
+ functions[name] = decl
+ }
+
</file context>
| // MARK: - ViewBuilder statements | ||
|
|
||
| private func evalItems(_ items: CodeBlockItemListSyntax, _ env: Environment) -> [RenderNode] { | ||
| registerFunctions(items, env) |
There was a problem hiding this comment.
P2: registerFunctions(items, env) inside evalItems writes function declarations into whichever Environment is passed — but for VStack, HStack, ZStack, and HSplitView, evalCall passes the parent env directly (not a child scope). This means any func defined inside a stack's trailing closure body leaks to the outer lexical scope and silently overwrites any function with the same name declared outside the stack, creating a scoping bug.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift, line 154:
<comment>`registerFunctions(items, env)` inside `evalItems` writes function declarations into whichever `Environment` is passed — but for VStack, HStack, ZStack, and HSplitView, `evalCall` passes the **parent** `env` directly (not a child scope). This means any `func` defined inside a stack's trailing closure body leaks to the outer lexical scope and silently overwrites any function with the same name declared outside the stack, creating a scoping bug.</comment>
<file context>
@@ -114,16 +129,29 @@ public struct SwiftViewInterpreter: Sendable {
// MARK: - ViewBuilder statements
private func evalItems(_ items: CodeBlockItemListSyntax, _ env: Environment) -> [RenderNode] {
+ registerFunctions(items, env)
var out: [RenderNode] = []
for item in items {
</file context>
| /// called from anywhere in the same or a nested scope. | ||
| final class Environment { | ||
| private var values: [String: SwiftValue] | ||
| private var functions: [String: FunctionDeclSyntax] |
There was a problem hiding this comment.
P2: Concurrency model gap widened: adding private var functions and mutating methods to a non-Sendable final class under Swift 6 strict mode. The class holds mutable state (values, functions) without @MainActor, actor isolation, or any concurrency primitive, violating the project's Swift 6 concurrency requirement.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift, line 14:
<comment>Concurrency model gap widened: adding `private var functions` and mutating methods to a non-Sendable `final class` under Swift 6 strict mode. The class holds mutable state (`values`, `functions`) without `@MainActor`, actor isolation, or any concurrency primitive, violating the project's Swift 6 concurrency requirement.</comment>
<file context>
@@ -1,16 +1,22 @@
+/// called from anywhere in the same or a nested scope.
final class Environment {
private var values: [String: SwiftValue]
+ private var functions: [String: FunctionDeclSyntax]
private let parent: Environment?
</file context>
| if names.count > 1 { scope.define(names[1].name.text, b) } | ||
| } | ||
| for item in closure.statements { | ||
| if let expr = item.item.as(ExprSyntax.self) { return eval(expr, scope) } |
There was a problem hiding this comment.
P2: evalClosure2 only handles single-expression closures. Multi-statement closures with let declarations, return statements, or if expressions silently return nil. This is inconsistent with evalBlockValue which supports full multi-statement function bodies, and could silently corrupt reduce results.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift, line 366:
<comment>`evalClosure2` only handles single-expression closures. Multi-statement closures with `let` declarations, `return` statements, or `if` expressions silently return `nil`. This is inconsistent with `evalBlockValue` which supports full multi-statement function bodies, and could silently corrupt `reduce` results.</comment>
<file context>
@@ -209,6 +241,133 @@ struct ExpressionEvaluator {
+ if names.count > 1 { scope.define(names[1].name.text, b) }
+ }
+ for item in closure.statements {
+ if let expr = item.item.as(ExprSyntax.self) { return eval(expr, scope) }
+ }
+ return nil
</file context>
|
|
||
| private func evalIfValue(_ ifExpr: IfExprSyntax, _ scope: Environment) -> SwiftValue? { | ||
| let taken = ifExpr.conditions.allSatisfy { element in | ||
| guard let expr = element.condition.as(ExprSyntax.self) else { return false } |
There was a problem hiding this comment.
P2: evalIfValue treats non-expression conditions (e.g., if let x = optional, if #available(...)) as always false. Optional-binding conditions silently skip the if-body, which will confound authors who use them in sidebar functions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift, line 300:
<comment>`evalIfValue` treats non-expression conditions (e.g., `if let x = optional`, `if #available(...)`) as always false. Optional-binding conditions silently skip the if-body, which will confound authors who use them in sidebar functions.</comment>
<file context>
@@ -209,6 +241,133 @@ struct ExpressionEvaluator {
+
+ private func evalIfValue(_ ifExpr: IfExprSyntax, _ scope: Environment) -> SwiftValue? {
+ let taken = ifExpr.conditions.allSatisfy { element in
+ guard let expr = element.condition.as(ExprSyntax.self) else { return false }
+ return eval(expr, scope)?.isTruthy ?? false
+ }
</file context>
| if let range = token.range(of: "size:") { | ||
| let digits = token[range.upperBound...].drop(while: { $0 == " " }) | ||
| .prefix(while: { $0.isNumber || $0 == "." }) | ||
| if let n = Double(digits) { return .system(size: CGFloat(n), design: design) } |
There was a problem hiding this comment.
P2: resolveFont ignores weight: for .system(size:weight:design:), so authored font weights are silently dropped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift, line 172:
<comment>`resolveFont` ignores `weight:` for `.system(size:weight:design:)`, so authored font weights are silently dropped.</comment>
<file context>
@@ -158,6 +160,28 @@ struct RenderNodeView: View {
+ if let range = token.range(of: "size:") {
+ let digits = token[range.upperBound...].drop(while: { $0 == " " })
+ .prefix(while: { $0.isNumber || $0 == "." })
+ if let n = Double(digits) { return .system(size: CGFloat(n), design: design) }
+ }
+ let styles: [(String, Font.TextStyle)] = [
</file context>
Suggested change
| if let n = Double(digits) { return .system(size: CGFloat(n), design: design) } | |
| if let n = Double(digits) { | |
| let weight: Font.Weight = { | |
| if token.contains("ultraLight") { return .ultraLight } | |
| if token.contains("thin") { return .thin } | |
| if token.contains("light") { return .light } | |
| if token.contains("regular") { return .regular } | |
| if token.contains("medium") { return .medium } | |
| if token.contains("semibold") { return .semibold } | |
| if token.contains("bold") { return .bold } | |
| if token.contains("heavy") { return .heavy } | |
| if token.contains("black") { return .black } | |
| return .regular | |
| }() | |
| return .system(size: CGFloat(n), weight: weight, design: design) | |
| } |
| continue | ||
| } | ||
| if let expr = node.as(ExprSyntax.self) { | ||
| if let ifExpr = expr.as(IfExprSyntax.self) { |
There was a problem hiding this comment.
P3: Dead code: unreachable IfExprSyntax check inside ExprSyntax branch of evalBlockValue. The preceding check (node.as(ExpressionStmtSyntax.self)?.expression.as(IfExprSyntax.self) ?? node.as(IfExprSyntax.self)) already catches all IfExprSyntax nodes and either returns or continues past this point.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift, line 288:
<comment>Dead code: unreachable `IfExprSyntax` check inside `ExprSyntax` branch of `evalBlockValue`. The preceding check (`node.as(ExpressionStmtSyntax.self)?.expression.as(IfExprSyntax.self) ?? node.as(IfExprSyntax.self)`) already catches all `IfExprSyntax` nodes and either returns or continues past this point.</comment>
<file context>
@@ -209,6 +241,133 @@ struct ExpressionEvaluator {
+ continue
+ }
+ if let expr = node.as(ExprSyntax.self) {
+ if let ifExpr = expr.as(IfExprSyntax.self) {
+ if let value = evalIfValue(ifExpr, scope) { return value }
+ continue
</file context>
… mask The interpreted sidebar content (both the non-split scroll wrapper and each ResizableHSplit column) started flush at y=0, so headers like "Agent Ops" / "Task Queue" underlapped the window's titlebar accessory strip, and content clipped sharply at the top edge instead of dissolving into the sidebar's fade mask the way the default workspace sidebar does. Add a host-injected CustomSidebarContentInsets (top/bottom), carried through the SwiftUI environment so the nested ResizableHSplit (built inside the generic RenderNodeView) picks it up without threading params through the IR renderer. Both scroll regions now apply matching top/bottom safeAreaInsets, mirroring the default sidebar: ContentView passes SidebarWorkspaceScrollInsets.workspaceList top/bottom, and the existing top fade mask makes scrolled content dissolve under the accessory bar instead of clipping. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment on lines
+57
to
+63
| public var iterationValues: [SwiftValue]? { | ||
| switch self { | ||
| case let .range(lower, upper, inclusive): | ||
| let end = inclusive ? upper + 1 : upper | ||
| guard end >= lower else { return [] } | ||
| return (lower..<end).map(SwiftValue.int) | ||
| case let .array(values): |
Contributor
There was a problem hiding this comment.
iterationValues eagerly materializes every integer in a range as a SwiftValue array before the caller can iterate. The previous thread flagged the Int.max + 1 trap for inclusive ranges; there is a second failure mode for exclusive ranges: any for i in 0..<N { } with a large-but-sub-overflow N (e.g. 0..<100_000) allocates 100 K heap objects per evaluation, and because the interpreter runs inside the 1-second TimelineView body on the main actor, a single authoring mistake stalls UI on every tick until the file is edited and saved. A safety cap (e.g. min(end - lower, 50_000) with a failed node emitted when the cap fires) prevents both the main-thread freeze and gives the author feedback without crashing the host.
Replace the 4-key custom-sidebar data context with a full read-surface
projection of cmux runtime state so interpreted sidebars can be fully
data-driven. Each workspace now exposes: pinned, index, directory, ports +
portCount, unread, tabs + tabCount, and (when present) description, color hex,
git branch + dirty, pull request {number,label,url,status,stale,branch},
progress {value,label}, latest agent message / submitted prompt / timestamp,
and remote {target,state,connected}. Each surface exposes focused, pinned, and
when available directory, branch + dirty, ports. Top level adds selectedId and
unreadTotal; clock adds weekday. Optional fields are omitted when absent so
interpreted `if let` / ternary truthiness works.
Also lands the design artifacts from the multi-agent surface study:
docs/swiftui-interpreter-surface.md (14-domain capability matrix + roadmap)
and docs/data-driven-sidebar-plan.md (data/command/hook architecture + waves),
and documents the new data keys in docs/custom-sidebars.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…gap report The app-target file now contains only makeCmuxSidebarActionDispatch (the cmux-coupled action sink); its old "playground" name no longer fits. Rename the file and rewire its four project.pbxproj references (file IDs unchanged), normalized + check-pbxproj clean. Remove docs/custom-sidebar-gap-report.md, superseded by docs/swiftui-interpreter-surface.md (full capability matrix) and docs/data-driven-sidebar-plan.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment on lines
+174
to
+179
| let styles: [(String, Font.TextStyle)] = [ | ||
| ("largeTitle", .largeTitle), ("title3", .title3), ("title2", .title2), ("title", .title), | ||
| ("headline", .headline), ("subheadline", .subheadline), ("body", .body), ("callout", .callout), | ||
| ("footnote", .footnote), ("caption2", .caption2), ("caption", .caption), | ||
| ] | ||
| for (name, style) in styles where token.contains(name) { |
Contributor
There was a problem hiding this comment.
.subheadline silently resolves to .headline. The loop uses token.contains(name) for matching, and "subheadline" contains "headline" as a substring — so the earlier ("headline", .headline) entry fires first for any .system(.subheadline) modifier argument. Moving "subheadline" before "headline" in the list ensures the more-specific name is checked first.
Suggested change
| let styles: [(String, Font.TextStyle)] = [ | |
| ("largeTitle", .largeTitle), ("title3", .title3), ("title2", .title2), ("title", .title), | |
| ("headline", .headline), ("subheadline", .subheadline), ("body", .body), ("callout", .callout), | |
| ("footnote", .footnote), ("caption2", .caption2), ("caption", .caption), | |
| ] | |
| for (name, style) in styles where token.contains(name) { | |
| let styles: [(String, Font.TextStyle)] = [ | |
| ("largeTitle", .largeTitle), ("title3", .title3), ("title2", .title2), ("title", .title), | |
| ("subheadline", .subheadline), ("headline", .headline), ("body", .body), ("callout", .callout), | |
| ("footnote", .footnote), ("caption2", .caption2), ("caption", .caption), | |
| ] | |
| for (name, style) in styles where token.contains(name) { |
Comment on lines
+130
to
+132
| case "+": return bothInt ? .int(Int(l + r)) : .double(l + r) | ||
| case "-": return bothInt ? .int(Int(l - r)) : .double(l - r) | ||
| case "*": return bothInt ? .int(Int(l * r)) : .double(l * r) |
Contributor
There was a problem hiding this comment.
+, -, and * is computed in Double (via numericPair) and then narrowed back with Int(...). Int(someDouble) is an unconditional trap when the value falls outside Int range — for example Int.max + 1 as two Int literals in user sidebar code will crash the host process. The already-flagged division-by-zero guard should accompany an Int.init(exactly:) check here so overflow silently returns nil instead of trapping.
Suggested change
| case "+": return bothInt ? .int(Int(l + r)) : .double(l + r) | |
| case "-": return bothInt ? .int(Int(l - r)) : .double(l - r) | |
| case "*": return bothInt ? .int(Int(l * r)) : .double(l * r) | |
| case "+": return bothInt ? Int(exactly: l + r).map(SwiftValue.int) ?? .double(l + r) : .double(l + r) | |
| case "-": return bothInt ? Int(exactly: l - r).map(SwiftValue.int) ?? .double(l - r) : .double(l - r) | |
| case "*": return bothInt ? Int(exactly: l * r).map(SwiftValue.int) ?? .double(l * r) : .double(l * r) |
There was a problem hiding this comment.
4 issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift:172">
P2: `resolveFont` ignores `weight:` for `.system(size:weight:design:)`, so authored font weights are silently dropped.</violation>
<violation number="2" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift:179">
P1: Font-style lookup matches "headline" instead of "subheadline" due to substring collision. `token.contains("headline")` returns true for the string `"subheadline"`, so `.system(.subheadline)` is resolved to `.headline`.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:75">
P1: `Button("title", action: { … })` drops the title string because the "label form" branch (triggered by finding an `action:` labeled closure argument) returns a node with no text and an empty children array, ignoring the unlabeled first argument.</violation>
<violation number="2" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:154">
P2: `registerFunctions(items, env)` inside `evalItems` writes function declarations into whichever `Environment` is passed — but for VStack, HStack, ZStack, and HSplitView, `evalCall` passes the **parent** `env` directly (not a child scope). This means any `func` defined inside a stack's trailing closure body leaks to the outer lexical scope and silently overwrites any function with the same name declared outside the stack, creating a scoping bug.</violation>
<violation number="3" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:178">
P3: `closureParameterName(_:)` is duplicated verbatim in both `SwiftViewInterpreter` and `ExpressionEvaluator`, each as a private method doing the same closure-signature parameter-name extraction.</violation>
<violation number="4" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:178">
P1: Empty-string sentinel for missing item IDs in Reorderable: `item.member(idField)?.displayString ?? ""` inserts `""` into `itemIds` when the id field is absent, causing invalid reorder commands (e.g., `workspace_id: ""`) instead of a clear failure or graceful skip.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift:14">
P2: Concurrency model gap widened: adding `private var functions` and mutating methods to a non-Sendable `final class` under Swift 6 strict mode. The class holds mutable state (`values`, `functions`) without `@MainActor`, actor isolation, or any concurrency primitive, violating the project's Swift 6 concurrency requirement.</violation>
<violation number="2" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift:35">
P1: Capture and reuse the function’s defining environment when registering user functions; binding from the caller scope here gives dynamic scoping and can resolve shadowed/free variables incorrectly.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNodeKind.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNodeKind.swift:4">
P1: Missing fallback for unknown raw values during JSON decoding — user-authored sidebar files with an unrecognized node kind (e.g., a typo like `"vstak"` or a future plugin authoring a new kind before this code adds it) will throw a hard `DecodingError.dataCorrupted` and fail to load the entire sidebar. Add a custom `Decodable` that maps unknown strings to a fallback `.unknown` case.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift:66">
P2: Drag gesture writes to `@AppStorage` on every `.onChanged` event (~60 writes/sec), causing unnecessary `UserDefaults` synchronization and potential UI jank. Use a local `@State` for the transient drag value and persist only on `.onEnded`.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift:51">
P2: Uses `FileManager.default` directly instead of an injected dependency, making the model untestable without real file I/O. The repository guidance (`CLAUDE.md`) requires package APIs to be testable without launching the app or relying on `FileManager.default`. The `reload()` method also reads files via `Data(contentsOf:)` and `String(contentsOf:encoding:)` which bypass any controllable FileManager.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swift:51">
P2: JSON sidebar action handler silently discards all button actions. The `onAction:` closure is `{ _ in }`, so any interactive element (button, tap) in a `.json` sidebar produces zero side effects — the buttons render but do nothing when tapped. Meanwhile, interpreted Swift sidebars correctly dispatch actions through the environment's `sidebarActionDispatch`.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLSidebarRenderer.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLSidebarRenderer.swift:33">
P2: Button node does not apply `resolvedFont`, unlike `.text` and `.image` cases. When a JSON DSL button specifies `font` or `size`, those properties are silently ignored, while the same properties work correctly on text and image nodes. This means the button's title text will always appear at the system default font regardless of the authored style.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/OptionalStyleModifiers.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/OptionalStyleModifiers.swift:4">
P3: Three ViewModifier types in one file violates the repo's one-major-type-per-file convention. Split `OptionalForeground`, `OptionalPadding`, and `OptionalBackground` into separate files for consistency with the codebase convention.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNode.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNode.swift:9">
P2: Auto-synthesized `Equatable` includes `id` (`UUID()`) in the comparison, making equality checks always return `false` for distinct instances even when semantically identical. The doc comment explicitly says `id` is "Not decoded; a stable identity per decoded node" — it is a runtime identity marker, not part of the value. Including it in `Equatable` means `==` can never be true for two separately-initialized or decoded nodes with identical content, breaking any code that relies on value equality (SwiftUI `EquatableView`, test assertions, collection diffing).</violation>
</file>
<file name="cmux.xcodeproj/project.pbxproj">
<violation number="1" location="cmux.xcodeproj/project.pbxproj:1932">
P2: CmuxSwiftRenderUI (a SwiftUI+AppKit UI-rendering package) is added as a dependency of the cmux-cli command-line tool target. Every source file in this package imports SwiftUI, and ResizableHSplit.swift imports AppKit. The cmux-cli target (product-type tool) has no UI runtime and cannot meaningfully use SwiftUI View types, which will unnecessarily link SwiftUI/AppKit frameworks into the CLI binary and create a maintenance burden.</violation>
</file>
<file name="Sources/DSLSidebarPlayground.swift">
<violation number="1" location="Sources/DSLSidebarPlayground.swift:31">
P2: Unconditionally coercing all numeric-looking string params to Int can break commands that require string-typed values.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift:18">
P1: ForEach uses index-based identity (`id: \.offset`) in a reorderable list, even though stable per-item identifiers are available via `spec.itemIds`. When a row is dragged to a new position and `rows` updates, SwiftUI identifies each row by its index (0, 1, 2...) rather than by its stable id. This prevents SwiftUI from tracking which view corresponds to which item across the reorder, losing view state, breaking transition animations, and making the reorder feel unresponsive or glitchy.
Use stable IDs from `spec.itemIds` as the ForEach identity, paired with each row, so SwiftUI correctly preserves view identity when items move.</violation>
<violation number="2" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift:37">
P2: Validate `ReorderSpec` command/key strings (and `draggedId`) before dispatching `.cmux`; currently malformed or empty dynamic keys can produce invalid reorder commands.
(Based on your team's feedback about avoiding sentinel/missing-key patterns in cmux param construction.) [FEEDBACK_USED].</violation>
</file>
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:31283">
P2: Inconsistent docs topic list: main CLI help advertises `sidebars` but the `docs` command's own usage string and error message still show the old topic list without it. Users running `cmux docs --help` or triggering the extra-args error path will see `docs [settings|shortcuts|api|browser|agents|dock]` — no mention of sidebars.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:288">
P3: Dead code: unreachable `IfExprSyntax` check inside `ExprSyntax` branch of `evalBlockValue`. The preceding check (`node.as(ExpressionStmtSyntax.self)?.expression.as(IfExprSyntax.self) ?? node.as(IfExprSyntax.self)`) already catches all `IfExprSyntax` nodes and either returns or continues past this point.</violation>
<violation number="2" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:300">
P2: `evalIfValue` treats non-expression conditions (e.g., `if let x = optional`, `if #available(...)`) as always false. Optional-binding conditions silently skip the if-body, which will confound authors who use them in sidebar functions.</violation>
<violation number="3" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:366">
P2: `evalClosure2` only handles single-expression closures. Multi-statement closures with `let` declarations, `return` statements, or `if` expressions silently return `nil`. This is inconsistent with `evalBlockValue` which supports full multi-statement function bodies, and could silently corrupt `reduce` results.</violation>
</file>
<file name="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:10498">
P2: `selectedId` uses an empty-string sentinel for “no selection” instead of omission, which breaks the key-presence pattern this sidebar data model relies on.
(Based on your team's feedback about checking key presence instead of string sentinels.) [FEEDBACK_USED].</violation>
<violation number="2" location="Sources/ContentView.swift:10521">
P2: tabCount counts all bonsplit tabs unconditionally, but the `tabs` array filters out entries where `panelIdFromSurfaceId(tab.id)` returns nil. When a surface has no panel mapping (possible during lifecycle transitions), `tabCount` will be greater than `tabs.count`, exposing a misleading raw count that doesn't match the actual array. Sidebar authors iterating over `tabs` by index or comparing counts will encounter off-by-one or out-of-bounds bugs.</violation>
<violation number="3" location="Sources/ContentView.swift:10521">
P2: The new `tabCount` line rescans all panes/tabs even though `tabs` was just built from the same traversal, adding avoidable repeated work in a ~1s refresh path.</violation>
</file>
<file name="docs/data-driven-sidebar-plan.md">
<violation number="1" location="docs/data-driven-sidebar-plan.md:56">
P2: Agent-hook coverage list is inaccurate: it omits `amp` and uses `hermes` instead of the wired `hermes-agent` identifier, which can mislead hook setup and review scope.
(Based on your team's feedback about including Hermes/Amp hook wiring in scope.) [FEEDBACK_USED].</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| "portCount": .int(workspace.listeningPorts.count), | ||
| "unread": .int(notificationStore.unreadCount(forTabId: workspace.id)), | ||
| "tabs": .array(customSidebarSurfaceValues(workspace, focusedPanelId: focusedPanelId)), | ||
| "tabCount": .int(workspace.bonsplitController.allPaneIds.reduce(0) { $0 + workspace.bonsplitController.tabs(inPane: $1).count }), |
There was a problem hiding this comment.
P2: tabCount counts all bonsplit tabs unconditionally, but the tabs array filters out entries where panelIdFromSurfaceId(tab.id) returns nil. When a surface has no panel mapping (possible during lifecycle transitions), tabCount will be greater than tabs.count, exposing a misleading raw count that doesn't match the actual array. Sidebar authors iterating over tabs by index or comparing counts will encounter off-by-one or out-of-bounds bugs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/ContentView.swift, line 10521:
<comment>tabCount counts all bonsplit tabs unconditionally, but the `tabs` array filters out entries where `panelIdFromSurfaceId(tab.id)` returns nil. When a surface has no panel mapping (possible during lifecycle transitions), `tabCount` will be greater than `tabs.count`, exposing a misleading raw count that doesn't match the actual array. Sidebar authors iterating over `tabs` by index or comparing counts will encounter off-by-one or out-of-bounds bugs.</comment>
<file context>
@@ -10506,15 +10488,102 @@ struct VerticalTabsSidebar: View {
+ "portCount": .int(workspace.listeningPorts.count),
+ "unread": .int(notificationStore.unreadCount(forTabId: workspace.id)),
+ "tabs": .array(customSidebarSurfaceValues(workspace, focusedPanelId: focusedPanelId)),
+ "tabCount": .int(workspace.bonsplitController.allPaneIds.reduce(0) { $0 + workspace.bonsplitController.tabs(inPane: $1).count }),
+ ]
+ if let description = workspace.customDescription, !description.isEmpty { fields["description"] = .string(description) }
</file context>
Suggested change
| "tabCount": .int(workspace.bonsplitController.allPaneIds.reduce(0) { $0 + workspace.bonsplitController.tabs(inPane: $1).count }), | |
| "tabCount": .int(customSidebarSurfaceValues(workspace, focusedPanelId: focusedPanelId).count), |
| "portCount": .int(workspace.listeningPorts.count), | ||
| "unread": .int(notificationStore.unreadCount(forTabId: workspace.id)), | ||
| "tabs": .array(customSidebarSurfaceValues(workspace, focusedPanelId: focusedPanelId)), | ||
| "tabCount": .int(workspace.bonsplitController.allPaneIds.reduce(0) { $0 + workspace.bonsplitController.tabs(inPane: $1).count }), |
There was a problem hiding this comment.
P2: The new tabCount line rescans all panes/tabs even though tabs was just built from the same traversal, adding avoidable repeated work in a ~1s refresh path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/ContentView.swift, line 10521:
<comment>The new `tabCount` line rescans all panes/tabs even though `tabs` was just built from the same traversal, adding avoidable repeated work in a ~1s refresh path.</comment>
<file context>
@@ -10506,15 +10488,102 @@ struct VerticalTabsSidebar: View {
+ "portCount": .int(workspace.listeningPorts.count),
+ "unread": .int(notificationStore.unreadCount(forTabId: workspace.id)),
+ "tabs": .array(customSidebarSurfaceValues(workspace, focusedPanelId: focusedPanelId)),
+ "tabCount": .int(workspace.bonsplitController.allPaneIds.reduce(0) { $0 + workspace.bonsplitController.tabs(inPane: $1).count }),
+ ]
+ if let description = workspace.customDescription, !description.isEmpty { fields["description"] = .string(description) }
</file context>
| "workspaces": .array(workspaces), | ||
| "workspaceCount": .int(tabManager.tabs.count), | ||
| "selectedTitle": .string(selectedWorkspace?.customTitle ?? selectedWorkspace?.title ?? ""), | ||
| "selectedId": .string(selectedId?.uuidString ?? ""), |
There was a problem hiding this comment.
P2: selectedId uses an empty-string sentinel for “no selection” instead of omission, which breaks the key-presence pattern this sidebar data model relies on.
(Based on your team's feedback about checking key presence instead of string sentinels.) .
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/ContentView.swift, line 10498:
<comment>`selectedId` uses an empty-string sentinel for “no selection” instead of omission, which breaks the key-presence pattern this sidebar data model relies on.
(Based on your team's feedback about checking key presence instead of string sentinels.) .</comment>
<file context>
@@ -10506,15 +10488,102 @@ struct VerticalTabsSidebar: View {
"workspaces": .array(workspaces),
"workspaceCount": .int(tabManager.tabs.count),
"selectedTitle": .string(selectedWorkspace?.customTitle ?? selectedWorkspace?.title ?? ""),
+ "selectedId": .string(selectedId?.uuidString ?? ""),
+ "unreadTotal": .int(notificationStore.unreadCount),
"clock": clock,
</file context>
| - `window.*` — lifecycle | ||
| - `workstream.*` — start / progress / complete | ||
|
|
||
| Subscribe via `CmuxEventBus.subscribe(afterSequence:names:categories:)`; the socket `events` command streams them. Agent lifecycle hooks (`CLI/CMUXCLI+AgentHookDefinitions.swift`) exist for codex/grok/cursor/gemini/kiro/antigravity/hermes — session-start/prompt-submit/stop/notification/session-end/shell-exec — recorded to `~/.cmuxterm/{agent}-hook-sessions.json`. |
There was a problem hiding this comment.
P2: Agent-hook coverage list is inaccurate: it omits amp and uses hermes instead of the wired hermes-agent identifier, which can mislead hook setup and review scope.
(Based on your team's feedback about including Hermes/Amp hook wiring in scope.) .
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/data-driven-sidebar-plan.md, line 56:
<comment>Agent-hook coverage list is inaccurate: it omits `amp` and uses `hermes` instead of the wired `hermes-agent` identifier, which can mislead hook setup and review scope.
(Based on your team's feedback about including Hermes/Amp hook wiring in scope.) .</comment>
<file context>
@@ -0,0 +1,84 @@
+- `window.*` — lifecycle
+- `workstream.*` — start / progress / complete
+
+Subscribe via `CmuxEventBus.subscribe(afterSequence:names:categories:)`; the socket `events` command streams them. Agent lifecycle hooks (`CLI/CMUXCLI+AgentHookDefinitions.swift`) exist for codex/grok/cursor/gemini/kiro/antigravity/hermes — session-start/prompt-submit/stop/notification/session-end/shell-exec — recorded to `~/.cmuxterm/{agent}-hook-sessions.json`.
+
+## Architecture
</file context>
There was a problem hiding this comment.
2 issues found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:246">
P1: Comparators with an explicit `return` are evaluated as `nil`/`false` here, so `sorted { a, b in return ... }` can yield incorrect ordering. Handle `ReturnStmtSyntax` in closure evaluation before consuming the comparator result.</violation>
<violation number="2" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:288">
P3: Dead code: unreachable `IfExprSyntax` check inside `ExprSyntax` branch of `evalBlockValue`. The preceding check (`node.as(ExpressionStmtSyntax.self)?.expression.as(IfExprSyntax.self) ?? node.as(IfExprSyntax.self)`) already catches all `IfExprSyntax` nodes and either returns or continues past this point.</violation>
<violation number="3" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:300">
P2: `evalIfValue` treats non-expression conditions (e.g., `if let x = optional`, `if #available(...)`) as always false. Optional-binding conditions silently skip the if-body, which will confound authors who use them in sidebar functions.</violation>
<violation number="4" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift:366">
P2: `evalClosure2` only handles single-expression closures. Multi-statement closures with `let` declarations, `return` statements, or `if` expressions silently return `nil`. This is inconsistent with `evalBlockValue` which supports full multi-statement function bodies, and could silently corrupt `reduce` results.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift:172">
P2: `resolveFont` ignores `weight:` for `.system(size:weight:design:)`, so authored font weights are silently dropped.</violation>
<violation number="2" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/RenderNodeView.swift:179">
P1: Font-style lookup matches "headline" instead of "subheadline" due to substring collision. `token.contains("headline")` returns true for the string `"subheadline"`, so `.system(.subheadline)` is resolved to `.headline`.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:75">
P1: `Button("title", action: { … })` drops the title string because the "label form" branch (triggered by finding an `action:` labeled closure argument) returns a node with no text and an empty children array, ignoring the unlabeled first argument.</violation>
<violation number="2" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:154">
P2: `registerFunctions(items, env)` inside `evalItems` writes function declarations into whichever `Environment` is passed — but for VStack, HStack, ZStack, and HSplitView, `evalCall` passes the **parent** `env` directly (not a child scope). This means any `func` defined inside a stack's trailing closure body leaks to the outer lexical scope and silently overwrites any function with the same name declared outside the stack, creating a scoping bug.</violation>
<violation number="3" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:164">
P2: `return` in view-helper bodies does not terminate statement evaluation, so subsequent statements can still render and produce incorrect duplicate output.</violation>
<violation number="4" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:178">
P3: `closureParameterName(_:)` is duplicated verbatim in both `SwiftViewInterpreter` and `ExpressionEvaluator`, each as a private method doing the same closure-signature parameter-name extraction.</violation>
<violation number="5" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift:178">
P1: Empty-string sentinel for missing item IDs in Reorderable: `item.member(idField)?.displayString ?? ""` inserts `""` into `itemIds` when the id field is absent, causing invalid reorder commands (e.g., `workspace_id: ""`) instead of a clear failure or graceful skip.</violation>
</file>
<file name="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift">
<violation number="1" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift:14">
P2: Concurrency model gap widened: adding `private var functions` and mutating methods to a non-Sendable `final class` under Swift 6 strict mode. The class holds mutable state (`values`, `functions`) without `@MainActor`, actor isolation, or any concurrency primitive, violating the project's Swift 6 concurrency requirement.</violation>
<violation number="2" location="Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/Environment.swift:35">
P1: Capture and reuse the function’s defining environment when registering user functions; binding from the caller scope here gives dynamic scoping and can resolve shadowed/free variables incorrectly.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNodeKind.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNodeKind.swift:4">
P1: Missing fallback for unknown raw values during JSON decoding — user-authored sidebar files with an unrecognized node kind (e.g., a typo like `"vstak"` or a future plugin authoring a new kind before this code adds it) will throw a hard `DecodingError.dataCorrupted` and fail to load the entire sidebar. Add a custom `Decodable` that maps unknown strings to a fallback `.unknown` case.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift:66">
P2: Drag gesture writes to `@AppStorage` on every `.onChanged` event (~60 writes/sec), causing unnecessary `UserDefaults` synchronization and potential UI jank. Use a local `@State` for the transient drag value and persist only on `.onEnded`.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift:51">
P2: Uses `FileManager.default` directly instead of an injected dependency, making the model untestable without real file I/O. The repository guidance (`CLAUDE.md`) requires package APIs to be testable without launching the app or relying on `FileManager.default`. The `reload()` method also reads files via `Data(contentsOf:)` and `String(contentsOf:encoding:)` which bypass any controllable FileManager.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLSidebarRenderer.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLSidebarRenderer.swift:33">
P2: Button node does not apply `resolvedFont`, unlike `.text` and `.image` cases. When a JSON DSL button specifies `font` or `size`, those properties are silently ignored, while the same properties work correctly on text and image nodes. This means the button's title text will always appear at the system default font regardless of the authored style.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/OptionalStyleModifiers.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/OptionalStyleModifiers.swift:4">
P3: Three ViewModifier types in one file violates the repo's one-major-type-per-file convention. Split `OptionalForeground`, `OptionalPadding`, and `OptionalBackground` into separate files for consistency with the codebase convention.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNode.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLNode.swift:9">
P2: Auto-synthesized `Equatable` includes `id` (`UUID()`) in the comparison, making equality checks always return `false` for distinct instances even when semantically identical. The doc comment explicitly says `id` is "Not decoded; a stable identity per decoded node" — it is a runtime identity marker, not part of the value. Including it in `Equatable` means `==` can never be true for two separately-initialized or decoded nodes with identical content, breaking any code that relies on value equality (SwiftUI `EquatableView`, test assertions, collection diffing).</violation>
</file>
<file name="cmux.xcodeproj/project.pbxproj">
<violation number="1" location="cmux.xcodeproj/project.pbxproj:1932">
P2: CmuxSwiftRenderUI (a SwiftUI+AppKit UI-rendering package) is added as a dependency of the cmux-cli command-line tool target. Every source file in this package imports SwiftUI, and ResizableHSplit.swift imports AppKit. The cmux-cli target (product-type tool) has no UI runtime and cannot meaningfully use SwiftUI View types, which will unnecessarily link SwiftUI/AppKit frameworks into the CLI binary and create a maintenance burden.</violation>
</file>
<file name="Sources/DSLSidebarPlayground.swift">
<violation number="1" location="Sources/DSLSidebarPlayground.swift:31">
P2: Unconditionally coercing all numeric-looking string params to Int can break commands that require string-typed values.</violation>
</file>
<file name="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift">
<violation number="1" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift:18">
P1: ForEach uses index-based identity (`id: \.offset`) in a reorderable list, even though stable per-item identifiers are available via `spec.itemIds`. When a row is dragged to a new position and `rows` updates, SwiftUI identifies each row by its index (0, 1, 2...) rather than by its stable id. This prevents SwiftUI from tracking which view corresponds to which item across the reorder, losing view state, breaking transition animations, and making the reorder feel unresponsive or glitchy.
Use stable IDs from `spec.itemIds` as the ForEach identity, paired with each row, so SwiftUI correctly preserves view identity when items move.</violation>
<violation number="2" location="Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ReorderableList.swift:37">
P2: Validate `ReorderSpec` command/key strings (and `draggedId`) before dispatching `.cmux`; currently malformed or empty dynamic keys can produce invalid reorder commands.
(Based on your team's feedback about avoiding sentinel/missing-key patterns in cmux param construction.) [FEEDBACK_USED].</violation>
</file>
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:31283">
P2: Inconsistent docs topic list: main CLI help advertises `sidebars` but the `docs` command's own usage string and error message still show the old topic list without it. Users running `cmux docs --help` or triggering the extra-args error path will see `docs [settings|shortcuts|api|browser|agents|dock]` — no mention of sidebars.</violation>
</file>
<file name="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:10498">
P2: `selectedId` uses an empty-string sentinel for “no selection” instead of omission, which breaks the key-presence pattern this sidebar data model relies on.
(Based on your team's feedback about checking key presence instead of string sentinels.) [FEEDBACK_USED].</violation>
<violation number="2" location="Sources/ContentView.swift:10521">
P2: tabCount counts all bonsplit tabs unconditionally, but the `tabs` array filters out entries where `panelIdFromSurfaceId(tab.id)` returns nil. When a surface has no panel mapping (possible during lifecycle transitions), `tabCount` will be greater than `tabs.count`, exposing a misleading raw count that doesn't match the actual array. Sidebar authors iterating over `tabs` by index or comparing counts will encounter off-by-one or out-of-bounds bugs.</violation>
<violation number="3" location="Sources/ContentView.swift:10521">
P2: The new `tabCount` line rescans all panes/tabs even though `tabs` was just built from the same traversal, adding avoidable repeated work in a ~1s refresh path.</violation>
</file>
<file name="docs/data-driven-sidebar-plan.md">
<violation number="1" location="docs/data-driven-sidebar-plan.md:56">
P2: Agent-hook coverage list is inaccurate: it omits `amp` and uses `hermes` instead of the wired `hermes-agent` identifier, which can mislead hook setup and review scope.
(Based on your team's feedback about including Hermes/Amp hook wiring in scope.) [FEEDBACK_USED].</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| var result: [SwiftValue] = [] | ||
| for value in values { | ||
| var insertAt = result.count | ||
| for index in result.indices where evalClosure2(closure, value, result[index], env)?.isTruthy ?? false { |
There was a problem hiding this comment.
P1: Comparators with an explicit return are evaluated as nil/false here, so sorted { a, b in return ... } can yield incorrect ordering. Handle ReturnStmtSyntax in closure evaluation before consuming the comparator result.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swift, line 246:
<comment>Comparators with an explicit `return` are evaluated as `nil`/`false` here, so `sorted { a, b in return ... }` can yield incorrect ordering. Handle `ReturnStmtSyntax` in closure evaluation before consuming the comparator result.</comment>
<file context>
@@ -211,7 +235,21 @@ struct ExpressionEvaluator {
+ var result: [SwiftValue] = []
+ for value in values {
+ var insertAt = result.count
+ for index in result.indices where evalClosure2(closure, value, result[index], env)?.isTruthy ?? false {
+ insertAt = index
+ break
</file context>
Comment on lines
+164
to
+172
| } else if let ret = node.as(ReturnStmtSyntax.self), let expr = ret.expression { | ||
| // A view helper with an explicit `return SomeView` (or | ||
| // `return ForEach(...) { }`) renders its returned expression, | ||
| // not nothing. | ||
| if let call = expr.as(FunctionCallExprSyntax.self), isForEach(call) { | ||
| out += evalForEach(call, env) | ||
| } else if let child = evalView(expr, env) { | ||
| out.append(child) | ||
| } |
There was a problem hiding this comment.
P2: return in view-helper bodies does not terminate statement evaluation, so subsequent statements can still render and produce incorrect duplicate output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift, line 164:
<comment>`return` in view-helper bodies does not terminate statement evaluation, so subsequent statements can still render and produce incorrect duplicate output.</comment>
<file context>
@@ -161,6 +161,15 @@ public struct SwiftViewInterpreter: Sendable {
out += evalFor(loop, env)
} else if let ifExpr = ifExpression(node) {
out += evalIf(ifExpr, env)
+ } else if let ret = node.as(ReturnStmtSyntax.self), let expr = ret.expression {
+ // A view helper with an explicit `return SomeView` (or
+ // `return ForEach(...) { }`) renders its returned expression,
</file context>
Suggested change
| } else if let ret = node.as(ReturnStmtSyntax.self), let expr = ret.expression { | |
| // A view helper with an explicit `return SomeView` (or | |
| // `return ForEach(...) { }`) renders its returned expression, | |
| // not nothing. | |
| if let call = expr.as(FunctionCallExprSyntax.self), isForEach(call) { | |
| out += evalForEach(call, env) | |
| } else if let child = evalView(expr, env) { | |
| out.append(child) | |
| } | |
| } else if let ret = node.as(ReturnStmtSyntax.self), let expr = ret.expression { | |
| // A view helper with an explicit `return SomeView` (or | |
| // `return ForEach(...) { }`) renders its returned expression, | |
| // then exits the helper body. | |
| if let call = expr.as(FunctionCallExprSyntax.self), isForEach(call) { | |
| return out + evalForEach(call, env) | |
| } else if let child = evalView(expr, env) { | |
| out.append(child) | |
| } | |
| return out |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swift (2)
62-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winJSON sidebar actions are silently dropped.
The
.jsonpath constructsDSLSidebarRenderer(node: document.root) { _ in }, discarding anyDSLActionfrom interactive JSON nodes. Route JSON actions through the samedispatchmechanism used by the.swiftSourcepath.🤖 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/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swift` around lines 62 - 63, The .json case currently creates DSLSidebarRenderer(node: document.root) with an empty closure, dropping DSLAction events; change it to forward actions to the same dispatch used in the .swiftSource path by passing the dispatch function as the renderer's action handler (i.e., replace the { _ in } closure with the existing dispatch handler used for .swiftSource so DSLSidebarRenderer(node: document.root) calls dispatch when it receives a DSLAction).
64-78:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSwift source is re-parsed on every body render.
SwiftViewInterpreter().evaluate(source, state: dataContext)parses and folds the AST on eachdataContextchange. SinceTimelineViewticks every second (per the host mounting atSources/ContentView.swift:11141), this re-parses every second even when the source hasn't changed.Cache the parsed AST in
CustomSidebarModeland evaluate only againstdataContexthere.🤖 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/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swift` around lines 64 - 78, The view is reparsing Swift source on every render via SwiftViewInterpreter().evaluate(source, state: dataContext); move parsing/folding into CustomSidebarModel so the AST/node is cached and only re-evaluated against dataContext here. Add a cachedParsedNode (or similar) on CustomSidebarModel that performs the expensive parse when the source changes, expose a method/property like evaluateCachedNode(state:) or evaluate(node:cachedNode, state:dataContext) so CustomSidebarView uses model.cachedParsedNode (or model.evaluateCachedNode(dataContext)) instead of creating a new SwiftViewInterpreter each render; update CustomSidebarView to call that cached-evaluate path and keep the scrollWrap/hsplit logic unchanged.Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift (1)
78-78:⚠️ Potential issue | 🟡 Minor | 💤 Low valueCursor may remain stuck after drag ends outside hover bounds.
The
onHoverhandler won't fire an exit event if the user drags outside the divider bounds, leaving the cursor asresizeLeftRight. Reset the cursor in.onEnded:Proposed fix
.onEnded { _ in + NSCursor.arrow.set() dragStartFraction = 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 `@Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift` at line 78, The drag end handler only clears dragStartFraction but doesn't reset the cursor if the user releases outside hover bounds, so update the .onEnded closure (where dragStartFraction is set to nil) to also restore the cursor to the default by undoing whatever the onHover handler did (e.g., if onHover pushes NSCursor.resizeLeftRight, pop or set NSCursor.arrow back). Reference the existing drag end closure and the onHover cursor logic (the resizeLeftRight cursor) and ensure both dragStartFraction = nil and the cursor reset are performed.Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift (1)
80-96:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the user-facing decoding-error messages.
describe(_:)returns hardcoded English ("Missing key …","Type mismatch at …", etc.) that flows intostate.failedand is rendered verbatim byCustomSidebarView.errorView. As per coding guidelines, Swift UI text must useString(localized:defaultValue:).🤖 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/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift` around lines 80 - 96, The decoding error messages in describe(_:) are user-facing and must be localized; update describe(_ error: Error) to wrap each returned message using String(localized: , defaultValue: ) (e.g., for the "Missing key …", "Type mismatch …", "Missing value …", "Invalid JSON …" branches and the `@unknown/default` case) so the strings are localized before they flow into state.failed and are shown by CustomSidebarView.errorView; keep the existing interpolation (key.stringValue, path(ctx), ctx.debugDescription) but pass the composed message as the localized string's defaultValue and provide a suitable localization key or comment as needed.Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift (1)
37-49:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd recursion-depth guard to prevent stack overflow from user input.
The interpreter still has no depth limit:
evaluate→evalView→evalCall→evalItems→evalFor/evalIf/evalForEach→ back toevalItems, andevalIfrecursively handles else-if chains. Deeply nested sidebar source (200-levelif/else ifor member chains) will overflow the stack and crash the app. Thread a depth counter through the call chain and fail gracefully past a threshold (e.g., 100).🤖 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/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift` around lines 37 - 49, Add a recursion-depth guard propagated through the interpreter call chain: extend evaluate to accept/initialize a depth counter (start at 0) and thread it into evalView, evalCall, evalItems, evalFor, evalIf, evalForEach (and any other recursive eval functions), incrementing at each recursive entry and returning a graceful failure (nil or a specific error node) once a threshold (e.g., 100) is exceeded; ensure every call site of evalView/evalCall/evalItems/etc. is updated to pass the incremented depth so deep user input cannot overflow the stack.
🤖 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/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swift`:
- Around line 344-356: The test integerDivisionByZeroDoesNotCrash currently only
asserts the VStack exists and the last child is "ok"; update it to assert the
exact children behavior for node (the value returned by interp.evaluate) so the
soft-fail is verified: inspect the current interpreter behavior and then add
either (A) an assertion that node?.children.count == 1 and
node?.children.first?.text == "ok" if zero-divisions are dropped, or (B)
assertions that node?.children.count == 3 and node?.children[0].text == "v=",
node?.children[1].text == "m=", and node?.children[2].text == "ok" if the
interpolations render empty strings; use the test function
integerDivisionByZeroDoesNotCrash and the node/children/text properties to
locate and implement the correct assertion variant.
---
Duplicate comments:
In `@Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift`:
- Around line 37-49: Add a recursion-depth guard propagated through the
interpreter call chain: extend evaluate to accept/initialize a depth counter
(start at 0) and thread it into evalView, evalCall, evalItems, evalFor, evalIf,
evalForEach (and any other recursive eval functions), incrementing at each
recursive entry and returning a graceful failure (nil or a specific error node)
once a threshold (e.g., 100) is exceeded; ensure every call site of
evalView/evalCall/evalItems/etc. is updated to pass the incremented depth so
deep user input cannot overflow the stack.
In
`@Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swift`:
- Line 78: The drag end handler only clears dragStartFraction but doesn't reset
the cursor if the user releases outside hover bounds, so update the .onEnded
closure (where dragStartFraction is set to nil) to also restore the cursor to
the default by undoing whatever the onHover handler did (e.g., if onHover pushes
NSCursor.resizeLeftRight, pop or set NSCursor.arrow back). Reference the
existing drag end closure and the onHover cursor logic (the resizeLeftRight
cursor) and ensure both dragStartFraction = nil and the cursor reset are
performed.
In
`@Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swift`:
- Around line 80-96: The decoding error messages in describe(_:) are user-facing
and must be localized; update describe(_ error: Error) to wrap each returned
message using String(localized: , defaultValue: ) (e.g., for the "Missing key
…", "Type mismatch …", "Missing value …", "Invalid JSON …" branches and the
`@unknown/default` case) so the strings are localized before they flow into
state.failed and are shown by CustomSidebarView.errorView; keep the existing
interpolation (key.stringValue, path(ctx), ctx.debugDescription) but pass the
composed message as the localized string's defaultValue and provide a suitable
localization key or comment as needed.
In
`@Packages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swift`:
- Around line 62-63: The .json case currently creates DSLSidebarRenderer(node:
document.root) with an empty closure, dropping DSLAction events; change it to
forward actions to the same dispatch used in the .swiftSource path by passing
the dispatch function as the renderer's action handler (i.e., replace the { _ in
} closure with the existing dispatch handler used for .swiftSource so
DSLSidebarRenderer(node: document.root) calls dispatch when it receives a
DSLAction).
- Around line 64-78: The view is reparsing Swift source on every render via
SwiftViewInterpreter().evaluate(source, state: dataContext); move
parsing/folding into CustomSidebarModel so the AST/node is cached and only
re-evaluated against dataContext here. Add a cachedParsedNode (or similar) on
CustomSidebarModel that performs the expensive parse when the source changes,
expose a method/property like evaluateCachedNode(state:) or
evaluate(node:cachedNode, state:dataContext) so CustomSidebarView uses
model.cachedParsedNode (or model.evaluateCachedNode(dataContext)) instead of
creating a new SwiftViewInterpreter each render; update CustomSidebarView to
call that cached-evaluate path and keep the scrollWrap/hsplit logic unchanged.
🪄 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: f2a0e23e-852f-454a-a4a9-a3faaab08aa2
📒 Files selected for processing (23)
CLI/cmux.swiftPackages/CmuxSettings/Sources/CmuxSettings/Keys/BetaFeaturesCatalogSection.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/ActionCommand.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/ButtonAction.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/ExpressionEvaluator.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/ModifierArg.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/RenderModifier.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/RenderNode.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/ReorderSpec.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swiftPackages/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swiftPackages/CmuxSwiftRenderUI/Package.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/CustomSidebarContentInsets.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Rendering/ResizableHSplit.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swiftSources/CmuxSidebarActionDispatch.swiftSources/ContentView.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojdocs/custom-sidebars.mddocs/data-driven-sidebar-plan.mddocs/swiftui-interpreter-surface.md
💤 Files with no reviewable changes (6)
- docs/data-driven-sidebar-plan.md
- docs/custom-sidebars.md
- docs/swiftui-interpreter-surface.md
- Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/RenderNode.swift
- Sources/CmuxSidebarActionDispatch.swift
- cmux.xcodeproj/project.pbxproj
Assert the interpolation drops the nil division/modulo segment ("v=", "m=")
rather than only checking a later row renders.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
Strengthened the div-by-zero test to assert the soft-fail output ( |
|
(ﾐ･ω･)ﾐ 🧠 Learnings used✅ Action performedReview finished.
|
Comment on lines
+312
to
+328
| private func evalIf(_ ifExpr: IfExprSyntax, _ env: Environment) -> [RenderNode] { | ||
| let taken = ifExpr.conditions.allSatisfy { element in | ||
| guard let expr = element.condition.as(ExprSyntax.self) else { return false } | ||
| return expressions.eval(expr, env)?.isTruthy ?? false | ||
| } | ||
| if taken { | ||
| return evalItems(ifExpr.body.statements, env.makeChild()) | ||
| } | ||
| guard let elseBody = ifExpr.elseBody else { return [] } | ||
| if let block = elseBody.as(CodeBlockSyntax.self) { | ||
| return evalItems(block.statements, env.makeChild()) | ||
| } | ||
| if let elseIf = elseBody.as(IfExprSyntax.self) { | ||
| return evalIf(elseIf, env) | ||
| } | ||
| return [] | ||
| } |
Contributor
There was a problem hiding this comment.
if let optional-binding conditions always evaluate to false
evalIf casts every condition element to ExprSyntax. An if let condition uses OptionalBindingConditionSyntax, so the cast fails and allSatisfy returns false, which means the body of any if let block is silently never rendered. The docs at docs/custom-sidebars.md explicitly recommend if let b = w.branch { ... } as the idiom for optional workspace fields — authors following that pattern will always get an empty view with no feedback. The ternary form (w.branch != nil ? ... : ...) is the only working alternative today, but it isn't mentioned as the only supported form and can't bind the unwrapped value cleanly.
…ions Two review findings on the custom-sidebar render path: - The Swift source was re-parsed every render. The host mounts the sidebar under a TimelineView that ticks every second, so SwiftViewInterpreter re-parsed and operator-folded the AST on every tick even when the source hadn't changed. Split the interpreter into parse() -> ParsedProgram and evaluate(ParsedProgram, state:); CustomSidebarModel caches the parse keyed by source and re-evaluates only against the live dataContext. File-change reload swaps in new source, which invalidates the cache on next render. - JSON sidebar actions were silently dropped: the .json path constructed DSLSidebarRenderer with an empty action closure. Route them through the same SidebarActionDispatch the interpreted path uses, via a DSLAction.buttonAction mapping (log/openURL/cmux-method). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
Collaborator
Author
|
Addressed both substantive findings in c87fdf7:
Both packages build clean; 34 CmuxSwiftRender tests pass. @coderabbitai review |
|
Kicking off a fresh review now. (ﾐ･ω･)ﾐ 🧠 Learnings used✅ Action performedReview finished.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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/CmuxSwiftRender/Sources/CmuxSwiftRender/ParsedProgram.swift`:
- Around line 17-24: ParsedProgram lacks Sendable conformance while
SwiftViewInterpreter (which produces ParsedProgram via parse(_:)) is Sendable
and ParsedProgram instances are cached and passed across actor boundaries (e.g.
CustomSidebarModel); make ParsedProgram conform to Sendable by declaring it
Sendable (add : Sendable to the ParsedProgram declaration) — since
SourceFileSyntax is already Sendable this is safe and requires no further
changes to the init or stored property `file`.
🪄 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: 04a84abf-486b-48ce-8b97-17a35d2f47fe
📒 Files selected for processing (5)
Packages/CmuxSwiftRender/Sources/CmuxSwiftRender/ParsedProgram.swiftPackages/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/JSON/DSLAction.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarModel.swiftPackages/CmuxSwiftRenderUI/Sources/CmuxSwiftRenderUI/Sidebar/CustomSidebarView.swift
Comment on lines
+17
to
+24
| public struct ParsedProgram { | ||
| /// The folded syntax tree the interpreter walks. | ||
| let file: SourceFileSyntax | ||
|
|
||
| init(file: SourceFileSyntax) { | ||
| self.file = file | ||
| } | ||
| } |
There was a problem hiding this comment.
Add Sendable conformance to match SwiftViewInterpreter.
SwiftViewInterpreter is Sendable and produces ParsedProgram via parse(_:). Downstream, CustomSidebarModel caches this value and may pass it across actor boundaries. Since SourceFileSyntax from swift-syntax 600 is already Sendable, the fix is trivial:
🛠️ Proposed fix
-public struct ParsedProgram {
+public struct ParsedProgram: Sendable {🧰 Tools
🪛 SwiftLint (0.63.3)
[Warning] 21-21: This memberwise initializer would be synthesized automatically - you do not need to define it
(unneeded_synthesized_initializer)
🤖 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/CmuxSwiftRender/Sources/CmuxSwiftRender/ParsedProgram.swift` around
lines 17 - 24, ParsedProgram lacks Sendable conformance while
SwiftViewInterpreter (which produces ParsedProgram via parse(_:)) is Sendable
and ParsedProgram instances are cached and passed across actor boundaries (e.g.
CustomSidebarModel); make ParsedProgram conform to Sendable by declaring it
Sendable (add : Sendable to the ParsedProgram declaration) — since
SourceFileSyntax is already Sendable this is safe and requires no further
changes to the init or stored property `file`.
azooz2003-bit
dismissed
coderabbitai[bot]’s stale review
June 3, 2026 16:02
Substantive findings (re-parse-every-tick perf, dropped JSON actions) resolved in c87fdf7. Remaining items: minor localization nit + low-value nit, tracked as fast-follow. User authorized dismissal.
azooz2003-bit
added a commit
that referenced
this pull request
Jun 3, 2026
#5254 merged the parse()->ParsedProgram / evaluate(program:) split into main. Reconcile with this branch's 16MB-stack robustness fix: both parse() and evaluate(program:) now run their recursive work on a shared onLargeStack worker (LargeStackResultBox), and ParsedProgram is Sendable so it can cross the worker boundary (also addresses CodeRabbit's Sendable suggestion). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
azooz2003-bit
added a commit
that referenced
this pull request
Jun 3, 2026
…in-package CmuxSwiftRenderUI gains its own Localizable.xcstrings (en + ja) for the sidebar.custom.* strings, including the previously unlocalized decode-error descriptions flagged on #5254; String(localized:) call sites now read from .module. Also document the SidebarTapTarget/SidebarTapTargetsKey public members and DSLDocument, and annotate the render worker's -> Never backstop exit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 4, 2026
Merged
azooz2003-bit
added a commit
that referenced
this pull request
Jun 4, 2026
The customSidebars.beta.enabled flag existed in the catalog and gated the picker and rendering since #5254, but the Beta Features section never grew a row for it, so the only way to flip it was a raw defaults write. Add the toggle (with search entry and en/ja strings) mirroring the Extensions row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
azooz2003-bit
added a commit
that referenced
this pull request
Jun 4, 2026
…ng (#5294) * Out-of-process sidebar interpreter: crash-isolating worker + client Untrusted vibe-coded sidebars run through a hand-written tree-walking interpreter. No matter how well-guarded, an interpreter bug (an unguarded path, a parser edge case, stack/memory exhaustion) running in-process can crash the whole terminal app. This isolates that risk behind a process boundary. - Make the RenderNode IR Codable (RenderNode/Kind, RenderModifier, ModifierArg, ActionCommand, ButtonAction, ReorderSpec, SwiftValue) so it crosses the process boundary. - New package CmuxSidebarInterpreterService: - cmux-sidebar-interpreter: a worker executable running the interpreter in a read-eval-write loop over a length-prefixed stdin/stdout channel. - InterpreterClient: a host-side actor that spawns/supervises the worker, correlates responses by id, and on a crash (closed pipe) or per-render timeout returns nil + relaunches the worker on the next render. The host never crashes regardless of what the source does. - LengthPrefixedMessageChannel: fd-backed (Sendable) framing shared by both. Verified by swift test: valid render, host data-context binding, worker reuse, and the two isolation guarantees — a crashing worker and a hanging worker both return nil to the host and recover transparently (8 tests). Not NSRemoteView: the goal is guarding against interpreter bugs, so the interpreter (not the rendering) goes out-of-process; the validated RenderNode IR renders in-process. This needs no private API and is verifiable headlessly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Render custom sidebars through an injected SidebarInterpreting seam Adds the seam the app uses to run interpreted sidebars out-of-process, without the UI layer depending on the worker/service: - SidebarInterpreting protocol (CmuxSwiftRender) abstracts where interpretation happens; InProcessSidebarInterpreter is the in-process default, Interpreter Client (out-of-process, crash-isolating) conforms. - CustomSidebarModel renders asynchronously through the injected interpreter (no longer interprets in-process in the view body); publishes the result to swiftRender, keeping the last view until the next render lands so live re-renders don't flicker. - CustomSidebarView drives renderSwift via .task(id:) keyed on source revision + data context, and reads the published node. - The worker's RenderInterpreterRunner caches the last parse so per-tick re-renders against changing data don't re-parse unchanged source. UI package builds; the app injects the out-of-process client next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Wire the out-of-process interpreter into the app (re-exec-self worker) - Link CmuxSidebarInterpreterClient into the cmux and cmux-cli targets (package ref + product dependency, mirroring CmuxSwiftRenderUI's wiring). - Entry point: CmuxMain.main() checks for the worker-mode flag before any AppKit/SwiftUI setup; in worker mode it runs the shared interpreter loop and exits. The worker is this same binary re-executed with the flag, so no separate helper needs to be bundled or signed. - InterpreterClient gains worker arguments + a reexecingCurrentBinary() factory; the shared worker loop moves into the library so the standalone executable and the re-exec path run identical code. - VerticalTabsSidebar holds one InterpreterClient and injects it into CustomSidebarView, so custom sidebars now interpret out-of-process: an interpreter crash kills only the worker, which relaunches on the next render. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix strict-concurrency error: no optional-chained actor calls in Tasks CI's Swift compiler rejects `Task { await self?.method() }` (the closure infers as non-Sendable `() async -> ()?`). Use `guard let self` inside the task closures instead. Behavior unchanged; 8 package tests still pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Extract value-driven CustomSidebarContentView and report tap targets Split the pure presentation out of CustomSidebarView so the out-of-process render worker can mount the exact same sidebar UI from value snapshots, and publish CustomSidebarModel (+ State, DSLDocument) for reuse by the worker. Buttons and tap-gesture nodes now report their global frames through a SidebarTapTargets preference, letting a remote host hit-test forwarded clicks geometrically (SwiftUI control gestures never fire in a never-on-screen window). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add the remote render worker: protocol, supervised client, faceless renderer Stage 2 of out-of-process custom sidebars: the worker now interprets AND renders the untrusted sidebar file, sharing its layer tree with the host via a remote CoreAnimation context (CAContext.remoteContextWithOptions: in the worker, CALayerHost in the host; the contextId is a plain uint32 over the existing framed channel, so no mach-port handoff is needed). The public CARemoteLayerServer/Client API renders blank on current macOS and is a verified dead end (docs/remote-sidebar-rendering/spike/ in the hq repo). - RenderWorkerInbound/Outbound wire protocol: scenes (file + data context + insets, acked by seq for the hang watchdog), resizes, pointers in; context announcements and ButtonActions out. - RenderWorkerClient actor: lazy spawn, crash/hang supervision, scene replay on respawn, retry-once sends, multicast event subscriptions. - CmuxSidebarRemoteRender target: runSidebarRenderWorker() (faceless AppKit loop), RenderWorkerCoordinator (offscreen never-ordered window + NSHostingView pump; AppKit re-claims the backing layer on every window layout pass, so the pump steals it back before each commit), geometric tap-target activation, NSScrollView-direct scrolling, contentsScale walk; host-side RemoteCustomSidebarView (CALayerHost + input forwarding). - cmux-sidebar-render-fixture + supervision tests (announce/ack/action/ crash-respawn/hang-discard through a real process boundary). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Render custom sidebars fully out-of-process in the app CmuxMain grows a render-worker branch (re-exec-self with --cmux-sidebar-render-worker, before any app startup), and the sidebar mounts RemoteCustomSidebarView: interpretation and rendering both happen in the supervised worker; the host only displays the worker's remote layer, forwards input, and dispatches returned ButtonActions. The stage-1 in-process interpreter path stays behind the SidebarInterpreting seam as the fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review: DocC on new public surface, localize sidebar strings in-package CmuxSwiftRenderUI gains its own Localizable.xcstrings (en + ja) for the sidebar.custom.* strings, including the previously unlocalized decode-error descriptions flagged on #5254; String(localized:) call sites now read from .module. Also document the SidebarTapTarget/SidebarTapTargetsKey public members and DSLDocument, and annotate the render worker's -> Never backstop exit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix remote sidebar scroll pinning under the titlebar inset The worker's scroll path clamped the clip origin to y >= 0, but with the host's safe-area top inset the resting origin is -topInset — the first scroll snapped content up under the traffic lights and the clamp never let it return. Clamp through NSClipView.constrainBoundsRect instead, which honors content insets and document bounds exactly like a real wheel event. Harness-verified: scroll down reveals later rows, over-scrolling back up settles at the inset rest position instead of pinning to the window top. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Keep the remote sidebar surface mounted across picker switches Dropping the .id(fileURL) remount: the worker swaps files in place on the next scene message, so remounting the surface only flashed the previous sidebar's pixels (the fresh CALayerHost adopts the live context, which still shows the old file for the IPC beat, and SwiftUI can overlap the outgoing/incoming views). One surface, one hosted layer, content switches in the worker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Adopt the live remote layer synchronously on sidebar surface mount Dogfood (Codex-driven) flagged transient blank sidebars when cycling providers: a remounting surface waited on the async subscribe round-trip before adopting the worker's layer. RenderWorkerClient now mirrors the live context id into a main-actor RemoteContextCache the surface reads in init, so provider switches and sidebar toggles show the worker's last rendered frame in the same frame the surface appears. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Make hot reload last-good sticky: broken saves never replace a working sidebar A save now only replaces what's on screen when it actually interprets to a view (or decodes, for JSON). Broken intermediate saves — editors writing mid-edit states, atomic saves' transient missing file — keep the previous working render, with the last GOOD source re-interpreted against fresh data each tick so live values keep flowing while the file is broken. Error states still show when a file is broken from the start (nothing good to fall back to), and switching files resets the fallback. Same stickiness in the in-process fallback path. Harness-verified: valid -> garbage keeps rendering the valid version with data ticks continuing; fixing the file swaps in the new content; a file broken from first selection still shows the error state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Show the Custom Sidebars beta toggle in Settings The customSidebars.beta.enabled flag existed in the catalog and gated the picker and rendering since #5254, but the Beta Features section never grew a row for it, so the only way to flip it was a raw defaults write. Add the toggle (with search entry and en/ja strings) mirroring the Extensions row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix CI Swift 6.1 sendability in the reload-notification forwarder CI's Xcode 16.4 rejects awaiting NotificationCenter's async stream from a MainActor task (non-Sendable Notification crossing isolation in next()). Consume the stream in a nonisolated task; only the Sendable names array leaves it through the outbox. Also add the parameter docs Codex review flagged on the public wire-type initializers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Use a token observer for reload forwarding (CI Swift 6.1) Task {} in a MainActor init inherits MainActor isolation, so the nonisolated rewrite still tripped 6.1's non-Sendable Notification check on the async stream's next(). Use the closure observer API instead — the same pattern CustomSidebarModel ships, already proven on CI's toolchain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address CodeRabbit re-review: frame guards, watcher re-arm, worker reaping - LengthPrefixedMessageChannel caps frames at 64 MB in both directions: an inbound peer-controlled length beyond the cap reads as EOF instead of an allocation, and an oversized outbound payload throws (regression tests for both). - The hot-reload watcher follows .swift/.json extension flips (reload re-resolves the file by name; the kqueue watcher previously stayed on the original path and went silently stale). - SidebarTapTargetsKey moves to its own file (public major type). - The render worker is terminated when its window closes (surface observes NSWindow.willCloseNotification); provider switches and sidebar toggles still keep it warm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 4, 2026
Merged
azooz2003-bit
added a commit
that referenced
this pull request
Jun 5, 2026
…5275) * Interpreter primitives B1a: List/Section/LazyVStack/LazyHStack/Group/EmptyView + horizontal ScrollView Leaf-tier container coverage from docs/swiftui-interpreter-surface.md: - New RenderNode kinds: lazyVStack, lazyHStack, group, list, section, hscroll. - Interpreter constructors for LazyVStack/LazyHStack, Group, EmptyView (-> empty group), List, Section("Header") { } (leading string literal becomes header), and ScrollView(.horizontal) -> hscroll (vertical stays passthrough so it does not double-scroll inside the already-scrolling sidebar). - RenderNodeView renders each: LazyVStack/LazyHStack, transparent Group, plain chrome-light List (.plain + hidden scroll background), Section with a styled caption header above its content, and a horizontal ScrollView+HStack. - Harness allow-list + 3 behavior tests (37 interpreter tests pass; UI package builds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter primitives B1b: ForEach enumerated/indices/2-arg + array slicing + conversions - ForEach over `Array(xs.enumerated())` with a 2-arg closure `{ index, item in }` now destructures the pair (offset/element). `.enumerated()` yields pairs addressable as `.offset`/`.element` or `$0`/`$1`. - `.indices` resolves as a property (bare, no parens) on arrays, so `ForEach(xs.indices) { i in ... }` works; also `.first`/`.last` properties. - Array `.dropFirst(_:)`, `.dropLast(_:)`, `.suffix(_:)` methods. - `Array(seq)` identity passthrough and `Int()/Double()/String()` conversions in value position. - Allow-list + 3 behavior tests (40 interpreter tests pass). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter primitives B2: text/typography modifiers + Label - New view-level text modifiers in the bridge: .italic, .underline, .monospaced, .monospacedDigit, .fontDesign, .multilineTextAlignment, .textCase, .truncationMode (resolvers in RenderStyle). All apply to the erased view, so they affect contained text. - Label(title, systemImage:) -> new .label kind rendered as a native Label. - Allow-list (Label/Array/Int/Double/String + the new members) + 2 tests (42 interpreter tests pass; UI builds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter primitives B3: shapes + semantic colors + cosmetic/transform modifiers - New shape kinds: Ellipse, UnevenRoundedRectangle (uniform-radius approx). - Widened dslColor palette: tertiary/quaternary + mint/teal/cyan/indigo/brown. - New bridge modifiers: shadow(color:radius:x:y:), border(_,width:), blur(radius:), offset(x:y:), scaleEffect, rotationEffect(.degrees/.radians), zIndex, brightness/contrast/saturation/grayscale, clipShape(<shape>), clipped, fixedSize, layoutPriority. Angle token parser for rotationEffect. - Allow-list + 2 tests (44 interpreter tests pass; UI builds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter primitives B5: arbitrary-child modifiers (.overlay/.background/.mask/.safeAreaInset) The composability keystone: RenderModifier now carries an optional child RenderNode subtree captured from a modifier's trailing closure. The interpreter lowers `.overlay { ... }`, `.background { ... }`, `.mask { ... }`, and `.safeAreaInset(edge:) { ... }` content through evalItems, and the bridge renders it (alignment-aware for overlay/background; ZStack for multiple children). This lets interpreted views nest arbitrary content inside modifiers, not just colors. Background/overlay still accept a plain color token when there is no closure. Allow-list + a test (45 interpreter tests pass; UI builds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter primitives B6: SF Symbol modifiers + min/max/abs builtins - Bridge modifiers: .imageScale, .symbolRenderingMode (hierarchical/multicolor/ palette/monochrome), .symbolVariant (fill/circle/square/slash) — the symbol controls SF-symbol sidebars actually use. - Numeric builtins min(_,_)/max(_,_)/abs(_) in value position (int-preserving). - Allow-list + 2 tests (47 interpreter tests pass; UI builds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter primitives B7: Grid/GridRow, LazyVGrid/LazyHGrid, ViewThatFits - New kinds grid/gridRow/lazyVGrid/lazyHGrid/viewThatFits with interpreter constructors and native bridge rendering. Grid uses leading alignment; LazyV/HGrid use adaptive columns/rows (sidebar-appropriate approximation of the GridItem spec); ViewThatFits lets SwiftUI pick the first fitting child. - Allow-list + a test (48 interpreter tests pass; UI builds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter primitives B8: ProgressView/Gauge/Menu + contextMenu/help/disabled - New kinds progressView (determinate via normalized value:/total: or indeterminate), gauge, menu (Menu(title) { items }). RenderNode gains a `value` field for determinate controls. - contextMenu joins the child-bearing modifiers (its closure items render as the menu); .help (tooltip) and .disabled bridge modifiers. - Allow-list + 2 tests (49 interpreter tests pass; UI builds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter primitives B9: redaction, accessibility, scroll/list tokens, aspectRatio Generic view-level bridge modifiers (no IR change; the interpreter already captures them): .redacted(reason:)/.unredacted, .accessibilityLabel/Hint/Value/ Hidden, .scrollIndicators, .scrollContentBackground, .aspectRatio(contentMode:), .scaledToFit/.scaledToFill. Allow-list updated; UI builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter robustness: large-stack worker + recursion budget; stress corpus Deeply-composed sidebars (and pathological nesting) overflowed the caller stack during parse/interpret and crashed the process (SIGBUS). Fixes: - evaluate() now runs parse + interpret on a dedicated 16MB-stack Thread, so realistic deep nesting renders instead of overflowing the small caller stack. - A per-Environment RecursionBudget (shared down the scope chain) backstops genuinely unbounded interpreter recursion (mutually-recursive view helpers), guarding evalView/eval/evalItems/callValueFunction. - Add 8 multi-agent-authored stress sidebars to the corpus (deeply nested overlays/backgrounds, grids, menus, progress, data-driven). All 22 corpus sidebars now render; 50 interpreter tests pass, incl a 600-deep nesting test. Surfaced next gap: shape `.stroke` (13 uses) / `.trim` / `.strokeBorder`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter primitives B-stroke: shape .stroke/.strokeBorder/.trim The stress corpus surfaced .stroke as the top gap (13 uses). Apply stroke/trim at the concrete-shape level (before AnyView erasure) via AnyShape: shapes (Circle/Rectangle/RoundedRectangle/Capsule/Ellipse) now honor `.trim(from:to:)` then `.stroke(color, lineWidth:)` / `.strokeBorder`, rendering real outlines; with no stroke the plain shape renders so .fill/.foregroundColor still fill it. Allow-list + a test (51 interpreter tests pass; UI builds clean). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter: AnyView(_:) passthrough renders the wrapped view Common in author/agent helpers that erase to AnyView; render the wrapped expression instead of dropping it. 52 interpreter tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: expand custom-sidebars authoring guide to the full interpreter surface Document the ~80 added views/modifiers/language features (lists/grids/lazy stacks, full text/typography, shapes + stroke/trim, arbitrary-child overlay/ background/mask/contextMenu, ProgressView/Gauge/Menu, symbol modifiers, value methods) and narrow the 'not yet supported' list to the real remaining gaps (@State + input controls, gradients, navigation, custom structs). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter primitives: Linear/Radial/Angular gradients New gradient view kinds with interpreter constructors (parse `colors: [...]` or `gradient: Gradient(colors:)` stops + start/end/center UnitPoint tokens) and native rendering via dslUnitPoint. Usable standalone in ZStack and inside the arbitrary-child `.background { }` / `.overlay { }` modifiers. RenderNode gains `colors`/`points`. Allow-list + a test (53 interpreter tests pass; UI builds). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: @State engine design note (the interactivity tier) Concrete implementation plan for the remaining big capability (@State / $bindings / input controls): mutable host-owned state bag keyed by declaration site, binding values, action executor for assignments, re-walk on change, staged S1-S3. Left as a design (not half-built) because interactive behavior needs real dogfooding. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter: .keyboardShortcut(key, modifiers:) Bridge .keyboardShortcut with a KeyEquivalent resolver (.return/.escape/arrows/ single char) and EventModifiers parser (command/shift/option/control). Closes the last non-niche modifier gap from the stress corpus. UI builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter language tier: if let / guard let binding + view-position switch - evalIf now evaluates condition lists via conditionsPass, binding `if let name = expr` (and shorthand `if let name`) optionals into the then-branch scope (previously only boolean conditions were honored, so `if let` never rendered). - View-position `switch subject { case "x": ... default: ... }` selects the first matching case; matches string/int literal patterns and bare `.member` enum-style patterns against the subject. The type-system tier begins. - 2 tests (55 interpreter tests pass). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter language tier: value-position switch + if let in user value funcs evalBlockValue/evalIfValue now mirror the view-side language support: a value helper can use `switch x { case ...: return ... }` and `if let y = opt { return y }` (conditionsPass binds optionals into the branch scope). Lets authors write natural value helpers (e.g. a status->color switch). 56 interpreter tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Corpus: add 6 stress2 sidebars exercising switch/if-let/gradients/trim Second multi-agent stress round, now leaning on the language tier (value-func `switch` mapping status->color/symbol, `if let pr = w.pr`), gradients in `.background { }`, `.trim` arcs, AngularGradient. All 28 corpus sidebars render (0 NIL); permanent regression fixtures for the language + gradient tiers. Only remaining unsupported symbol is `.resizable` (+ undefined persona helper funcs, which are authoring gaps, not interpreter gaps). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter: Image .resizable() (applied before erasure) styledImage applies .resizable() on the concrete Image (an Image method, not available post-AnyView-erasure); .scaledToFit/.aspectRatio from the generic pass apply on top. Closes the last unsupported modifier from the stress corpus — the only remaining gaps are AsyncImage/URL (async/file) and the @State tier. UI builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter: fix bugs from adversarial correctness review Confirmed findings from a multi-agent review of tonight's changes: - CRITICAL: evalClosure/evalClosure2 returned the first expression, skipping local `let` bindings — `xs.map { x in let d = x*2; d+1 }` (and reduce/sorted with locals) silently broke. Both now route the body through evalBlockValue, honoring let/if/switch + trailing expression. - HIGH: Color(red:green:blue:) channel did `Int(d*255)` which TRAPS on Double.infinity/NaN — guard `isFinite` + clamp to [0,1] before converting. - range iterationValues: overflow-safe end (addingReportingOverflow) + a 100k materialization cap so `0...Int.max` can't overflow or exhaust memory. - .disabled() now disables only when the arg is explicitly true (unresolved expr -> enabled, not disabled). - .aspectRatio applies an explicit ratio only when positive (zero/negative is invalid in SwiftUI) -> falls back to mode-only. Regression tests for the closure-let and non-finite-color cases (58 tests pass). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter: array .joined, string .capitalized / .replacingOccurrences Common value-method long-tail: array `.joined(separator:)`, string `.replacingOccurrences(of:with:)` (methods, via evalMethod), and `.capitalized` (a property — resolved in SwiftValue.member like .indices, alongside .uppercased/.lowercased property forms). 59 interpreter tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Interpreter: string .trimmingCharacters(in:) Common everyday helper (trim titles/branches): .trimmingCharacters(in: .whitespaces / .whitespacesAndNewlines / .newlines). 60 interpreter tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Rebase onto main: thread parse/evaluate split through large-stack worker #5254 merged the parse()->ParsedProgram / evaluate(program:) split into main. Reconcile with this branch's 16MB-stack robustness fix: both parse() and evaluate(program:) now run their recursive work on a shared onLargeStack worker (LargeStackResultBox), and ParsedProgram is Sendable so it can cross the worker boundary (also addresses CodeRabbit's Sendable suggestion). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add failing tests: Int() of non-finite double traps; Gauge ignores total: Int(1.0 / 0.0) in authored sidebar source crashes the interpreting process (Swift's Int(_: Double) traps on non-finite/overflow), and Gauge(value:total:) renders full because only the raw value: is captured. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix Int() trap on non-finite doubles and Gauge total: normalization Int(exactly:) on the truncated value returns nil for NaN/infinity/overflow instead of trapping, so authored source like Int(1.0 / 0.0) renders best-effort rather than killing the interpreting process. Gauge(value:total:) now normalizes through the same progressValue logic as ProgressView, so total-relative gauges render proportionally instead of pinned full. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
ShubhamPatilsd
pushed a commit
to emergent-inc/mosaic
that referenced
this pull request
Jul 9, 2026
…ng (#5294)
* Out-of-process sidebar interpreter: crash-isolating worker + client
Untrusted vibe-coded sidebars run through a hand-written tree-walking
interpreter. No matter how well-guarded, an interpreter bug (an unguarded
path, a parser edge case, stack/memory exhaustion) running in-process can
crash the whole terminal app. This isolates that risk behind a process
boundary.
- Make the RenderNode IR Codable (RenderNode/Kind, RenderModifier, ModifierArg,
ActionCommand, ButtonAction, ReorderSpec, SwiftValue) so it crosses the
process boundary.
- New package CmuxSidebarInterpreterService:
- cmux-sidebar-interpreter: a worker executable running the interpreter in a
read-eval-write loop over a length-prefixed stdin/stdout channel.
- InterpreterClient: a host-side actor that spawns/supervises the worker,
correlates responses by id, and on a crash (closed pipe) or per-render
timeout returns nil + relaunches the worker on the next render. The host
never crashes regardless of what the source does.
- LengthPrefixedMessageChannel: fd-backed (Sendable) framing shared by both.
Verified by swift test: valid render, host data-context binding, worker reuse,
and the two isolation guarantees — a crashing worker and a hanging worker both
return nil to the host and recover transparently (8 tests).
Not NSRemoteView: the goal is guarding against interpreter bugs, so the
interpreter (not the rendering) goes out-of-process; the validated RenderNode
IR renders in-process. This needs no private API and is verifiable headlessly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Render custom sidebars through an injected SidebarInterpreting seam
Adds the seam the app uses to run interpreted sidebars out-of-process, without
the UI layer depending on the worker/service:
- SidebarInterpreting protocol (CmuxSwiftRender) abstracts where interpretation
happens; InProcessSidebarInterpreter is the in-process default, Interpreter
Client (out-of-process, crash-isolating) conforms.
- CustomSidebarModel renders asynchronously through the injected interpreter
(no longer interprets in-process in the view body); publishes the result to
swiftRender, keeping the last view until the next render lands so live
re-renders don't flicker.
- CustomSidebarView drives renderSwift via .task(id:) keyed on source revision
+ data context, and reads the published node.
- The worker's RenderInterpreterRunner caches the last parse so per-tick
re-renders against changing data don't re-parse unchanged source.
UI package builds; the app injects the out-of-process client next.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Wire the out-of-process interpreter into the app (re-exec-self worker)
- Link CmuxSidebarInterpreterClient into the cmux and cmux-cli targets
(package ref + product dependency, mirroring CmuxSwiftRenderUI's wiring).
- Entry point: CmuxMain.main() checks for the worker-mode flag before any
AppKit/SwiftUI setup; in worker mode it runs the shared interpreter loop and
exits. The worker is this same binary re-executed with the flag, so no
separate helper needs to be bundled or signed.
- InterpreterClient gains worker arguments + a reexecingCurrentBinary()
factory; the shared worker loop moves into the library so the standalone
executable and the re-exec path run identical code.
- VerticalTabsSidebar holds one InterpreterClient and injects it into
CustomSidebarView, so custom sidebars now interpret out-of-process: an
interpreter crash kills only the worker, which relaunches on the next render.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Fix strict-concurrency error: no optional-chained actor calls in Tasks
CI's Swift compiler rejects `Task { await self?.method() }` (the closure
infers as non-Sendable `() async -> ()?`). Use `guard let self` inside the
task closures instead. Behavior unchanged; 8 package tests still pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Extract value-driven CustomSidebarContentView and report tap targets
Split the pure presentation out of CustomSidebarView so the out-of-process
render worker can mount the exact same sidebar UI from value snapshots, and
publish CustomSidebarModel (+ State, DSLDocument) for reuse by the worker.
Buttons and tap-gesture nodes now report their global frames through a
SidebarTapTargets preference, letting a remote host hit-test forwarded
clicks geometrically (SwiftUI control gestures never fire in a
never-on-screen window).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Add the remote render worker: protocol, supervised client, faceless renderer
Stage 2 of out-of-process custom sidebars: the worker now interprets AND
renders the untrusted sidebar file, sharing its layer tree with the host
via a remote CoreAnimation context (CAContext.remoteContextWithOptions: in
the worker, CALayerHost in the host; the contextId is a plain uint32 over
the existing framed channel, so no mach-port handoff is needed). The public
CARemoteLayerServer/Client API renders blank on current macOS and is a
verified dead end (docs/remote-sidebar-rendering/spike/ in the hq repo).
- RenderWorkerInbound/Outbound wire protocol: scenes (file + data context +
insets, acked by seq for the hang watchdog), resizes, pointers in;
context announcements and ButtonActions out.
- RenderWorkerClient actor: lazy spawn, crash/hang supervision, scene
replay on respawn, retry-once sends, multicast event subscriptions.
- CmuxSidebarRemoteRender target: runSidebarRenderWorker() (faceless AppKit
loop), RenderWorkerCoordinator (offscreen never-ordered window +
NSHostingView pump; AppKit re-claims the backing layer on every window
layout pass, so the pump steals it back before each commit), geometric
tap-target activation, NSScrollView-direct scrolling, contentsScale walk;
host-side RemoteCustomSidebarView (CALayerHost + input forwarding).
- cmux-sidebar-render-fixture + supervision tests (announce/ack/action/
crash-respawn/hang-discard through a real process boundary).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Render custom sidebars fully out-of-process in the app
CmuxMain grows a render-worker branch (re-exec-self with
--cmux-sidebar-render-worker, before any app startup), and the sidebar
mounts RemoteCustomSidebarView: interpretation and rendering both happen in
the supervised worker; the host only displays the worker's remote layer,
forwards input, and dispatches returned ButtonActions. The stage-1
in-process interpreter path stays behind the SidebarInterpreting seam as
the fallback.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Address review: DocC on new public surface, localize sidebar strings in-package
CmuxSwiftRenderUI gains its own Localizable.xcstrings (en + ja) for the
sidebar.custom.* strings, including the previously unlocalized decode-error
descriptions flagged on manaflow-ai/cmux#5254;
String(localized:) call sites now read from .module. Also document the
SidebarTapTarget/SidebarTapTargetsKey public members and DSLDocument, and
annotate the render worker's -> Never backstop exit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix remote sidebar scroll pinning under the titlebar inset
The worker's scroll path clamped the clip origin to y >= 0, but with the
host's safe-area top inset the resting origin is -topInset — the first
scroll snapped content up under the traffic lights and the clamp never let
it return. Clamp through NSClipView.constrainBoundsRect instead, which
honors content insets and document bounds exactly like a real wheel event.
Harness-verified: scroll down reveals later rows, over-scrolling back up
settles at the inset rest position instead of pinning to the window top.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Keep the remote sidebar surface mounted across picker switches
Dropping the .id(fileURL) remount: the worker swaps files in place on the
next scene message, so remounting the surface only flashed the previous
sidebar's pixels (the fresh CALayerHost adopts the live context, which
still shows the old file for the IPC beat, and SwiftUI can overlap the
outgoing/incoming views). One surface, one hosted layer, content switches
in the worker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Adopt the live remote layer synchronously on sidebar surface mount
Dogfood (Codex-driven) flagged transient blank sidebars when cycling
providers: a remounting surface waited on the async subscribe round-trip
before adopting the worker's layer. RenderWorkerClient now mirrors the live
context id into a main-actor RemoteContextCache the surface reads in init,
so provider switches and sidebar toggles show the worker's last rendered
frame in the same frame the surface appears.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Make hot reload last-good sticky: broken saves never replace a working sidebar
A save now only replaces what's on screen when it actually interprets to a
view (or decodes, for JSON). Broken intermediate saves — editors writing
mid-edit states, atomic saves' transient missing file — keep the previous
working render, with the last GOOD source re-interpreted against fresh data
each tick so live values keep flowing while the file is broken. Error
states still show when a file is broken from the start (nothing good to
fall back to), and switching files resets the fallback. Same stickiness in
the in-process fallback path.
Harness-verified: valid -> garbage keeps rendering the valid version with
data ticks continuing; fixing the file swaps in the new content; a file
broken from first selection still shows the error state.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Show the Custom Sidebars beta toggle in Settings
The customSidebars.beta.enabled flag existed in the catalog and gated the
picker and rendering since manaflow-ai/cmux#5254,
but the Beta Features section never grew a row for it, so the only way to
flip it was a raw defaults write. Add the toggle (with search entry and
en/ja strings) mirroring the Extensions row.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix CI Swift 6.1 sendability in the reload-notification forwarder
CI's Xcode 16.4 rejects awaiting NotificationCenter's async stream from a
MainActor task (non-Sendable Notification crossing isolation in next()).
Consume the stream in a nonisolated task; only the Sendable names array
leaves it through the outbox. Also add the parameter docs Codex review
flagged on the public wire-type initializers.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Use a token observer for reload forwarding (CI Swift 6.1)
Task {} in a MainActor init inherits MainActor isolation, so the
nonisolated rewrite still tripped 6.1's non-Sendable Notification check on
the async stream's next(). Use the closure observer API instead — the same
pattern CustomSidebarModel ships, already proven on CI's toolchain.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Address CodeRabbit re-review: frame guards, watcher re-arm, worker reaping
- LengthPrefixedMessageChannel caps frames at 64 MB in both directions:
an inbound peer-controlled length beyond the cap reads as EOF instead of
an allocation, and an oversized outbound payload throws (regression
tests for both).
- The hot-reload watcher follows .swift/.json extension flips (reload
re-resolves the file by name; the kqueue watcher previously stayed on
the original path and went silently stale).
- SidebarTapTargetsKey moves to its own file (public major type).
- The render worker is terminated when its window closes (surface
observes NSWindow.willCloseNotification); provider switches and sidebar
toggles still keep it warm.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This branch was successfully deployed
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.
What
Adds user/agent-authored custom sidebars: a single SwiftUI-style view, interpreted at runtime (no toolchain, no signing, no compile step), discovered from
~/.config/cmux/sidebars/<name>.{swift,json}and selectable in the sidebar toggle button's right-click provider picker. It renders in the real sidebar area, hot-reloads on save, binds to a live workspace data context, and runs cmux commands on tap.Everything is gated by
betaFeatures.customSidebars(default off); while off, nothing changes in the app.How it's structured (mirrors CmuxSettings / CmuxSettingsUI)
Packages/CmuxSwiftRender(logic, pure, no SwiftUI, unit-tested):swift-syntaxparser + tree-walking interpreter for a growing Swift subset lowering to aRenderNodeIR. CoversText/VStack/HStack/ZStack/HSplitView/Button(string + label forms)/Image/shapes/Spacer/Divider;for/if/let/ForEach/ternary; member access, subscript, array + string methods; data binding andcmux()action capture.Packages/CmuxSwiftRenderUI(UI):RenderNodeto native SwiftUI (RenderNodeView, resizableHSplitView, JSON DSL renderer,CustomSidebarModel/CustomSidebarView). The cmux-coupled executor is injected viaSidebarActionDispatchthrough the SwiftUI environment, so the package has zeroTerminalControllerdependency.ContentViewadds the provider, the live data context (workspaces/tabs/clock), and aTimelineViewauto-refresh;makeCmuxSidebarActionDispatch()runs actions viaTerminalController; a smallrunV2CommandLineseam exposes the v2 dispatcher in-process.Notes for review
CmuxSwiftRender.docs/custom-sidebar-gap-report.mdis a multi-agent survey of 14 opinionated sidebar concepts plus the prioritized interpreter-feature gaps that drive the remaining roadmap (richer data provider, more actions,@State/inputs, popover for per-workspace notes).Sources/DSLSidebarPlayground.swift(legacy from the JSON-DSL spike); worth renaming.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds runtime‑interpreted custom sidebars (SwiftUI‑style or JSON) behind a beta flag. They hot‑reload, bind to rich workspace data, support actions and drag‑and‑drop reordering, respect titlebar insets, and now cache the parsed Swift program for smoother updates while JSON actions dispatch through the same sink.
New Features
customSidebars(default off). Sidebars are discovered in~/.config/cmux/sidebars/<name>.{swift,json}, selectable in the provider picker, and hot‑reload on save.cmux docs sidebarsopens the authoring guide.pinned,index,directory,ports/portCount,unread,tabs/tabCount,description,color,git {branch,dirty},pullRequest {number,label,url,status,stale,branch},progress {value,label}, latest agent message/prompt/timestamp,remote {target,state,connected}), plusselectedId,unreadTotal, andclock.weekday; absent fields are omitted.CmuxSwiftRender:swift-syntaxinterpreter toRenderNodeIR; supports user-definedfuncs (value and view, including explicitreturn),flatMap/reduce,.formatted(.currency/.notation/...)with real currency codes,Color(...),ScrollView,.strikethrough,.font(.system(...)), andopenURL(...).CmuxSwiftRenderUI: renders IR to native SwiftUI (resizable HSplit, JSON DSL, reorderable rows viaSidebarActionDispatch); host‑injected content insets keep content below the titlebar accessory and fade into the top mask.&&/||, guards integer/and%by zero (tests assert soft‑fail output), and honorssorted { ... }comparators via a stable insertion sort. Tests updated (34 passing).parse→ParsedProgram) avoids per‑tick re‑parsing; JSON DSL actions now route throughSidebarActionDispatch(log/openURL/cmux method) instead of being dropped.cmux docs sidebarsauthoring guide; corpus of 14 persona sidebars with a coverage harness. Minor cleanup: renamed the app‑side action sink toCmuxSidebarActionDispatch, removed a stale gap report, and splitRenderNode.swiftinto dedicated files (no behavior change).Migration
~/.config/cmux/sidebars/<name>.swiftor.json, then pick it from the sidebar button’s provider picker (right‑click). Usecmux docs sidebarsfor the authoring guide.Written for commit c87fdf7. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests