Bugfix: Show all commands in the shortcuts editor

vdavid · vdavid · commit 73766c9ee7fb · 2026-06-07T19:12:55.000+02:00
- Group commands by their `CommandScope` (one titled group per scope) so the 51 compound-scope commands (`Main window/File list`, `Main window/Brief mode`, every F-key and Quick Look command) now render and are rebindable. Previously they matched no group and never appeared.
- Extract the pure grouping into `keyboard-shortcuts-grouping.ts` with a set-equality regression guard: union of grouped commands === all registry commands.
- Deep links to compound-scope rows (`shortcut-file.quickLook` from the Quick Look toast) now land and flash; the deep-link E2E spec targets `file.quickLook` to pin it.
- Update the section CLAUDE.md gotcha to describe the new grouping.
diff --git a/apps/desktop/src/lib/settings/sections/CLAUDE.md b/apps/desktop/src/lib/settings/sections/CLAUDE.md
@@ -34,6 +34,7 @@ Parents: [`../CLAUDE.md`](../CLAUDE.md) (registry, store, applier, search) and
 | `ai-secret-error.ts`               | Pure mapper from OS secret-store error variants to user-facing strings. Used by `AiCloudSection`                                                                                                                                                                                                                                                                                                                                            |
 | `license-section-utils.ts`         | Pure label/status formatters extracted from `LicenseSection` for testability                                                                                                                                                                                                                                                                                                                                                                |
 | `ram-gauge-utils.ts`               | Pure stacked-bar segment math for `AiLocalSection`'s memory gauge (used → projected → free, plus warning thresholds)                                                                                                                                                                                                                                                                                                                        |
+| `keyboard-shortcuts-grouping.ts`   | Pure scope→group logic for `KeyboardShortcutsSection` (one titled group per `CommandScope`, fixed order). Tested by the set-equality regression guard                                                                                                                                                                                                                                                                                       |
 
 Each section ships with an `*.a11y.test.ts` (axe-core tier-3). `McpServerSection`, `UpdatesSection`, `SearchSection`,
 and `FileSystemWatchingSection` also have functional `*.test.ts` / `*.svelte.test.ts` files; the three pure-helper `.ts`
@@ -102,16 +103,19 @@ Settings AI changes hot-apply because `settings-applier.ts` routes `ai.provider`
 `ai.cloudProviderConfigs` to `ai-config.ts::pushConfigToBackend()`, which re-reads everything fresh. Sections just call
 `setSetting(...)`; don't try to push the AI config from the section component.
 
-### Compound-scope rows don't render in any group
+### Every command groups by scope (one group per `CommandScope`)
 
-`KeyboardShortcutsSection` groups commands by **exact** `command.scope` match against a fixed `scopes` list
-(`['App', 'Main window', 'File list', …]`). A command whose registry scope is a compound path like
-`'Main window/File list'` (e.g. `file.quickLook`, `file.contextMenu`) matches NEITHER `'Main window'` NOR `'File list'`,
-so it lands in no group and never renders here. A deep link to such a row (`shortcut-file.quickLook`) still opens the
-section, but the scroll silently no-ops because the row element doesn't exist. This is a pre-existing display gap, not
-something the deep-link work introduced; fixing it (group compound scopes under their parent, or widen the match) is
-separate work. The Quick Look toast deep-links to `file.quickLook` anyway because that's the truthful target — when the
-display gap is fixed, it lands on the row with no further change.
+`KeyboardShortcutsSection` renders one titled group per `CommandScope`, in a fixed reading order, via the pure
+`groupCommandsByScope` (`keyboard-shortcuts-grouping.ts`). Compound scopes (`'Main window/File list'`,
+`'Main window/Brief mode'`, `'Main window/Volume chooser'`, …) each become their own group titled by the last segment
+("File list", "Brief mode", …). So every registry command lands in exactly one group and is rebindable here, including
+`file.quickLook` and the F-key commands. Don't reintroduce an ad-hoc title list matched against scopes: the group set
+must stay the scope union, or whole groups of commands silently vanish from the rebinding UI. The
+`keyboard-shortcuts-grouping.test.ts` set-equality test (union of grouped commands === all registry commands) is the
+guard; it also fails if a new `CommandScope` is added without a `scopeOrder` entry.
+
+Deep links to compound-scope rows now land + flash like any other (`shortcut-file.quickLook` from the Quick Look toast,
+F-key chips from the F-bar).
 
 ## Deep-link arrival into a shortcut row
 
diff --git a/apps/desktop/src/lib/settings/sections/KeyboardShortcutsSection.svelte b/apps/desktop/src/lib/settings/sections/KeyboardShortcutsSection.svelte
@@ -27,6 +27,7 @@
     } from '$lib/shortcuts'
     import { confirmDialog } from '$lib/utils/confirm-dialog'
     import GlobalShortcutRow from '$lib/downloads/GlobalShortcutRow.svelte'
