jellytau/docs/specs/catalog-index-search.md

# Spec: Locally-indexed search

**Status:** Implemented
**Requirements:** UR-065 → DR-108, DR-109, DR-110, DR-111; IR-030
**UX spec:** [ux-flows.md §6.1](../ux-flows.md) (search surface is unchanged)
**Revises:** [scoped-search.md](scoped-search.md) and
[scoped-search-boundary.md](scoped-search-boundary.md) — scope semantics are
untouched; this changes only *which corpus* the cache leg searches.

## Summary

Search stops depending on a per-keystroke round trip to Jellyfin. The local
SQLite catalog — which is already synced and already FTS5-indexed — becomes the
corpus the instant leg of search reads, so results appear as fast as SQLite can
answer, online or offline. A background indexer keeps that catalog fresh on a
schedule instead of only at app start, prunes content deleted on the server, and
covers the item types search groups results by. The server query stays, demoted
to a background reconciliation that merges in late results for anything indexed
since the last pass.

## Motivation

The pieces are already built and simply not wired together:

- [`sync_full_catalog`](../../src-tauri/src/commands/catalog.rs) already walks
  every library `Recursive=true` and persists items with `synced_at`.
- `items_fts` (schema.rs migration 001) already indexes `name`, `overview`,
  `album_name`, `album_artist`, `artists`, `series_name` with keep-in-sync
  triggers.
- `repository_search` is already two-phase — synchronous cache result, then a
  spawned server query merged in via the `search-event`.

What breaks the chain is that the cache leg is hard-restricted to *downloaded*
items. `OfflineRepository::search` wraps its FTS query in a `downloaded_items`
CTE requiring `d.status = 'completed'`:

```sql
FROM items i
 JOIN items_fts fts ON fts.rowid = i.rowid
 INNER JOIN downloaded_items di ON i.id = di.id
 WHERE i.server_id = ? AND items_fts MATCH ?
```

So for a user with no downloads, phase 1 returns nothing on every query, and
every debounced keystroke falls through to a full `Recursive=true` server
request with `Limit=10000`. The populated local index is never read.

`get_items` does not have this problem — it gates a third `synced_at IS NOT NULL`
branch on `include_catalog_browse()` (offline.rs, the "Show all server media"
toggle). The asymmetry is the bug: **offline you can already browse the whole
catalog but cannot search it.**

Three further defects found while confirming the above:

1. **The FTS index grows without bound.** `save_to_cache` uses
   `INSERT OR REPLACE INTO items`, but `recursive_triggers` is never enabled
   (`storage/mod.rs` sets only `foreign_keys` and `journal_mode`). SQLite fires
   `AFTER DELETE` triggers on a REPLACE *only* with recursive triggers on — so
   `items_ad` never runs, the old FTS row is orphaned, and because `items.id` is
   a `TEXT PRIMARY KEY` the replacement row takes a **new rowid** and inserts a
   second FTS entry. Every sync appends a duplicate index. Results stay correct
   (the `INNER JOIN … ON fts.rowid = i.rowid` hides orphans, and no rowid is ever
   reused because nothing is deleted) but `MATCH` degrades permanently.
2. **Server-side deletions never propagate.** There is no `DELETE FROM items`
   anywhere in the codebase. The local catalog is append-only, so media removed
   from the server would stay searchable forever — tolerable when the cache was
   only a browse accelerator, not acceptable when it is the search corpus.
3. **The index omits types search groups by.** `CATALOG_ITEM_TYPES` is
   `MusicAlbum, Movie, Series, Season, Episode, Audio, BoxSet` — no
   `MusicArtist`, no `Playlist`, and People live in a separate `people` table
   with no FTS at all. UR-060 mandates Artists and People result groups, so today
   those can *only* come from the server.

## Layer assignment

