Skip to content

Settings: group sidebar navigation by taxonomy - #13039

Closed
teamleaderleo wants to merge 1 commit into
manaflow-ai:mainfrom
teamleaderleo:lane-b-settings-taxonomy
Closed

teamleaderleo wants to merge 1 commit into
manaflow-ai:mainfrom
teamleaderleo:lane-b-settings-taxonomy

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator

Reviewer summary

Groups Settings navigation by the way people look for a setting, with clearer labels and descriptions.

What changed

  • group the existing Settings destinations into eight browse categories: General, Terminal, Workspace, Sidebar & Dock, Agents & Automation, Browser & Files, Remote & Devices, and Keyboard & Advanced
  • keep every existing SettingsSectionID, section title, setting ID, stored value, default, persistence path, and feature implementation unchanged
  • keep search flat and relevance-ranked while a query is active, preserving the existing search result IDs and row anchors
  • add focused taxonomy tests plus a small CmuxSettingsUI-localized catalog for the new group headers

Navigation compatibility

The taxonomy is presentation-only. Existing leaf destinations still use the same section:<rawValue> IDs and cmux.settings.navigate targets, so restored selection, cmux settings open <target>, search hits, and deep row anchors continue through the existing paths.

Tests

  • SettingsTaxonomyTests verifies every existing navigation leaf appears exactly once in the taxonomy
  • verifies the intended group membership/order
  • verifies empty-query search retains the original section entry IDs/order and matching anchor IDs

Coordinates Lane B from teamleaderleo/Tact#79.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Groups the Settings sidebar into eight browse categories so the default view is easier to scan, implementing Lane B of teamleaderleo/Tact#79. Search stays flat and relevance-ranked while a query is active, preserving existing search hit IDs, row anchors, and navigation targets.

Compatibility

  • Existing SettingsSectionID values, section titles, setting IDs, defaults, and persistence paths are unchanged.
  • Group titles are localized through a new CmuxSettingsUI resource catalog; the package now bundles its Resources directory.
  • No migration or rollout action is needed because the taxonomy is presentation-only.

Tests

  • SettingsTaxonomyTests verifies every leaf appears exactly once, group membership/order, empty-query search IDs, and localized group titles.

Written for commit 7c5e1ff. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Settings navigation is now organized into clearly labeled groups, making sections easier to browse.
    • Added localized group titles in Arabic, German, English, Spanish, French, Japanese, Korean, Simplified Chinese, and Traditional Chinese.
    • Search results remain flat and relevance-ranked for faster discovery.
    • Settings resources are now included in the application package.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8a8a6a24-bbde-4b1d-a9f1-2d54253ab63a

📥 Commits

Reviewing files that changed from the base of the PR and between 4c67b4d and a7c910a.

📒 Files selected for processing (5)
  • Packages/macOS/CmuxSettingsUI/Package.swift
  • Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/SettingsTaxonomy.swift
  • Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Resources/Localizable.xcstrings
  • Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Scene/SettingsWindowScene.swift
  • Packages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsTaxonomyTests.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The settings package now defines eight localized taxonomy groups, renders grouped sidebar entries outside search, preserves existing section identities, and validates coverage, ordering, search identity, and localized titles.

Changes

Settings taxonomy

Layer / File(s) Summary
Taxonomy contract and resources
Packages/macOS/CmuxSettingsUI/Package.swift, Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/SettingsTaxonomy.swift, Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Resources/Localizable.xcstrings
The package processes resources. SettingsTaxonomyGroup defines eight groups and maps them to existing section IDs. Localized group titles are added for nine locales.
Grouped sidebar rendering
Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Scene/SettingsWindowScene.swift
The default sidebar renders non-empty taxonomy groups with headers and ordered rows. Search results remain flat and relevance-ranked.
Taxonomy validation
Packages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsTaxonomyTests.swift
Tests verify complete section coverage, group membership, preserved search identities, and non-empty localized titles.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant SettingsSearchIndex
  participant SettingsWindowScene
  participant SettingsTaxonomyGroup
  SettingsSearchIndex->>SettingsWindowScene: provide section entries
  SettingsWindowScene->>SettingsTaxonomyGroup: request taxonomy order
  SettingsTaxonomyGroup-->>SettingsWindowScene: return grouped section IDs
  SettingsWindowScene->>SettingsWindowScene: render grouped sidebar rows
  SettingsSearchIndex->>SettingsWindowScene: provide flat results for active search