+    import { groupCommandsByScope } from './keyboard-shortcuts-grouping'
     import { shortcutAnchorId } from '$lib/settings/settings-window'
     import {
         getPendingShortcutHighlight,
@@ -102,9 +103,6 @@
         }
     })
 
-    // Group commands by scope
-    const scopes = ['App', 'Main window', 'File list', 'Navigation', 'Selection', 'Edit', 'View', 'Help']
-
     // Get conflict count for badge
     const conflictCount = $derived.by(() => {
         // Trigger on shortcut changes
@@ -162,17 +160,9 @@
         return 'go to latest download global'.includes(nameSearchQuery.trim().toLowerCase())
     })
 
-    // Group filtered commands by scope
-    const groupedCommands = $derived.by(() => {
-        const groups: Record<string, Command[]> = {}
-        for (const scope of scopes) {
-            const scopeCommands = filteredCommands.filter((c) => c.scope === scope)
-            if (scopeCommands.length > 0) {
-                groups[scope] = scopeCommands
-            }
-        }
-        return groups
-    })
+    // Group filtered commands by scope into the fixed display order. Every command
+    // renders in exactly one group (keyed by its scope), so all are rebindable here.
+    const groupedCommands = $derived(groupCommandsByScope(filteredCommands))
 
     function handleKeyCapture(event: KeyboardEvent) {
         if (!editingShortcut) return
@@ -501,10 +491,10 @@
     {/if}
 
     <div class="commands-list">
-        {#each Object.entries(groupedCommands) as [scope, scopeCommands] (scope)}
+        {#each groupedCommands as group (group.scope)}
             <div class="scope-group">
-                <h3 class="scope-title">{scope}</h3>
-                {#each scopeCommands as command (`${command.id}-${String(shortcutChangeCounter)}`)}
+                <h3 class="scope-title">{group.title}</h3>
+                {#each group.commands as command (`${command.id}-${String(shortcutChangeCounter)}`)}
                     {@const shortcuts = getEffectiveShortcuts(command.id)}
                     {@const isModified = isShortcutModified(command.id)}
                     {@const hasConflicts = conflictingIds.has(command.id)}
diff --git a/apps/desktop/src/lib/settings/sections/keyboard-shortcuts-grouping.test.ts b/apps/desktop/src/lib/settings/sections/keyboard-shortcuts-grouping.test.ts
@@ -0,0 +1,43 @@
+import { describe, it, expect } from 'vitest'
+import { commands } from '$lib/commands/command-registry'
+import { groupCommandsByScope, groupedScopes } from './keyboard-shortcuts-grouping'
+
+describe('groupCommandsByScope', () => {
+  it('renders every registry command in exactly one group', () => {
+    const groups = groupCommandsByScope(commands)
+    const grouped = groups.flatMap((g) => g.commands)
+
+    // No command appears twice.
+    const ids = grouped.map((c) => c.id)
+    expect(new Set(ids).size).toBe(ids.length)
+
+    // The union of grouped commands === all registry commands. This is the
+    // regression guard: a compound-scope command (e.g. `file.quickLook` on
+    // `'Main window/File list'`) that falls into no group would shrink this set
+    // and become unrebindable through the UI.
+    expect(new Set(ids)).toEqual(new Set(commands.map((c) => c.id)))
+  })
+
+  it('covers every scope present in the registry (no command silently dropped)', () => {
+    const registryScopes = new Set(commands.map((c) => c.scope))
+    for (const scope of registryScopes) {
+      expect(groupedScopes).toContain(scope)
+    }
+  })
+
+  it('drops empty groups and preserves the fixed scope order', () => {
+    const groups = groupCommandsByScope(commands)
+    expect(groups.every((g) => g.commands.length > 0)).toBe(true)
+
+    const orderIndex = (scope: string) => groupedScopes.indexOf(scope as (typeof groupedScopes)[number])
+    for (let i = 1; i < groups.length; i++) {
+      expect(orderIndex(groups[i].scope)).toBeGreaterThan(orderIndex(groups[i - 1].scope))
+    }
+  })
+
+  it('groups file.quickLook (a compound-scope command) so the Quick Look deep link lands', () => {
+    const groups = groupCommandsByScope(commands)
+    const quickLook = groups.flatMap((g) => g.commands).find((c) => c.id === 'file.quickLook')
+    expect(quickLook).toBeDefined()
+  })
+})
diff --git a/apps/desktop/src/lib/settings/sections/keyboard-shortcuts-grouping.ts b/apps/desktop/src/lib/settings/sections/keyboard-shortcuts-grouping.ts
@@ -0,0 +1,69 @@
+/**
+ * Pure grouping logic for the Keyboard shortcuts section.
+ *
+ * Every registry command carries a `CommandScope` (`'App'`, `'Main window'`,
+ * `'Main window/File list'`, …). The section renders one titled group per scope,
+ * in a fixed reading order, so EVERY command shows up in exactly one group and is
+ * rebindable through the UI.
+ *
+ * Group straight off `CommandScope` (don't reintroduce an ad-hoc title list that
+ * has to be matched against scopes): commands on compound scopes like
+ * `'Main window/File list'` only render because the group set IS the scope union.
+ * A title list that drifts from the scopes silently hides whole groups of
+ * commands from the rebinding UI.
+ */
+import type { Command } from '$lib/commands/types'
+import type { CommandScope } from '$lib/commands/types'
+
+/** A titled group of commands sharing one scope, ready to render. */
+export interface ShortcutGroup {
+  /** The scope this group renders (stable key for the `{#each}`). */
+  scope: CommandScope
+  /** User-facing group heading (sentence case). */
+  title: string
+  commands: Command[]
+}
+
+/**
+ * Display order + heading for each scope. Reads top-down the way a user scans:
+ * global app commands first, then the main window and its inner contexts (file
+ * list is the workhorse, so it leads), then the auxiliary windows.
+ *
+ * Keep this in sync with `CommandScope` in `lib/commands/types.ts`: the
+ * exhaustiveness test (`union of grouped commands === all registry commands`)
+ * fails if a new scope is added without an entry here.
+ */
+const scopeOrder: readonly { scope: CommandScope; title: string }[] = [
+  { scope: 'App', title: 'App' },
+  { scope: 'Main window', title: 'Main window' },
+  { scope: 'Main window/File list', title: 'File list' },
+  { scope: 'Main window/Brief mode', title: 'Brief mode' },
+  { scope: 'Main window/Full mode', title: 'Full mode' },
+  { scope: 'Main window/Volume chooser', title: 'Volume chooser' },
+  { scope: 'Main window/Network', title: 'Network' },
+  { scope: 'Main window/Share browser', title: 'Share browser' },
+  { scope: 'Command palette', title: 'Command palette' },
+  { scope: 'About window', title: 'About window' },
+  { scope: 'Onboarding', title: 'Onboarding' },
+]
+
+/**
+ * Group commands by scope into the fixed display order, dropping empty groups.
+ *
+ * Every command lands in exactly one group (its `scope`); a command whose scope
+ * isn't in `scopeOrder` would silently vanish, which the exhaustiveness test
+ * guards against.
+ */
+export function groupCommandsByScope(commands: Command[]): ShortcutGroup[] {
+  const groups: ShortcutGroup[] = []
+  for (const { scope, title } of scopeOrder) {
+    const scopeCommands = commands.filter((c) => c.scope === scope)
+    if (scopeCommands.length > 0) {
+      groups.push({ scope, title, commands: scopeCommands })
+    }
+  }
+  return groups
+}
+
+/** Every scope the grouping renders, for the exhaustiveness test. */
+export const groupedScopes: readonly CommandScope[] = scopeOrder.map((s) => s.scope)
diff --git a/apps/desktop/test/e2e-playwright/settings.spec.ts b/apps/desktop/test/e2e-playwright/settings.spec.ts
@@ -353,14 +353,13 @@ test.describe('Settings keyboard-shortcut deep link', () => {
 
   test('lands on Keyboard shortcuts with the target row scrolled into view', async ({ tauriPage }) => {
     const main = tauriPage as TauriPage
-    // `downloads.goToLatest` is a regular registry row (scope `Main window`) that
-    // lives far enough down the list to need a scroll, so it exercises the nested
-    // `.commands-list` scroll path. (We can't target `file.quickLook` — the Quick
-    // Look toast's deep-link target — because its compound `Main window/File list`
-    // scope keeps it out of the section's scope groups entirely; that's a
-    // pre-existing display bug, see `sections/CLAUDE.md` § "Compound-scope rows
-    // don't render in any group".)
-    const targetAnchor = 'shortcut-downloads.goToLatest'
+    // `file.quickLook` is the Quick Look toast's deep-link target. Its scope is the
+    // compound `Main window/File list`, so this also guards that compound-scope rows
+    // render and are reachable (the section groups every command by scope; see
+    // `sections/CLAUDE.md` § "Every command groups by scope"). The row lives far
+    // enough down the list to need a scroll, exercising the nested `.commands-list`
+    // scroll path.
+    const targetAnchor = 'shortcut-file.quickLook'
     const section = JSON.stringify(['Keyboard shortcuts'])
     const url = `/settings?section=${encodeURIComponent(section)}&anchor=${encodeURIComponent(targetAnchor)}`
     const urlJson = JSON.stringify(url)
