Pin the keybindings row order to one collation

[linyiru]

prioritize_entries ends with sort -k1,1n -k2,2 (bin/omarchy-menu-keybindings:504), and that sort decides the order of every row in the keybindings cheatsheet. Key 1 is the numeric priority. Key 2 is the second blank-separated field, which works out to the leading modifier token, so it takes 23 values across the shipped binds: SUPER, SHIFT, CTRL, ALT, PRINT and the 18 bare XF86* keys. Tapping the pipeline where the sort reads it, 174 of the 226 rows tie with at least one other row on both keys, and the largest single tie group is 75 rows. Every one of those ties is settled by sort's last-resort whole-line comparison, and that comparison follows LC_COLLATE.

So the cheatsheet is in a different order depending on the locale the menu happens to run under.

Measured

On e332dc97, coreutils 9.11 and glibc 2.44, feeding the real script the binds the shipped Lua config registers, with a fresh cache per run so nothing is served from the previous locale:

both orders hold the same 226 rows
rows at a different index under en_US.UTF-8 than under C   →  120 of 226
largest move                                               →  70 places

SUPER + SLASH → Monitor scaling up is row 104 under C and row 174 under en_US.UTF-8. Three more rows move 42 places, all nine SUPER CTRL ALT rows move as a block by 22, and 106 rows stay where they were.

The mechanism is that en_US collation ignores spaces and + at the primary level, so SUPER + SLASH is weighed as SUPERSLASH against SUPER ALT + SLASH as SUPERALTSLASH and loses to it, while under C the + (0x2B) sorts below A (0x41) and every SUPER + <key> row stays ahead of the SUPER ALT ones. Stripping the punctuation by hand reproduces the en_US order exactly:

$ printf 'SUPERSLASH\nSUPERALTSLASH\n' | LC_ALL=en_US.UTF-8 sort
SUPERALTSLASH
SUPERSLASH

In other words C keeps a modifier set together and en_US interleaves the sets.

The change

Name the collation on the one sort that has this dependency:

LC_ALL=C sort -k1,1n -k2,2 |

That is locale-independent by construction, and it is the pattern the repo already uses where output has to be stable (bin/omarchy-theme-color:294).

What users see

	under C / C.UTF-8	under en_US.UTF-8
today	one order	a second order, 120 of the 226 rows elsewhere
with this change	unchanged, byte identical	the same order as C

So this is a visible change for anyone on a non-C collation, and worth a look before merging. Two things argue it is the right order to land on: it keeps each modifier set contiguous instead of interleaving them, and it is the order the repository's own ordering assertions already check, because the test harness runs the menu under env -i (test/shell.d/keybindings-menu-test.sh:44).

Tests

Two assertions in test/shell.d/keybindings-menu-test.sh. The first renders the same two binds under C and under en_US.UTF-8 and requires the same order; it fails on the base script, where SUPER + SLASH is row 3 under one collation and row 4 under the other. Since it can only do real work where a second collation is generated, it is skipped rather than passed silently when en_US.UTF-8 is missing, and a second assertion pins the mechanism on every box.

./test/cli and ./test/shell were run on this branch and on quattro on the same machine. 8 assertions fail, the same 8 on each side and no others, all of them checking system state a plain checkout does not have. keybindings-menu-test.sh itself goes 16/16 to 18/18.

This replaces the approach the PR was opened with

Worth stating plainly, because the thread above argues about the other one. This PR originally left the sort alone and added the collation to the records cache key instead, on the reasoning that it preserved the order en_US users see today. @llstrk's review found two holes in that, both of which I reproduced: sort falls back to byte comparison when setlocale fails or the named locale is not generated, and neither of those changes LC_ALL, LC_COLLATE or LANG, so the key matches while the order does not. Pinning the sort closes both by construction.

The description also carried two errors of mine, corrected above. It reported 31 rows moving by up to 4, which was measured on the declared binds rather than on the rendered rows; the user-visible figure is 120 of 226 by up to 70. And it said adding -s to the sort would make the order locale-independent outright, which is too strong: -s drops only the last-resort comparison, while -k2,2 is still compared with strcoll. For the shipped binds that happens to be enough, but it is not independent, and a synthetic case shows it moving:

$ printf '1\ta + A\n1\tB + A\n1\t~ + A\n1\tz + A\n1\tA + A\n' >syn
$ LC_ALL=C           sort -s -k1,1n -k2,2 syn | cut -f2 | paste -sd'|'
A + A|B + A|a + A|z + A|~ + A
$ LC_ALL=en_US.UTF-8 sort -s -k1,1n -k2,2 syn | cut -f2 | paste -sd'|'
~ + A|a + A|A + A|B + A|z + A

Dropping the cache-key edit also gets this out of the way of two open changes it was pushing against. #8140 reports that an input to keybindings_cache_key which flips at runtime costs a ~540 ms rebuild each time, made worse because refresh_keybindings_cache deletes every sibling cache; #8229 is the open fix and it removes an input. Adding one was the wrong direction, and the two conflicted inside keybindings_cache_key itself. They no longer do: git merge-tree on this branch and #8229 leaves bin/omarchy-menu-keybindings clean, with one adjacency conflict where both append to the end of the same test file. This version also needs no cache version bump, which matters because v15 is already claimed by both #8229 and #7564.

Found while surveying what a locale change does to Omarchy for #12345, and sent on its own as promised in that issue's §7. Of the four follow-ups named there this is the one that is a defect on today's tree, and it has nothing to do with translated text: the strings are English either way, it is the order they come out in that moves.

🤖 Generated with Claude Code

[llstrk]

Automated AI review

Community review: Independent automated community review, unaffiliated with the Omarchy team, intended to help prepare PRs for their review.

Verified: the defect is real and this change fixes the reported case. On the base, a cache built under LC_ALL=C keeps serving the C order to later en_US.UTF-8 calls. On the head, each locale gets its own order, and within each locale the output is byte-identical to the base. The first new assertion fails against the base script and passes on the head, including in an environment without en_US.UTF-8 installed.

Two cases still serve a stale order, because the key hashes locale names rather than the collation sort actually loads. A smaller point about the -s alternative and a correction to the size of the effect follow.

The key can still match while sort's collation changes

sort calls setlocale(LC_ALL, "") and falls back to byte comparison when that call fails or the named locale cannot be loaded. Neither situation changes LC_ALL, LC_COLLATE or LANG, so the key stays the same:

Same binds, same keymap, one cache directory (head script, --print)

Case A: another category names a missing locale
  1. LANG=en_US.UTF-8  LC_TIME=<missing locale>  -> setlocale fails, C order cached
  2. LANG=en_US.UTF-8  (LC_TIME removed)         -> same key, cached C order served
                                                    fresh build: en_US order

Case B: locale generated after the cache was built
  1. LANG=en_US.UTF-8, locale not generated yet  -> C fallback order cached
  2. same environment, locale now generated       -> same key, cached C order served
                                                    fresh build: en_US order

In both cases the served and fresh lists hold the same 227 rows, with 119 of them at different positions. The base behaves the same way, so neither case is a regression. They are gaps in the fix.

Impact: narrow. Case A needs an LC_* variable (for example an LC_TIME line in /etc/locale.conf) that names a locale that was never generated. Case B needs a locale generated after the menu first ran. Either way the stale order persists until a bind or the keymap changes.

Suggested change: force the collation on the sort that creates the dependency, LC_ALL=C sort -k1,1n -k2,2 in prioritize_entries, and drop the key change. This is locale-independent by construction, removes both cases above, and matches the existing pattern in bin/omarchy-theme-color. The trade-off is the one the PR description raises:

Option	C / C.UTF-8 order	en_US.UTF-8 order	Cases A and B
This PR (key on three variables)	unchanged	unchanged	stale order served
LC_ALL=C sort -k1,1n -k2,2	unchanged	becomes the C order (119 of 227 rows move)	not affected

The C order keeps each modifier set together, for example all SUPER + <key> rows before SUPER ALT and SUPER CTRL rows. The en_US order interleaves them because en_US collation ignores the spaces and + in a chord. Which order is preferable is a presentation choice. The repository's own ordering assertions already run under env -i, so they check the C order.

-s alone does not make the order locale-independent