Loading

Suggested reviewers: austinywang

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS. The PR changes only CmuxSettingsUI package resources, localized taxonomy labels, settings-sidebar grouping, and taxonomy tests. The authoritative diff contains no Cloud terminal creation, cmux-t…
Cmux Swift Actor Isolation ✅ Passed No actor-isolation failure is introduced. The new SettingsTaxonomyGroup is a small Sendable value enum with no @MainActor annotation, and the CmuxSettingsUI package has no default MainActor se…
Cmux Swift Blocking Runtime ✅ Passed The PR adds taxonomy and sidebar presentation code, localized resources, and taxonomy tests. The authoritative diff introduces no semaphores, blocking waits, sleeps, delayed dispatch, polling loops, m…
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request does not change browser socket automation. The authoritative diff changes only CmuxSettingsUI settings navigation, localization, package resources, and taxonomy tests. No rule-s…
Cmux Expensive Synchronous Load ✅ Passed PASS. The PR adds settings taxonomy data, localized group titles, and sidebar grouping only. The changed production Swift files contain no RestorableAgentSessionIndex, SharedLiveAgentIndex, agent/…
Cmux Cache Substitution Correctness ✅ Passed PASS. The diff adds presentation-only taxonomy and localized resources. The changed sidebar still reads the existing SettingsSearchIndex and only groups or reorders its entries; the prior implementa…
Cmux No Hacky Sleeps ✅ Passed PASS. The pull request changes only Swift, .xcstrings, and Swift test files. It introduces no sleep, usleep, timers, polling, backoff, retry, or delayed dispatch. The only timing-related match i…
Cmux Algorithmic Complexity ✅ Passed PASS. The only new production lookup is taxonomyEntries in SettingsWindowScene.swift, where each fixed taxonomy section performs entries.first(where:). This runs only when isSearching is false…
Cmux Swift Concurrency ✅ Passed The pull request adds only taxonomy, resource, and SwiftUI sidebar presentation code plus tests. The added Swift lines contain no DispatchQueue/DispatchGroup, Combine state, completion-handler API, or…
Cmux Swift @Concurrent ✅ Passed The PR introduces no new or changed async, nonisolated, @concurrent, actor, or explicit hop behavior. The new taxonomy properties and taxonomyEntries helper are synchronous. The changed sideba…
Cmux Swift Package Boundaries ✅ Passed No package-boundary violation is introduced. The diff changes the existing CmuxSettingsUI SwiftPM target, not an app target. SettingsTaxonomyGroup is an internal, presentation-only sidebar groupin…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The only package-manifest change is in Packages/macOS/CmuxSettingsUI/Package.swift: it adds .process("Resources"). The existing path dependencies are unchanged, and the PR adds no external d…
Cmux Swift Logging ✅ Passed PASS — The pull-request diff adds taxonomy, resource wiring, sidebar grouping, and tests. It adds no print, debugPrint, dump, NSLog, ad hoc file/stdout logging, Logger declaration, or sensitiv…
Cmux User-Facing Error Privacy ✅ Passed The pull request adds settings taxonomy headers and reorganizes existing sidebar rows. It does not add or change user-facing errors, alerts, command output, API error bodies, or recovery copy. The new…
Cmux Full Internationalization ✅ Passed The PR adds seven user-facing taxonomy labels. Each uses String(localized:defaultValue:bundle:) with a stable key in SettingsTaxonomy.swift. The new `CmuxSettingsUI/Resources/Localizable.xcstrings…
Cmux Swiftui State Layout ✅ Passed The PR does not introduce any prohibited SwiftUI state or layout pattern. The changed sidebar adds value-based SettingsSearchIndex.Entry rows and pure taxonomyEntries/sidebarEntryRow helpers. It…
Cmux Architecture Rethink ✅ Passed The diff adds an immutable, presentation-only SettingsTaxonomyGroup and uses it from the existing SettingsWindowRoot sidebar. It does not add sleeps, delayed dispatch, polling, locks, new observer…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The PR changes the existing SettingsWindowRoot sidebar presentation only. It does not add or alter NSWindow, NSPanel, NSWindowController, Window, or WindowGroup construction. The exi…
Cmux Source Artifacts ✅ Passed All five changed paths are intentional product files: Swift source, package configuration, a localization catalog, and focused tests. The localization catalog is valid JSON with seven taxonomy keys, a…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The pull request adds no test/debug seam in production Swift source. SettingsTaxonomy.swift adds the internal SettingsTaxonomyGroup used by the production sidebar, with no test/debug naming or bui…
Title check ✅ Passed The title clearly and concisely describes the main change: grouping Settings sidebar navigation by taxonomy.
Description check ✅ Passed The description provides a clear summary, compatibility details, testing coverage, and rationale. It omits the template's Demo Video, Review Trigger, and Checklist sections, but the core required info…
Full details: Docstring Coverage

Explanation

Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor

cursor Bot commented Sep 19, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@teamleaderleo
teamleaderleo force-pushed the lane-b-settings-taxonomy branch from eb808f4 to a7c910a Compare September 19, 2026 18:09
@cursor

cursor Bot commented Sep 19, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo
teamleaderleo force-pushed the lane-b-settings-taxonomy branch from 1de6be3 to 43c6eb6 Compare September 19, 2026 18:12
@teamleaderleo
teamleaderleo force-pushed the lane-b-settings-taxonomy branch from a6342de to 7c5e1ff Compare September 19, 2026 18:13

@teamleaderleo teamleaderleo left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual pass on current head 7c5e1ffa6a7cff5393763b585821b66585662fe3 because the automated review was partly rate-limited.

I checked the taxonomy against search/deep-link behavior specifically:

  • whitespace-only search still counts as browse mode;
  • clearing search resets the selected row to the current section, matching the existing behavior;
  • active search stays flat and keeps the original entry IDs/anchors;
  • external navigation still preserves an active setting hit when it targets the same parent section;
  • runtime-hidden Cloud entries are filtered before taxonomy grouping.

I found no additional issue in those navigation seams.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Local UI verification — September 19, 2026

Tested 7c5e1ffa6a7cff5393763b585821b66585662fe3.

  • Tagged app build passed.
  • Full CmuxSettingsUI package suite: 173 tests in 31 suites passed, including all four taxonomy tests.
  • All eight group headers are present; upper and lower groups render without clipped headers.
  • Native section selection, flat search results, search-to-row navigation/highlight, No Results, and clearing search back to grouped browsing worked.

Observation for follow-up: searching scrollbar ranks Adapt Default Theme above Show Terminal Scroll Bar and includes many unrelated results. This was not established as a regression introduced by this PR.

Built as an isolated tagged Debug app. The only local overlay was the two build-script files from #12973 to enable CMUX_DEV_BACKEND_MODE=local; application and Settings source matched the commit above.

Screenshots

Grouped Settings

Grouped Settings

Lower groups and Keyboard Shortcuts

Lower groups and Keyboard Shortcuts

Search result navigating to the setting

Search result navigating to the setting

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Replaced by #13222: same commits, head branch moved into the org.

@teamleaderleo
teamleaderleo deleted the lane-b-settings-taxonomy branch September 23, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant