Repository navigation
Conversation
Phase 1 of the Aurean appearance system. Introduces a leaf package of pure, testable value types with no app-target coupling: - AureanColor: hex->sRGB parsing (3/6/8-digit, optional hash, case-insensitive), opacity projection, Color/NSColor bridges - AureanOpacity: 1/phi^n opacity ladder stops - AppearancePalette: protocol with 8 semantic roles + opacity helpers - AureanPaletteVariant: cool/dune/warm/obsidian enum - AureanPalette: token values, cool default, warn/crit fixed across variants - AureanMetrics: phi, Fibonacci, scale, motion, geometry, golden split 10 Swift Testing cases cover hex parsing/fallback, palette defaults, signal invariants, variant distinctness, opacity ladder, and golden split. All green via `swift test` (isolated, no app launch). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 2 of the Aurean appearance system. Adds the writable owner and the view-tree distribution seam, still inside the leaf package with no app-target coupling: - AureanTheme: @observable @mainactor owner of the active variant; resolves to a concrete palette and notifies observers on change. No shared/default instance — constructed at the app root and injected. - EnvironmentValues.aureanPalette: value-type palette in the environment, defaulting to the cool palette so views read sane colors even unthemed. - View.aureanTheme(_:): injects the owner and its resolved palette; reading the palette from the observable owner makes a variant flip re-skin the subtree. 6 Swift Testing cases cover default resolution, variant switching, observation firing (via confirmation), signal invariants surviving a switch, and the environment default/override. All 16 package tests green via `swift test`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 3 of the Aurean appearance system. Links the CmuxAppearance package to the cmux app target (mirroring the CmuxSettings local-package wiring across the six pbxproj entries) and injects the theme at the window root: - Construct AureanTheme once at launch (@State, defaults to cool) - Apply .aureanTheme(_:) on MainWindowBootstrapView so descendant chrome can read @Environment(\.aureanPalette) No visible change yet — this only establishes the seam. Verified the Debug app builds clean via `./scripts/reload.sh --tag aurean` (BUILD SUCCEEDED). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 4 of the Aurean appearance system. The central accent color (selection, focus rings, drop targets — every caller of cmuxAccentNSColor) now resolves from the active Aurean palette in dark mode, replacing the hardcoded blue. - Add AureanAppearanceSettings: reads the persisted palette variant from UserDefaults (defaults to cool), so ambient AppKit color helpers rendered outside the SwiftUI environment still track the chosen temperature. This is the same key the settings picker and root AureanTheme will write in Phase 6. - cmuxAccentNSColor(for:) returns the palette accent in dark mode; light mode keeps the original blue (the pale Aurean accent lacks contrast on light). Verified the Debug app builds clean via reload.sh --tag aurean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 5 of the Aurean appearance system. The window chrome (sidebar, titlebar, backdrop) mirrors the terminal background by design, so a single lever recolors the whole app: the terminal canvas now resolves from the active Aurean palette. - Seed GhosttyApp.defaultBackgroundColor with the active palette's surfacePrimary so the first paint matches the chrome that mirrors it. - In applyDefaultBackground, drive the stored background from the active Aurean surface while still honoring the user's Ghostty theme for foreground, cursor, and selection. Opacity/blur are preserved, so terminal transparency still works. This is a runtime, in-app override — it does not write the user's ghostty config, so it only affects this build. applyDefaultBackground runs on theme/appearance changes, not per keystroke, so the typing hot path is untouched. Verified the Debug app builds clean via reload.sh --tag aurean. Visual verification deferred: the screen was locked at capture time. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 6 of the Aurean appearance system. Lets the user pick the palette temperature (cool / dune / warm / obsidian) in Settings → App, persists it, and re-skins the whole app live. - CmuxSettingsUI depends on CmuxAppearance (acyclic: CmuxAppearance is a leaf). - AureanPaletteVariant: SettingCodable (free via String RawRepresentable) so a DefaultsKey can back the picker; key declared inline (the enum lives in CmuxAppearance, which the schema layer does not depend on). userDefaultsKey matches the app's ambient AureanAppearanceSettings reader. - AureanPalettePickerRow: presentational swatch picker mirroring ThemePickerRow, one tile per variant previewing surface + sand + signal dots. - AppSection wires a DefaultsValueModel<AureanPaletteVariant> below the Theme row. - GhosttyApp.reapplyAureanSurface(): forces the canvas to re-resolve from the new surface (reusing the current scope so precedence is preserved, forceNotify so every workspace + the key window re-skin). applyDefaultBackground already overrides the stored background with the active Aurean surface. - cmuxApp observes the persisted variant: seeds AureanTheme on launch and, on change, updates the SwiftUI theme owner and calls reapplyAureanSurface. Verified the Debug app builds clean via reload.sh --tag aurean. Visual/live-switch verification deferred (screen locked). New picker strings carry English defaultValues; xcstrings/Japanese entries are a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to Phase 5. The window-root backdrop previously inherited the terminal's background opacity, so a translucent terminal let the desktop wallpaper bleed through and diluted the Aurean canvas. Render the window-root backdrop as the opaque Aurean surface on the window host layer. A translucent terminal now composites over the palette canvas instead of the wallpaper, so the chrome reads solid Aurean while the terminal keeps its own background opacity (the "opaque chrome, transparent terminal" choice). Glass/blur styles still short-circuit to .clear, so this only affects the standard backdrop path. Verified by launching reload.sh --tag aurean and sampling the rendered window: canvas reads ~(26,28,28) ≈ Aurean cool surfacePrimary #161819, no warm wallpaper bleed; sidebar dark with the pale-blue Aurean accent on the selected workspace. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Completes the "opaque chrome" appearance: the sidebar backdrop was a translucent vibrancy material that let the desktop wallpaper bleed through. Resolve the sidebar material policy to a solid Aurean surfaceOff fill — material nil (no vibrancy is built) plus an opaque tint, which ContentView.backdrop paints as a flat Color over the backdrop layer. The sidebar now reads as part of the cohesive opaque Aurean chrome with no wallpaper bleed. (The sidebar list view still contributes its own slightly lighter background above this backdrop; theming that internal fill to exactly surfaceOff is a separate, more invasive change and is left as follow-up.) Verified by launching reload.sh --tag aurean: window reads as a uniform dark Aurean surface across terminal and sidebar, with the pale-blue Aurean accent on the selected workspace and no warm wallpaper bleed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the five user-facing strings introduced by the Aurean palette picker to
Localizable.xcstrings with English and Japanese, per the localization rule:
- settings.app.aureanPalette ("Palette" / パレット)
- aurean.palette.cool / dune / warm / obsidian (Cool/Dune/Warm/Obsidian with
katakana transliterations)
Only en + ja are provided (the documented required set); the other 18 languages
in the catalog fall back to the English source until translated. Verified the
app builds clean via reload.sh --tag aurean (xcstrings compiles).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@leosar is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds the Aurean appearance package, palette/theme types, settings UI, app wiring, runtime palette-driven appearance updates, tests, and documentation. ChangesAurean Protocol Theme System
Sequence Diagram(s)sequenceDiagram
participant User
participant AppSection
participant cmuxApp
participant GhosttyApp
participant Windowing
User->>AppSection: Select Aurean palette variant
AppSection->>cmuxApp: Persist aureanVariantRaw
cmuxApp->>cmuxApp: applyAureanVariant()
cmuxApp->>GhosttyApp: reapplyAureanSurface()
GhosttyApp->>Windowing: applyDefaultBackground()
GhosttyApp->>Windowing: applyBackgroundToKeyWindow()
Windowing->>Windowing: Resolve Aurean backdrop and terminal policies
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR introduces the Aurean φ-based appearance system end-to-end: a new
Confidence Score: 5/5Safe to merge; the new package is well-isolated with 16 passing tests, dark-mode gating is consistently applied, and no production code paths are changed outside the appearance system. All pre-existing review concerns (actor isolation on AureanAppearanceSettings, UserDefaults key duplication, dual source-of-truth, appKitMutationID staleness, missing locale translations) were surfaced in prior review threads. No new correctness or data-integrity issues were found in this pass. The CmuxAppearance package is a clean leaf with appropriate Sendable/Hashable/Codable conformances, and the app-side integration is dark-mode-gated throughout. No files require special attention beyond what was already flagged in prior threads. The integration files (SidebarAppearanceSupport.swift, GhosttyTerminalView.swift, cmuxApp.swift) carry the known actor-isolation and key-duplication concerns from those earlier discussions. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AureanPalettePickerRow
participant DefaultsValueModel
participant UserDefaults
participant cmuxApp_AppStorage as cmuxApp @AppStorage
participant applyAureanVariant
participant AureanTheme
participant GhosttyApp
User->>AureanPalettePickerRow: selects variant (e.g. warm)
AureanPalettePickerRow->>DefaultsValueModel: aureanPalette.set(.warm)
DefaultsValueModel->>UserDefaults: "write aureanPaletteVariant = warm"
UserDefaults-->>cmuxApp_AppStorage: onChange(aureanVariantRaw)
cmuxApp_AppStorage->>applyAureanVariant: applyAureanVariant()
applyAureanVariant->>AureanTheme: "variant = .warm"
Note over AureanTheme: @Observable fires — SwiftUI subtree re-renders via @Environment(\.aureanPalette)
applyAureanVariant->>GhosttyApp: reapplyAureanSurface()
GhosttyApp->>UserDefaults: AureanAppearanceSettings.activePalette (reads new variant)
GhosttyApp->>GhosttyApp: applyDefaultBackground(aureanBackground: warm.surfacePrimary)
GhosttyApp->>GhosttyApp: applyBackgroundToKeyWindow()
Note over GhosttyApp: AppKit chrome re-skins to warm palette
Reviews (5): Last reviewed commit: "Default pane splits to the golden ratio ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+5
to
90
| "settings.app.aureanPalette": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Palette" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "パレット" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "aurean.palette.cool": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Cool" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "クール" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "aurean.palette.dune": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Dune" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "デューン" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "aurean.palette.warm": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Warm" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "ウォーム" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "aurean.palette.obsidian": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Obsidian" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "オブシディアン" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.error.alert.dismiss": { |
Contributor
There was a problem hiding this comment.
The catalog already carries translations in ar, bs, da, de, es, fr, it, km, ko, nb, pl, pt-BR, ru, th, tr, and uk for every other string. All five new keys (settings.app.aureanPalette, aurean.palette.cool, aurean.palette.dune, aurean.palette.warm, aurean.palette.obsidian) only ship en and ja. Any user whose device is set to one of those 16 locales will see raw English labels in the picker.
Rule Used: Flag production user-facing text that is not fully... (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!
| @StateObject private var sidebarState = SidebarState() | ||
| @StateObject private var keyboardShortcutSettingsObserver = KeyboardShortcutSettingsObserver.shared | ||
| @AppStorage(AppearanceSettings.appearanceModeKey) private var appearanceMode = AppearanceSettings.defaultMode.rawValue | ||
| @AppStorage("aureanPaletteVariant") private var aureanVariantRaw = AureanPaletteVariant.cool.rawValue |
Contributor
There was a problem hiding this comment.
"aureanPaletteVariant" is duplicated as a string literal in three places: here in cmuxApp.swift, in AppSection.swift's inline DefaultsKey, and in AureanAppearanceSettings.userDefaultsKey (which defines the canonical constant but is unused at both call sites). If the constant's value is ever updated, @AppStorage here will silently read a different key and the persisted palette will stop loading.
Suggested change
| @AppStorage("aureanPaletteVariant") private var aureanVariantRaw = AureanPaletteVariant.cool.rawValue | |
| @AppStorage(AureanAppearanceSettings.userDefaultsKey) private var aureanVariantRaw = AureanPaletteVariant.cool.rawValue |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Windowing/WindowAppearanceSnapshot.swift (1)
315-328: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
terminalBackdropPolicyignores snapshot fields.The method now hard-wires
opacity: 1andrenderingMode: .windowHostBackdrop, ignoring the snapshot'sterminalBackgroundOpacityandterminalRenderingModefields (lines 208, 210). While the Aurean opaque chrome design is clearly documented in the comment (lines 319-322), this creates a mismatch where:
- The snapshot still stores and computes these fields (lines 234, 236-238)
WindowBackdropController.backdropPlan(context snippet) checksterminalBackgroundOpacity < 0.999for hosting phase decisions- But
terminalBackdropPolicy()always returnsopacity: 1Consider either:
- Documenting in the snapshot struct that
terminalBackgroundOpacityis used only for hosting phase decisions, not the actual policy- Or refactoring to make the opaque-chrome override explicit at the call site rather than inside
terminalBackdropPolicy()🤖 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/Windowing/WindowAppearanceSnapshot.swift` around lines 315 - 328, The terminalBackdropPolicy() currently hard-codes opacity: 1 and renderingMode: .windowHostBackdrop, which ignores the snapshot's terminalBackgroundOpacity and terminalRenderingMode and causes a mismatch with WindowBackdropController.backdropPlan; update terminalBackdropPolicy() to use the snapshot fields (replace opacity: 1 with terminalBackgroundOpacity and renderingMode: .windowHostBackdrop with terminalRenderingMode) so the returned WindowBackdropPolicy reflects the snapshot, or alternatively move the Aurean "opaque chrome" override out of terminalBackdropPolicy() and make it an explicit decision at the call site (e.g., in WindowBackdropController.backdropPlan) while documenting that terminalBackgroundOpacity and terminalRenderingMode are used for the final policy when not overridden.
🤖 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/CmuxAppearance/Sources/CmuxAppearance/AureanMetrics.swift`:
- Around line 23-36: Add Swift-DocC triple-slash comments to each public static
constant in the Spacing struct (Spacing.s1 through Spacing.s233) in
AureanMetrics.swift: for every public static let (s1, s2, s3, s5, s8, s13, s21,
s34, s55, s89, s144, s233) add a one-line /// summary documenting the spacing
value or role (e.g., "/// 8pt spacing — small gap" or "/// Fibonacci spacing:
21pt") immediately above the declaration so every public symbol in Spacing is
documented per package guidelines.
In `@Packages/CmuxAppearance/Sources/CmuxAppearance/AureanPalette.swift`:
- Around line 15-22: The public stored properties surfacePrimary, surfaceOff,
surfaceAbyssal, text, accent, ok, warn, and crit on AureanPalette are missing
Swift-DocC triple-slash comments; add concise /// comments above each public
property in AureanPalette.swift (similar to the existing documentation for
variant) that describe the role or intended use of each AureanColor (e.g.,
primary surface, secondary/off surface, abyssal/background, primary text, accent
color, success/ok, warning, critical/error) so every public symbol is documented
per package guidelines.
In `@Sources/Windowing/WindowAppearanceSnapshot.swift`:
- Line 121: AureanAppearanceSettings.activePalette.surfaceOff is performing a
UserDefaults lookup on every access and is being used inside the hot-path
computed property materialPolicy in WindowAppearanceSnapshot; change the code to
resolve and cache the palette once outside the per-frame/per-row path (e.g.
capture let resolvedPalette = AureanAppearanceSettings.activePalette or cache
surfaceOff value) and then use resolvedPalette.surfaceOff.nsColor for tintColor
inside materialPolicy/backdrop construction, or modify activePalette to memoize
the UserDefaults lookup so repeated accesses (from
materialPolicy/WindowAppearanceSnapshot) do not call UserDefaults repeatedly.
- Around line 109-126: The appKitMutationID currently incorporates fields that
materialPolicy no longer uses (materialRawValue, tintHex, tintHexLight,
tintHexDark, tintOpacity, blurOpacity), causing needless backdrop invalidations;
update the appKitMutationID computation (the var appKitMutationID) to only
include the identity-relevant values that materialPolicy actually uses—e.g.,
blendModeRawValue, stateRawValue and cornerRadius (and any other fields still
read by SidebarBackdropMaterialPolicy) —or add a concise comment documenting why
those unused fields must remain if you choose not to remove them.
---
Outside diff comments:
In `@Sources/Windowing/WindowAppearanceSnapshot.swift`:
- Around line 315-328: The terminalBackdropPolicy() currently hard-codes
opacity: 1 and renderingMode: .windowHostBackdrop, which ignores the snapshot's
terminalBackgroundOpacity and terminalRenderingMode and causes a mismatch with
WindowBackdropController.backdropPlan; update terminalBackdropPolicy() to use
the snapshot fields (replace opacity: 1 with terminalBackgroundOpacity and
renderingMode: .windowHostBackdrop with terminalRenderingMode) so the returned
WindowBackdropPolicy reflects the snapshot, or alternatively move the Aurean
"opaque chrome" override out of terminalBackdropPolicy() and make it an explicit
decision at the call site (e.g., in WindowBackdropController.backdropPlan) while
documenting that terminalBackgroundOpacity and terminalRenderingMode are used
for the final policy when not overridden.
🪄 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: fc255879-fbe7-498e-930a-eec48fe900db
📒 Files selected for processing (22)
Packages/CmuxAppearance/Package.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AppearancePalette.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanColor.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanMetrics.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanOpacity.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanPalette.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanPaletteVariant.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanTheme.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/EnvironmentValues+AureanPalette.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/View+AureanTheme.swiftPackages/CmuxAppearance/Tests/CmuxAppearanceTests/AureanPaletteTests.swiftPackages/CmuxAppearance/Tests/CmuxAppearanceTests/AureanThemeTests.swiftPackages/CmuxSettingsUI/Package.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/AureanPalettePickerRow.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/AureanPaletteVariant+SettingCodable.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swiftResources/Localizable.xcstringsSources/GhosttyTerminalView.swiftSources/Sidebar/SidebarAppearanceSupport.swiftSources/Windowing/WindowAppearanceSnapshot.swiftSources/cmuxApp.swiftcmux.xcodeproj/project.pbxproj
👮 Files not reviewed due to content moderation or server errors (1)
- Sources/GhosttyTerminalView.swift
Comment on lines
+23
to
+36
| public struct Spacing: Sendable { | ||
| public static let s1: CGFloat = 1 | ||
| public static let s2: CGFloat = 2 | ||
| public static let s3: CGFloat = 3 | ||
| public static let s5: CGFloat = 5 | ||
| public static let s8: CGFloat = 8 | ||
| public static let s13: CGFloat = 13 | ||
| public static let s21: CGFloat = 21 | ||
| public static let s34: CGFloat = 34 | ||
| public static let s55: CGFloat = 55 | ||
| public static let s89: CGFloat = 89 | ||
| public static let s144: CGFloat = 144 | ||
| public static let s233: CGFloat = 233 | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add DocC comments to the Spacing ladder constants.
The Spacing struct is documented, but each public static constant (s1…s233) is undocumented. As per coding guidelines, "Every public symbol in any new Swift package under Packages/ is documented with a Swift-DocC triple-slash comment (///) at the time of writing". A one-line summary per rung (e.g. the corresponding Fibonacci point value/role) satisfies this.
🤖 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/CmuxAppearance/Sources/CmuxAppearance/AureanMetrics.swift` around
lines 23 - 36, Add Swift-DocC triple-slash comments to each public static
constant in the Spacing struct (Spacing.s1 through Spacing.s233) in
AureanMetrics.swift: for every public static let (s1, s2, s3, s5, s8, s13, s21,
s34, s55, s89, s144, s233) add a one-line /// summary documenting the spacing
value or role (e.g., "/// 8pt spacing — small gap" or "/// Fibonacci spacing:
21pt") immediately above the declaration so every public symbol in Spacing is
documented per package guidelines.
Comment on lines
+15
to
+22
| public let surfacePrimary: AureanColor | ||
| public let surfaceOff: AureanColor | ||
| public let surfaceAbyssal: AureanColor | ||
| public let text: AureanColor | ||
| public let accent: AureanColor | ||
| public let ok: AureanColor | ||
| public let warn: AureanColor | ||
| public let crit: AureanColor |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Public stored properties are missing DocC comments.
surfacePrimary…crit are public symbols without /// comments (only variant is documented). Although they satisfy AppearancePalette requirements, the package documentation rule applies at the declaration site. As per coding guidelines, "Every public symbol in any new Swift package under Packages/ is documented with a Swift-DocC triple-slash comment (///) at the time of writing".
🤖 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/CmuxAppearance/Sources/CmuxAppearance/AureanPalette.swift` around
lines 15 - 22, The public stored properties surfacePrimary, surfaceOff,
surfaceAbyssal, text, accent, ok, warn, and crit on AureanPalette are missing
Swift-DocC triple-slash comments; add concise /// comments above each public
property in AureanPalette.swift (similar to the existing documentation for
variant) that describe the role or intended use of each AureanColor (e.g.,
primary surface, secondary/off surface, abyssal/background, primary text, accent
color, success/ok, warning, critical/error) so every public symbol is documented
per package guidelines.
| opacity: blurOpacity, | ||
| tintColor: tintColor, | ||
| opacity: 1, | ||
| tintColor: AureanAppearanceSettings.activePalette.surfaceOff.nsColor, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
UserDefaults lookup in computed property.
Same concern as SidebarAppearanceSupport.swift line 17-23: AureanAppearanceSettings.activePalette.surfaceOff performs a UserDefaults.standard.string(forKey:) lookup on every access. Since materialPolicy is a computed property accessed during backdrop plan construction, verify this is not called in hot rendering paths.
Based on coding guidelines: avoid repeated work in frequently re-rendered UI paths; consider caching the resolved palette if this computed property is accessed per-frame or per-row.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Windowing/WindowAppearanceSnapshot.swift` at line 121,
AureanAppearanceSettings.activePalette.surfaceOff is performing a UserDefaults
lookup on every access and is being used inside the hot-path computed property
materialPolicy in WindowAppearanceSnapshot; change the code to resolve and cache
the palette once outside the per-frame/per-row path (e.g. capture let
resolvedPalette = AureanAppearanceSettings.activePalette or cache surfaceOff
value) and then use resolvedPalette.surfaceOff.nsColor for tintColor inside
materialPolicy/backdrop construction, or modify activePalette to memoize the
UserDefaults lookup so repeated accesses (from
materialPolicy/WindowAppearanceSnapshot) do not call UserDefaults repeatedly.
Self-review pass on the branch before merge: 1. Light mode (HIGH): the Aurean surface override was unconditional, so in light mode it forced a dark canvas/chrome under light-mode foreground text (unreadable) and was inconsistent with the light-aware accent. Gate all surface overrides on AureanAppearanceSettings.isActiveForCurrentAppearance (dark only); light mode keeps the user's Ghostty theme, opacity, rendering mode, and sidebar material. This also restores honoring OSC 11 / dynamic terminal backgrounds in light mode. 2. Sidebar stale on variant switch (MED): SidebarBackdropSettingsSnapshot .appKitMutationID omitted the active variant, so the AppKit backdrop could stay stale after a palette switch. Fold the active variant and the appearance-active flag into the id. 3. OSC 11 in dark mode is intentionally owned by the Aurean canvas (the headline feature); the light-mode guard above restores normal behavior where Aurean stands down. 4. Hot-path allocation: AureanPaletteVariant.palette now returns a shared cached value instead of re-parsing tokens on every accent/drop-target read. 5. Dead stored props: resolved by (1) — the light-mode branch uses tintHex/ tintOpacity/blurOpacity/material again. 6. Single source of truth: the UserDefaults key now lives once as AureanPaletteVariant.userDefaultsKey, referenced by the ambient settings, the @AppStorage binding, and the settings DefaultsKey. 7. Redundant variant seeding: AureanTheme is seeded from the persisted variant at construction, dropping the duplicate onAppear assignment. Verified: CmuxAppearance 16/16 tests green; app builds clean via reload.sh --tag aurean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| // Persisted selection of the active Aurean palette temperature. The settings UI and the | ||
| // root `AureanTheme` write `userDefaultsKey`; ambient AppKit color helpers read it here so | ||
| // chrome rendered outside the SwiftUI environment still tracks the chosen variant. | ||
| enum AureanAppearanceSettings { |
Contributor
There was a problem hiding this comment.
isActiveForCurrentAppearance reads NSApp?.effectiveAppearance, which is a @MainActor-isolated property of NSApplication. Because AureanAppearanceSettings carries no actor annotation, this static is callable from any isolation domain. In Swift 6 strict concurrency this is a compile error; in older modes it is a silent AppKit data race if ever reached off the main thread. The fix is to mark the whole helper enum @MainActor, which matches every confirmed call site (applyDefaultBackground, materialPolicy, the GhosttyApp property initializer).
Suggested change
| enum AureanAppearanceSettings { | |
| @MainActor | |
| enum AureanAppearanceSettings { |
Rule Used: Flag new or materially worsened Swift 6 actor isol... (source)
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/GhosttyTerminalView.swift (1)
4098-4107:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReapply the Aurean backdrop to every open main window.
reapplyAureanSurface()only callsapplyBackgroundToKeyWindow(), and that path resolves a singlecmux.main*window viaactiveMainWindow(). After an app-wide palette change, any other open main windows keep the old chrome until they become key.♻️ Proposed fix
`@MainActor` func reapplyAureanSurface(source: String = "aurean.variantChanged") { applyDefaultBackground( color: defaultBackgroundColor, opacity: defaultBackgroundOpacity, backgroundBlur: defaultBackgroundBlur, source: source, scope: defaultBackgroundUpdateScope, forceNotify: true ) - applyBackgroundToKeyWindow() + applyBackgroundToAllMainWindows() } + +private func applyBackgroundToAllMainWindows() { + let snapshot = WindowAppearanceSnapshot.currentFromUserDefaults(app: self) + let plan = snapshot.backdropPlan() + for window in NSApp.windows { + guard let raw = window.identifier?.rawValue, + raw == "cmux.main" || raw.hasPrefix("cmux.main.") else { continue } + _ = WindowBackdropController.apply(plan: plan, to: window) + } +}🤖 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/GhosttyTerminalView.swift` around lines 4098 - 4107, reapplyAureanSurface currently only updates the key window by calling applyBackgroundToKeyWindow which uses activeMainWindow(), so other open main windows keep the old chrome; modify reapplyAureanSurface to iterate all app main windows (e.g., via NSApp.windows or the app's window registry), identify each cmux.main* / main window instead of a single activeMainWindow(), and call the existing background-application logic for each window (or introduce a helper like applyBackgroundToWindow(window:) and call it for every matching window) so the new Aurean palette is applied to every open main window immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/cmuxApp.swift`:
- Line 33: When reading the stored palette string into aureanVariantRaw and
mapping it to AureanPaletteVariant, detect when the raw value does not
correspond to any case and replace it with the fallback
(AureanPaletteVariant.cool) and write that normalized rawValue back into
UserDefaults so the invalid string is removed; update the same logic wherever
similar fallback mapping occurs (the other AureanPaletteVariant deserialization
sites) to ensure the picker always has a matching selection and bad values are
normalized.
- Around line 18-25: Only the main WindowGroup gets the
`.aureanTheme(aureanTheme)` modifier, so Settings and Config scenes won't
update; wrap all scenes in a single container and apply the modifier there.
Modify the App body to put WindowGroup, Settings, and Config inside a shared
Group (or top-level container) and move `.aureanTheme(aureanTheme)` onto that
Group so every Scene inherits the same AureanTheme (keep the existing `@State`
private var aureanTheme and the `.aureanTheme(aureanTheme)` call but attached to
the shared Group rather than just the main WindowGroup).
In `@Sources/Windowing/WindowAppearanceSnapshot.swift`:
- Around line 117-128: The snapshot currently calls
AureanAppearanceSettings.isActiveForCurrentAppearance (which reads NSApp) inside
SidebarBackdropMaterialPolicy creation, causing nondeterminism; change it to
derive activation from the captured colorScheme instead (e.g. call an API such
as AureanAppearanceSettings.isActive(for: colorScheme) or otherwise evaluate the
same activation logic using the WindowAppearanceSnapshot.colorScheme) so the
Sidebar backdrop path in WindowAppearanceSnapshot (where
SidebarBackdropMaterialPolicy is returned) is consistent with the earlier
resolvedHex decisions; apply the identical change to terminalBackdropPolicy() so
both policies use the snapshot's colorScheme rather than live NSApp state.
---
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 4098-4107: reapplyAureanSurface currently only updates the key
window by calling applyBackgroundToKeyWindow which uses activeMainWindow(), so
other open main windows keep the old chrome; modify reapplyAureanSurface to
iterate all app main windows (e.g., via NSApp.windows or the app's window
registry), identify each cmux.main* / main window instead of a single
activeMainWindow(), and call the existing background-application logic for each
window (or introduce a helper like applyBackgroundToWindow(window:) and call it
for every matching window) so the new Aurean palette is applied to every open
main window immediately.
🪄 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: 6e043378-3309-44f9-acb1-9844706c358a
📒 Files selected for processing (5)
Packages/CmuxAppearance/Sources/CmuxAppearance/AureanPaletteVariant.swiftSources/GhosttyTerminalView.swiftSources/Sidebar/SidebarAppearanceSupport.swiftSources/Windowing/WindowAppearanceSnapshot.swiftSources/cmuxApp.swift
Comment on lines
+18
to
+25
| /// Owner of the active Aurean appearance. Created once at launch (seeded from the | ||
| /// persisted variant) and injected into the window's environment via `.aureanTheme(_:)`; | ||
| /// descendant chrome reads colors through `@Environment(\.aureanPalette)`. | ||
| @State private var aureanTheme = AureanTheme( | ||
| variant: AureanPaletteVariant( | ||
| rawValue: UserDefaults.standard.string(forKey: AureanPaletteVariant.userDefaultsKey) ?? "" | ||
| ) ?? .cool | ||
| ) |
There was a problem hiding this comment.
Propagate the shared Aurean theme to every app scene.
This creates a single app-scoped AureanTheme, but only the main WindowGroup receives .aureanTheme(aureanTheme). The Settings and Config scenes in this file will stay on the default/stale palette, so changing the palette from Settings will not live-update those windows.
💡 Suggested fix
Window(String(localized: "settings.title", defaultValue: "Settings"), id: SettingsWindowPresenter.windowID) {
SettingsWindowRoot(runtime: settingsRuntime)
.settingsRuntime(settingsRuntime)
+ .aureanTheme(aureanTheme)
.background(WindowAccessor(dedupeByWindow: false) { window in
SettingsWindowPresenter.configure(window: window)
})
.cmuxAppearanceColorScheme(appearanceMode)
}
@@
Window(String(localized: "settings.config.windowTitle", defaultValue: "Config"), id: ConfigSettingsView.windowID) {
ConfigSettingsView()
.settingsRuntime(settingsRuntime)
+ .aureanTheme(aureanTheme)
.cmuxAppearanceColorScheme(appearanceMode)
}Also applies to: 279-281
🤖 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/cmuxApp.swift` around lines 18 - 25, Only the main WindowGroup gets
the `.aureanTheme(aureanTheme)` modifier, so Settings and Config scenes won't
update; wrap all scenes in a single container and apply the modifier there.
Modify the App body to put WindowGroup, Settings, and Config inside a shared
Group (or top-level container) and move `.aureanTheme(aureanTheme)` onto that
Group so every Scene inherits the same AureanTheme (keep the existing `@State`
private var aureanTheme and the `.aureanTheme(aureanTheme)` call but attached to
the shared Group rather than just the main WindowGroup).
| @StateObject private var sidebarState = SidebarState() | ||
| @StateObject private var keyboardShortcutSettingsObserver = KeyboardShortcutSettingsObserver.shared | ||
| @AppStorage(AppearanceSettings.appearanceModeKey) private var appearanceMode = AppearanceSettings.defaultMode.rawValue | ||
| @AppStorage(AureanPaletteVariant.userDefaultsKey) private var aureanVariantRaw = AureanPaletteVariant.cool.rawValue |
There was a problem hiding this comment.
Normalize invalid stored palette values when falling back.
If aureanVariantRaw contains an unknown value, the theme falls back to .cool, but the bad raw string stays in UserDefaults. That can leave the picker without a matching selection and reintroduce the invalid value on the next launch.
💡 Suggested fix
private func applyAureanVariant() {
- aureanTheme.variant = AureanPaletteVariant(rawValue: aureanVariantRaw) ?? .cool
+ let resolved = AureanPaletteVariant(rawValue: aureanVariantRaw) ?? .cool
+ if aureanVariantRaw != resolved.rawValue {
+ aureanVariantRaw = resolved.rawValue
+ }
+ aureanTheme.variant = resolved
GhosttyApp.shared.reapplyAureanSurface()
}Also applies to: 279-281
🤖 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/cmuxApp.swift` at line 33, When reading the stored palette string
into aureanVariantRaw and mapping it to AureanPaletteVariant, detect when the
raw value does not correspond to any case and replace it with the fallback
(AureanPaletteVariant.cool) and write that normalized rawValue back into
UserDefaults so the invalid string is removed; update the same logic wherever
similar fallback mapping occurs (the other AureanPaletteVariant deserialization
sites) to ensure the picker always has a matching selection and bad values are
normalized.
Comment on lines
+117
to
+128
| if AureanAppearanceSettings.isActiveForCurrentAppearance { | ||
| return SidebarBackdropMaterialPolicy( | ||
| material: nil, | ||
| blendingMode: blendingMode, | ||
| state: state, | ||
| opacity: 1, | ||
| tintColor: AureanAppearanceSettings.activePalette.surfaceOff.nsColor, | ||
| cornerRadius: CGFloat(max(0, cornerRadius)), | ||
| preferLiquidGlass: false, | ||
| usesWindowLevelGlass: false | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# How colorScheme is supplied to WindowAppearanceSnapshot.current(...)
rg -nP --type=swift -C3 'WindowAppearanceSnapshot\.current\('
rg -nP --type=swift -C3 'isActiveForCurrentAppearance'Repository: manaflow-ai/cmux
Length of output: 5393
🏁 Script executed:
cd /repo && wc -l Sources/Windowing/WindowAppearanceSnapshot.swiftRepository: manaflow-ai/cmux
Length of output: 116
🏁 Script executed:
# Get the full context of WindowAppearanceSnapshot class definition and constructor
sed -n '1,80p' Sources/Windowing/WindowAppearanceSnapshot.swiftRepository: manaflow-ai/cmux
Length of output: 1874
🏁 Script executed:
# Get lines 110-175 to see both sidebarBackdropPolicy and the mutation id usage
sed -n '110,175p' Sources/Windowing/WindowAppearanceSnapshot.swiftRepository: manaflow-ai/cmux
Length of output: 3191
🏁 Script executed:
# Get lines 132-145 to see the resolvedHex decision mentioned in the review
sed -n '130,145p' Sources/Windowing/WindowAppearanceSnapshot.swiftRepository: manaflow-ai/cmux
Length of output: 845
🏁 Script executed:
# Get lines 350-365 to see terminalBackdropPolicy usage
sed -n '350,365p' Sources/Windowing/WindowAppearanceSnapshot.swiftRepository: manaflow-ai/cmux
Length of output: 909
🏁 Script executed:
# Check if there's an init or current method that seeds colorScheme
rg -nP --type=swift -A20 'static func current' Sources/Windowing/WindowAppearanceSnapshot.swift | head -40Repository: manaflow-ai/cmux
Length of output: 933
🏁 Script executed:
# Check AppearanceSettings.colorScheme to see how it's derived from effectiveAppearance
rg -nP --type=swift -A15 'func colorScheme.*appearanceMode' Sources/Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Check the ColorScheme enum and how it's defined
rg -nP --type=swift 'enum ColorScheme' Sources/ -A5Repository: manaflow-ai/cmux
Length of output: 419
🏁 Script executed:
# Verify how the colorScheme parameter flows from ContentView to the snapshot
rg -nP --type=swift -B5 -A5 'AppearanceSettings.colorScheme' Sources/ContentView.swift | head -30Repository: manaflow-ai/cmux
Length of output: 756
🏁 Script executed:
# Find AppearanceSettings and the colorScheme function
rg -nP --type=swift 'AppearanceSettings' Sources/ | grep -E '(class|struct|enum) AppearanceSettings'Repository: manaflow-ai/cmux
Length of output: 212
🏁 Script executed:
# Search for the colorScheme function definition in AppearanceSettings
rg -nP --type=swift -A10 'static.*colorScheme.*appearanceMode|func colorScheme' Sources/Repository: manaflow-ai/cmux
Length of output: 2465
🏁 Script executed:
# Look for ColorScheme type definition (from SwiftUI, it's typically imported)
grep -r 'ColorScheme' Sources/Windowing/WindowAppearanceSnapshot.swift | head -5Repository: manaflow-ai/cmux
Length of output: 289
🏁 Script executed:
# Check if ColorScheme in WindowAppearanceSnapshot is SwiftUI.ColorScheme
sed -n '1,30p' Sources/Windowing/WindowAppearanceSnapshot.swift | grep -E 'import|ColorScheme'Repository: manaflow-ai/cmux
Length of output: 111
🏁 Script executed:
# Check how the snapshot is used and if it's recreated when appearance changes
rg -nP --type=swift -B5 -A10 'windowAppearanceSnapshot' Sources/ContentView.swift | head -50Repository: manaflow-ai/cmux
Length of output: 1757
🏁 Script executed:
# Check what triggers the windowAppearanceSnapshot property to recompute
rg -nP --type=swift 'titlebarThemeGeneration|appearanceMode' Sources/ContentView.swift | head -20Repository: manaflow-ai/cmux
Length of output: 1036
🏁 Script executed:
# Look at the actual property definition and dependencies
sed -n '2270,2285p' Sources/ContentView.swiftRepository: manaflow-ai/cmux
Length of output: 919
🏁 Script executed:
# Check if AureanAppearanceSettings.isActiveForCurrentAppearance is truly reading live state every time
sed -n '28,35p' Sources/Sidebar/SidebarAppearanceSupport.swiftRepository: manaflow-ai/cmux
Length of output: 470
🏁 Script executed:
# Check what the colorScheme fallback parameter is in ContentView
rg -nP --type=swift -B10 'windowAppearanceSnapshot.*colorScheme.*fallback' Sources/ContentView.swift | head -30Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Look for where colorScheme is defined in ContentView to understand the fallback
rg -nP --type=swift 'var colorScheme.*{|`@Environment`.*colorScheme' Sources/ContentView.swiftRepository: manaflow-ai/cmux
Length of output: 183
🏁 Script executed:
# Check the full context of how appearanceMode is used and when it updates
rg -nP --type=swift 'appearanceMode.*=|appearanceMode.*@' Sources/ContentView.swift | head -20Repository: manaflow-ai/cmux
Length of output: 188
🏁 Script executed:
# Check how Aurean should activate based on colorScheme vs effectiveAppearance
rg -nP --type=swift 'AureanAppearanceSettings' Sources/ | grep -E '(init|setup|activate)' | head -20Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Look for any tests or documentation about Aurean appearance synchronization
find . -type f \( -name '*Test*.swift' -o -name '*.md' \) | xargs grep -l 'Aurean' 2>/dev/null | head -5Repository: manaflow-ai/cmux
Length of output: 212
🏁 Script executed:
# Check if colorScheme is always in sync with NSApp effectiveAppearance when not overridden
rg -nP --type=swift 'effectiveAppearance|colorScheme' Sources/AppearanceSettings.swift | head -30Repository: manaflow-ai/cmux
Length of output: 733
🏁 Script executed:
# Check when titlebarThemeGeneration is incremented and what triggers it
sed -n '2468,2480p' Sources/ContentView.swiftRepository: manaflow-ai/cmux
Length of output: 891
🏁 Script executed:
# Check the cmuxAppearanceColorScheme extension to understand appearance setup
rg -nP --type=swift 'cmuxAppearanceColorScheme' Sources/Repository: manaflow-ai/cmux
Length of output: 618
🏁 Script executed:
# Look for cases where appearance might not sync properly
rg -nP --type=swift -B3 -A3 'effectiveAppearance' Sources/ | grep -v node_modules | head -50Repository: manaflow-ai/cmux
Length of output: 3613
🏁 Script executed:
# Check if colorScheme and isActiveForCurrentAppearance should always agree
rg -nP --type=swift 'appearanceMode.*auto|appearanceMode.*system' Sources/AppearanceSettings.swiftRepository: manaflow-ai/cmux
Length of output: 42
Snapshot determinism: activation reads live NSApp instead of the captured colorScheme.
isActiveForCurrentAppearance resolves against NSApp?.effectiveAppearance (live global) while this snapshot captures and carries colorScheme as a parameter. The same effect repeats in terminalBackdropPolicy() (line 356). Two consequences:
- A
WindowAppearanceSnapshotvalue can return different results if re-evaluated, since the liveNSAppstate may change between accesses. - The Aurean activation decision (live
NSApp) and theresolvedHexdecision (capturedcolorScheme, lines 132–138) can disagree during appearance transitions.
Since the snapshot already captures the appearance via colorScheme, derive activation from it to keep the snapshot internally consistent.
Suggested direction
- if AureanAppearanceSettings.isActiveForCurrentAppearance {
+ if colorScheme == .dark {Apply the same fix to terminalBackdropPolicy() at line 356.
📝 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
| if AureanAppearanceSettings.isActiveForCurrentAppearance { | |
| return SidebarBackdropMaterialPolicy( | |
| material: nil, | |
| blendingMode: blendingMode, | |
| state: state, | |
| opacity: 1, | |
| tintColor: AureanAppearanceSettings.activePalette.surfaceOff.nsColor, | |
| cornerRadius: CGFloat(max(0, cornerRadius)), | |
| preferLiquidGlass: false, | |
| usesWindowLevelGlass: false | |
| ) | |
| } | |
| if colorScheme == .dark { | |
| return SidebarBackdropMaterialPolicy( | |
| material: nil, | |
| blendingMode: blendingMode, | |
| state: state, | |
| opacity: 1, | |
| tintColor: AureanAppearanceSettings.activePalette.surfaceOff.nsColor, | |
| cornerRadius: CGFloat(max(0, cornerRadius)), | |
| preferLiquidGlass: false, | |
| usesWindowLevelGlass: false | |
| ) | |
| } |
🤖 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/Windowing/WindowAppearanceSnapshot.swift` around lines 117 - 128, The
snapshot currently calls AureanAppearanceSettings.isActiveForCurrentAppearance
(which reads NSApp) inside SidebarBackdropMaterialPolicy creation, causing
nondeterminism; change it to derive activation from the captured colorScheme
instead (e.g. call an API such as AureanAppearanceSettings.isActive(for:
colorScheme) or otherwise evaluate the same activation logic using the
WindowAppearanceSnapshot.colorScheme) so the Sidebar backdrop path in
WindowAppearanceSnapshot (where SidebarBackdropMaterialPolicy is returned) is
consistent with the earlier resolvedHex decisions; apply the identical change to
terminalBackdropPolicy() so both policies use the snapshot's colorScheme rather
than live NSApp state.
- CHANGELOG: add an Unreleased entry for the Aurean appearance (manaflow-ai#5050). - docs/aurean-appearance.md: how to use the palette picker, the light-mode and terminal-transparency behavior, the CmuxAppearance package architecture, and how the theme reaches the app (root injection, ambient AppKit, live switch). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
leosar
added a commit
to leosar/cmux
that referenced
this pull request
May 31, 2026
- CHANGELOG: add an Unreleased entry for the Aurean appearance (manaflow-ai#5050). - docs/aurean-appearance.md: how to use the palette picker, the light-mode and terminal-transparency behavior, the CmuxAppearance package architecture, and how the theme reaches the app (root injection, ambient AppKit, live switch). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
Reviewer notesWhere to focus
Design decisions (deliberate)
Verification
Known follow-ups
Docs: |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/aurean-appearance.md (1)
1-70:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRendered markdown was added without required locale coverage.
This new user-facing docs page is English-only in this diff, but policy requires rendered markdown changes to be localized via the project locale system and updated across every locale listed in
web/i18n/routing.ts.As per coding guidelines: “
**/*.{swift,ts,tsx,js,jsx,md}: Fail when diff violates.github/review-bot-rules/full-internationalization.md… web UI, metadata, API response, rendered markdown, changelog, or user-facing data changes must read fromnext-intlor locale-specific source and update every locale inweb/i18n/routing.tswith matchingweb/messages/entries.”🤖 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/aurean-appearance.md` around lines 1 - 70, The rendered English-only docs page (docs/aurean-appearance.md) was added without locale coverage; convert the content to use the project's localization system by moving strings into the next-intl/messages files and replacing the hard-coded markdown with a locale-aware renderer (or per-locale markdown entries) that pulls from web/messages/<locale> keys, then update web/i18n/routing.ts to include the new route/key for every locale it lists and add corresponding entries to each web/messages/<locale>.json (preserve the same keys across locales) so the page is present for every locale required by the project rules.
🤖 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 `@CHANGELOG.md`:
- Around line 5-9: The changelog addition in CHANGELOG.md introduces user-facing
copy but lacks per-locale localized entries required by the repo i18n policy;
update the PR to add matching localized changelog messages for every locale
listed in web/i18n/routing.ts (follow the guidance in
.github/review-bot-rules/full-internationalization.md), e.g., add
locale-specific entries or translation keys corresponding to the new “Aurean
appearance” changelog line and ensure the docs rendering pipeline picks them up
so the docs site shows localized changelog entries for each supported locale.
---
Outside diff comments:
In `@docs/aurean-appearance.md`:
- Around line 1-70: The rendered English-only docs page
(docs/aurean-appearance.md) was added without locale coverage; convert the
content to use the project's localization system by moving strings into the
next-intl/messages files and replacing the hard-coded markdown with a
locale-aware renderer (or per-locale markdown entries) that pulls from
web/messages/<locale> keys, then update web/i18n/routing.ts to include the new
route/key for every locale it lists and add corresponding entries to each
web/messages/<locale>.json (preserve the same keys across locales) so the page
is present for every locale required by the project rules.
🪄 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: 0f7d3579-3951-4bdb-86cd-234d416c60bc
📒 Files selected for processing (2)
CHANGELOG.mddocs/aurean-appearance.md
Comment on lines
+5
to
+9
| ## [Unreleased] | ||
|
|
||
| ### Added | ||
| - **Aurean appearance**: a golden-ratio (φ) based theme. cmux ships a cohesive dark palette across the terminal canvas and the window chrome, with a palette-temperature picker (Cool / Dune / Warm / Obsidian) in Settings → App that re-skins the app live. Light mode keeps the existing theme. ([#5050](https://github.com/manaflow-ai/cmux/pull/5050)) | ||
|
|
There was a problem hiding this comment.
Missing required per-locale changelog localization coverage.
Line 8 adds new user-facing changelog copy, but there’s no corresponding evidence in this PR layer that docs-localized entries were added for every locale supported by web/i18n/routing.ts. This should block merge under the repo i18n policy for changelog updates.
As per coding guidelines: “CHANGELOG.md: Apply .github/review-bot-rules/full-internationalization.md during review. Verify changelog copy rendered on the docs site has matching localized message coverage for every locale listed in web/i18n/routing.ts.”
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 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 `@CHANGELOG.md` around lines 5 - 9, The changelog addition in CHANGELOG.md
introduces user-facing copy but lacks per-locale localized entries required by
the repo i18n policy; update the PR to add matching localized changelog messages
for every locale listed in web/i18n/routing.ts (follow the guidance in
.github/review-bot-rules/full-internationalization.md), e.g., add
locale-specific entries or translation keys corresponding to the new “Aurean
appearance” changelog line and ensure the docs rendering pipeline picks them up
so the docs site shows localized changelog entries for each supported locale.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
CHANGELOG.md (1)
5-9:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing required per-locale changelog localization coverage.
Line 8 adds new user-facing changelog copy, but there's no corresponding evidence in this PR layer that docs-localized entries were added for every locale supported by
web/i18n/routing.ts. This should block merge under the repo i18n policy for changelog updates.As per coding guidelines: "
CHANGELOG.md: Apply.github/review-bot-rules/full-internationalization.mdduring review. Verify changelog copy rendered on the docs site has matching localized message coverage for every locale listed inweb/i18n/routing.ts."🤖 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 `@CHANGELOG.md` around lines 5 - 9, The new user-facing changelog entry added in CHANGELOG.md for "Aurean appearance" lacks localized counterparts; update the localization layer so the same changelog copy is provided for every locale defined in web/i18n/routing.ts by adding matching localized messages (per-locale keys) and corresponding entries used by the docs rendering system, following the `.github/review-bot-rules/full-internationalization.md` checklist to ensure each locale in routing.ts has a translated changelog string and that the docs site rendering picks up those translations.Sources/cmuxApp.swift (2)
258-258:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope the shared Aurean theme above the main scene.
aureanThemeis injected and observed only inside the mainWindowGroup. The Settings and Config scenes below still never receive that shared theme, and if the main scene is gone thisonChangepath stops driving live palette updates entirely.Also applies to: 279-281
🤖 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/cmuxApp.swift` at line 258, The Aurean theme (aureanTheme) is only injected/observed inside the main WindowGroup, so Settings and Config scenes (and onChange-driven live palette updates) don't receive updates; move the shared aureanTheme state and its .onChange handler out of the main WindowGroup and scope it above all Scene declarations so you inject the same observed/injected theme into the Settings and Config scenes (and preserve the .aureanTheme(aureanTheme) modifier and onChange logic at the app root) to ensure all scenes share live palette updates.
900-902:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize invalid stored Aurean variants when falling back.
If
aureanVariantRawis unknown, this falls back to.coolbut leaves the bad raw value inUserDefaults. That can leave downstream pickers without a matching selection and reintroduce the invalid value on next launch.Suggested fix
private func applyAureanVariant() { - aureanTheme.variant = AureanPaletteVariant(rawValue: aureanVariantRaw) ?? .cool + let resolved = AureanPaletteVariant(rawValue: aureanVariantRaw) ?? .cool + if aureanVariantRaw != resolved.rawValue { + aureanVariantRaw = resolved.rawValue + } + aureanTheme.variant = resolved GhosttyApp.shared.reapplyAureanSurface() }🤖 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/cmuxApp.swift` around lines 900 - 902, The applyAureanVariant function uses AureanPaletteVariant(rawValue: aureanVariantRaw) and falls back to .cool but leaves the invalid aureanVariantRaw in persistent storage; change applyAureanVariant so after computing aureanTheme.variant (using AureanPaletteVariant(rawValue: aureanVariantRaw) ?? .cool) you check whether aureanTheme.variant.rawValue != aureanVariantRaw and, if so, write the normalized value back to persistence (update the stored aureanVariantRaw in UserDefaults or the app's settings to aureanTheme.variant.rawValue) before calling GhosttyApp.shared.reapplyAureanSurface() so the bad raw value is normalized for future launches.
🤖 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/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/AureanPalettePickerRow.swift`:
- Line 20: The localization keys used in AureanPalettePickerRow.swift
(Text(String(localized: "settings.app.aureanPalette", ...)) and the palette
labels aurean.palette.cool, aurean.palette.dune, aurean.palette.warm,
aurean.palette.obsidian) currently only have en/ja entries; add translated
strings for every locale already present in Resources/Localizable.xcstrings
(e.g., ar, de, fr, zh-Hans, etc.) by updating that .xcstrings catalog to include
those keys for each locale with appropriate translations so the UI displays
localized palette names across all supported locales.
In `@Sources/Sidebar/SidebarAppearanceSupport.swift`:
- Around line 17-23: activeVariant/activePalette currently read UserDefaults on
every access which can be invoked repeatedly during SwiftUI renders (e.g., in
cmuxAccentNSColor(for:), SidebarBackdropSettingsSnapshot.materialPolicy, and
SidebarBackdropSettingsSnapshot.appKitMutationID), so resolve and cache the
AureanPaletteVariant/AureanPalette once when creating a snapshot instead of
calling AureanAppearanceSettings.activeVariant/activePalette repeatedly; update
the snapshot type (SidebarBackdropSettingsSnapshot) to carry the resolved
variant.rawValue and palette-derived NSColor values (or the
AureanPalette/AureanPaletteVariant themselves) and change callers
(WindowAppearanceSnapshot.policy(for:), WindowAccessor/ContentView path) to use
those snapshot fields; alternatively implement memoization in
AureanAppearanceSettings with a short-lived cache invalidated when the
UserDefaults key changes, but prefer storing resolved values on snapshot
creation to ensure no per-render UserDefaults reads.
---
Duplicate comments:
In `@CHANGELOG.md`:
- Around line 5-9: The new user-facing changelog entry added in CHANGELOG.md for
"Aurean appearance" lacks localized counterparts; update the localization layer
so the same changelog copy is provided for every locale defined in
web/i18n/routing.ts by adding matching localized messages (per-locale keys) and
corresponding entries used by the docs rendering system, following the
`.github/review-bot-rules/full-internationalization.md` checklist to ensure each
locale in routing.ts has a translated changelog string and that the docs site
rendering picks up those translations.
In `@Sources/cmuxApp.swift`:
- Line 258: The Aurean theme (aureanTheme) is only injected/observed inside the
main WindowGroup, so Settings and Config scenes (and onChange-driven live
palette updates) don't receive updates; move the shared aureanTheme state and
its .onChange handler out of the main WindowGroup and scope it above all Scene
declarations so you inject the same observed/injected theme into the Settings
and Config scenes (and preserve the .aureanTheme(aureanTheme) modifier and
onChange logic at the app root) to ensure all scenes share live palette updates.
- Around line 900-902: The applyAureanVariant function uses
AureanPaletteVariant(rawValue: aureanVariantRaw) and falls back to .cool but
leaves the invalid aureanVariantRaw in persistent storage; change
applyAureanVariant so after computing aureanTheme.variant (using
AureanPaletteVariant(rawValue: aureanVariantRaw) ?? .cool) you check whether
aureanTheme.variant.rawValue != aureanVariantRaw and, if so, write the
normalized value back to persistence (update the stored aureanVariantRaw in
UserDefaults or the app's settings to aureanTheme.variant.rawValue) before
calling GhosttyApp.shared.reapplyAureanSurface() so the bad raw value is
normalized for future launches.
🪄 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: 31528505-7176-4c1b-93a5-009ce1a760ff
📒 Files selected for processing (24)
CHANGELOG.mdPackages/CmuxAppearance/Package.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AppearancePalette.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanColor.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanMetrics.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanOpacity.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanPalette.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanPaletteVariant.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/AureanTheme.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/EnvironmentValues+AureanPalette.swiftPackages/CmuxAppearance/Sources/CmuxAppearance/View+AureanTheme.swiftPackages/CmuxAppearance/Tests/CmuxAppearanceTests/AureanPaletteTests.swiftPackages/CmuxAppearance/Tests/CmuxAppearanceTests/AureanThemeTests.swiftPackages/CmuxSettingsUI/Package.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/AureanPalettePickerRow.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/AureanPaletteVariant+SettingCodable.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swiftResources/Localizable.xcstringsSources/GhosttyTerminalView.swiftSources/Sidebar/SidebarAppearanceSupport.swiftSources/Windowing/WindowAppearanceSnapshot.swiftSources/cmuxApp.swiftcmux.xcodeproj/project.pbxprojdocs/aurean-appearance.md
👮 Files not reviewed due to content moderation or server errors (1)
- Sources/GhosttyTerminalView.swift
|
|
||
| var body: some View { | ||
| HStack(alignment: .top, spacing: 12) { | ||
| Text(String(localized: "settings.app.aureanPalette", defaultValue: "Palette")) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the catalog and inspect locale coverage for the new keys.
fd -t f 'Localizable.xcstrings' Resources
CATALOG=$(fd -t f 'Localizable.xcstrings' Resources | head -n1)
for key in "settings.app.aureanPalette" "aurean.palette.cool" "aurean.palette.dune" "aurean.palette.warm" "aurean.palette.obsidian"; do
echo "=== $key ==="
jq --arg k "$key" '.strings[$k].localizations | keys' "$CATALOG" 2>/dev/null || echo "MISSING"
done
# Reference: full set of locales declared anywhere in the catalog.
echo "=== all locales present in catalog ==="
jq -r '[.strings[].localizations? // {} | keys[]] | unique | .[]' "$CATALOG" 2>/dev/null | sort -uRepository: manaflow-ai/cmux
Length of output: 450
Add full locale coverage for Aurean palette localization keys
The string catalog Resources/Localizable.xcstrings supports locales beyond en/ja (e.g., ar, de, fr, zh-Hans, etc.), but these keys only have en and ja entries: settings.app.aureanPalette and aurean.palette.{cool,dune,warm,obsidian} (used in AureanPalettePickerRow.swift around lines ~20 and ~91-94). Add translated values for every locale the catalog already supports.
🤖 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/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/AureanPalettePickerRow.swift`
at line 20, The localization keys used in AureanPalettePickerRow.swift
(Text(String(localized: "settings.app.aureanPalette", ...)) and the palette
labels aurean.palette.cool, aurean.palette.dune, aurean.palette.warm,
aurean.palette.obsidian) currently only have en/ja entries; add translated
strings for every locale already present in Resources/Localizable.xcstrings
(e.g., ar, de, fr, zh-Hans, etc.) by updating that .xcstrings catalog to include
those keys for each locale with appropriate translations so the UI displays
localized palette names across all supported locales.
Comment on lines
+17
to
+23
| static var activeVariant: AureanPaletteVariant { | ||
| guard let raw = UserDefaults.standard.string(forKey: userDefaultsKey), | ||
| let variant = AureanPaletteVariant(rawValue: raw) else { return .cool } | ||
| return variant | ||
| } | ||
|
|
||
| static var activePalette: AureanPalette { activeVariant.palette } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how frequently activePalette and activeVariant are accessed
rg -nP --type=swift -C5 'AureanAppearanceSettings\.(activeVariant|activePalette)' Sources/
# Check if cmuxAccentNSColor or materialPolicy are in hot paths
rg -nP --type=swift -C3 'cmuxAccentNSColor|\.materialPolicy' Sources/ | rg -i 'foreach|loop|\.map|\.filter|draw|render|layout'Repository: manaflow-ai/cmux
Length of output: 5824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate the AureanAppearanceSettings definition
fd -i "SidebarAppearanceSupport.swift" Sources/ | head -n 20
rg -n "enum AureanAppearanceSettings|struct AureanAppearanceSettings|class AureanAppearanceSettings|AureanAppearanceSettings\s*[\{\(]" Sources/Sidebar/SidebarAppearanceSupport.swift
# Show the definition region
sed -n '1,220p' Sources/Sidebar/SidebarAppearanceSupport.swift | nl -ba | sed -n '1,220p'
# 2) Inspect WindowAppearanceSnapshot.materialPolicy implementation and its call surface
rg -n "materialPolicy|SidebarBackdropMaterialPolicy|tintColor:\s*AureanAppearanceSettings\.activePalette" Sources/Windowing/WindowAppearanceSnapshot.swift
sed -n '1,260p' Sources/Windowing/WindowAppearanceSnapshot.swift | nl -ba | sed -n '1,260p'
# 3) Find other materialPolicy usages and WindowAppearanceSnapshot creation/update call sites
rg -n --type=swift "WindowAppearanceSnapshot|materialPolicy" Sources/ | head -n 200
# 4) Check whether cmuxAccentNSColor is called inside any loops/render callbacks
rg -n --type=swift "cmuxAccentNSColor\(" Sources/ | head -n 200
# 5) For the main call sites found by the first script, inspect a small neighborhood to see if they're inside animation/layout loops
for f in Sources/GhosttyTerminalView.swift Sources/Windowing/WindowAppearanceSnapshot.swift Sources/Sidebar/SidebarAppearanceSupport.swift; do
echo "==== $f (activePalette activeVariant neighborhoods) ===="
rg -n --type=swift "AureanAppearanceSettings\.(activePalette|activeVariant)" "$f" | head -n 50
doneRepository: manaflow-ai/cmux
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== AureanAppearanceSettings definition =="
sed -n '1,220p' Sources/Sidebar/SidebarAppearanceSupport.swift | cat -n | sed -n '1,220p'
echo
echo "== activePalette/activeVariant usages in SidebarAppearanceSupport.swift =="
rg -n --type=swift "AureanAppearanceSettings\.(activeVariant|activePalette)" Sources/Sidebar/SidebarAppearanceSupport.swift
echo
echo "== materialPolicy + tintColor using activePalette =="
rg -n --type=swift "materialPolicy|SidebarBackdropMaterialPolicy|tintColor:\s*AureanAppearanceSettings\.activePalette" Sources/Windowing/WindowAppearanceSnapshot.swift
sed -n '1,260p' Sources/Windowing/WindowAppearanceSnapshot.swift | cat -n | sed -n '1,260p'
echo
echo "== other WindowAppearanceSnapshot / materialPolicy call sites =="
rg -n --type=swift "WindowAppearanceSnapshot|materialPolicy" Sources/ | head -n 200
echo
echo "== cmuxAccentNSColor call sites =="
rg -n --type=swift "cmuxAccentNSColor\(" Sources/ | head -n 200Repository: manaflow-ai/cmux
Length of output: 28207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=swift "currentFromUserDefaults\(" Sources/ | head -n 200
rg -n --type=swift "var\s+windowAppearanceSnapshot:|windowAppearanceSnapshot\s*\{" Sources/ContentView.swift
sed -n '2230,2355p' Sources/ContentView.swift | cat -n
# Inspect the call sites we saw in GhosttyTerminalView.swift around the snapshot creation lines
sed -n '4920,5055p' Sources/GhosttyTerminalView.swift | cat -n
sed -n '7720,7810p' Sources/GhosttyTerminalView.swift | cat -n
sed -n '7760,7840p' Sources/GhosttyTerminalView.swift | cat -n
# Inspect WindowAppearanceSnapshot.currentFromUserDefaults implementation
rg -n --type=swift "static func currentFromUserDefaults" Sources/Windowing/WindowAppearanceSnapshot.swift
sed -n '238,420p' Sources/Windowing/WindowAppearanceSnapshot.swift | cat -n
# Inspect WindowBackdropController usage to infer whether materialPolicy is rebuilt per frame
sed -n '1,260p' Sources/Windowing/WindowBackdropController.swift | cat -n
# Check if SidebarBackdropSettingsSnapshot.materialPolicy is computed once per snapshot
rg -n --type=swift "struct SidebarBackdropSettingsSnapshot|materialPolicy\s*:" Sources/Windowing/WindowAppearanceSnapshot.swift
sed -n '90,190p' Sources/Windowing/WindowAppearanceSnapshot.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 24465
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show backdropPlan implementation
rg -n --type=swift "func\s+backdropPlan\s*\(" Sources/Windowing/WindowAppearanceSnapshot.swift
sed -n '260,520p' Sources/Windowing/WindowAppearanceSnapshot.swift | cat -n
# 2) Find all call sites of backdropPlan()
rg -n --type=swift "backdropPlan\(" Sources/ | head -n 200
# 3) Inspect WindowAppearanceSnapshot.current(...) to see whether it constructs SidebarBackdropSettingsSnapshot
rg -n --type=swift "static func current\(" Sources/Windowing/WindowAppearanceSnapshot.swift
sed -n '238,360p' Sources/Windowing/WindowAppearanceSnapshot.swift | cat -n
sed -n '360,520p' Sources/Windowing/WindowAppearanceSnapshot.swift | cat -n
# 4) Check how windowAppearanceSnapshot computed property is used in ContentView
rg -n --type=swift "windowAppearanceSnapshot" Sources/ContentView.swift
# show nearby regions around the declaration and typical usage
sed -n '2240,2335p' Sources/ContentView.swift | cat -n
sed -n '2600,2660p' Sources/ContentView.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
#!/bin/bash
set -u
file="Sources/Windowing/WindowAppearanceSnapshot.swift"
echo "== file = $file =="
wc -l "$file"
echo
echo "== backdropPlan definition =="
rg -n --type=swift "func\s+backdropPlan\s*\(" "$file" || true
# print only the neighborhood around the definition if found
line="$(rg -n --type=swift -m1 "func\s+backdropPlan\s*\(" "$file" | cut -d: -f1 || true)"
if [ -n "${line:-}" ]; then
start=$((line-40)); end=$((line+140))
sed -n "${start},${end}p" "$file" | cat -n
fi
echo
echo "== WindowAppearanceSnapshot.current definition =="
rg -n --type=swift "static func current\(" "$file" || true
line="$(rg -n --type=swift -m1 "static func current\(" "$file" | cut -d: -f1 || true)"
if [ -n "${line:-}" ]; then
start=$((line-40)); end=$((line+220))
sed -n "${start},${end}p" "$file" | cat -n
fi
echo
echo "== currentFromUserDefaults definition =="
rg -n --type=swift "currentFromUserDefaults" "$file" || true
# ContentView snapshot usage
cv="Sources/ContentView.swift"
echo
echo "== ContentView usage of windowAppearanceSnapshot =="
rg -n --type=swift "windowAppearanceSnapshot" "$cv" || true
# print sections around the computed var and where it's used for backdrop/sidebar
rg -n --type=swift "var\s+windowAppearanceSnapshot|sidebarPanelWithBackdrop|backdrop" "$cv" | head -n 100 || trueRepository: manaflow-ai/cmux
Length of output: 8937
🏁 Script executed:
#!/bin/bash
set -u
# Locate backdropPlan definition and appKitMutationID usage
rg -n --type=swift "backdropPlan\s*\(" Sources/Windowing Sources/ContentView.swift Sources/ | head -n 200
rg -n --type=swift "appKitMutationID" Sources/Windowing Sources/ContentView.swift Sources/ | head -n 200
# Inspect ContentView where snapshot.policy(for:) and backdrop(for:) are used
cv="Sources/ContentView.swift"
sed -n '17580,17760p' "$cv" | cat -n
# Inspect ContentView where windowAppearanceSnapshot is computed and used (around the computed property + assignment)
sed -n '2230,2365p' "$cv" | cat -n
sed -n '2580,2680p' "$cv" | cat -n
# Inspect where backdropPlan is called/applied
sed -n '3270,3360p' "$cv" | cat -n
# Inspect WindowBackdropController apply to see how frequently mutation ids are computed
wbc="Sources/Windowing/WindowBackdropController.swift"
sed -n '1,280p' "$wbc" | cat -n
rg -n --type=swift "appKitMutationID|mutation|didChangeGlassRoot|hostingPhase" "$wbc" | head -n 200Repository: manaflow-ai/cmux
Length of output: 39652
Cache resolved Aurean palette/variant instead of re-reading UserDefaults during backdrop rendering.
AureanAppearanceSettings.activeVariant / activePalette (UserDefaults-backed) are consumed by:
cmuxAccentNSColor(for:)viaactivePalette.accent.nsColorSidebarBackdropSettingsSnapshot.materialPolicyviaactivePalette.surfaceOff.nsColorSidebarBackdropSettingsSnapshot.appKitMutationIDviaactiveVariant.rawValue
materialPolicy/appKitWindowMutationID feed into WindowAppearanceSnapshot.policy(for:) and WindowAccessor/backdrop updates in ContentView, so these UserDefaults reads can happen repeatedly during SwiftUI view refreshes (not just when the palette changes). Consider resolving AureanPaletteVariant/AureanPalette once at snapshot-creation (or memoizing for the lifetime of a render/update) and storing it in the snapshot fields rather than recomputing via activeVariant/activePalette.
🤖 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/Sidebar/SidebarAppearanceSupport.swift` around lines 17 - 23,
activeVariant/activePalette currently read UserDefaults on every access which
can be invoked repeatedly during SwiftUI renders (e.g., in
cmuxAccentNSColor(for:), SidebarBackdropSettingsSnapshot.materialPolicy, and
SidebarBackdropSettingsSnapshot.appKitMutationID), so resolve and cache the
AureanPaletteVariant/AureanPalette once when creating a snapshot instead of
calling AureanAppearanceSettings.activeVariant/activePalette repeatedly; update
the snapshot type (SidebarBackdropSettingsSnapshot) to carry the resolved
variant.rawValue and palette-derived NSColor values (or the
AureanPalette/AureanPaletteVariant themselves) and change callers
(WindowAppearanceSnapshot.policy(for:), WindowAccessor/ContentView path) to use
those snapshot fields; alternatively implement memoization in
AureanAppearanceSettings with a short-lived cache invalidated when the
UserDefaults key changes, but prefer storing resolved values on snapshot
creation to ensure no per-render UserDefaults reads.
Aurean: space divides on Φ, never 50/50. A split node with no explicit ratio now resolves to 0.618 instead of 0.5, so freshly created splits open at the golden ratio. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/CmuxConfig.swift`:
- Around line 1765-1767: Replace the duplicated magic number with the shared
constant: add "import CmuxAppearance" at the top of the file and change the
fallback in the initializer where "let value = split ?? 0.618033988749" to use
"AureanMetrics.phiInverse" instead of the hardcoded number so the code
references the single source of truth.
- Line 1767: The test expectation and name are out of sync with the new
nil-default used in CmuxConfig: replace the test assertion and rename the test
from testClampedSplitPositionDefaultsToHalf to reflect the golden-ratio fallback
(e.g., testClampedSplitPositionDefaultsToAureanPhiInverse) and assert that
clampedSplitPosition equals the AureanMetrics.phiInverse value (or the literal
0.618033988749) used when split is nil; update any references to the default in
CmuxConfig (the expression split ?? 0.618033988749) so the test checks against
the same constant/symbol (preferably AureanMetrics.phiInverse) to keep
implementation and test consistent.
🪄 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: a777aa48-5f25-4ca0-a0e1-dbcab0002586
📒 Files selected for processing (1)
Sources/CmuxConfig.swift
Comment on lines
+1765
to
+1767
| // Aurean: space divides on the golden ratio (61.8 / 38.2), never 50/50, when a | ||
| // split carries no explicit ratio. | ||
| let value = split ?? 0.618033988749 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the AureanMetrics.phiInverse constant instead of the magic number.
The magic number 0.618033988749 duplicates the AureanMetrics.phiInverse constant defined in the CmuxAppearance package. Using the constant maintains a single source of truth and makes the connection to the Aurean system explicit.
♻️ Proposed fix
Add the import at the top of the file:
import Bonsplit
+import CmuxAppearance
import Combine
import CryptoKit
import FoundationThen use the constant:
var clampedSplitPosition: Double {
// Aurean: space divides on the golden ratio (61.8 / 38.2), never 50/50, when a
// split carries no explicit ratio.
- let value = split ?? 0.618033988749
+ let value = split ?? AureanMetrics.phiInverse
return min(0.9, max(0.1, value))
}📝 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
| // Aurean: space divides on the golden ratio (61.8 / 38.2), never 50/50, when a | |
| // split carries no explicit ratio. | |
| let value = split ?? 0.618033988749 | |
| import Bonsplit | |
| import CmuxAppearance | |
| import Combine | |
| import CryptoKit | |
| import Foundation | |
| ... | |
| var clampedSplitPosition: Double { | |
| // Aurean: space divides on the golden ratio (61.8 / 38.2), never 50/50, when a | |
| // split carries no explicit ratio. | |
| let value = split ?? AureanMetrics.phiInverse | |
| return min(0.9, max(0.1, value)) | |
| } |
🤖 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/CmuxConfig.swift` around lines 1765 - 1767, Replace the duplicated
magic number with the shared constant: add "import CmuxAppearance" at the top of
the file and change the fallback in the initializer where "let value = split ??
0.618033988749" to use "AureanMetrics.phiInverse" instead of the hardcoded
number so the code references the single source of truth.
| let value = split ?? 0.5 | ||
| // Aurean: space divides on the golden ratio (61.8 / 38.2), never 50/50, when a | ||
| // split carries no explicit ratio. | ||
| let value = split ?? 0.618033988749 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find the test that expects 0.5 and check for other hardcoded references
# Check for the test assertion
echo "=== Test expecting 0.5 default ==="
rg -n "clampedSplitPosition.*0\.5" --type=swift
# Check for references to the test name
echo -e "\n=== Test function ==="
rg -n "testClampedSplitPositionDefaultsToHalf" --type=swift
# Check for other potential usages of the default split value
echo -e "\n=== Other references to 0.5 split ==="
rg -nP "split.*=.*0\.5|0\.5.*split" --type=swift -C2Repository: manaflow-ai/cmux
Length of output: 1836
Fix CI failure: update nil-default split test expectation
cmuxTests/CmuxConfigTests.swift:testClampedSplitPositionDefaultsToHalf expects split.clampedSplitPosition == 0.5, but Sources/CmuxConfig.swift uses split ?? 0.618033988749 when split is nil, so the test will fail. Update the expectation to the golden-ratio fallback (or AureanMetrics.phiInverse) and rename the test to reflect the new default.
🤖 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/CmuxConfig.swift` at line 1767, The test expectation and name are out
of sync with the new nil-default used in CmuxConfig: replace the test assertion
and rename the test from testClampedSplitPositionDefaultsToHalf to reflect the
golden-ratio fallback (e.g., testClampedSplitPositionDefaultsToAureanPhiInverse)
and assert that clampedSplitPosition equals the AureanMetrics.phiInverse value
(or the literal 0.618033988749) used when split is nil; update any references to
the default in CmuxConfig (the expression split ?? 0.618033988749) so the test
checks against the same constant/symbol (preferably AureanMetrics.phiInverse) to
keep implementation and test consistent.
teamleaderleo
added
S3: minor
This branch has not been 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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Introduces Aurean, a golden-ratio (φ) based appearance system, and dresses cmux in it end-to-end: a cohesive dark palette across the terminal canvas and chrome, a settings picker to switch temperature, and a reusable theme package.
How (commit-by-commit)
New package
CmuxAppearance(leaf, no deps; 16 Swift Testing cases green viaswift test):AureanColor(hex→sRGB, opacity ladder),AureanPalette(cool/dune/warm/obsidian, signals fixed across variants),AureanMetrics(φ, Fibonacci, golden split).@Observable @MainActor AureanTheme+EnvironmentValues.aureanPalette+View.aureanTheme(_:). No singletons; injected at the app root.App integration:
CmuxAppearanceinto the app target (pbxproj, normalized;check-pbxproj.shgreen) and injectedAureanThemeat the window root.~/.config/ghostty.GhosttyApp.reapplyAureanSurface()(reuses the current scope so precedence is preserved). Localized (en + ja).Verification
swift testonCmuxAppearance: 16/16 green../scripts/reload.sh --tag aurean: BUILD SUCCEEDED at every step../scripts/check-pbxproj.sh: exit 0.#161819, no wallpaper bleed; sidebar dark; pale-blue Aurean accent on the selected workspace; live cool↔warm switch confirmed.Notes / follow-ups
surfaceOffis a separate, more invasive change left as follow-up.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds the Aurean φ-based appearance system with a selectable palette (cool/dune/warm/obsidian) and defaults pane splits to the golden ratio (61.8/38.2). Delivers a cohesive dark theme across the terminal canvas and chrome with live re-skin from Settings; in light mode, Aurean stands down to preserve Ghostty/system colors and sidebar materials.
New Features
CmuxAppearance: token layer (AureanColor,AureanOpacity,AureanMetrics), palettes/variants,AureanThemeprovider, and env plumbing; 16 tests.AureanThemeat the window root; views read via@Environment(\.aureanPalette).aureanPaletteVariant; live re-skin viaGhosttyApp.reapplyAureanSurface(). Strings localized in en and ja.docs/aurean-appearance.mdand CHANGELOG entry under Unreleased.Bug Fixes / Refactors
AureanPaletteVariant.paletteto avoid hot-path allocations; centralized theUserDefaultskey onAureanPaletteVariant.userDefaultsKey.AureanThemefrom the persisted variant at init (no redundant onAppear wiring).Written for commit ad41bed. Summary will update on new commits.
Summary by CodeRabbit
New Features
Localization
Documentation
Tests