| Logic / responsibility | Layer | Why it belongs there |
|------------------------|-------|----------------------|
| Which corpus search reads (downloads-only vs full synced catalog) | **Rust** | Sync/availability policy over domain data. Changes if Jellyfin's API or the offline rules change, not if the UI is redesigned. Reuses the existing `include_catalog_browse()` flag so search and browse cannot diverge again. |
| Index freshness policy — TTL, when a re-index is due, skip-while-offline | **Rust** | Explicitly named as domain policy in [SPEC-REVIEW-CHECKLIST.md](SPEC-REVIEW-CHECKLIST.md) ("reachability/sync policy"). It is currently frontend-driven in `offlineCatalog.ts`; this spec moves it. |
| Which Jellyfin item types get indexed (`CATALOG_ITEM_TYPES`) | **Rust** | Textbook domain taxonomy — a category→item-type set. Must never appear in `src/`. |
| Reconciling a crawl against local rows (what to prune) | **Rust** | Operates on domain data and depends on crawl completeness semantics. |
| FTS query construction, ranking, scope→type expansion | **Rust** | Already there (`search_rank.rs`, `SearchScope::item_types()`); unchanged by this spec. |
| Rendering a "catalog last indexed N ago" hint and any re-index button | **Frontend** | Pure presentation of a backend-supplied timestamp. |
| Debounce interval, result group order, scope chips | **Frontend** | Input handling and view preference; changes only if the UI is redesigned. |

Borderline call, recorded: the **TTL value itself** (how many hours before a
re-index is due) could be argued as a user preference and therefore frontend. It
is placed in Rust because the frontend must not be able to decide *whether the
cache is authoritative* — that is the same class of decision as
`include_catalog_browse`, which already lives in Rust. If the TTL later becomes
user-configurable it stays a Rust-owned setting the frontend edits through a
command, not a frontend constant. Borderline defaults to Rust.

## Design

### 1. Search the full synced catalog (DR-108)

`OfflineRepository::search` mirrors `get_items` exactly: rename the CTE to
`available_items` and add the same third branch, gated on the same flag.

```rust
let catalog_branch = if include_catalog_browse() {
    "UNION

        -- Synced catalog: fast online search, or the offline 'Show all
        -- server media' view. Mirrors get_items; see set_include_catalog_browse.
        SELECT DISTINCT i.id
        FROM items i
        WHERE i.synced_at IS NOT NULL"
} else {
    ""
};
```

No new IPC surface and no frontend change: `set_include_catalog_browse` is
already called with `true` when online or when the offline toggle is on, and
`false` only when offline with the toggle off. Search inherits the correct
behaviour in all three states, and the "search is restricted to downloads" case
survives for users who deliberately asked for downloads-only.

Also fix, in the same function, the `type_filter` built by **string
interpolation** of `include_item_types` rather than bound parameters. It is
currently safe only because callers pass `SearchScope`-derived values, but
`SearchOptions.include_item_types` is settable directly from the frontend (as
`GenericMediaListPage` does). Bind the values.

Phase 2 (the server query) is unchanged and still merges via `search-event`, so
content added to the server since the last index still surfaces — just late
rather than first.

### 2. Scheduled background indexer (DR-109, IR-030)

A Rust-owned task replaces the frontend's startup-only trigger.

```rust
/// How long a full-catalog index stays fresh before a re-index is due.
const CATALOG_INDEX_TTL: Duration = Duration::from_secs(6 * 60 * 60);
```

Behaviour:

- On app setup, spawn a tokio task that ticks every 30 min.
- Each tick: if a repository is active **and** the server is reachable **and**
  `now - last_catalog_sync > CATALOG_INDEX_TTL`, run a full index pass.
- On the existing `ConnectivityMonitor` reconnect signal, evaluate the same
  staleness condition immediately rather than waiting for the next tick.
- Never run two passes concurrently (the existing `syncInProgress` guard moves
  into Rust as an `AtomicBool`).

`last_catalog_sync` is already written to `app_settings` by `sync_full_catalog`
and is currently read only for a UI hint; this makes it load-bearing.

`RepositoryManager` (`commands/repository.rs`) is a `HashMap<String, …>` with no
notion of an active handle, so the task has nothing to run against. Add:

```rust
pub struct RepositoryManager {
    repositories: Arc<Mutex<HashMap<String, Arc<HybridRepository>>>>,
    active: Arc<Mutex<Option<String>>>,   // set in create(), cleared in destroy()
}
```