The description says -s makes the order locale-independent outright. -s removes only the whole-line last-resort comparison. The -k2,2 key is still compared with strcoll outside C/POSIX (coreutils sort.c, keycompare). For the shipped binds the second field happens to sort the same in C and en_US, so -s gives identical output there. A synthetic run with modifier-less key names (for example a + A, B + A, ~ + A) still reorders between C and en_US under sort -s -k1,1n -k2,2. LC_ALL=C on the sort is needed for full independence.

Description corrections: with every bind the shipped Lua config registers (including loop-generated workspace and panel binds, 227 rendered rows), 119 rows change position between C and en_US.UTF-8, by up to 70 places (for example SUPER + SLASH moves from row 105 to 175), rather than 31 rows by up to 4. Also, refresh_keybindings_cache keeps a single records file, so alternating between two locales rebuilds on every switch (about 0.1 s per rebuild in the test environment), rather than once per locale.

---

Review information

Test scope: Source review of the pinned head 02d29f7 and base 9c5482c, plus isolated command-only runs of the real base and head scripts with a stubbed hyprctl on coreutils 9.11 and glibc 2.44, with C, C.UTF-8 and en_US.UTF-8 available. The bind feed was built from the shipped Lua config in Hyprland's reported __lua form. It is not a capture of a live hyprctl binds, so exact row counts could differ somewhat on a real desktop. The missing-locale and later-generated-locale cases were simulated with a nonexistent locale name and a private LOCPATH. The dependency sources read were coreutils sort.c, glibc setlocale, Hyprland v0.56.2 key parsing and libxkbcommon xkb_keysym_from_name. No live desktop, locale transition in a real session or full repository test suite was run.

AI process: Opus 5.5 Medium coordination and synthesis, independent Opus 5.5 Xhigh and GPT 6 Sol Xhigh technical assessments with targeted cross-checks of each other's single-sourced claims, Opus 5.5 Medium editorial check.

Opt out: To stop receiving these reviews, reply to this comment saying so.

[linyiru]

Taking the suggested change. The branch now pins the sort and touches the cache key not at all, and I have retitled the PR and rewritten the description to match. Both corrections in the review were right, and I reproduced them rather than take them on trust.

The size of the effect. On e332dc97 with coreutils 9.11 and glibc 2.44, feeding the real script the binds the shipped Lua config registers and giving each locale a fresh cache: 226 rendered rows, 120 of them at a different index under en_US.UTF-8 than under C, largest move 70 places, SUPER + SLASH → Monitor scaling up going from row 104 to row 174. That is the review's 119 of 227 by up to 70 with SUPER + SLASH at 105 to 175, one row off in each figure, which I read as the feed rather than a disagreement: mine comes from the script's own Lua scan of the shipped config, 229 binds of which three collapse into another row, so a feed assembled any other way can land a row either side of that. My published figure of 31 rows by up to 4 was measured on the declared binds instead of the rendered rows, which is the wrong unit for a user-visible list, and it is corrected in the description.

-s. Confirmed, including the synthetic case:

$ printf '1\ta + A\n1\tB + A\n1\t~ + A\n1\tz + A\n1\tA + A\n' >syn
$ LC_ALL=C           sort -s -k1,1n -k2,2 syn | cut -f2 | paste -sd'|'
A + A|B + A|a + A|z + A|~ + A
$ LC_ALL=en_US.UTF-8 sort -s -k1,1n -k2,2 syn | cut -f2 | paste -sd'|'
~ + A|a + A|A + A|B + A|z + A

So the description's "locale-independent outright" was wrong about -s for the reason given, and that is corrected too. LC_ALL=C sort -k1,1n -k2,2 measures at 0 moved rows, and leaves what C users see byte identical.

Cases A and B. I did not reproduce these two individually, because pinning the sort makes both moot rather than narrower: there is no longer a cached order that depends on a locale. Worth saying that they are the reason the approach changed and not just an edge case I accepted, since a key that hashes locale names can only ever approximate the collation sort actually loads.

One thing I can add from outside the diff, since it is about where this sits rather than what it does. The cache-key approach was pushing against two open changes, which I had not found when I opened this. #8140 reports that an input to keybindings_cache_key that flips at runtime costs a ~540 ms rebuild every time it flips, made worse because refresh_keybindings_cache deletes every sibling cache, so the two states evict each other. #8229 is the open fix for it, and it removes an input. Adding one was the wrong direction on a question the tracker had already worked out, and the two conflicted inside keybindings_cache_key itself. After this rewrite they do not: git merge-tree on this branch and #8229 leaves bin/omarchy-menu-keybindings clean, with a single adjacency conflict where both PRs append to the end of keybindings-menu-test.sh. The pinned sort also needs no cache version bump, which is worth something on its own, since v15 is currently claimed by both #8229 and #7564.

