Repository navigation
fix(sidebar): order sidebar names the same on desktop, phone and server - #29946
Conversation
Sidebar names, project labels, project-group names and folder workspace names were ordered with localeCompare, whose result depends on the ICU data bundled with Electron, the phone WebView or Node. Order them with one comparator in src/shared/sidebar-list/text-order.ts instead: case-folded and NFC-normalized, digit runs by value, the rest by code point, and equal text broken by host-qualified id. Co-Authored-By: Claude <noreply@anthropic.com>
…w identity helper Co-Authored-By: Claude <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 11 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly and concisely describes the main change: consistent sidebar name ordering across desktop, phone, and server. Description check The description is complete and focused. It explains the user impact, implementation, rationale, testing, performance, cross-platform considerations, and checklist status. The linked issue is marked N… Full details: Description check
-
Explanation
The description is complete and focused. It explains the user impact, implementation, rationale, testing, performance, cross-platform considerations, and checklist status. The linked issue is marked N/A while referencing
#29906, which is a minor deviation from the template but does not make the description incomplete.
-
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving 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 @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
f309729d-aef8-4ddb-94c2-015039138fab
📒 Files selected for processing: 15
mobile/src/worktree/workspace-list-ordering.tssrc/renderer/src/components/sidebar/__goldens__/sidebar-list/group-by-none.txtsrc/renderer/src/components/sidebar/__goldens__/sidebar-list/group-by-pr-status.txtsrc/renderer/src/components/sidebar/__goldens__/sidebar-list/group-by-repo.txtsrc/renderer/src/components/sidebar/__goldens__/sidebar-list/group-by-workspace-status.txtsrc/renderer/src/components/sidebar/smart-sort.tssrc/renderer/src/components/sidebar/worktree-list/grouping/recent-project-rank-scan-budget.test.tssrc/renderer/src/components/sidebar/worktree-sort-label-ordering.test.tssrc/shared/project-section-order.tssrc/shared/sidebar-list/folder-workspace-lanes.tssrc/shared/sidebar-list/project-group-sections.tssrc/shared/sidebar-list/text-order.test.tssrc/shared/sidebar-list/text-order.tssrc/shared/workspace-view/workspace-row-filters.test.tssrc/shared/workspace-view/workspace-row-sort.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
| const compareRecent = (a: Worktree, b: Worktree): number => | ||
| recentActivity(b) - recentActivity(a) || a.displayName.localeCompare(b.displayName) | ||
| recentActivity(b) - recentActivity(a) || | ||
| compareSidebarText(a.displayName, b.displayName) || |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
rg -n 'getWorktreeSortLabel|trim' src/renderer/src/components/sidebar/smart-sort.ts
sed -n '1,70p' mobile/src/worktree/workspace-list-ordering.tsRepository: stablyai/orca
Length of output: 3508
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- desktop smart-sort relevant block ---'
nl -ba src/renderer/src/components/sidebar/smart-sort.ts | sed -n '25,110p'
printf '%s\n' '--- workspace row comparator declarations and label use ---'
rg -n -F --glob '*.ts' -e 'function buildWorkspaceRowComparator' -e 'label:' -e 'options.label' -e 'label(' src/shared mobile
nl -ba src/shared/workspace-view/workspace-row-sort.ts | sed -n '1,220p'
printf '%s\n' '--- mobile ordering at merge base ---'
git show 776c791554bf4a4dfd4298cb57cddf60316c9d2c:mobile/src/worktree/workspace-list-ordering.ts | nl -ba | sed -n '1,95p'
printf '%s\n' '--- mobile changes in the PR ---'
git diff --unified=20 776c791554bf4a4dfd4298cb57cddf60316c9d2c 5f61ae95741dd47ec9e15fe5d9d19992daa4f56e -- mobile/src/worktree/workspace-list-ordering.ts
printf '%s\n' '--- desktop label history across base and head ---'
git diff --unified=12 776c791554bf4a4dfd4298cb57cddf60316c9d2c 5f61ae95741dd47ec9e15fe5d9d19992daa4f56e -- src/renderer/src/components/sidebar/smart-sort.ts src/shared/workspace-view/workspace-row-sort.tsRepository: stablyai/orca
Length of output: 41569
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- shared text ordering ---'
nl -ba src/shared/sidebar-list/text-order.ts | sed -n '1,220p'
printf '%s\n' '--- mobile Worktree displayName declaration ---'
rg -n -F --glob '*.ts' -e 'export type Worktree' -e 'interface Worktree' -e 'displayName:' -e 'displayName?' mobile/src/worktree src/shared/worktree
printf '%s\n' '--- relevant Worktree type block ---'
for f in mobile/src/worktree/workspace-list-types.ts src/shared/worktree/types.ts; do
if test -f "$f"; then
echo "--- $f ---"
nl -ba "$f" | sed -n '1,180p'
fi
doneRepository: stablyai/orca
Length of output: 30324
Use the desktop-resolved label for mobile sorting.
getWorktreeSortLabel trims displayName, but mobile passes the raw value to both compareRecent and the row comparator. A leading-space name can therefore sort before Alpha on mobile while its trimmed label sorts after Alpha on desktop.
Resolve the mobile label with the same trim and fallback rules, then use it for both comparisons. The raw accessors predate this PR, but the ordering mismatch remains.
Accents and case now only break exact ties, so "Éclair" sorts beside "eclair". Co-Authored-By: Claude <noreply@anthropic.com>
The workspace-list builder moved to src/shared/sidebar-list (stablyai#29906), and the sidebar now draws one header per project group id when two hosts publish the same group (stablyai#30128). The branch linked each host's copy to a parent on its own host, which assumed every copy was drawn; with copies collapsed, parents now resolve by id over the drawn groups, the same links reveal walks. Same-id copies from two hosts can still close a parent loop, so section emission and getProjectGroupOccurrencePaths share one parent map that cuts it with breakProjectGroupParentCycles. compareProjectGroupSiblings adopts the device-independent sidebar order from stablyai#29946 instead of localeCompare. createOnTarget moves to project-group-create-on-host.ts, and two repo RPC test expectations fold into one, to keep both files under max-lines.
ELI5
The sidebar sorts workspace names, project names and group names alphabetically. It used the browser's built-in "alphabetical" rule, and that rule is not the same everywhere: the desktop app, the phone, and a headless server each carry their own copy of the language data behind it. So the same names could come out in a slightly different order on the phone than on the desktop. This PR gives the sidebar one fixed alphabetical rule that gives the same answer on every device.
What Changed
Before: names were ordered with
localeCompare, whose answer depends on the language data bundled with Electron, the phone's web view, or Node. Rows with exactly the same name kept whatever order they happened to arrive in.After: one comparator,
src/shared/sidebar-list/text-order.ts:It is used for workspace names in every sort mode (desktop and phone share
workspace-row-sort.ts), project header names, project group names, and folder workspace names. Host headers already follow the user's saved host order and never sort by name, so nothing changed there. OtherlocaleCompareusers outside the sidebar list are untouched.Golden fixture changes (
src/renderer/src/components/sidebar/__goldens__/sidebar-list/)Only ties moved. The fixtures have no accented or numbered names, so the only rows that moved are rows whose names are identical: every repo's main checkout is called "main", and it has the same recent activity time as the others.
group-by-none.txt,group-by-pr-status.txt,group-by-workspace-status.txt: in Name, Recent and Repo sort, the five "main" rows were in store order: orca, orca-ssh, api, lab, ext. They are now in id order: api (paired server), ext, lab, orca-ssh, orca. In Recent sort the local orca "main" has newer activity and stays where it was, so only the other four reorder. Shortcut targets (Cmd+1 to 9) and selection order follow the same rows.group-by-repo.txt: same change. Inside the shared "orca" project, its two "main" checkouts (local and GPU VM) swap so the GPU VM one comes first. Every combination of project order and pin setting is affected the same way. Project headers did not move.Why
The sidebar is moving toward one builder that the desktop and the phone both use, and they have to agree on order down to the row. A built-in collator can't promise that because each runtime ships its own data. A fixed rule that uses only case folding, Unicode normalization and code points needs no language data, so every device gets the same order. I considered reusing the existing
fileNameCollator(src/shared/file-name-sort.ts), but it is a built-in collator too, so it has the same problem.Sorting 10,000 rows takes about 12 to 18 ms with this comparator, versus about 4 to 6 ms before. Each sort folds each label once and reuses the result, and plain ASCII names skip the accent step. A normal sidebar of a few hundred rows takes well under a millisecond.
Linked Issue
N/A, part of the sidebar list refactor (follows #29906).
Visual Proof
N/A: this only reorders rows whose names tie, or names with accents and numbers. The golden fixture diffs above show the exact before and after order.
Testing
text-order.test.ts: accents ignored then used as a tie-break, case, "w2" < "w10", leading zeros, NFC vs NFD treated as equal, astral characters, ties, plus an antisymmetry check over a mixed set of names.worktree-sort-label-ordering.test.ts: two rows with the same name on two hosts now sort the same way whatever order they arrive in.recent-project-rank-scan-budget.test.ts: projects with equal labels now order by section key.Review
Reviewed for correctness (strict ordering, fallback when folded text ties, tie-break ids on desktop and phone, missed call sites) and performance. Applied: cache each label's folded form for the length of one sort, and reuse the phone's existing row-identity helper.
Agent skill upstream boundary
Notes
Cross-platform: no platform-specific code. SSH and paired servers: the tie-break includes the host, so the same workspace id on two hosts orders the same way. Mobile: the phone list uses the same shared comparator.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)