fix: sort numbered graph-switch shortcuts alphabetically - #824
Conversation
WalkthroughThe change adds alphabetical, case-insensitive ordering for recent graphs with root-path tie-breaking, tests the ordering helper, and updates graph-switching shortcuts to use the resulting graph-root order. ChangesAlphabetical graph shortcut ordering
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Shall we just sort graphs alphabetically? |
Per review: instead of keeping numbered graph slots (Cmd+1..9) in first-seen order, sort them alphabetically by graph name so a given number maps to a predictable graph. Case-insensitive with the root path as a stable tiebreak. This also drops the first-seen reconciliation bookkeeping.
|
Good call - switched to sorting alphabetically by graph name in 166f923, so Cmd+1 maps to the alphabetically-first graph and so on. More predictable than first-seen order, and it let me drop the stable-order reconciliation entirely (net simpler). Tests + typecheck green. I'll update the PR title/description to match the alphabetical approach. |
mvanhorn
changed the title
|
Update: the repo |
|
Thanks for the heads-up @ocavue — retargeted this PR to |
There was a problem hiding this comment.
Pull request overview
This PR makes Cmd+1 through Cmd+9 graph-switch shortcuts deterministic by mapping shortcut slots to graphs ordered alphabetically (case-insensitive) by graph name instead of MRU recents ordering, addressing the unpredictability described in #816.
Changes:
- Added
sortGraphsByNamehelper to compute stable alphabetical ordering for recent graphs (root path tiebreak). - Updated app shortcut handling to use the sorted ordering for numbered graph switching.
- Added regression tests for the new sorting behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/desktop/src/routing/app-shortcuts.ts | Switches numbered graph shortcut selection from MRU order to alphabetical graph-name order via sortGraphsByName. |
| apps/desktop/src/lib/commands/app-commands.ts | Introduces sortGraphsByName helper used to derive stable Cmd+1..9 graph slot ordering. |
| apps/desktop/src/lib/commands/app-commands.test.ts | Adds unit tests verifying alphabetical, case-insensitive sorting with root-path tiebreak. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| generationRef.current = graph?.generation ?? null | ||
| graphRootRef.current = graph?.root ?? null | ||
| recentsRef.current = recents | ||
| graphOrderRef.current = sortGraphsByName(recents) |
Summary
Command+1 through Command+9 now switch to graphs sorted alphabetically by graph name instead of following the most-recently-used ordering, so a given number maps to a predictable graph.
Why this matters
#816 reported that Command+1 / Command+2 graph switching behaved unpredictably. The shortcuts indexed into the graph list in MRU order, so every switch reordered the list and the number-to-graph mapping changed on each use. Pressing Command+2 twice could land on two different graphs.
What changed
sortGraphsByNameinapp-commands.ts, applied inapp-shortcuts.ts.app-commands.test.ts).Testing
Targeted Vitest for the changed command module: 36 passed. Typecheck + oxlint clean.
Note
There is a separate, pre-existing collision between the Meta+digit graph shortcuts and the Meowdown heading shortcuts. That is out of scope for this fix and left unchanged; happy to file or address it separately if useful.
Closes #816