Skip to content

Stabilize sidebar directory ordering when split focus changes - #1798

Merged
lawrencecchen merged 2 commits into
mainfrom
task-stable-sidebar-dir-order
Mar 19, 2026
Merged

lawrencecchen merged 2 commits into
mainfrom
task-stable-sidebar-dir-order

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Mar 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add a regression test covering sidebar directory ordering across focused-split cwd churn
  • resolve sidebar directory display order from per-panel live cwd or requested working directory before falling back to the focused panel cwd
  • reuse the same stable ordered directory list for the sidebar path summary text

Testing

  • xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -derivedDataPath /tmp/cmux-task-stable-sidebar-dir-order-red test -only-testing:cmuxTests/WorkspacePanelGitBranchTests/testSidebarBranchDirectoryEntriesStayStableAcrossFocusedSplitChanges
  • xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -derivedDataPath /tmp/cmux-task-stable-sidebar-dir-order-green test -only-testing:cmuxTests/WorkspacePanelGitBranchTests/testSidebarBranchDirectoryEntriesStayStableAcrossFocusedSplitChanges
  • ./scripts/reload.sh --tag task-stable-sidebar-dir-order

Summary by cubic

Stabilizes sidebar directory ordering so it stays consistent when the focused split changes. Removes flicker and keeps path summary and branch entries in a stable order.

  • Bug Fixes
    • Resolve per-panel directories from live cwd or requested working directory; only fall back to focused cwd if needed.
    • Canonicalize and dedupe directories (expand tilde, standardize paths) to keep ordering stable.
    • Add sidebarDirectoriesInDisplayOrder(...) and reuse it for the sidebar path summary and branch directory entries.
    • Add regression test testSidebarBranchDirectoryEntriesStayStableAcrossFocusedSplitChanges.

Written for commit c8b495e. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed inconsistent sidebar directory ordering when switching between multiple panels
  • Tests

    • Added verification for sidebar directory stability across panel focus changes

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@vercel

vercel Bot commented Mar 19, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Mar 19, 2026 7:58am

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai

coderabbitai Bot commented Mar 19, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Refactored sidebar directory handling by introducing new workspace methods (sidebarDirectoriesInDisplayOrder) that consolidate directory resolution with improved normalization logic. Updated ContentView's directorySummaryText to delegate to the new method, simplifying its implementation. Added per-panel directory fallback chains and deduplication while preserving display order.

Changes

Cohort / File(s) Summary
View Simplification
Sources/ContentView.swift
Simplified TabItemView.directorySummaryText(orderedPanelIds:) by replacing manual loop with delegation to new tab.sidebarDirectoriesInDisplayOrder(orderedPanelIds:), removing early guard and de-duplication logic.
Workspace Directory Logic
Sources/Workspace.swift
Added sidebarDirectoriesInDisplayOrder methods and new helpers for directory canonicalization (expand ~, standardize paths), per-panel directory resolution with fallback chain (stored → requested → current), and deduplication while preserving display order. Updated sidebar directory entry computation.
Test Coverage
cmuxTests/WorkspaceUnitTests.swift
Added testSidebarBranchDirectoryEntriesStayStableAcrossFocusedSplitChanges to verify sidebar directories remain stable across currentDirectory changes and properly deduplicate while maintaining order.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 Hop skip and a bound! ✨
Directories dance in proper order,
No duplicates here—just clean display,
Sidebar springs organized and bright,
A rabbit's delight! 🌿

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: stabilizing sidebar directory ordering when split focus changes, which matches the core objective of the PR.
Description check ✅ Passed The description includes a comprehensive summary explaining what changed and why, detailed testing commands demonstrating verification, and addresses most required checklist items, though the demo video section is not applicable.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task-stable-sidebar-dir-order
📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 3 files

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Sources/Workspace.swift`:
- Around line 5942-5947: The current canonicalization uses
NSString.expandingTildeInPath which expands ~ against the local machine; update
canonicalSidebarDirectoryKey(_:) and the matching canonicalizer inside
SidebarBranchOrdering.orderedUniqueBranchDirectoryEntries(...) to perform tilde
expansion against the workspace's remote/home context when the workspace is
remote (use the workspace/connection's remoteHome or live CWD to resolve "~"),
then continue to standardizePath and trim whitespace as before so both sidebar
keys and branch-directory entries canonicalize identically in remote workspaces.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: de2faa34-7c26-44ce-b388-e99cf25b3d09

📥 Commits

Reviewing files that changed from the base of the PR and between 5cab7c4 and c8b495e.

📒 Files selected for processing (3)
  • Sources/ContentView.swift
  • Sources/Workspace.swift
  • cmuxTests/WorkspaceUnitTests.swift

Comment thread Sources/Workspace.swift
Comment on lines +5942 to +5947
private func canonicalSidebarDirectoryKey(_ directory: String?) -> String? {
guard let directory = normalizedSidebarDirectory(directory) else { return nil }
let expanded = NSString(string: directory).expandingTildeInPath
let standardized = NSString(string: expanded).standardizingPath
let cleaned = standardized.trimmingCharacters(in: .whitespacesAndNewlines)
return cleaned.isEmpty ? nil : cleaned

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make directory canonicalization remote-aware.

Line 5944 expands ~ against the local machine. In a remote workspace, that makes a requested cwd like ~/repo canonicalize differently from the later live cwd /home/remoteuser/repo, so the sidebar can still show duplicate entries for the same remote directory.

Suggested fix
     private func canonicalSidebarDirectoryKey(_ directory: String?) -> String? {
         guard let directory = normalizedSidebarDirectory(directory) else { return nil }
-        let expanded = NSString(string: directory).expandingTildeInPath
-        let standardized = NSString(string: expanded).standardizingPath
+        let pathForKey = isRemoteWorkspace
+            ? directory
+            : NSString(string: directory).expandingTildeInPath
+        let standardized = NSString(string: pathForKey).standardizingPath
         let cleaned = standardized.trimmingCharacters(in: .whitespacesAndNewlines)
         return cleaned.isEmpty ? nil : cleaned
     }

Please apply the same remote-aware rule to the matching canonicalizer inside SidebarBranchOrdering.orderedUniqueBranchDirectoryEntries(...) so the branch rows and plain directory summary stay consistent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Workspace.swift` around lines 5942 - 5947, The current
canonicalization uses NSString.expandingTildeInPath which expands ~ against the
local machine; update canonicalSidebarDirectoryKey(_:) and the matching
canonicalizer inside
SidebarBranchOrdering.orderedUniqueBranchDirectoryEntries(...) to perform tilde
expansion against the workspace's remote/home context when the workspace is
remote (use the workspace/connection's remoteHome or live CWD to resolve "~"),
then continue to standardizePath and trim whitespace as before so both sidebar
keys and branch-directory entries canonicalize identically in remote workspaces.

@lawrencecchen
lawrencecchen merged commit 0010e10 into main Mar 19, 2026
16 of 18 checks passed
@lawrencecchen
lawrencecchen deleted the task-stable-sidebar-dir-order branch March 19, 2026 08:06
bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026
…ow-ai#1798)

* Add sidebar directory ordering regression test

* Stabilize sidebar directory ordering

---------

Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>

This branch was successfully deployed

1 active deployment
Preview — c8b495e0 Deployed Mar 19, 2026 by vercel[bot]
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