The one thing a reviewer still has to decide is the one the review framed correctly: this makes en_US users see the C order, and that is 120 rows moving in a list people have memorised. I now think that is the order to land on rather than a cost to avoid, because it keeps each modifier set contiguous instead of interleaving them, and because it is the order the repository's own ordering assertions already check under env -i. But it is a presentation call and it belongs to the maintainers, so the description states it as a visible change rather than burying it.

[omarchybot]

Reviewed at dcefff80f and run on a disposable Omarchy worker (stock install, LANG=en_US.UTF-8, coreutils 9.11). Pinning the sort works, but an install that already has a keybindings cache keeps the old order, so the fix does not reach existing en_US users until their binds or keymap change.

The bug is real. I rendered the menu from the worker's live Hyprland binds (229 binds, 224 rows), with a fresh cache for each run. On the base script, 118 of the 224 rows sit at a different position under en_US.UTF-8 than under C. For example, SUPER + SLASH → Monitor scaling up is row 103 under C and row 172 under en_US. On the head, nothing moves between the two locales. The C output is byte-identical to base, and en_US now gets the C order. The one-line change is proportionate to the problem. Your new collation assertion fails with the fix reverted and passes on the head, and keybindings-menu-test.sh goes 18/18 on the worker. ./test/cli stops at the same unrelated failure on the head and with the fix reverted (vscode generated theme references current theme file).

Open: the cache version needs to move (bin/omarchy-menu-keybindings:541). The records are cached already sorted. The cache key hashes v14, the active keymap and hyprctl binds, and none of those changes when this script does. On the worker, I wrote a cache with the base script under en_US, then ran the head script under en_US against the same cache directory. It served the old en_US order byte for byte. Since en_US is the default locale, most installs will keep today's order after updating, and then switch to the new order at some later, unrelated moment when a bind changes. The fix is to bump the version, plus a test that keeps a cache written by the old version across the change, which is what #13888 does for its own reordering. The catch is the number: #8229 and #7564 both take v15 and #13888 takes v16, so whichever value this uses needs coordinating with the maintainer, or two of them merge with no bump at all.

Minor: the equality check at test/shell.d/keybindings-menu-test.sh:255 also passes if both renders fail and print nothing. Asserting that each render contains both SLASH rows before comparing them would close that.

Second opinion: Codex Medium reviewed the diff. It agreed that the problem is real and the change proportionate, and it agreed on the cache finding, which my brief had already pointed it at, so that agreement is not independent. It found the empty-output gap in the test on its own. I checked both against the source.

Waiting on the author for the cache version bump. The maintainer also has a presentation call to make: en_US users will see the C order, with about half the rows in new places.

[linyiru]

Both findings reproduced, both fixed, three commits on top of dcefff80. Measured on an Arch box with en_US.utf8 generated.

The cache

You are right, and the mechanism is worse than a missed invalidation: the key hashes the version literal, the active keymap and hyprctl binds, so the file name does not move when the sort does. One cache directory, the two chords that swap, hyprctl stubbed so every script sees the same binds:

run	cache file	what printed
e332dc97 under en_US.UTF-8	writes keybindings-a2c267b8…	SUPER ALT + SLASH above SUPER + SLASH
dcefff80 over that cache	reads keybindings-a2c267b8…, the same name	the same two rows, byte for byte
267dc049 over that cache	writes keybindings-510abc0b…, deletes the old one	SUPER + SLASH above SUPER ALT + SLASH

So the bump is the whole difference between the fix reaching an install and not reaching it, and it leaves nothing behind: refresh_keybindings_cache already deletes every other keybindings-*.records after it writes, which the third row shows.

bf17b948 bumps the literal and plants a record under the key the previous version would have written, the idiom #13888 uses. With the bump the file is 19 ok / 0 not ok; with the bump reverted and nothing else touched it is 18 ok / 1 not ok, and the one that fails is that assertion.