Progress is reported with a **kebab-case** event (per the project convention):

```rust
// event name: "catalog-index-event"
#[derive(specta::Type, Serialize, Clone)]
#[serde(rename_all = "camelCase")]
pub struct CatalogIndexEvent {
    pub state: CatalogIndexState,   // #[serde(tag = "type")] Idle | Running | Complete | Failed
    pub libraries_done: usize,
    pub libraries_total: usize,
    pub items_indexed: usize,
}
```

`sync_full_catalog` stays a command so the UI can still force a pass; it and the
scheduler share one internal `run_index_pass()`.

### 3. Index hygiene — no orphans, and deletions propagate (DR-110)

**Orphan growth.** Replace `INSERT OR REPLACE INTO items (…)` in `save_to_cache`
with a true upsert:

```sql
INSERT INTO items (id, server_id, …) VALUES (…)
ON CONFLICT(id) DO UPDATE SET
    name = excluded.name, overview = excluded.overview, …,
    synced_at = excluded.synced_at
```

This preserves the rowid (which `items_fts` keys on via `content_rowid`) and
fires `items_au` instead of silently orphaning a row. Preferred over
`PRAGMA recursive_triggers = ON` because it also stops the rowid churn, and the
three FTS triggers are the only triggers in the schema so nothing else depends
on REPLACE semantics.

A new migration `021_rebuild_items_fts` clears the orphans already accumulated on
existing installs:

```sql
INSERT INTO items_fts(items_fts) VALUES('rebuild');
```

**Deletions.** After a library crawls *successfully and completely*, reconcile:
delete local rows for that library whose `id` was not seen in the crawl. Two
constraints the implementation must respect:

- Skip any item with a completed download — the user has the file; removing the
  row would orphan it. Prune only synced-but-not-downloaded rows.
- Only sweep libraries whose crawl succeeded. `sync_full_catalog` is
  deliberately best-effort per library, and `items.parent_id` is
  `ON DELETE CASCADE` — sweeping on a partial crawl would cascade a whole series
  away because one request timed out.

### 4. Index the types search groups by (DR-111)

Add `MusicArtist` and `Playlist` to `CATALOG_ITEM_TYPES`.

People need a different mechanism: they live in `people` (`id`, `server_id`,
`name`, `overview`, `primary_image_tag`, `synced_at`), populated incidentally by
item-detail fetches, with no FTS table. Migration `022_people_fts` adds one
mirroring the `items_fts` pattern:

```sql
CREATE VIRTUAL TABLE IF NOT EXISTS people_fts USING fts5(
    name, overview, content='people', content_rowid='rowid'
);
-- plus people_ai / people_ad / people_au triggers
```

`OfflineRepository::search` UNIONs `people_fts` matches into its result set as
`Person`-typed items when the resolved scope permits them (i.e. when
`include_item_types` is `None` — `SearchScope::All`). `search_rank.rs` already
handles `MediaKind::Person`, so ranking needs no change.

## Out of scope

