dtourolle/jellytau
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 37 Wiki Activity

feat(search): answer search from a local index; tier downloads by lifetime

Browse Source 
Search's instant leg read only downloaded items, so with no downloads it
returned nothing and every keystroke fell through to a full Recursive=true
server query. It now reads the whole synced catalog through the same
availability CTE get_items uses, gated on the same include_catalog_browse
flag so search and browse cannot diverge. (UR-065, DR-108)

Also fixes three defects found while confirming that:

- items_fts grew by a full duplicate index every catalog pass. INSERT OR
  REPLACE fires no AFTER DELETE trigger without recursive_triggers, so the
  old index row was orphaned, and a TEXT PRIMARY KEY meant the replacement
  took a fresh rowid and inserted a second entry. Now a real upsert, with
  migration 021 rebuilding existing indexes. (DR-110)
- DELETE FROM items existed nowhere, so server-side deletions never
  propagated. Adds a post-crawl mark-and-sweep, scoped to crawled types,
  skipping downloaded items, and refusing to run after a partial crawl
  because items.parent_id cascades. (DR-110)
- The index omitted MusicArtist, Playlist and People, which search groups
  results by. Adds them plus people_fts (migration 022). (DR-111)

Re-indexing moves from a frontend startup call to a Rust background task
with a 6h TTL, so a long session no longer searches a stale catalog and a
restart no longer forces a crawl regardless of freshness. (DR-109, IR-030)

Downloads gain a lifetime tier. Eviction selected every completed row by
age with no download_source filter, so hitting the storage limit deleted
the oldest download -- typically one saved deliberately for offline -- to
make room for a precached track. It now reclaims only 'auto' rows, and
expired ones are reclaimed first, before live cache is evicted.
(DR-126, DR-127)

Downloaded video and audio-only handoffs now play from disk instead of
streaming; the video path had never consulted downloads at all. No
transcode is involved: MPV runs video=no and ExoPlayer has no surface for
an Audio item. (DR-123 in part, DR-128)

FTS queries are built as quoted phrases so apostrophes, hyphens and
slashes are data rather than operator syntax, and the item-type filter is
bound rather than interpolated.

Specs: docs/specs/catalog-index-search.md,
docs/specs/read-through-media-cache.md

Includes concurrently-developed favourites browsing and background-audio
stream-end handling; the two workstreams share offline.rs, lib.rs and
online.rs, so no subset of files builds independently.
This commit is contained in:
dtourolle
2026-08-04 17:35:17 +02:00
parent c55ff45692
commit 62873cab3d
52 changed files with 6110 additions and 191 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/specs/catalog-index-search.md
+334
View File
@@ -0,0 +1,334 @@
# 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.