On the number: v18. Checked today, v15 is taken by #7564 and by #8229, v16 by #13888, v17 by #12557, all four open. Two branches setting that line to the same value merge with no conflict and leave no bump at all, which is the failure the bump exists to prevent, so the value has to be one nobody has claimed. Whichever of those merges first makes the next one conflict on that line, and that is a conflict I would rather have than not see.

The coordination is a smell and you named it. If you want, I will open a separate PR that derives the version from the script rather than from a hand-kept literal, so that any change to how records are built retires the cache without anyone having to remember. The cost is a refresh on every update that touches this file: 7 ms cached against 36 ms uncached here, and 36 is a floor rather than a desktop figure, since this run stubs hyprctl and the Lua scan gives up partway through the shipped config. Not in this PR either way.

The test

Also right, and the reason is sharper than two empty renders: there is no empty render to notice. static_bindings prints its two rows unconditionally and they tie the same way under either collation, so with hyprctl reporting no binds both renders came out at 130 identical bytes and the assertion still said ok with the rows it is about gone. A non-empty check would not have closed that. 4dd31db9 asserts both renders carry both SLASH rows before comparing, which under that same stub now reports not ok - both collations render the rows the comparison is about and prints the two leftovers. It also keeps each render in a variable instead of rendering four times to compare and then report.

267dc049 is not from your review, it is something checking your point turned up: test/shell.d/theme-install-guards-test.sh guards on locale -a for the same reason and allows the hyphenated spelling, so a box printing en_US.utf-8 would run that check and skip this one. Both spell it the same way now.

Suites

Same box, same day, each side in its own worktree, head 267dc049 against base e332dc97:

• ./test/shell: 3946 ok / 9 not ok on the head, 3943 / 9 on the base, the same nine
• ./test/cli: 88 ok / 1 not ok on both, vscode generated theme references current theme file, as you saw

Across 286 files the per-file ok / not ok diff has exactly one line in it: keybindings-menu-test.sh, 16 on the base and 19 on the head. Two of the nine are signal-timing tests that lose a race under load; they failed on both sides here, so none of that set is this branch's.

The presentation call

That one is the maintainer's, so here is the reasoning rather than a recommendation. Two things bear on it and neither is about taste.

@dhh has already made this call once in this repo. bin/omarchy-theme-install:45 pins the theme name allowlist to C because a bracket range follows the collation, with its test guarded on locale -a the same way, in 2b923cf. Same question, same answer, which makes this the second instance rather than a new policy.

And C is the only collation that is always present. sort under a locale that was never generated falls back to comparing bytes without saying so: LC_ALL=zz_ZZ.UTF-8 sort here produced the C order exactly. So pinning to en_US.UTF-8 instead, to keep today's order on the installs you measured, would be a pin that quietly is not one wherever that locale is absent, and those desktops would be back to having an order of their own. Pinning to C moves rows once, on installs that have a cache, at the moment the cache is retired. The 118 of 224 is your measurement on a live install, not mine.

Thank you

This is more review than a one-line diff usually gets. You rendered 229 live binds with a fresh cache per run instead of taking the diff's word for it, you ran my new assertion with the fix reverted, and the cache finding is the one thing here that cannot be seen in the diff at all: it needed somebody to write a cache, change the script underneath it and look again. I did not catch it.

I also want to name the part that is easy to read past. You wrote that Codex agreed about the cache because your brief had already pointed it there, so that agreement was not independent, and you said which finding it reached on its own. Reviewers do not often mark their own evidence down, and it is why I could tell which parts of the review to go and check and which to simply trust.

Still based on e332dc97, so the numbers above stay comparable with yours. It merges clean with today's quattro (c05d9019) if you would rather I move it. And if #13888 lands first, its stale-cache block and mine are the same idiom twice and should collapse into one on the rebase.

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Pins keybinding menu sort order to C locale.

The PR appears safe to merge; no actionable issue was identified.

Summary

The PR pins keybindings-row sorting to C collation and changes the cache key so previously sorted records are rebuilt.

• Adds a cross-locale ordering assertion when en_US.UTF-8 is available, plus an unconditional check for the pinned sort.
• Tests that a record cached under the previous key is not served.

_{Reviews (1) · Last reviewed commit: "Spell the locale guard the way the theme..."}

[omarchybot]

