Skip to content

Unify Cmd+K palette and dock workspace search (#912)#915

Merged
srid merged 39 commits into
masterfrom
feat/unified-palette
May 17, 2026
Merged

Unify Cmd+K palette and dock workspace search (#912)#915
srid merged 39 commits into
masterfrom
feat/unified-palette

Conversation

@srid
Copy link
Copy Markdown
Member

@srid srid commented May 15, 2026

Workspace search now lives inside the command paletteCmd+Shift+K and the dock's search-icon button both open the palette pre-drilled into a "Search workspaces" group whose body renders the same facet sidebar + agent-state column grid (Idle / Awaiting / Working / No agent with idle sub-buckets) the standalone mega panel used to host. One engine, one home for live-terminal navigation, both keyboard- and mouse-reachable from the same surface.

The data side unifies cleanly: one shared search.ts (tokenize + AND-token match) drives both the palette filter and the workspace grid. The presentation side specializes — most groups (Theme, Debug, New terminal) keep the flat-list drill-in; the new "Search workspaces" group opts into a per-group body slot that paints columns instead.

How it fits together

Cmd+K           ─┐
Cmd+Shift+K     ─┼─▶ CommandPalette ─▶ Search workspaces drill-in
Dock 🔍 button  ─┘                       ├─ filter:  search.ts (AND-token)
                                          └─ body:    WorkspaceGrid (columns)

Other drill-ins (Theme, Debug, New terminal, Recent agents):
                  CommandPalette ─▶ default <For> filtered list

The PaletteGroup.body?: Component<{ query; closePalette }> slot is the seam — palette engine forwards the typed query and a closePalette callback; the body owns its own rendering.

Raycast-feel chrome

  • Input bar carries a leading caret and lives inside a flex row so the slot reads as a command line, not a generic text field
  • Breadcrumb chipsCommands › Theme rendered as clickable pills; the deepest segment is emphasized
  • Bottom action bar shows the current ⏎ <verb> (Open / Run / Submit / per-group bodyHint) and esc Back when drilled
  • Dialog dimensions scale to min(80vh, 40rem) × min(90vw, 44rem) — the workspace grid's 4-column body needs the breathing room

What changed outside the palette

File / area Change
packages/client/src/search.ts (new) tokenize + matchesAllTokens — one home for AND-token matching
packages/client/src/canvas/dock/WorkspaceGrid.tsx (new) The column grid + facet sidebar, mounted via the palette's body slot
packages/client/src/canvas/dock/Dock.tsx Mega mode removed — DockMode = "rail" | "cards". The dock's 🔍 button calls onOpenWorkspaceSearch (passed in)
packages/client/src/canvas/dock/DockMega.tsx (deleted) Mega-as-a-second-surface goes away; the grid lives in WorkspaceGrid now
packages/client/src/App.tsx openWorkspaceSwitcheropenPaletteGroup("Search workspaces"); dockMegaOpenRequest impulse removed
packages/client/src/canvas/TerminalCanvas.tsx Drops the openMegaRequest / workspaceEntries / getRecency props the dock no longer needs
packages/client/src/commands.{ts → tsx} Renamed for the JSX body factory; registers WorkspaceGrid as the Search workspaces body

Notable bug found and fixed during review

Two separate createEffect(on(() => props.open, ...)) blocks both watched props.open. When the palette closed, the path-reset effect's setPath([]) could fire before the close-effect read path() for onCancel propagation — swallowing the Theme preview reset (and any other group's clean-up hook). Surfaced by the existing theme-preview e2e and fixed in 3f907bc8 by merging the two effects so the read of path() is ordered explicitly before the reset.

Tests

  • workspace-switcher.feature — 14 scenarios kept, repointed at palette selectors; column / idle-sub-bucket / repo-facet scenarios restored after a brief detour through a column-less design
  • command-palette.feature — migrated 4 scenarios (Switch terminal → Theme or Search workspaces); dropped the now-obsolete "Shortcut hints in nested group" scenario (Cmd+1..9 keybinds still fire from the keyboard dispatcher, just not as palette items)
  • All 75 affected scenarios pass locally via just test-quick

Closes #912.

Generated by /do on Claude Code (model claude-opus-4-7).

srid added 11 commits May 15, 2026 13:41
Workspace search now lives in the command palette as the "Search
workspaces" group. Cmd+Shift+K (and the dock's search-icon button)
open the palette pre-drilled into that group. Each row carries
repo · #pr · agent · cwd · recency in human-readable form, while a
hidden searchText field exposes 20+ fields to the filter's
AND-token query. The dock's standalone mega mode (faceted card
grid with agent-state columns) is removed — the unified palette is
the single workspace-search surface.

