Files
jellytau/docs/specs/scoped-search-boundary-implementation.md
dtourolle 32043a2152 docs: fold shipped specs into the architecture docs and delete them
A spec was a promise; sixteen of them had become descriptions of code that
already shipped, sitting beside four that describe work still outstanding, with
nothing in the file telling the two apart. Half the statuses were also wrong —
audio-equalizer read "Accepted" with the EQ live on both platforms, the native
video spec said the flag stays off after the default was flipped on.

The shipped designs move into docs/architecture, which is the maintained
description of the build, and the spec files go. Git history keeps the
originals; what a future change still needs is carried across:

- 01-rust-backend: favourites rewritten (the old section named a file that no
  longer exists and called shipped buttons "planned"), domain vocabulary owned
  by Rust (SearchScope, exclusions, the bitrate ladder), background workers
- 02-svelte-frontend: app shell and chrome, library mosaic, series/episode
  navigation, downloaded browse, safe-area insets, native-video store, logging
- 03-data-flow: locally-indexed search
- 05-platform-backends: audio settings on ExoPlayer, the equalizer's band
  vocabulary, native video compositing, the background-audio handoff
- 06-downloads-and-offline: one storage model, offline catalog visibility
- 09-security: path confinement and input binding

docs/specs/README.md now says what the directory is for and where each shipped
design went. Deferred work the specs recorded is kept beside the code it
concerns rather than lost: season-bounded autoplay, the two dead search
commands, why indexing is a full crawl.

requirements.md had fourteen stale statuses — Android audio parity still read
"Linux only", DR-150 still said the native-video default was off, DR-190 was
Proposed after DR-196 implemented it, and five tooling requirements were
Proposed after landing. Three unbuilt specs suggested requirement ids that have
since been allocated to other work; each now carries a warning.
2026-08-21 18:15:58 +02:00

255 lines
13 KiB
Markdown