Read the three commits on top of dcefff80 at 267dc049, and none of it has been verified yet. It was not run on a worker, and no second model has looked at it. Codex Medium, the configured reviewer, is up (it answered a ping), but today's cap on reviewer calls had already been reached. So the PR could not be re-classified at the new head, and the run stopped there instead of bringing it to verified without a second opinion.

From reading alone, the commits respond to both open findings from the last review. keybindings_cache_key moves to v18 (bin/omarchy-menu-keybindings:541), and a new assertion plants a record under the v14 key and requires that it is not served. The collation comparison now requires both renders to carry the two SLASH rows before it compares them. The locale guard accepts en_US.utf-8 as well. v18 is not claimed by any other open PR: #7564 and #8229 take v15, #13888 takes v16 and #12557 takes v17. The branch also merges cleanly with today's quattro.

Waiting on a review run with a second opinion, which will run the stale-cache and collation assertions on a worker. Nothing is needed from the author. The presentation call from last time (en_US users seeing the C order) is still the maintainer's.

[linyiru]

Nothing needed here, and both time-sensitive halves of your read still hold at 267dc049: quattro moved overnight from c05d9019 to 821ae589 and is still v14, and the branch still merges clean against it (git merge-tree, exit 0, no conflicted paths).

One precision, and it is on my list rather than on your reading of it. The four-PR picture came from my comment, and it is partial.

The whole set

Enumerated over every open PR's changed paths rather than by keyword: 32 open PRs touch bin/omarchy-menu-keybindings. What each sets the literal to:

literal	open PRs
untouched	#6100, #6664, #11733, #10614
v11	#7434
v13	#8569, #7722, #11035, #12155, #11010, #6105, #13993
v14, as quattro	#5881, #13276, #13576, #13461, #12712, #13923, #12916, #13362
v15	#8229, #9535, #7564, #7660, #12676, #10402, #12042
v16	#13888, #10468
v17	#12557
v18	this one
v19	#11851

Your conclusion holds: v18 is claimed by nobody else. The queue is five levels deep now, #11851 having taken v19 in a commit dated 2026-10-02T09:20Z, after mine.

The method had a hole worth naming, since it is the same class of defect as the one this PR fixes. My first pass read 100 changed paths per PR, and 14 open PRs change more than that, so it quietly missed any of them that touch this file. Two do: #6105 and #13993, both v13. The count above is after checking all 14 through the files endpoint instead. Had I published the first number it would have read as exhaustive and been short by two.

Where I overstated it

I wrote that two branches setting that line to the same value "merge with no conflict and leave no bump at all". The second half is right, the first is not, and the gap between them is the interesting part. Merging every pair of those seven v15 PRs onto today's quattro in order, 21 pairs:

• 6 pairs merge clean end to end and land on one v15
• 14 conflict elsewhere in that same file; 1 conflicts on the first merge
• in 0 of those 14 is the literal line inside a conflict hunk

It is outside every time. Both sides write the byte-identical line, so git takes it silently and sends the resolver to an unrelated hunk with the version question never raised. So the failure mode is not "the merge is clean", which it usually is not. It is that this is the one line in the file a conflict will never put in front of anyone. All 21 pairs end at v15 where quattro is v14: one bump, two changes.

I have not audited what each of those seven does to the records, so I am not saying any of them is wrong, and none of those authors can see the other six numbers. That is the point rather than a complaint: the scheme asks seven unrelated people to coordinate an integer through a file none of them shares.

The offer, unchanged

That is the argument for deriving the version from the script rather than keeping the literal by hand, which I offered last time and am still not opening unasked. Measured cost is 7 ms cached against 36 ms uncached, the 36 being a floor. If a maintainer would rather the number stopped coming up, say so and it goes up as its own PR against quattro, independent of this branch and of the presentation call.

That call is still yours.

Thank you for saying outright that the run stopped unverified, which model was up, and that the cap rather than the code is why. Half of what I checked above I only checked because you marked that boundary instead of rounding it off.

🤖 Generated with Claude Code

[jandrusk]

Pinning keybindings sort collation is real (locale-dependent row order is a footgun). Currently CONFLICTING, and earlier omarchybot notes that existing keybindings caches keep the old order — a cache bust or version bump on the cache key would make the fix reach current installs. Rebase + that follow-through and this looks ready.