- Extracted shared search.ts (tokenize + matchesAllTokens)
- Simplified dockModel.ts: replaced buildDockModel/columns/facets
  with searchWorkspaceEntries returning a flat DockEntry list
- Deleted DockMega.tsx and the mega-mode plumbing in Dock.tsx
  (DockMode is now "rail" | "cards" only)
- Migrated workspace-switcher.feature scenarios and step
  definitions to assert against the palette surface
The palette filter (CommandPalette.tsx#filtered) runs AND-token
matching against the row's `name + description + searchText`, so
the workspace path's `query` option was never invoked in
production. Removing it eliminates a misleading API surface where
a future caller passing a query would get subtly different
semantics (only `searchText` checked, not `name`/`description`).

Cross-validation refinement: the four corpus-coverage tests are
kept here — rewritten as `searchText`-contains assertions on the
pure `DockEntry` output — rather than migrated up to the palette
layer. The 20-field corpus belongs to `searchTextFor`, not to the
palette's filter memo.
After the palette unification, `dockRowChrome.ts` had a single
remaining export (`resolvedPr`) — a pure narrower on
`TerminalMetadata["pr"]` with zero dock-row dependency. Three
consumers (`Dock.tsx`, `MobileDockDrawer.tsx`, `commands.ts`)
imported it; for `commands.ts` that meant the app-level palette
registry reaching into a `canvas/dock/` chrome file for a metadata
helper — a layering inversion.

`dockModel.ts` already wraps `TerminalMetadata` for workspace
search (`prSearchFields` packs the same union into the search
corpus), so co-locating the resolver removes the misnamed module
without spawning a new single-function file.
User redirect: "Search workspaces" should *in effect* still show
the multi-column Trello-like layout — the unification belongs at the
data layer (one engine, one home), not at the rendering layer (one
flat list everywhere).

- PaletteGroup gains an optional `body` slot. When the user drills
  in, the body component replaces the default `<For>` filtered list
  while the breadcrumb, input, and bottom action bar stay palette-
  owned. The body receives the typed query and a `closePalette`
  callback.
- New `canvas/dock/WorkspaceGrid.tsx` renders the facet sidebar +
  agent-state columns (Idle/Awaiting/Working/None) + idle sub-bucket
  ladder (4–12h, 12–24h, 24–48h, 48h+) — the same shape DockMega
  used, now mounted inside the palette via the `body` slot.
- `dockModel.ts`: restore `buildDockModel` and the column/facet/
  idle-sub-bucket types (still authoritative; consumed by
  WorkspaceGrid via the body). `searchWorkspaceEntries` keeps its
  `query` option — it now has a real consumer.
- `dockRowChrome.ts`: restore `agentLabel` / `metaLine` /
  `prSummary` / `tokenLine` (used by the grid cards). `resolvedPr`
  stays in `dockModel.ts` per the prior hickey/lowy ruling.
- `CommandPalette.tsx`: Raycast row chrome — input gets a leading
  ⏵ caret, breadcrumb renders as clickable chips, bottom action bar
  shows `⏎ Open/Run/Submit` plus `esc Back` when drilled. Dialog
  caps at `min(80vh, 40rem)` so the column grid has room to breathe.
- `commands.tsx` (renamed from `.ts` to host the body JSX): the
  Search workspaces group registers `WorkspaceGrid` as its body and
  `Pick a workspace to switch` as its bodyHint.
- Tests: restore the column / idle-sub-bucket / repo-facet
  scenarios — they assert against the same `workspace-switcher-*`
  testids the grid carries, now nested inside the command-palette
  dialog. The close-button scenario becomes Escape.
The body Component reference must be stable across the createMemo's
reactive re-runs; otherwise SolidJS's <Dynamic> sees a fresh
component identity on every terminal-state update and unmounts the
WorkspaceGrid, losing its repoFilter signal and scroll position.
…th() read before reset

Two separate `on()` effects both depending on `props.open` raced:
the initialGroup-effect's `setPath([])` could fire before the
close-effect read `path()` for onCancel propagation, swallowing
the Theme preview reset (and any other group with a clean-up
hook). Merging into one effect makes the ordering explicit —
read path() for onCancel first, then setPath([]).

Surfaced by the Theme preview e2e scenario, which expected the
header to revert from "Dracula" to "Tomorrow Night" after Escape.
The 'Switch terminal' group rename to 'Search workspaces' (with a
column-grid body) means the old scenarios that drilled into it as a
flat list no longer apply. Updated scenarios:

- 'Filter commands by typing' — switched to the Theme group (a
  flat-list drill-in that still narrows via the filter)
- 'Switch terminal via command palette' — drills into Search
  workspaces and clicks a workspace card in the body
- 'Backspace drills out of nested group' / 'Breadcrumb click
  navigates back to root' — switched to Theme as the drill target
- 'Shortcut hints shown in nested group' — removed; the
  Switch-to-terminal-N actions no longer appear as palette items
  (Cmd+1..9 keybinds still fire from the keyboard dispatcher)
@srid
Copy link
Copy Markdown
Member Author

srid commented May 15, 2026

Hickey/Lowy Analysis

# Lens Finding Disposition
1 Hickey resolvedPr stranded in wrong-layer dockRowChrome.ts Fixed in this PR
2 Hickey Workspace-row helpers placed in commands.ts No-op (per Lowy cross-val)
3 Hickey agentDisplayLabel duplication across 3 files No-op (per Lowy cross-val)
4 Lowy Workspace presentation helpers in orchestration registry No-op (single consumer, no volatility)
5 Lowy Dead query parameter in searchWorkspaceEntries Fixed in this PR (then re-added when the column body became its consumer)
6 Lowy resolvedPr home wrong for its consumer set Fixed in this PR
7 Hickey (cross-val) Agent summary multiplication No-op (one inline + one component)

Two reviewer passes (initial + cross-validation) ran against the diff post-implement. The convergent findings landed as standalone refactor commits (refactor(hickey) / refactor(lowy) prefixes in the log); the divergent ones were resolved in favor of the more conservative call (keeping single-consumer helpers in commands.tsx rather than spawning a new module for them).

The user then redirected the design ("Search workspaces should still show the multi-column Trello-like layout"). That redirect resurrected buildDockModel + column types + the query option on searchWorkspaceEntries — F2's deletion was correct for the prior design (no consumer) but wrong for the new design (the body has a real query consumer). The reviewers' methodology stays sound; the design changed under them.

Hickey rationale

Fragmentation findings (Layer 2) — Three invariants surfaced. Agent display label reconstructed independently across commands.ts, AgentIndicator.tsx, MetadataInspector.tsx (Finding C). searchText lowercased at both construction and match time — idempotent, no action. Workspace presentation helpers as fragmentation across dockModel.ts (owns the type) and commands.ts (formats it) (Finding B).

Concept multiplication (Layer 3) — None. search.ts consolidates two previously separate substring scans into one shared primitive — concept count drops.

Structural pattern matches (Layers 4–5)resolvedPr stranded in dockRowChrome.ts after the diff left the file with one export (Finding A). Workspace-row presentation helpers in commands.ts against the AGENTS.md "commands stay declarative" rule (Finding B). Agent-label inline assembly across three sites (Finding C).

Lowy rationale

Volatility mapsearch.ts is a real volatility seam (fuzzy/ranked scoring is a known roadmap item). searchText?: string on PaletteBase is a healthy seam (rendered content vs. searchable corpus is a legitimate independent axis). The "Search workspaces" group placement encapsulates a genuine sequence: get entries → sort by recency → hydrate → map to actions → delegate filter+render to the engine.

Findings — Workspace presentation helpers in the orchestration registry add a "how to format a DockEntry" axis to a file that should stay declarative (F1). The query option on searchWorkspaceEntries was misleading API surface in the prior design (F2). resolvedPr had no dock dependency but was buried in a dock-internal file with three consumers including app-level commands.ts (F3).

Cross-validation — Lowy's lens audited Hickey's destinations and pushed back on B (single-consumer helpers don't warrant a new module) and C (the three "duplication" sites actually consume the tables differently — one joins, others render in separate slots with fallbacks).

Reviewers ran on Sonnet 4.6 via Agent Skill frontmatter.

srid added 2 commits May 15, 2026 15:13
AND-token semantics matched 'Split terminal' query against both
'Toggle terminal split' and 'Split terminal' commands — the test's
`paletteCommand("Split terminal")` then clicked the first option
in DOM order ('Toggle terminal split'), which toggles rather than
creates, breaking sub-terminal e2e scenarios.

Substring on `name | description | searchText` is what master had,
just extended to also probe `searchText` for rich rows. Workspace
search inside the WorkspaceGrid body keeps AND-token semantics on
the 20-field corpus via `searchWorkspaceEntries` — that's where
multi-field AND matching actually helps.
…orkspaces

The canvas.feature:111 scenario asserts that activating a terminal
via the palette pans the canvas to its tile. The original path
drilled into 'Switch terminal' → 'Switch to terminal 1'. After
#912 the same activation happens through 'Search workspaces' →
click a workspace card; store.activate(id) still pans.
@srid
Copy link
Copy Markdown
Member Author

srid commented May 15, 2026

Evidence

Screenshot 1 — Cmd+K unified palette (top-level commands)
Raycast-style chrome: caret on the input, command list with group entries (Theme →, Search workspaces →, Debug →), and a bottom action bar showing ⏎ Open.

Cmd+K unified palette


Screenshot 2 — Theme drill-in with breadcrumb chip
After selecting Theme: breadcrumb chip Commands › Theme above the input, the full themes list, action bar now reads ⏎ Run + esc Back.

Theme drill-in


Screenshot 3 — Cmd+Shift+K opens the workspace grid inside the palette
Palette opens pre-drilled into Commands › Search workspaces. The body is the column grid + repo facet sidebar — same layout DockMega used to host, now mounted inside the palette frame.

Workspace grid


Screenshot 4 — Workspace grid filtered live by the palette input
Typing in the palette input narrows the visible cards across all columns; the facet sidebar updates to count the post-filter matches per repo.

Workspace grid filtered

@srid
Copy link
Copy Markdown
Member Author

srid commented May 15, 2026

/do results

Step Status Duration Verification
sync 1s git fetch ok; forge=github
research 6m 15s Mapped CommandPalette/commands/Dock/dockModel/dockRowChrome/App wiring; located DockMega-only consumers; identified workspace_switcher tests for migration
branch 5s Created feat/unified-palette branch from origin/master
implement 17m 7s Initial unification: search.ts extracted; dockModel simplified; commands.ts gained Search workspaces; CommandPalette filter; App.tsx wired; Dock mega mode removed; DockMega.tsx deleted; tests migrated
check 0s just check — exit 0, 50 pre-existing warnings (none in touched files)
docs 49s README dock description updated (3-level → 2-level), mega prose replaced with unified-palette pointer
fmt 17s biome formatted 3 files; nixpkgs-fmt clean
commit 23s Initial feature commit pushed
hickey+lowy 14m 59s 2 parallel reviews + cross-validation. 2 findings fixed (resolvedPr placement, dead query param), 4 no-op (helpers placement, agentDisplayLabel dedup, agent summary multiplication)
police 24m 36s 6 violations fixed across rules + fact-check + elegance: dead code (repoColorDot), dedup (tokenize, resolvedPr), dead typeLabel, bodyGroup memo extraction, workspaceGridBody reactive stability
test 38m 21s 75 scenarios across 8 features pass. Found + fixed a SolidJS effect-ordering race between two on(() => props.open, ...) blocks that swallowed onCancel hooks (commit 3f907bc); migrated 4 command-palette scenarios off the renamed "Switch terminal" group
create-pr 1m 39s Draft PR #915 + hickey/lowy analysis comment
ci 51m 23s All 14 expected contexts pass on HEAD 10e1f15. Discovered + fixed canvas.feature:111 panning scenario (drilled into the renamed group). Linux unit/e2e/surface-example-build had transient ENOSPC during parallel fanout; each passed on individual retry from a fresh devshell
evidence 4m 56s 4 screenshots posted: Cmd+K palette chrome, Theme drill-in, Cmd+Shift+K workspace grid, filtered grid
Total 162m

Slowest step: ci (51m 23s)

Optimization suggestions

  • Two real-bug fixes surfaced in CI, not earlier. Switch terminal rename impacted scenarios in two feature files I missed (command-palette.feature — caught in test) and canvas.feature:111 — caught in ci. A pre-commit grep for the renamed string (grep -rn "Switch terminal" packages/tests/features/) would have caught both at the implement step and saved one CI lap.
  • Filter semantics regression also caught only in CI. The AND-token filter change matched "Toggle terminal split" for the query "Split terminal" and broke the sub-terminal e2e. A unit test asserting the palette filter's behaviour against close-name action pairs (or a deliberate "filter exclusivity" test) would have caught this without an e2e cycle.
  • Linux ENOSPC on the remote builder cost ~10m of CI wall clock across two runs. Probably outside my control, but a smaller-scoped --from ci-only retry after a real fix avoids paying the full _summary overhead — worth invoking next time only the linux fanout needs to rerun.
  • test (38m) and ci (51m) together are ~92% of the run. For a UI-heavy palette change, that's largely the e2e suite's natural cost; the only meaningful saving would be just test-quick against a tighter set during implement before letting full CI take it.

Workflow completed at 2026-05-15T19:58:14Z.

srid and others added 14 commits May 15, 2026 16:23
Three issues from PR #915 review:

1. The palette dialog was capped at `max-w-md` (28rem) by the
   shared `ModalDialog` wrapper, which then forced the
   workspace-grid body's 4 columns into ~80px each — card titles
   truncated to "post-buil…". Added an `lg` size (max-w-3xl,
   48rem) to ModalDialog and switched the palette to use it. The
   Dialog.Content's inline `width: 44rem` cap is dropped in favor
   of `w-full` against the wider wrapper.

2. The palette listed both a "Workspace switcher" action *and* a
   "Search workspaces" group at the top level — same destination,
   two entries. Dropped the action; gave the group the
   `Cmd+Shift+K` keybind so its discoverability survives.

3. CommandPalette's row renderer only painted keybinds on
   `kind === "action"`. Generalized to render the keybind on any
   row that has one (group included), so the keybind chip now
   sits next to the group's chevron.
Two issues raised in #915 review:

1. **Arrow keys / Enter inside the Search workspaces body did
   nothing** — the body mounted via `PaletteGroup.body` was
   click-only because the palette engine's keydown listener
   short-circuited on body mode (filtered() was empty) AND still
   `preventDefault`'d Enter, swallowing the body's input. Fixed
   by having the engine bail early for nav keys when mode is
   `body` (Backspace still drills out, Escape still closes via
   Corvu), and giving WorkspaceGrid its own capture-phase
   listener:
   - ArrowDown/Right step forward; ArrowUp/Left step back.
   - Enter activates the highlighted entry and closes the
     palette.
   - The "flat" nav order is column-major DOM order (idle
     sub-buckets first, then awaiting/working/none) — matches
     what the user sees scanning left to right.
   - selectedIndex resets to 0 when the visible-entry id list
     changes (typing narrows; switching repo facet).
   - The highlighted card paints a `ring-2 ring-accent` overlay
     so the keyboard cursor reads as a distinct status from
     `active` (which keeps its repo-color left rail).
   - `scrollIntoView({ block: "nearest" })` keeps the cursor
     onscreen as the user navigates.

2. **Active dock row was easy to lose against tile-themed
   variant rows** — the 0.5-px accent strip on the left edge
   was the only visual cue. Added an accent-tinted background
   wash + a 2-px inset accent ring + an inset border highlight
   on the row, and widened the left strip from 0.5 to 1 (4 px).
   The active row now pops against any combination of repo
   color, tile theme, and bucket border.
The previous fix put the active treatment as `ring-2 ring-inset` +
`bg-accent/15` on the row's outer flex container. That worked for
QuietRowBody (which uses a translucent `bg-surface-1/40`) but the
ring was getting completely covered on AwaitingCardBody and
WorkingPillBody — both paint a solid `theme().bg` over the row body
that sat above the ring's inset shadow.

Move the indicator to an absolutely-positioned overlay span at
z-30, so it sits above every variant body's background regardless
of opacity. `pointer-events-none` keeps clicks reaching the
underlying body. The 4-px left strip stays as the legacy testid
hook and to give rail mode (no body width) a visible cue.
Previously all four arrow keys stepped through a single flat list
in column-major DOM order, so Left/Right behaved identically to
Up/Down. Split the cursor into `(selectedColumn, selectedRow)`:

- ↑ / ↓  step within the current column (clamped, no wrap)
- ← / →  jump to the previous / next non-empty column at the same
         row (clamped to that column's length; empty columns are
         skipped over rather than entered)
- Enter  activates the highlighted entry

Idle's sub-buckets (4-12h → 48h+) collapse into one flat column
for nav — the user just scans top-to-bottom inside Idle, matching
what they see.

scrollIntoView now looks up the card by `data-terminal-id` rather
than by index, so the (column, row) cursor doesn't have to match
the column-major DOM order.
…iny)

A fixed max-w-3xl (48rem / 768px) leaves a lot of empty space on
27"+ monitors. Switched the lg size to a viewport-aware cap:
`max-w-[min(95vw,80rem)]` — 1280px ceiling on ultrawide, 95vw
fallback on laptops so the workspace grid still gets breathing
room across screen sizes.
…the keyboard-cursor ring

The leftmost column's leftmost card had its `ring-2 ring-accent
ring-offset-1` left edge clipped — `overflow-y-auto` on the grid
implicitly clips `overflow-x` per CSS spec, and the grid's only
horizontal padding was `pr-1` for the scrollbar. Swapped to `px-1.5`
so both edges have ~6px of gutter, enough to fit the 3-px ring +
1-px offset.
Two new scenarios in workspace-switcher.feature, plus two step
defs (`workspace switcher card N should be highlighted` and
`exactly one workspace switcher card should be highlighted`):

- "Arrow keys move the keyboard cursor between workspace cards" —
  cursor defaults to card 1, ArrowDown advances to card 2,
  ArrowUp returns to card 1, exactly one card carries
  data-highlighted at any time.
- "Enter on the keyboard-highlighted workspace activates it" —
  the body's Enter handler activates the cursor's card and closes
  the palette, mirroring the click-card-1 path of the existing
  selection scenario through the keyboard path.

The step defs probe `data-highlighted=""` (set by `props.highlighted`
on WorkspaceCard) rather than visual class — semantic selector per
.claude/rules/e2e-testing.md.
No command in the registry set the field, and workspace search (the use
case the field's JSDoc cited) runs through the WorkspaceGrid body slot,
which bypasses filtered() entirely. Shrinking the interface to what is
actually used.
WorkspaceGrid already runs all production workspace search through
buildDockModel. searchWorkspaceEntries was a parallel hydration loop
exported solely for the test that drives it — same searchTextFor +
agentBucket pipeline, minus idleClassifier — so it was a drift surface
the next dockModel field would need to be added to in two places. The
remaining "searches foreground..." buildDockModel test already covers
the AND-token corpus.
The slot string was duplicated between the card's data-in-grid render
attribute and the scroll-into-view querySelector. Extract the literal
into a single constant so a typo at either site can't silently break
keyboard-cursor scrolling.
commands.tsx is the declarative command registry; "Trigger server
error" and "Clear localStorage" reached past that boundary into
client.terminal.resize and direct localStorage / location.reload calls.
Both are different volatility axes than "what commands exist" — RPC
shape and browser lifecycle. Move them behind CommandDeps handlers in
App.tsx so the registry stays declarative.
The body + children: [] invariant on PaletteGroup was enforced by
author discipline at the single call site, not by the type. Split into
a discrete "body-group" kind that carries body + bodyHint without
children — so a future contributor cannot silently populate both, and
the engine narrows body access through ts-pattern's exhaustive switch
instead of an optional runtime check.

The Mode union's body arm now references PaletteBodyGroup directly;
drillInto, isDrillable (renamed from isGroup for clarity now that three
variants drill), and the body-render branch all narrow through the
new discriminator.
ActionBar redeclared the three-arm discriminated union inline. Hoist
Mode to module scope and export it so the engine and the action bar
read from one declaration — a future arm forces a single edit and
both consumers pick it up through the import.
srid added 11 commits May 17, 2026 10:05
Only one consumer reads the body leaf — the JSX <Show when={...}>. A
memo creates a signal node for a single-read derivation; per the
SolidJS rule, plain functions cover that case.
WorkspaceGrid stored the keyboard cursor as (selectedColumn,
selectedRow) plus a selectedEntry memo with a divergent fallback. Three
pieces for one logical concept — and the fallback could disagree with
the stored signals, so the user's next keypress was relative to a
stale position rather than the visible highlight.

Store the cursor as a single highlightedId. stepRow/stepColumn locate
the id in the column layout, compute the next position by delta, and
write the new id back. The reconcile effect runs when the visible
set changes: if the id is no longer present, fall back to the first
available — one place that decides, no drift surface.
The two props were threaded from EntryList into WorkspaceCard but
never read in the body. Removing them along with the now-unused
useTileTheme import in EntryList.
ActionBar inlined the three-arm kind check (group / value / body-group)
that the module-scope isDrillable helper already encapsulates. Calling
the helper means a future drillable arm shows up once, in isDrillable,
and every consumer picks it up.
…yLabel

Three-arm if/else if cascade on PaletteMode.kind reads through three
branches without the compiler enforcing exhaustiveness. ts-pattern's
match(...).exhaustive() forces a new mode arm to register here, mirroring
the dispatch style already used elsewhere in CommandPalette (execute,
the close-effect onCancel walk).
activateSelected() already null-checks selectedEntry() internally;
calling it from the Enter handler is safe without an outer guard, and
removing the guard avoids a double read of the memo.
… spread

The manual flat accumulator (`flat: DockEntry[]; for (...) flat.push(...)`)
is just flatMap with extra moving parts. byBucket arrays in
buildDockModel are freshly allocated and never mutated downstream, so
the [...column.entries] defensive copy is unnecessary.
The outer guard already returns when `last` is a body-group. drillInto
is the only path-mutator and it pushes to the end; the engine's
body-mode key-bail keeps the user from drilling further, so a
body-group can never be a non-final segment. The inner guard could
never fire.
firstAvailableId's loop body runs only when col.length > 0, so col[0]
cannot be undefined. ! asserts the invariant directly instead of
threading ?? null through a known-non-null lookup.
The doc said 48rem but the actual Tailwind cap is min(95vw, 80rem).
Aligning the doc with the class so a future reader doesn't size a
container assuming 48rem.
The block explained what "used to live here" before the unification.
That belongs in git history (commit a85fb44 and the body of #912),
not in source. A future reader doesn't need the diff narrative.
@srid
Copy link
Copy Markdown
Member Author

srid commented May 17, 2026

Hickey/Lowy Analysis

Polish-pass review on the post-merge diff. Findings deduped across the two lenses (Hickey F5 and Lowy F1 were the same finding). Cross-validation pass surfaced zero new findings — both reviewers' recommendations were structurally sound under the other lens.

# Lens Finding Disposition
1 Lowy PaletteBase.searchText is a dead contract field Fixed in this PR
2 Hickey searchWorkspaceEntries duplicates buildDockModel hydration Fixed in this PR
3 Hickey data-in-grid="palette-body" string literal duplicated Fixed in this PR
4 Hickey commands.tsx contains direct RPC and DOM side effects Fixed in this PR
5 Hickey PaletteGroup.body+children:[] invariant held by convention Fixed in this PR
6 Hickey Mode type duplicated across CommandPalette and ActionBar Fixed in this PR
7 Hickey bodyGroup memo is redundant single-consumer derivation Fixed in this PR
8 Hickey (selectedColumn, selectedRow) split cursor fragments concept Fixed in this PR

Hickey rationale

Seven findings against the post-merge diff, all "Fix in this PR":

  • Mode type duplicated across CommandPalette and ActionBar. CommandPalette.tsx declared a local type Mode (three-variant discriminated union); ActionBar redeclared the same union inline as a prop type. Structurally compatible but two coordinated edits with no compile-time guard against drift. Exported as PaletteMode and referenced from both ends.
  • bodyGroup memo is a redundant single-consumer derivation. The memo was read in exactly one <Show when>. Demoted to a plain function per the SolidJS rule for single-consumer derivations.
  • commands.tsx contains direct RPC and DOM side effects. After the .ts.tsx rename, commands.tsx still called client.terminal.resize(...) and inline localStorage.clear() + location.reload(). The project rule (commands stay declarative) and Lowy (RPC + browser-lifecycle volatility are different axes than "what commands exist") both want these behind CommandDeps handlers. Routed via handleTriggerServerError / handleClearLocalStorage in App.tsx.
  • PaletteGroup.body+children:[] invariant held by convention, not types. Split into a discrete PaletteBodyGroup kind that carries body+bodyHint without children — so a future contributor cannot silently populate both.
  • searchWorkspaceEntries duplicates buildDockModel's hydration loop with no production callers. Same searchTextFor + agentBucket pipeline, exported solely for the test that drove it. Deleted; the remaining "searches foreground..." buildDockModel test already covers the AND-token corpus.
  • data-in-grid="palette-body" string literal couples WorkspaceGrid render to query. Extracted to a WORKSPACE_GRID_SLOT constant so the render-site attribute and the scroll-into-view query selector can't drift.
  • (selectedColumn, selectedRow) split cursor with diverging selectedEntry fallback. Three pieces for one logical concept, and the fallback could disagree with the stored signals (cursor "jumped" from the displayed position on next nav key). Collapsed to a single highlightedId signal; stepRow/stepColumn locate the id, compute the next position by delta, and write the new id back.

Lowy rationale

Two findings, both "Fix in this PR":

  • searchWorkspaceEntries is a dead export. Same finding as Hickey Add cargo watch workflow #5 from a different angle: two parallel hydration loops in dockModel.ts (buildDockModel + searchWorkspaceEntries) computing the same baseFields + searchText + bucket projection means "what fields hydrate into a DockEntry" was split across two receptacles — a future contributor adding a field would need to remember to edit both. Collapses to one site.
  • PaletteBase.searchText is a dead contract field with misleading documentation. The JSDoc cited workspace entries as the motivating example, but workspace entries are delivered via the body slot of a PaletteGroup and bypass filtered() entirely. The field was never read for any current command. Shrunk the interface to what is actually used (removed the field, JSDoc, and the cmd.searchText?.toLowerCase().includes(q) filter arm).

Commits

a4a930fe refactor(hickey): collapse split cursor into single highlightedId
f6aae1e8 refactor(hickey): demote bodyGroup memo to plain function
047f7fbc refactor(hickey): export PaletteMode as the shared mode union
2dba177e refactor(hickey): introduce PaletteBodyGroup variant
4c26460e refactor(hickey): route debug actions through CommandDeps
5b37ad7a refactor(hickey): name the workspace-grid slot once
0598e987 refactor(hickey): delete dead searchWorkspaceEntries
202c116b refactor(lowy): drop dead PaletteBase.searchText field

A subsequent /code-police pass surfaced nine more refinements (dead props, exhaustive match over if-cascades, flatMap over manual accumulator, fact-checking doc strings, removing change-narration comments) — landed as fix(police): / refactor(police): commits.

@srid
Copy link
Copy Markdown
Member Author

srid commented May 17, 2026

/do --from polish results

Polish-pass slice (this run only):

Step Status Duration Verification
hickey+lowy 22m 28s 8 findings → 8 commits; cross-validation surfaced 0 new findings
police 29m 40s 9 fixes across 4 rounds (dead code, fact-check, elegance)
test 1m 42s workspace-switcher/command-palette/canvas/theme/dock/sub-terminal/right-panel: 123/123
create-pr 4m 35s hickey/lowy analysis posted as PR comment
ci 20m 38s All 14 ci/* contexts green at 734e988
evidence 19s Skipped: refactor-only diff, no UI impact
done 6s

Slowest step: ci (20m 38s).

Optimization suggestions

  • CI darwin e2e flakes were the cost driver: the darwin builder's e2e suite failed twice on unrelated scenarios (mobile-terminal-scroll.feature viewport-y; code-tab.feature pierre-diff-view locator). Both passed on the next retry. Cumulative retry cost was ~10 min. Worth investigating whether those two scenarios are persistently flaky on the darwin builder, or if it's a runner cold-start issue, and post to #320 if not already tracked.
  • Linux remote builder (srid1) was unreachable during this run — required swapping the entry out of ~/.config/ci-hosts.json to let the linux contexts run on the local host. If that instance is meant to be persistent, monitoring + auto-restart would have saved one just ci round-trip.
  • Police ran 4 rounds (workflow budget is 3): each subsequent pass surfaced legitimate but smaller refinements (flatMap, match exhaustive, fact-check on doc strings). Worth tuning the police skill's stopping criterion — e.g. an explicit "post-3rd-pass: only flag substantive findings" gate — so the workflow doesn't keep churning on the diff after the high-value findings have landed.
  • 17 commits in the polish pass (8 hickey/lowy + 9 police, each on its own commit) gave a clean linear story in git log but also expanded the commit graph noticeably. The one-commit-per-finding rule is the right default; just noting that combined with CI cost per round, large polish passes are slow.

Workflow completed at 2026-05-17 (local time).

@srid srid marked this pull request as ready for review May 17, 2026 16:50
Pre-#912 the dock had three levels (rail / cards / mega search); after
unification the dock is two levels and the workspace search lives
behind Cmd+Shift+K inside the command palette. Re-phrasing the canvas
feature blurb to match.
@srid srid merged commit ddbfca5 into master May 17, 2026
2 of 5 checks passed
@srid srid deleted the feat/unified-palette branch May 17, 2026 16:51
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.

Unified 'search anything' palette (Raycast-style)

1 participant