- **Incremental indexing** (e.g. Jellyfin's `MinDateLastSaved`). A full crawl is
  what makes the deletion sweep in §3 sound — it yields the authoritative id set
  per library. An incremental pass cannot detect deletions, so it would need a
  separate reconciliation strategy. Worth revisiting if full crawls prove too
  slow on large libraries; measure first.
- **Changing search UX** — scope chips, group order, the debounce, and the
  `/search` route are untouched.
- **Removing the server leg.** Phase 2 stays.
- The two dead search implementations (`storage_search_items` in
  `commands/storage/mod.rs`, `offline_search` in `commands/offline.rs`) — both
  registered in `lib.rs` and exported to `bindings.ts`, neither called from the
  frontend. Deleting them is correct but is cleanup, not this feature; file
  separately so this spec's diff stays reviewable.
- `GenericMediaListPage` passing raw `includeItemTypes` and re-implementing the
  store's request-id/event protocol. A real boundary smell, tracked separately.

## Acceptance criteria

- [ ] With a synced catalog and **zero downloads**, typing a query returns
      results from the local index before any server request completes.
- [ ] Offline with "Show all server media" **on**, search returns the full
      catalog (non-downloaded entries greyed out, matching browse).
- [ ] Offline with the toggle **off**, search returns downloaded media only —
      the behaviour that exists today.
- [ ] Re-running a full index pass N times does not grow `items_fts` row count
      beyond the `items` row count.
- [ ] An item deleted server-side disappears from local search after one index
      pass; a **downloaded** item deleted server-side does not.
- [ ] A library that fails mid-crawl prunes nothing.
- [ ] Searching an artist or actor name returns results with the server
      unreachable.
- [ ] `bun run check` and `bun run test` pass.
- [ ] `cargo fmt` clean, `cargo clippy` clean, `bun run test:rust` passes.
- [ ] `bun run check:boundary` passes.
- [ ] New requirement-implementing code carries `// TRACES:` comments.
- [ ] `bindings.ts` regenerated (new `CatalogIndexEvent` type).

## Testing

Per CLAUDE.md, each defect gets a **failing test first**.

Rust (`cargo test`), against an in-memory DB seeded with synced-but-not-
downloaded items:

- `search` returns synced items when `include_catalog_browse()` is true, and
  only downloaded items when false. *Fails today* — the current CTE returns
  empty in the first case.
- Upserting the same item twice leaves exactly one `items_fts` row. *Fails
  today.*
- The sweep removes a vanished synced item, retains a vanished downloaded item,
  and no-ops for a library whose crawl errored.
- `type_filter` binds parameters — a type string containing a quote does not
  alter the query.
- Staleness: a `last_catalog_sync` inside the TTL does not trigger a pass; one
  outside it does; offline never does.
- `people_fts` matches surface as `Person` items under `SearchScope::All` and
  are excluded under `Music`/`Movies`/`Tv`.

Frontend (`vitest`): the catalog-index event maps to the staleness hint; no
change to the search store's request-id/stale-response handling, which stays
covered by its existing tests.

## TRACES

| Piece | Tag |
|---|---|
| `OfflineRepository::search` availability CTE | `// TRACES: UR-065 \| DR-108` |
| Background indexer task + scheduling | `// TRACES: UR-065 \| DR-109, IR-030` |
| `save_to_cache` upsert + FTS rebuild migration | `// TRACES: UR-065 \| DR-110` |
| Deletion reconciliation | `// TRACES: UR-065 \| DR-110` |
| `CATALOG_ITEM_TYPES` widening + `people_fts` | `// TRACES: UR-065, UR-060 \| DR-111` |

## Notes for the implementer

- **A parallel Claude session may be active in this repo.** Run `git diff`
  before "repairing" changes you did not make (CLAUDE.md gotchas).
- The frontend's `offlineCatalog.ts` startup trigger should be **removed**, not
  left alongside the Rust scheduler — two independent triggers with one
  `syncInProgress` guard each is how double-crawls happen.
- `downloads` has a relaxed FK to `items` (migration 005). Verify the deletion
  sweep's interaction with it before enabling the sweep, and check whether
  `parent_id`'s `ON DELETE CASCADE` reaches further than intended.
- The existing 100 ms `cache_with_timeout` in `hybrid.rs` returns *empty* on
  timeout rather than erroring. Once the cache leg is the primary path, that
  budget may need raising — an FTS query over a large catalog on cold page cache
  can exceed it, and the failure mode is a silently empty result.
- Keep `SearchScope` semantics as-is: `All => None` (no filter), deliberately
  not a union, so People and folders are not filtered out (DR-063).
- Noted but deliberately not fixed here: `pushCatalogVisibility` in
  `offlineCatalog.ts` derives the flag as `connected || showCatalog` — the
  frontend computing an availability *policy*, even though the flag itself is
  Rust-stored. DR-108 depends on that derivation being correct and it is, so
  this spec leaves it alone. Once DR-109 has moved sync policy into Rust, the
  derivation belongs there too, with the frontend pushing only the raw user
  toggle. Folding it into this change would enlarge the diff for no behavioural
  gain — but do not add *new* policy on the frontend side of that line.