# Spec: Land the scoped-search boundary fix (implementation)
**Status:** Stage 1 Implemented — Stage 2 (result-side grouping) outstanding
**Requirements:** UR-049, UR-050 | DR-063, DR-066, DR-067 (existing — no new IDs)
**UX spec:** n/a — zero user-visible change is the point (see Acceptance criteria).
**Supersedes / revises:** implements [scoped-search-boundary.md](scoped-search-boundary.md),
which specified this fix but was never built. That spec remains the **design
authority**; this one is the delivery plan and status correction.
## Summary
[scoped-search-boundary.md](scoped-search-boundary.md) diagnosed a domain-taxonomy
leak, specified the fix in full detail, and became the justification for the
project's boundary rule in CLAUDE.md, the `check:boundary` tripwire, and the
spec-review checklist. **The fix was never implemented.** The leak it describes
is still live in `main`. This spec exists to close that gap and to correct the
record — the codebase currently enforces a rule against a violation it still
contains.
## Motivation
The mapping the rule forbids is present and in use:
```ts
// src/lib/utils/searchScope.ts:29-32
const SCOPE_ITEM_TYPES: Record<Exclude<SearchScope, "all">, string[]> = {
music: ["MusicAlbum", "MusicArtist", "Audio", "Playlist"],
movies: ["Movie"],
tv: ["Series", "Episode"],
};
```
This is not dead code. [library.ts:262](../../src/lib/stores/library.ts#L262)
calls `scopeItemTypes(scope)` and puts the result straight into
`options.includeItemTypes`. Meanwhile there is **no `SearchScope` anywhere in
`src-tauri/`**:
```console
$ grep -rn "SearchScope" src-tauri/src --include='*.rs'
(no output)
```
Three things make this the highest-value item found in the design-principles
audit:
1. **The rule's own founding incident is unremediated.** CLAUDE.md cites this
spec as "the incident this rule came from." A rule whose originating
violation is still shipping is not credible.
2. **The tripwire cannot see it.** `bun run check:boundary` passes — it greps for
a multi-type array literal *at the query site*, and this one is assigned to a
named const and dereferenced elsewhere. Broadening the tripwire is specified
separately by the tripwire hardening (DR-094, shipped);
note that hardening it **without** landing this fix would turn `master` red.
3. **The spec's own acceptance criterion fails today.** "Adding a hypothetical
new type to a scope requires editing only Rust" — adding a type to the Music
scope right now requires editing `searchScope.ts`.
## Layer assignment
Unchanged from [scoped-search-boundary.md](scoped-search-boundary.md) §Design;
restated so this spec is reviewable on its own.
| Logic / responsibility | Layer | Why it belongs there |
|------------------------|-------|----------------------|
| Scope → Jellyfin item types (`music``MusicAlbum`, `MusicArtist`, `Audio`, `Playlist`) | **Rust** | Domain vocabulary. Changes if Jellyfin adds/renames an item type — the litmus test's "yes" case. This is the leak being fixed. |
| Result item → search group bucketing | **Rust** | Same taxonomy, result side. Classifying a `MediaItem` as a Song vs Album is Jellyfin vocabulary, not layout. |
| `All` sends no filter at all (≠ union of enumerated types) | **Rust** | A query-shaping rule with a correctness consequence (Person/folder results would be silently dropped). Belongs with the expansion it qualifies. |
| Group display order, labels, reordering, persistence | Frontend | Pure presentation — changes only if the UI is redesigned. Explicitly retained frontend-side. |
| `resolveSearchScope(pathname)` — route → initial scope | Frontend | Routing/navigation, no Jellyfin vocabulary. Stays exactly as-is. |
| Chip labels (`SCOPE_LABELS`), scope order (`SEARCH_SCOPES`) | Frontend | Display strings over an opaque enum. |
| `GROUP_SCOPE` (which group belongs to which scope) | **Delete** | Borderline taxonomy, made redundant: once Rust filters by scope, out-of-scope groups arrive empty and drop via the empty-omit rule. Borderline defaults to Rust; here it defaults to *gone*. |
The `SearchScope` and `SearchGroupId` **types** come to the frontend from
generated `bindings.ts`. Naming an opaque enum variant is not taxonomy; knowing
what item types it expands to is.
## Design
**Follow [scoped-search-boundary.md](scoped-search-boundary.md) §Design as
written** — `SearchScope` enum + `item_types()` in `repository/types.rs`,
`SearchOptions.scope`, `SearchGroupId`/`SearchGroup`/`GroupedSearchResult`,
scope-wins precedence, `All``None` → no filter. It is not restated here;
duplicating it would create two drifting copies of the same design.
This spec adds only the delivery sequencing that the original left implicit.
### Staging: land it in two reviewable pieces
The original bundles the query side and the result side into one change. That is
a large diff touching Rust types, `bindings.ts`, the store, and a component, with
the `search-event` dual-payload hazard in the middle. Split it:
**Stage 1 — query side (closes the leak).**
`SearchScope` enum, `SearchOptions.scope`, command resolves scope →
`include_item_types` in Rust, `library.ts` sends `{ scope }`, delete
`SCOPE_ITEM_TYPES` and `scopeItemTypes()`. Result grouping stays as it is.
After Stage 1 the actual boundary violation is gone and
the hardened tripwire (DR-094) can pass.
**Stage 2 — result side.** `SearchGroupId`/`SearchGroup`/`GroupedSearchResult`,
Rust bucketing, both payloads converted, `composeSearchGroups()` shrunk,
`GROUP_ITEM_TYPES`/`groupItemTypes()`/`GROUP_SCOPE` deleted.
Both stages are required for the original spec's acceptance criteria to pass;
Stage 1 alone leaves `GROUP_ITEM_TYPES` in the frontend. **Stage 1 is not a
stopping point** — it is a review boundary. Do not mark the parent spec
Implemented until Stage 2 lands.
### Stage 1 — delivered (July 2026)
- `SearchScope` enum + `item_types()` in [repository/types.rs](../../src-tauri/src/repository/types.rs);
`All``None` → no filter.
- `SearchOptions.scope` with `resolve_scope()`; scope wins over
`include_item_types`, which stays for the non-search `get_items` callers.
- `repository_search` resolves the scope **once, before** the cache/server split,
so both phases filter identically.
- `SCOPE_ITEM_TYPES` and `scopeItemTypes()` deleted; `searchScope.ts` now
re-exports `SearchScope` from the generated bindings instead of a hand-written
union.
- [library.ts](../../src/lib/stores/library.ts) sends `{ scope }`.
- 8 Rust tests (`search_scope_tests`); the frontend suite now asserts the
*opaque scope* is sent rather than an item-type list.
Verified: adding `"AudioBook"` to the Music scope changed **zero** files under
`src/` — the criterion that failed before this work.
**Stage 2 remains open**: `GROUP_ITEM_TYPES` / `groupItemTypes()` (result-side
bucketing, single-type-per-group) are still in `searchScope.ts`, and both search
payloads still carry a flat `MediaItem[]` rather than `GroupedSearchResult`.
### 🔴 The `search-event` dual payload (Stage 2)
The original flags this as "the single largest part of the change and the
easiest to half-do." Restating because it is the one thing that silently breaks:
search resolves **twice** — the command returns instant cache results, then the
merged cache+server union arrives via `search-event`. Both payloads must carry
`GroupedSearchResult`. Convert one and the UI flickers between shapes as server
results land.
Write the failing test for the *event* payload first — the command return is the
obvious half, the event is the half that gets forgotten.
### Note on `SearchOptions.scope` and specta
`SearchOptions` is already `#[serde(rename_all = "camelCase")]` with
`skip_serializing_if = "Option::is_none"`. Add `scope: Option<SearchScope>`
following that pattern so `All`/absent omits the key. Regenerate `bindings.ts`
`SearchOptions` there is currently
`{ limit?, includeItemTypes?, searchTerm? }` and must gain `scope?`. Never
hand-edit it.
## Out of scope
- Redesigning anything in [scoped-search-boundary.md](scoped-search-boundary.md).
If implementation shows the design wrong, revise **that** spec, don't fork it.
- Online/offline `include_item_types` **filtering** — already correct; only the
source of the type list moves.
- Ranking within or across groups (DR-090 territory).
- Chip UX, scope persistence, group-order persistence — unchanged.
- The two lesser type-set sites in `DownloadedBrowse.svelte` and
`GenericMediaListPage.svelte`, handled in
the hardened tripwire (DR-094, see `scripts/check-frontend-boundary.sh`).
- Broadening the tripwire itself — same sibling spec.
## Acceptance criteria
Inherits every criterion from [scoped-search-boundary.md](scoped-search-boundary.md)
§Acceptance criteria. Additionally:
- [ ] `grep -rn "SearchScope" src-tauri/src --include='*.rs'` returns matches —
the enum exists in Rust (it does not today).
- [ ] `grep -n "SCOPE_ITEM_TYPES\|scopeItemTypes\|GROUP_ITEM_TYPES\|groupItemTypes" src/lib/utils/searchScope.ts`
returns nothing.
- [ ] `grep -rn "scopeItemTypes" src/` returns nothing — including the
`library.ts` import and call site.
- [ ] `SearchOptions` in `bindings.ts` includes `scope`; regenerated, not
hand-edited.
- [ ] **Behaviour is byte-identical for the user**: same scoping, same groups,
same order, same empty-group omission, offline included. This spec is a
pure refactor — any visible change is a defect.
- [ ] `All` scope sends no `includeItemTypes` (asserted in a Rust test, not by
inspection).
- [ ] Adding a type to the Music scope requires editing **only** Rust —
demonstrate by making the edit and confirming no `src/` file changes.
- [ ] `scoped-search-boundary.md` status flips to **Implemented**, and
`scoped-search.md`'s "frontend only, no Rust changes" framing gets a
banner pointing at the corrected design.
- [ ] `bun run check` and `bun run test` pass.
- [ ] `cargo fmt` clean, `cargo clippy` clean, `bun run test:rust` passes.
- [ ] `bun run check:boundary` passes.
- [ ] Changed code carries `// TRACES:` comments (IDs below).
## Testing
Follow [scoped-search-boundary.md](scoped-search-boundary.md) §Testing. Emphases:
**Rust** (`cargo test`):
- `SearchScope::item_types()` per scope; `All``None`.
- Scope resolution happens **before** the online/offline split, so both paths
get the same filter — a regression here is invisible until someone searches
offline.
- `scope` set + `include_item_types` set → scope wins (the documented
precedence; assert it rather than trusting the doc).
- Stage 2: mixed `Vec<MediaItem>` buckets correctly; unknown types dropped;
canonical group order; **the `search-event` payload is the grouped shape**.
**Frontend** (`bun run test`):
- `resolveSearchScope()` tests in `searchScope.test.ts` must pass **unchanged**
they cover the part that is not moving, and are the regression net proving the
refactor didn't disturb routing.
- `library.ts` sends `{ scope }` and never `includeItemTypes` for search.
- `composeSearchGroups()` over fixture `SearchGroup[]` with no `.type`
inspection in the implementation.
**Offline parity:** run a scoped search with the server unreachable and confirm
identical grouping. The offline repository path honours `include_item_types`
independently, and this is the case most likely to be missed.
## TRACES
No new requirement IDs — this implements existing ones. Retag as the code moves:
```rust
// src-tauri/src/repository/types.rs
/// TRACES: UR-049 | DR-063
pub enum SearchScope { }
```
```typescript
// src/lib/utils/searchScope.ts — keep the file header; it retains
// resolveSearchScope + group-order presentation logic.
// TRACES: UR-049, UR-050 | DR-063, DR-066, DR-067
```
Update DR-063's text in `requirements.md` to state that scope expansion is owned
by Rust, so the requirement stops describing the leaked design. New Rust tests
take `@req-test: UT-089` onward (next free UT is **UT-089**).
## Notes for the implementer
- A parallel Claude session may be active in this repo — `git diff` before
"repairing" unexpected changes (CLAUDE.md §Gotchas).
- **Read [scoped-search-boundary.md](scoped-search-boundary.md) first.** This
spec is deliberately thin on design; that one is the authority.
- Sequence with the sibling specs: **Stage 1 here → then
the hardened tripwire (DR-094)**. Hardening
the tripwire first turns `master` red on a known-unfixed violation.
- `git log --oneline -- docs/specs/scoped-search-boundary.md` is worth a look
before starting — understanding why the fix stalled may surface a constraint
the spec didn't record.
- The user-visible-change count for this spec is zero. If QA reports a
difference in search results, that is a bug in the refactor, not an
improvement.