Skip to content

Commit 69ca52e

Browse files
committed
Query dialogs: fix Search AI filter leak + a11y coverage gap, strip plan tags, trim docs
Final pre-merge cleanup for the query-dialogs overhaul. Two blockers and a docs sweep: - **Search AI stale-filter leak (correctness):** a second AI run that returned no size/date bound silently kept the first run's filter (for example a ≥ 5 MB lower bound survived "find PDFs"). `applySizeFromAi` / `applyDateFromAi` no-op on a null bound, so `applyAiSharedFilters` now resets `sizeFilter` / `dateFilter` to `any` before applying each translation, matching what Selection already does and the documented contract in `apply-ai-filters.ts`. `typeFilter` is deliberately left alone (the AI omitting type keeps the user's choice). Pinned by a red-first regression test plus a no-recall guard test (reopening in AI mode renders persisted results without re-calling the cloud translate). - **a11y coverage gap:** `FilterDropdown.svelte` shipped without a tier-3 a11y test, so `a11y-coverage` failed once uncached. Added `FilterDropdown.a11y.test.ts` (closed, span-heading grid, and `labelFor` single-control states); not allowlisted. - **Plan-tag strip:** removed the `M2`/`M4`/`M5`/`M6` milestone tags that leaked into shipped code, tests, and docs, replacing each with a descriptive reference. Pre-existing unrelated milestone tags (FilePane git browser, indexing, the older Open-in-pane test) are left as-is. - **Doc trim:** moved depth out of three over-length push-tier `CLAUDE.md` files into colocated `DETAILS.md` (new for `filter-chips/` and `selection-dialog/`), keeping every invariant/gotcha/guardrail: `filter-chips` 1836 → 543 words, `selection-dialog` 1619 → 501, `query-ui` 1066 → 928 (the rest is all must-knows; can't drop further without losing guardrails). The `claude-md-length` allowlist auto-shrink-wrapped the two now-under-threshold entries. Full `pnpm check --fresh` (uncached) is green.
1 parent 14abab0 commit 69ca52e

17 files changed

Lines changed: 652 additions & 386 deletions

apps/desktop/src-tauri/src/search/history.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,7 @@ mod tests {
665665
// The type filter is an additive `#[serde(default)]` field: a new value serializes
666666
// and deserializes cleanly, AND an old file missing the key still loads (as `None`),
667667
// all on schema v1. This pins "no schema bump needed".
668-
assert_eq!(CURRENT_SCHEMA_VERSION, 1, "M4's type filter must NOT bump the schema");
668+
assert_eq!(CURRENT_SCHEMA_VERSION, 1, "the type filter must NOT bump the schema");
669669

670670
let mut e = entry(HistoryMode::Filename, "*.png");
671671
e.filters.is_directory = Some(true);

apps/desktop/src/lib/query-ui/CLAUDE.md

Lines changed: 41 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -13,75 +13,68 @@ popover, and the `createQueryFilterState()` factory. Filter-chip internals live
1313
- `QueryBar`, `ModeChips`, `AiPromptStrip`, `QueryResults`, `EmptyState`, `PathPills`, `SearchRowMenu`,
1414
`recent-items/*`: UI pieces. Pure helpers: `enter-action.ts`, `path-pills-layout.ts`, `recent-chips-layout.ts`.
1515
- `query-filter-state.svelte.ts`: factory owning the cross-consumer state. `filter-chips/`: chip subsystem.
16-
- `apply-ai-filters.ts`: shared `applySizeFromAi` / `applyDateFromAi` / `applyTypeFromAi` over a `QueryFilterState`;
17-
both wrappers call them, so the AI-result-to-chip mapping is in one place.
18-
- `ai-summary.ts`: pure `buildAiSummary()` → the `AiPromptStrip`'s human-readable mirror of the produced pattern +
19-
Size/Modified/Type. The strip is a MIRROR; the live chips stay the editable truth.
16+
- `apply-ai-filters.ts`: shared `applySizeFromAi` / `applyDateFromAi` / `applyTypeFromAi` over a `QueryFilterState` (one
17+
place for the AI-result-to-chip mapping). `ai-summary.ts`: pure `buildAiSummary()` → the `AiPromptStrip`'s
18+
human-readable mirror (a MIRROR; the live chips stay the editable truth).
2019

2120
## Must-knows
2221

23-
- **Three pieces of state are QueryDialog's alone; consumer callbacks MUST NOT write them:** (1) `state.lastDialogEvent`
24-
(drives `deriveEnterAction` + the `` swap), (2) `state.lastAiPrompt` / `lastAiCaveat` (orchestrator sets the prompt
25-
before `translateAi`), (3) `state.results` / `totalCount` / `cursorIndex` (set after `runQuery` resolves). `runQuery`
26-
returns `{ entries, totalCount }`; `translateAi` returns `{ caveat, highlightedFields }`. Writing these from a
27-
consumer breaks the ⏎ ownership swap.
22+
- **Three pieces of state are QueryDialog's alone; consumer callbacks MUST NOT write them:** `state.lastDialogEvent`
23+
(drives `deriveEnterAction` + the `` swap), `state.lastAiPrompt` / `lastAiCaveat` (orchestrator sets these before
24+
`translateAi`), and `state.results` / `totalCount` / `cursorIndex` (set after `runQuery` resolves). `runQuery` returns
25+
`{ entries, totalCount }`; `translateAi` returns `{ caveat, highlightedFields }`. Writing these from a consumer breaks
26+
the ⏎ ownership swap. Full ownership table in DETAILS.md § Ownership contracts.
2827
- **AI translation errors surface once, in QueryDialog, for both consumers.** The consumer's `translateAi` must let the
2928
typed `AiTranslateError` throw (don't swallow it). QueryDialog catches and calls `showAiTranslateErrorToast`. A `null`
3029
return means a benign empty translation, distinct from a throw. Don't re-add a per-consumer catch.
3130
- **`createQueryFilterState()` owns ONLY cross-consumer fields.** When adding a field, ask "would Selection care?" Yes →
3231
core factory. No → the consumer's extras module (`createSearchExtrasState()` etc.). Don't share via the core when
3332
semantics diverge (`lastAiLabel` is the textbook "no").
3433
- **`typeFilter: 'both' | 'file' | 'folder'`** (core, default `'both'`) maps to the existing IPC
35-
`SearchQuery.isDirectory: Option<bool>` in `buildBaseSearchQuery` (`both → null`, `file → false`, `folder → true`): no
36-
new IPC field, no engine change. Selection's matcher reads it via `getIsDirFor`; it round-trips as
37-
`HistoryFilters.isDirectory` (additive `#[serde(default)]`, no schema bump).
34+
`SearchQuery.isDirectory: Option<bool>` (`both → null`, `file → false`, `folder → true`): no new IPC field, no engine
35+
change, no schema bump (`HistoryFilters.isDirectory` is additive `#[serde(default)]`). Mapping detail in DETAILS.md §
36+
State shape.
3837
- **`recordAiTranslation` (core) writes ONLY `handTyped[mode]`.** The Search-only label/pattern slots live in the extras
3938
and are written separately. Don't fold them into the core method.
4039
- **`stopPropagation()` on every dialog `keydown`** (shields the file explorer behind it; without it, keys trigger
4140
quick-search/nav). All `use:trapFocus` listeners run in the capture phase so this can't starve the trap.
4241
- **Don't wipe state from `onDestroy` / any lifecycle hook.** The dialog mounts on open and unmounts on close; state
4342
survives unmount by design. The ONLY sanctioned reset is `⌘N` (the consumer's clear hook). Wiping on unmount turns
4443
every close+reopen into lost work.
45-
- **Reopen re-derives results so they show immediately, not the empty state.** `hasSearched` is component-local (resets
46-
to `false` each mount), so it's seeded from `getLastRunQuery() !== null` (the precise "a prior run exists" flag,
47-
nulled by `⌘N`/`clearCore`) so persisted results render on mount. For a restored NON-AI session (a prior run + a
48-
non-empty query or active filter, via `hasRestorableQuery()`), `onMount` sets `runOnMount` so the query re-runs:
49-
Select re-derives against the freshly-snapshotted current folder (more correct than showing rows from the old folder),
50-
Search re-hits the index. AI restored sessions must NOT re-run (cloud cost): the `onMount` gate excludes
51-
`mode === 'ai'`, so the seeded `hasSearched` renders the persisted results without re-calling translate. A first-ever
52-
open (no prior run) still rests on the empty state.
53-
- **⌘⏎ and ⇧⏎ are explicit no-ops** (swallowed with `preventDefault`); bare Enter is the only key that runs a search or
54-
opens the cursor row, dispatched via `enterAction`. `⌘N` is captured before the dialog's `stopPropagation` so it
55-
doesn't reach the route-level new-tab handler.
44+
- **Reopen re-derives results so they show immediately, not the empty state.** `hasSearched` (component-local) is seeded
45+
from `getLastRunQuery() !== null` so persisted results render on mount. A restored NON-AI session (prior run +
46+
`hasRestorableQuery()`) sets `runOnMount` to re-run on reopen. AI restored sessions must NOT re-run (cloud cost): the
47+
`onMount` gate excludes `mode === 'ai'`, so the seeded `hasSearched` renders the persisted results without re-calling
48+
translate. Don't loosen the `mode !== 'ai'` gate. Full lifecycle (cold-open / hot-prefill / index-not-ready) in
49+
DETAILS.md § `runOnMount` consumer.
50+
- **⌘⏎ and ⇧⏎ are explicit no-ops** (`preventDefault`); bare Enter is the only key that runs a search or opens the
51+
cursor row, via `enterAction`. `⌘N` is captured before the dialog's `stopPropagation` so it doesn't reach the
52+
route-level new-tab handler.
5653
- **Path pills are mouse-only, `tabindex="-1"`** (keyboard equivalents `⌥←` / `⌥→`); making them tabbable breaks the
57-
row's arrow-down flow in the virtualized list. The `nested-interactive` axe rule is deliberately disabled on the
58-
populated-results a11y test with a comment pointing at the rationale; don't "fix" it by retabbing.
59-
- **Status bar stays empty whenever the content area shows a state message** (Searching / No files match / Loading).
60-
When you add a content-area state in `QueryResults`, make `getStatusText()` return `''` for it, or it reads as broken.
61-
- **`.results-container` carries `role="listbox"` ONLY when option rows actually render** (the `showingRows` derived:
62-
`isIndexAvailable && isIndexReady && !isSearching && results.length > 0`). Don't gate it on `results.length > 0`
63-
alone: on reopen the dialog re-runs with the persisted `results` still set while the spinner shows, so a length-only
64-
gate puts `role="listbox"` on a container with no `option` children = axe `aria-required-children` (critical). Pinned
54+
row's arrow-down flow. The `nested-interactive` axe rule is deliberately disabled on the populated-results a11y test;
55+
don't "fix" it by retabbing.
56+
- **Status bar stays empty whenever the content area shows a state message** (Searching / No files match / Loading):
57+
make `getStatusText()` return `''` for any new content-area state, or it reads as broken.
58+
- **`.results-container` carries `role="listbox"` ONLY when option rows actually render** (the `showingRows` derived,
59+
not `results.length > 0` alone): on reopen the dialog re-runs with persisted `results` set while the spinner shows, so
60+
a length-only gate yields `role="listbox"` with no `option` children = axe `aria-required-children` (critical). Pinned
6561
by the "searching with stale results" test in `QueryResults.a11y.test.ts`.
66-
- **Content chip is visible-disabled with NO shortcut** (`⌘4` reserved): wiring a shortcut to a disabled control is
67-
hostile UX. When Content ships it claims `⌘3` and Regex moves to `⌘4`.
62+
- **Content chip is visible-disabled with NO shortcut** (`⌘4` reserved): when Content ships it claims `⌘3` and Regex
63+
moves to `⌘4`.
6864
- **AI mode never auto-applies** (cost); filename/regex auto-apply behind `search.autoApply` (default on, 1,000 ms
69-
debounce), gated also by IME composition. The split lives in `scheduleSearch()`'s early-return chain.
70-
- **The AI translation overwrites `query` + `mode`.** The original prompt lives in `lastAiPrompt`; use
71-
`getLastAiPrompt()`, don't assume `query` still holds natural-language input after an AI run.
65+
debounce, IME-gated), in `scheduleSearch()`'s early-return chain.
66+
- **The AI translation overwrites `query` + `mode`.** Use `getLastAiPrompt()` for the original prompt; don't assume
67+
`query` still holds natural-language input after an AI run.
7268
- **The `AiPromptStrip` is a human-readable MIRROR, never the source of truth.** `buildAiSummary()` (`ai-summary.ts`)
73-
turns the current chip state (pattern + Size/Modified/Type) into the strip's "Here's what the agent did:" lines; the
74-
live chips stay the editable representation. The first-person agent voice is a SANCTIONED exception to the
75-
no-first-person copy rule (David-decided), alongside onboarding / About.
69+
renders the current chip state into the strip's "Here's what the agent did:" lines; the live chips stay editable. Its
70+
first-person agent voice is a SANCTIONED exception to the no-first-person copy rule (alongside onboarding / About).
7671
- **The spinner covers the AI translate round-trip.** `runAiSearch` sets `isSearching` true BEFORE the cloud translate
77-
(the slow part) and leaves it on through `executeQuery` (which clears it in `finally`); its early-returns reset it so
78-
it can't stick. `QueryResults` shows `.spinner` off `isSearching`, and `getStatusText()` returns `''` for it. The
79-
global `.spinner` honors `prefers-reduced-motion` in `app.css`.
80-
- **Type-in-AI is leave-alone-if-null; size/date are reset-first. Don't "consistency-fix" this.** The AI RECEIVES the
81-
current type as context (`translateSearchQuery` / `translateSelectionQuery` take a `currentType` arg, via
82-
`typeFilterToIsDirectory`) and may set or stay silent. `applyTypeFromAi` writes only on a non-null `isDirectory`; a
83-
`null` keeps the user's choice. Unlike `applySizeFromAi` / `applyDateFromAi` (callers reset to `any` first), callers
84-
must NOT pre-reset `typeFilter`. The `'type'` highlight flashes the toggle via a wrapper span (`ToggleGroup` has no
85-
`highlighted` prop).
72+
and leaves it on through `executeQuery` (cleared in `finally`; early-returns reset it so it can't stick). Detail in
73+
DETAILS.md § Shared UI behavior.
74+
- **Type-in-AI is leave-alone-if-null; size/date are reset-first. Don't "consistency-fix" this.** Each AI run resets
75+
`sizeFilter` / `dateFilter` to `'any'` before applying the translation (the helpers no-op on a null bound, so without
76+
the reset a prior run's filter leaks). Type is the deliberate asymmetry: `applyTypeFromAi` writes only on a non-null
77+
`isDirectory`, so the AI staying silent keeps the user's choice. Callers must NOT pre-reset `typeFilter`. Full
78+
contract in `apply-ai-filters.ts`.
8679

8780
Architecture, flows, and decision detail: [DETAILS.md](DETAILS.md). Read it in whole before structural changes here.

apps/desktop/src/lib/query-ui/DETAILS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ losing the user's words is worse than handing them text they may need to tweak.
218218
**Precedence on an empty target buffer** (reconciling the carry-over with the AI-pattern probe):
219219

220220
1. `aiPatternProbe(target)` first. It returns the AI's structured, kind-correct pattern (filename gets the glob, regex
221-
gets the regex) and is the post-AI editing handoff (M6's "tweak what the agent did" loop depends on it). The raw
221+
gets the regex) and is the post-AI editing handoff (the "tweak what the agent did" loop depends on it). The raw
222222
carry-over must NOT clobber it.
223223
2. The outgoing term second, as the fallback when there's no probed pattern.
224224

0 commit comments

Comments
 (0)