docs: specs, requirements, ux-flows and traceability for new features
Add specs for the account menu, downloads-as-offline-library, offline downloaded-only filter, and scoped search (+ boundary revision). Add the new UR/DR entries to requirements.md, update ux-flows, and regenerate the traceability matrix. TRACES: UR-049, UR-050, UR-052, UR-053, UR-054, UR-055, UR-056
This commit is contained in:
@@ -0,0 +1,68 @@
|
||||
# Spec review checklist
|
||||
|
||||
Run a spec past this before accepting it. It exists because JellyTau's
|
||||
backend/frontend boundary is a **stated rule with, historically, no gate** — the
|
||||
rule lived in the architecture docs, but nothing forced a spec author to check a
|
||||
new design against it, and a "minimal-change" spec quietly leaked domain
|
||||
taxonomy into the frontend (see [scoped-search-boundary.md](scoped-search-boundary.md)).
|
||||
This checklist is the human gate. The CI check
|
||||
(`scripts/check-frontend-boundary.sh`) is only a crude tripwire for one leak
|
||||
signature — it does **not** replace this.
|
||||
|
||||
Copy the boxes into the review comment (or the PR) and tick them.
|
||||
|
||||
## Boundary (the one that bites)
|
||||
|
||||
- [ ] **The spec has a filled-in "Layer assignment" table**, and it assigns
|
||||
*logic*, not files. A spec without this section is not ready to review.
|
||||
- [ ] **No domain vocabulary is placed in the frontend.** In particular: Jellyfin
|
||||
item-type sets that define a *category* (what "Music"/"TV"/"Movies" means),
|
||||
query-shaping rules, business rules, reachability/sync policy. If the
|
||||
frontend names a *set* of item types to define a category, that is a leak —
|
||||
it belongs behind an opaque enum the backend expands.
|
||||
- [ ] **"The backend already accepts this parameter" was not used as the reason**
|
||||
to place the deciding logic in the frontend. Accepting a parameter ≠ owning
|
||||
the decision of its value.
|
||||
- [ ] **The `Scope:` / effort framing is not optimizing for "least backend
|
||||
change."** "Frontend only, no Rust changes" is a description, never a goal.
|
||||
The goal is *correct layer placement*; sometimes that is more Rust work.
|
||||
- [ ] Ran the litmus test on each borderline responsibility: *would it change if
|
||||
Jellyfin's API changed?* → Rust. *Only if the UI were redesigned?* →
|
||||
frontend. Borderline defaults to Rust.
|
||||
- [ ] Single-type presentation (`itemType: "Movie"`, "this page shows albums")
|
||||
is **not** over-corrected into the backend. The rule targets category
|
||||
*taxonomy*, not every mention of a type. Don't invent a backend enum per
|
||||
list page.
|
||||
|
||||
## IPC contract
|
||||
|
||||
- [ ] Anything crossing the boundary has its wire shape specified.
|
||||
- [ ] camelCase rule accounted for: top-level params auto-convert; nested structs
|
||||
get `#[serde(rename_all = "camelCase")]`; tagged unions match tags on both
|
||||
sides; events are kebab-case. (CLAUDE.md §IPC,
|
||||
[04-type-sync-and-threading.md](../architecture/04-type-sync-and-threading.md).)
|
||||
- [ ] Any result that arrives *twice* (command return **and** a later event —
|
||||
e.g. the search cache/server merge) has **both** payloads in the new shape.
|
||||
- [ ] `bindings.ts` is regenerated from Rust, not hand-edited.
|
||||
|
||||
## Requirements & traceability
|
||||
|
||||
- [ ] Linked to existing URs, or new URs/DRs are allocated in
|
||||
[requirements.md](../requirements.md).
|
||||
- [ ] Requirement-implementing code will carry `// TRACES:` comments (CLAUDE.md).
|
||||
- [ ] Traceability coverage stays ≥ 50% (the CI gate).
|
||||
|
||||
## Conflicts & hygiene
|
||||
|
||||
- [ ] If this spec revises/supersedes another, the older spec gets a banner
|
||||
pointing here — no two specs silently contradicting.
|
||||
- [ ] Acceptance criteria include the standard gates: `bun run check`,
|
||||
`bun run test`, `bun run check:boundary`, and (if Rust changed)
|
||||
`cargo fmt`/`cargo clippy`/`bun run test:rust`.
|
||||
- [ ] Notes flag that a parallel Claude session may be active in the repo.
|
||||
|
||||
---
|
||||
|
||||
**If any Boundary box can't be ticked, the spec is not ready** — fix the layer
|
||||
assignment first. Every other section can be negotiated; that one is the whole
|
||||
reason this file exists.
|
||||
@@ -0,0 +1,105 @@
|
||||
# Spec: <feature name>
|
||||
|
||||
<!--
|
||||
Copy this file to docs/specs/<kebab-name>.md and fill it in. Delete the HTML
|
||||
comments as you go. The section that matters most for this project is
|
||||
"Layer assignment" — read its comment before writing it.
|
||||
|
||||
Before merging a spec, run it past docs/specs/SPEC-REVIEW-CHECKLIST.md.
|
||||
-->
|
||||
|
||||
**Status:** Proposed <!-- Proposed | Accepted | Implemented | Superseded -->
|
||||
**Requirements:** <!-- UR-xxx → DR-yyy; allocate new DRs in requirements.md. -->
|
||||
**UX spec:** <!-- link to the relevant ux-flows.md section, or "n/a". -->
|
||||
**Supersedes / revises:** <!-- link any spec this changes, or delete this line. -->
|
||||
|
||||
## Summary
|
||||
|
||||
<!-- 2–4 sentences. What changes for the user, in plain terms. -->
|
||||
|
||||
## Motivation
|
||||
|
||||
<!-- Why now. The problem being solved. -->
|
||||
|
||||
## Layer assignment
|
||||
|
||||
<!--
|
||||
🔴 THIS IS THE SECTION THAT KEEPS THE ARCHITECTURE HONEST. Do not skip it, and
|
||||
do NOT reframe it as "how little backend work can we get away with."
|
||||
|
||||
The project rule (CLAUDE.md, architecture/02-svelte-frontend.md): the Rust
|
||||
backend owns ALL business logic — auth, catalog, sessions, downloads, offline,
|
||||
playback, AND domain vocabulary (e.g. what Jellyfin item types the category
|
||||
"Music" means). The Svelte frontend is PRESENTATION ONLY: rendering, layout,
|
||||
navigation, view/order preferences, input handling.
|
||||
|
||||
For each distinct piece of *logic* this feature introduces, put it in the table
|
||||
and name the layer it belongs to and WHY. "It's less work in the frontend" and
|
||||
"the backend already accepts this parameter" are NOT reasons to place logic in
|
||||
the frontend — the backend accepting a parameter does not make deciding that
|
||||
parameter's value a presentation concern.
|
||||
|
||||
Litmus test for "does this belong in Rust?": Would this logic have to change if
|
||||
Jellyfin changed its API, added an item type, or altered a business rule? If
|
||||
yes, it is domain logic → Rust. Would it change if we redesigned the UI? If
|
||||
yes (and only yes), it is presentation → frontend.
|
||||
|
||||
A past incident: scoped-search.md placed the item-type taxonomy (what "Music"
|
||||
means as a set of Jellyfin types) in the frontend because the backend already
|
||||
accepted an includeItemTypes filter. That was a boundary leak; see
|
||||
scoped-search-boundary.md. This section exists to catch that class of mistake
|
||||
at spec time, not in review three features later.
|
||||
-->
|
||||
|
||||
| Logic / responsibility | Layer | Why it belongs there |
|
||||
|------------------------|-------|----------------------|
|
||||
| <!-- e.g. scope → item-types --> | Rust | <!-- domain vocabulary; changes with Jellyfin's API --> |
|
||||
| <!-- e.g. group display order --> | Frontend | <!-- pure presentation; changes only if UI is redesigned --> |
|
||||
|
||||
<!--
|
||||
If a row is genuinely borderline, say so and give the tie-breaker you used.
|
||||
Borderline defaults to Rust for anything touching domain data or vocabulary.
|
||||
-->
|
||||
|
||||
## Design
|
||||
|
||||
<!--
|
||||
How it works. Wire shapes for anything crossing the IPC boundary. Remember:
|
||||
- Command NAME must match the Rust fn name exactly.
|
||||
- Top-level params auto-convert snake_case → camelCase (Tauri v2).
|
||||
- Nested struct fields need #[serde(rename_all = "camelCase")].
|
||||
- Events are kebab-case.
|
||||
(See CLAUDE.md §IPC and architecture/04-type-sync-and-threading.md.)
|
||||
|
||||
Regenerate bindings.ts from Rust types; never hand-edit it.
|
||||
-->
|
||||
|
||||
## Out of scope
|
||||
|
||||
<!-- What this spec deliberately does NOT do. -->
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
<!-- Checkable statements. Include the standard gates: -->
|
||||
|
||||
- [ ] `bun run check` and `bun run test` pass.
|
||||
- [ ] `cargo fmt` clean, `cargo clippy` clean, `bun run test:rust` passes (if Rust changed).
|
||||
- [ ] `bun run check:boundary` passes (no taxonomy leak into the frontend).
|
||||
- [ ] New requirement-implementing code carries `// TRACES:` comments.
|
||||
- [ ] `bindings.ts` regenerated if Rust types changed.
|
||||
|
||||
## Testing
|
||||
|
||||
<!-- Rust: cargo test. Frontend: vitest, src/lib/**/*.test.ts. What to cover. -->
|
||||
|
||||
## TRACES
|
||||
|
||||
<!-- Suggested tags per new/changed piece: UR-xxx | DR-yyy | tests. -->
|
||||
|
||||
## Notes for the implementer
|
||||
|
||||
<!--
|
||||
- A parallel Claude session may be active in this repo — `git diff` before
|
||||
"repairing" unexpected changes (see project memory / CLAUDE.md gotchas).
|
||||
- Anything else non-obvious.
|
||||
-->
|
||||
@@ -0,0 +1,164 @@
|
||||
# Spec: Account menu and global chrome availability
|
||||
|
||||
**Status:** Implemented
|
||||
**Scope:** Frontend only. No Rust changes required.
|
||||
**Requirements:** UR-054 → DR-075, DR-076, DR-077 (see
|
||||
[requirements.md](../requirements.md)).
|
||||
**UX spec:** [ux-flows.md §1.2–1.4](../ux-flows.md).
|
||||
|
||||
## Summary
|
||||
|
||||
Account actions — Settings, Downloads, Display preferences, Sign out — are
|
||||
currently reachable **only from `/library/*`**. Move them into a single shared
|
||||
account menu anchored to the user's name, and make that menu available on every
|
||||
authenticated non-immersive screen.
|
||||
|
||||
## Motivation
|
||||
|
||||
A user sitting on the home screen cannot open Settings or sign out. The bottom
|
||||
nav offers Home / Search / Library only, and the header that hosts those actions
|
||||
belongs to the library layout. The user has to guess that account actions live
|
||||
*inside* Library — an unrelated section — and navigate there first.
|
||||
|
||||
Desktop and mobile also disagree today: desktop shows an unlabeled logout icon
|
||||
with no grouped menu, mobile shows a three-dot overflow with labelled items. The
|
||||
same two actions are found two different ways.
|
||||
|
||||
## Background: verified current state
|
||||
|
||||
1. **The header is not global.** It is defined in
|
||||
[library/+layout.svelte](../../src/routes/library/+layout.svelte). The root
|
||||
layout [+layout.svelte](../../src/routes/+layout.svelte) renders no header at
|
||||
all.
|
||||
|
||||
2. **`routeOwnsLayout`** in
|
||||
[layoutShell.ts](../../src/lib/utils/layoutShell.ts) returns true for
|
||||
`/library`, `/player/`, `/login` — those routes own their own full-height
|
||||
flex column. Everything else renders into the root scroller with the root's
|
||||
`BottomUi` below it.
|
||||
|
||||
3. **Bottom nav is Home / Search / Library only**
|
||||
([BottomNav.svelte](../../src/lib/components/BottomNav.svelte)) — no Settings
|
||||
or account entry.
|
||||
|
||||
4. **Net effect:** on `/`, `/search`, and `/downloads` there is no route to
|
||||
Settings or Sign out.
|
||||
|
||||
5. **Desktop username is inert text** — a `<span>` next to the icons, not a
|
||||
trigger.
|
||||
|
||||
6. **The mobile overflow menu already has the right contents** (Downloads,
|
||||
Settings, divider, Sign out) and the right dismissal behaviour (backdrop
|
||||
click, keyboard handler). **Extract and reuse it rather than rewriting it.**
|
||||
|
||||
7. **`viewMode` is already a persisted store** in
|
||||
[library.ts](../../src/lib/stores/library.ts) (`jellytau-view-mode`,
|
||||
`localStorage`). The Display setting is a second view onto it — **no new
|
||||
state, no migration.**
|
||||
|
||||
## Design
|
||||
|
||||
### `AccountMenu` component (DR-075)
|
||||
|
||||
One component used by both breakpoints. Contents in fixed order:
|
||||
|
||||
```
|
||||
Signed in as <name> ← identity block, not interactive
|
||||
<server host>
|
||||
────────────────────────
|
||||
Downloads
|
||||
Settings
|
||||
Display ← grid/list preference
|
||||
────────────────────────
|
||||
Sign out ← destructive, last, after a divider
|
||||
```
|
||||
|
||||
- **Trigger is the username/avatar**, not a bare three-dot icon. On mobile where
|
||||
horizontal space is tight, the avatar (or initial) alone is acceptable; the
|
||||
name shows inside the open menu regardless.
|
||||
- **Same items, same order, both platforms.**
|
||||
- Preserve the existing dismissal behaviour: click-outside backdrop, `Escape`,
|
||||
and focus return to the trigger on close.
|
||||
- Menu items are real links/buttons — keyboard reachable, correct roles,
|
||||
`aria-expanded` on the trigger.
|
||||
|
||||
"Display" may either navigate to the Settings Display section or expose the
|
||||
grid/list choice inline. Prefer navigating — it keeps one source of truth for
|
||||
preferences and avoids a nested control inside a dropdown.
|
||||
|
||||
### Global chrome (DR-076)
|
||||
|
||||
Make the header — and therefore the account menu — available on `/`, `/search`,
|
||||
and `/downloads`.
|
||||
|
||||
The cleanest route is to lift the header out of the library layout into a shared
|
||||
component rendered by the root layout, with the library layout consuming the
|
||||
same component rather than defining its own. **Do not duplicate the markup into
|
||||
each route.**
|
||||
|
||||
Constraints that must survive the change:
|
||||
|
||||
- `/player/*` and `/login` stay chrome-free.
|
||||
- `/settings` already owns its layout; it needs no account menu (the user is
|
||||
already there), but must not double up on chrome.
|
||||
- The root layout's flex/scroller structure is deliberate — the comments in
|
||||
[layoutShell.ts](../../src/lib/utils/layoutShell.ts) and
|
||||
[+layout.svelte](../../src/routes/+layout.svelte) explain why routes own their
|
||||
own column. Preserve the scroll containment; a regression here reintroduces
|
||||
the "last row hidden behind the nav" bug called out in those comments.
|
||||
- Mini-player and bottom-nav visibility rules (`showGlobalMiniPlayer`,
|
||||
`showBottomNav`) must be unchanged.
|
||||
|
||||
### Display section in Settings (DR-077)
|
||||
|
||||
Add a Display section to [settings/+page.svelte](../../src/routes/settings/+page.svelte)
|
||||
with the grid/list control bound to the existing `viewMode` store via
|
||||
`library.setViewMode(...)`. The page-header toggle in `LibraryGrid` stays — both
|
||||
controls drive the same store, so they stay in sync for free.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Redesigning the Settings page or reorganising its existing sections.
|
||||
- Multi-server / account switching (UR-047) — the identity block displays the
|
||||
active server but offers no switcher.
|
||||
- Changing the bottom nav's three destinations.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] Settings and Sign out are reachable from `/`, `/search`, and `/downloads`
|
||||
without first navigating into Library.
|
||||
- [ ] Desktop and mobile show the same account menu items in the same order.
|
||||
- [ ] The username/avatar opens the menu; it is a real button with
|
||||
`aria-expanded`.
|
||||
- [ ] Sign out is last, after a divider, and still logs out + resets library
|
||||
state + redirects as it does today.
|
||||
- [ ] `/player/*` and `/login` remain chrome-free.
|
||||
- [ ] Settings has a Display section that changes grid/list, and the change is
|
||||
immediately reflected by the library page-header toggle (same store).
|
||||
- [ ] No regression in scroll containment, mini-player visibility, or bottom-nav
|
||||
visibility on any route.
|
||||
- [ ] `bun run check` and `bun run test` pass.
|
||||
|
||||
## Testing
|
||||
|
||||
- Extend the existing `layoutShell` tests: chrome-visibility for `/`, `/search`,
|
||||
`/downloads` (now true) and `/player/*`, `/login` (still false).
|
||||
- `AccountMenu`: renders the documented items in order; trigger toggles
|
||||
`aria-expanded`; `Escape` and backdrop click close it; Sign out invokes the
|
||||
logout handler.
|
||||
- Display setting: writes through to the `viewMode` store and persists.
|
||||
|
||||
New requirement-implementing code needs `TRACES:` comments — see
|
||||
[CLAUDE.md](../../CLAUDE.md). Suggested: `AccountMenu` → `UR-054 | DR-075`,
|
||||
shell/header changes → `UR-054 | DR-076`, Settings Display section →
|
||||
`UR-054, UR-029 | DR-077`.
|
||||
|
||||
## Notes for the implementer
|
||||
|
||||
- Read [ux-flows.md §1.2–1.4](../ux-flows.md) first — behavioural spec; this is
|
||||
the implementation plan.
|
||||
- The layout shell is subtle and the existing comments record real bugs that
|
||||
were fixed there. Read them before restructuring.
|
||||
- Another session may be active in this repo, including in
|
||||
`src/routes/settings/+page.svelte`. Check `git diff` before "repairing"
|
||||
unexpected changes, and expect to coordinate on that file.
|
||||
@@ -0,0 +1,166 @@
|
||||
# Spec: Downloads as a browsable offline library
|
||||
|
||||
**Status:** Draft — ready to implement
|
||||
**Scope:** Frontend-heavy; one new repository-client browse path. Minimal Rust.
|
||||
**Requirements:** UR-055 → DR-081, DR-082, DR-083, DR-084; UR-056 → DR-085
|
||||
(see [requirements.md](../requirements.md)).
|
||||
**UX spec:** [ux-flows.md §7.2–7.7](../ux-flows.md).
|
||||
|
||||
## Summary
|
||||
|
||||
Replace the flat Active/Completed download list with two views under
|
||||
`/downloads`:
|
||||
|
||||
1. **Downloaded** (default) — the library, filtered to what's on the device,
|
||||
using the *same* browse screens as online (grids, cards, detail pages).
|
||||
2. **Transfers** — the existing progress-row list, demoted to a secondary tab,
|
||||
showing only in-flight transfers.
|
||||
|
||||
Plus per-item disk usage (UR-056) shown in familiar units on cards, detail
|
||||
pages, a device total, and the remove confirmation.
|
||||
|
||||
## Motivation
|
||||
|
||||
A user who downloaded three seasons and two albums sees ~70 individual transfer
|
||||
rows today, with no grouping and no reuse of the library UI. "What do I have
|
||||
offline" and "what is downloading" are different questions crammed into one flat
|
||||
list. Browsing offline should feel exactly like browsing online.
|
||||
|
||||
## Background: verified current state
|
||||
|
||||
1. **The offline repository is already a browsable tree.**
|
||||
[offline.rs](../../src-tauri/src/repository/offline.rs) — `get_items` returns
|
||||
downloaded items **plus** containers (MusicAlbum, Series, Season) that have at
|
||||
least one downloaded child. `get_libraries`, `get_item`, and `search` all
|
||||
filter to downloaded content via CTEs. This is the data source for
|
||||
Downloaded; **do not build a new query layer.**
|
||||
|
||||
2. **The client cannot reach it independently.**
|
||||
[repository-client.ts](../../src/lib/api/repository-client.ts) `getItems` →
|
||||
`repositoryGetItems` always goes through the **hybrid** repository
|
||||
([hybrid.rs](../../src-tauri/src/repository/hybrid.rs)), which merges cache and
|
||||
server. There is no "offline only" browse path exposed. This is the one real
|
||||
backend gap (DR-082).
|
||||
|
||||
3. **Downloads page is a flat two-tab list.**
|
||||
[downloads/+page.svelte](../../src/routes/downloads/+page.svelte) — Active /
|
||||
Completed tabs, one `DownloadItem` row per transfer, no browsing.
|
||||
|
||||
4. **Library browse components are reusable as-is.** `LibraryGrid`, `MediaCard`,
|
||||
the `/library/[id]` detail page (§5A/§5B) render whatever items they are
|
||||
given. Downloaded browse is those components with an offline-scoped source.
|
||||
|
||||
5. **A related fallthrough bug is already tracked** (DR-080, another session):
|
||||
`HybridRepository::get_items` treats an empty offline result as a cache miss
|
||||
and falls through to the server. The offline-only browse path (DR-082) must
|
||||
**not** share that behaviour — an empty result there is authoritative "nothing
|
||||
downloaded here."
|
||||
|
||||
6. **Concurrency, the 3-download cap, and the auto-pump are backend concerns.**
|
||||
Do not surface them as manual controls; do not loop `startDownload` from the
|
||||
frontend (see [CLAUDE.md](../../CLAUDE.md) gotchas).
|
||||
|
||||
## Design
|
||||
|
||||
### View split (DR-081)
|
||||
|
||||
`/downloads` renders a **Downloaded** / **Transfers** switch. Downloaded is the
|
||||
default. Transfers shows a count/badge only while transfers are active.
|
||||
Initiating downloads stays on item/album/series detail pages (§7.1) — this page
|
||||
does not start downloads.
|
||||
|
||||
### Offline-scoped browse source (DR-082, DR-083)
|
||||
|
||||
Add an explicit offline-only browse path so Downloaded never merges server
|
||||
results and never depends on reachability. Two viable shapes — pick per the
|
||||
codebase, do not do both:
|
||||
|
||||
- **(a)** A dedicated command (e.g. `repository_get_downloaded_items` /
|
||||
`_libraries`) that calls the offline repository directly, with a matching
|
||||
client method; or
|
||||
- **(b)** An explicit `offlineOnly`/scope flag on the existing get-items path
|
||||
that bypasses the hybrid merge and the empty→fallthrough behaviour.
|
||||
|
||||
Either way: an empty result is authoritative (do **not** reuse the DR-080
|
||||
fallthrough), and the path is available while the server is reachable (a user
|
||||
online still wants to browse their downloads).
|
||||
|
||||
Downloaded then reuses `LibraryGrid` / `MediaCard` / the detail page against this
|
||||
source. Omit libraries and containers with no downloaded content. Badge
|
||||
partially- vs fully-downloaded containers. Play uses the local file; remove is
|
||||
available at item / album / season / series level and removes a container from
|
||||
the browse when its last downloaded child goes.
|
||||
|
||||
### Transfers view (DR-084)
|
||||
|
||||
The existing list, filtered to in-flight rows only: downloading (with progress),
|
||||
queued, paused, failed, waiting-for-WiFi (the DR-074 state from the other
|
||||
session). Controls: Pause / Resume / Cancel / Retry. Completed transfers leave
|
||||
this view — they appear in Downloaded. Empty state points at the library.
|
||||
|
||||
### Disk usage (DR-085, UR-056)
|
||||
|
||||
- **Source the bytes from the download manager** — it writes the files and can
|
||||
stat them. Aggregate to album/season/series subtotals and a device total.
|
||||
This is display + aggregation, **not** new tracking.
|
||||
- **Format once, consistently.** One shared formatter, human units, 2–3
|
||||
significant figures (`1.2 GB`, `340 MB`). Binary vs decimal — pick one and use
|
||||
it everywhere.
|
||||
- **Surface it in familiar places:** a secondary size label on the card and
|
||||
detail page; a device total at the top of Downloaded (`3.4 GB · 12 items`)
|
||||
that reconciles with the listed sum; a reclaim figure in the remove
|
||||
confirmation ("frees 1.2 GB"). No separate "storage report" screen.
|
||||
- Sort/filter by size is a nice-to-have, not required for v1.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Changing download initiation, the 3-concurrent cap, or the auto-pump.
|
||||
- The catalog-browse / show-server-catalog toggle (UR-052, another session) —
|
||||
that governs the *online offline-fallback* library; this is the dedicated
|
||||
Downloads surface. They should be consistent but are separate work.
|
||||
- Fixing the DR-080 hybrid fallthrough bug (owned elsewhere) — just don't depend
|
||||
on that behaviour here.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] `/downloads` opens on Downloaded and can switch to Transfers.
|
||||
- [ ] Downloaded lists only libraries/containers with downloaded content, using
|
||||
the same grids/cards/detail pages as online browsing.
|
||||
- [ ] Browsing Downloaded never shows non-downloaded server items, online or off.
|
||||
- [ ] An empty Downloaded result reads as "nothing downloaded," never falls
|
||||
through to the server.
|
||||
- [ ] Play from Downloaded plays the local file.
|
||||
- [ ] Remove works at item/album/season/series level and updates the browse.
|
||||
- [ ] Transfers shows only in-flight rows with working controls; finished
|
||||
transfers move to Downloaded.
|
||||
- [ ] Each downloaded item/container shows its on-disk size; a device total is
|
||||
shown and reconciles with the sum; remove states the reclaim amount.
|
||||
- [ ] `bun run check`, `bun run test`, and (if Rust touched) `cargo test` +
|
||||
`cargo clippy` pass.
|
||||
|
||||
## Testing
|
||||
|
||||
- Repository client: the offline-only browse path returns downloaded content and
|
||||
its containers, and an empty result does **not** trigger server fallthrough.
|
||||
- Downloaded view: libraries/containers with no downloads are omitted;
|
||||
partial/full container badging.
|
||||
- Transfers: only in-flight statuses render; a completed transfer disappears.
|
||||
- Size formatter: rounding and unit thresholds; subtotal aggregation; device
|
||||
total reconciles with listed items.
|
||||
- If a Rust command is added, add the tauri IPC param-naming coverage per
|
||||
[CLAUDE.md](../../CLAUDE.md) (camelCase rule).
|
||||
|
||||
New requirement-implementing code needs `TRACES:` comments. Suggested tags:
|
||||
view split `UR-055 | DR-081`; offline browse path `UR-055 | DR-082, DR-083`;
|
||||
Transfers `UR-055 | DR-084`; size display `UR-056 | DR-085`.
|
||||
|
||||
## Notes for the implementer
|
||||
|
||||
- Read [ux-flows.md §7.2–7.7](../ux-flows.md) first — behavioural spec; this is
|
||||
the implementation plan.
|
||||
- The offline repository already does the hard part. The main work is a clean
|
||||
offline-only client path and reusing the library components — resist
|
||||
rebuilding browse UI.
|
||||
- Another session is active in downloads/offline/connectivity code (DR-074,
|
||||
DR-078–080). Coordinate on [downloads/+page.svelte](../../src/routes/downloads/+page.svelte)
|
||||
and the repository layer; check `git diff` before repairing unexpected changes.
|
||||
@@ -0,0 +1,235 @@
|
||||
# Spec: Offline "downloaded only" filtering (issue #10)
|
||||
|
||||
**Status:** Implemented
|
||||
**Scope:** Frontend (connectivity store) + Rust (hybrid repository). No new
|
||||
commands, no schema changes, no UI additions.
|
||||
**Requirements:** UR-052 → DR-078, DR-079, DR-080
|
||||
(see [requirements.md](../requirements.md)).
|
||||
**Tracking:** issue #10 — *"when offline the filter to show only downloaded
|
||||
media does not work."*
|
||||
|
||||
## Summary
|
||||
|
||||
Offline, a library page is supposed to show **only media on the device**, with a
|
||||
"Show all server media" toggle that additionally reveals the cached server
|
||||
catalog greyed out (queueable for download on reconnect). In practice the toggle
|
||||
does not gate the listing — every server item still appears. This spec fixes
|
||||
that with two independent changes; either one alone leaves the bug visible.
|
||||
|
||||
## Background: what already exists
|
||||
|
||||
Verified in code. **The feature is built and mostly correct — this is a
|
||||
two-point repair, not new infrastructure.** Do not rebuild the toggle, the
|
||||
command, or the SQL gate.
|
||||
|
||||
1. **The SQL gate works and is unit-tested.**
|
||||
[offline.rs](../../src-tauri/src/repository/offline.rs) — `get_items` appends
|
||||
the synced-catalog `UNION` branch only when `include_catalog_browse()` is
|
||||
true; with it false, only downloaded/local rows return. Guarded by
|
||||
`test_get_items_toggle_gates_synced_catalog` (UT-067). **Do not touch the
|
||||
query.**
|
||||
|
||||
2. **The toggle → backend path is wired.** The `showServerCatalog` store and the
|
||||
`set_show_server_catalog` command
|
||||
([catalog.rs](../../src-tauri/src/commands/catalog.rs)) drive the process-wide
|
||||
`INCLUDE_CATALOG_BROWSE` flag. `pushCatalogVisibility` in
|
||||
[offlineCatalog.ts](../../src/lib/services/offlineCatalog.ts) computes
|
||||
`include = connected || showCatalog` and pushes it on every change.
|
||||
|
||||
3. **Home-screen queries are already downloads-only.** `get_latest_items`,
|
||||
`get_resume_items`, `get_recently_played_audio`, `get_resume_movies` all
|
||||
`INNER JOIN downloads ... status = 'completed'`. They are unaffected — leave
|
||||
them.
|
||||
|
||||
4. **`MediaCard` already greys and queues.**
|
||||
[MediaCard.svelte](../../src/lib/components/library/MediaCard.svelte) —
|
||||
`isServerOnly` renders the greyed, inert card with a queue button; the queued
|
||||
row heals its `stream_url` on reconnect via the offlineCatalog service. Leave
|
||||
it.
|
||||
|
||||
## The two defects
|
||||
|
||||
### Defect A — offline is never actually entered (DR-079)
|
||||
|
||||
`pushCatalogVisibility` keys off `isConnected`, but
|
||||
[connectivity.ts](../../src/lib/stores/connectivity.ts) derives:
|
||||
|
||||
```ts
|
||||
isConnected = isOnline && isServerReachable // isOnline = navigator.onLine
|
||||
```
|
||||
|
||||
`navigator.onLine` is documented in that same file as **advisory only** — the
|
||||
Rust `ConnectivityMonitor` is the source of truth (principle: *reachability from
|
||||
real traffic*, DR-055). When the server is unreachable but the device link is
|
||||
up (server down, wrong LAN, VPN dropped), `isOnline` stays true, so `isConnected`
|
||||
stays true, so `include` stays true, so the backend keeps returning the full
|
||||
catalog. The user is "offline" in every meaningful sense but the toggle never
|
||||
gets a chance to gate anything.
|
||||
|
||||
This is the primary cause: it explains why the filter looks dead rather than
|
||||
merely inverted — the gate never closes.
|
||||
|
||||
### Defect B — an intentionally empty result falls through to the server (DR-080)
|
||||
|
||||
With the gate off and nothing downloaded in a library, offline `get_items`
|
||||
correctly returns few or zero rows. But
|
||||
[hybrid.rs](../../src-tauri/src/repository/hybrid.rs) treats a cache result as a
|
||||
hit only `if data.has_content()`. An empty offline result is indistinguishable
|
||||
from a cache miss, so `HybridRepository::get_items` (and `parallel_race`, used by
|
||||
~10 other reads) falls through to the server and returns the full server list —
|
||||
re-defeating the filter even after Defect A is fixed.
|
||||
|
||||
## Design
|
||||
|
||||
### Fix A: `isConnected` follows backend reachability alone (DR-079)
|
||||
|
||||
In [connectivity.ts](../../src/lib/stores/connectivity.ts), redefine the derived
|
||||
store:
|
||||
|
||||
```ts
|
||||
export const isConnected = derived(
|
||||
connectivity,
|
||||
($c) => $c.isServerReachable
|
||||
);
|
||||
```
|
||||
|
||||
`navigator.onLine` stays wired to what it is good for — a *trigger* for an
|
||||
immediate recheck (`online`/`offline` listeners already call
|
||||
`checkServerReachable()`); it must no longer be a *term* in the offline decision.
|
||||
Leave `isOnline` on the state object and the listeners intact.
|
||||
|
||||
Consider whether the optimistic `isServerReachable: true` startup default
|
||||
([connectivity.ts](../../src/lib/stores/connectivity.ts)) should hold until the
|
||||
first real check resolves. Keep it — flipping the app to "offline" on launch is a
|
||||
worse regression than a brief full-catalog flash before the first probe. Note the
|
||||
choice in a comment.
|
||||
|
||||
**Blast radius — this is the reason this is a spec, not a patch.** `isConnected`
|
||||
is consumed beyond this feature (banners, `MediaCard`, mini-player gating,
|
||||
anything importing it). Enumerate consumers first:
|
||||
|
||||
```
|
||||
grep -rn "isConnected" src/ | grep -v node_modules
|
||||
```
|
||||
|
||||
For each, confirm "server unreachable" (not "device link down") is the correct
|
||||
trigger. It almost always is — that is the whole point of the reachability model
|
||||
— but verify rather than assume, and call out anything that genuinely wanted the
|
||||
device link in the PR description.
|
||||
|
||||
### Fix B: an empty offline result is authoritative when the gate is off (DR-080)
|
||||
|
||||
The backend must distinguish "cache is cold, go ask the server" from "user asked
|
||||
for downloads only and there are none here." The gate flag already encodes intent
|
||||
— reuse it.
|
||||
|
||||
Add a getter beside the existing setter in
|
||||
[offline.rs](../../src-tauri/src/repository/offline.rs):
|
||||
|
||||
```rust
|
||||
pub fn include_catalog_browse() -> bool { /* pub, already exists privately */ }
|
||||
```
|
||||
|
||||
In [hybrid.rs](../../src-tauri/src/repository/hybrid.rs) `get_items`: when
|
||||
`!include_catalog_browse()`, treat the offline result as authoritative and return
|
||||
it **as-is even when empty** — do not spawn/await the server fallback for this
|
||||
call. When the flag is on (online fast-path, or offline with the toggle on),
|
||||
behaviour is unchanged: empty cache still falls through to the server.
|
||||
|
||||
Keep it surgical:
|
||||
|
||||
- Scope the change to `get_items`. The gate is a `get_items` concept; do not
|
||||
thread it into `parallel_race` or the other readers, which have no catalog
|
||||
gate and legitimately want the server on an empty cache.
|
||||
- Preserve the online path exactly: with the flag on (its default, and always so
|
||||
while reachable) the method behaves as it does today, including the background
|
||||
cache refresh on a hit.
|
||||
- The flag is process-global `Relaxed`; it is set from the frontend before the
|
||||
query. That ordering already holds for the SQL gate — no new synchronization.
|
||||
|
||||
### Why both
|
||||
|
||||
Fix A closes the gate; Fix B stops the hybrid from re-opening it. A alone: with
|
||||
downloads present the list still gets padded by the server fallback whenever a
|
||||
library's cache is thin. B alone: the gate never closes because `isConnected`
|
||||
never goes false on a live link. Ship them together.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- The SQL gate, the toggle, the command, `INCLUDE_CATALOG_BROWSE` — all correct.
|
||||
- `MediaCard` greying / queue-on-reconnect — correct.
|
||||
- Home-screen and resume queries — already downloads-only.
|
||||
- The Rust `ConnectivityMonitor` reachability logic itself — unchanged; this
|
||||
spec only stops the *frontend* from diluting its verdict with `navigator.onLine`.
|
||||
- Any new IPC command, DB column, or settings entry.
|
||||
- Making the "Show all server media" toggle reachable from Settings (that is a
|
||||
UX-placement question, tracked separately under UR-051's toggle note).
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [~] With the server unreachable on a live device link, a library page lists
|
||||
only downloaded media when the toggle is off (IT-016 — pending e2e; unit
|
||||
coverage via UT-069 + gate tests).
|
||||
- [x] Turning the toggle on reveals the greyed-out cached catalog; turning it off
|
||||
hides it again — without leaving/re-entering the page (SQL gate + toggle
|
||||
wiring unchanged; UT-068 confirms the flag is pushed on toggle change).
|
||||
- [x] A library with downloads and a thin cache does not get padded with
|
||||
non-downloaded server items when offline with the toggle off (Defect B —
|
||||
UT-070: gate off + empty offline result returned as-is, server not queried).
|
||||
- [x] `isConnected` is false whenever the server is unreachable, regardless of
|
||||
`navigator.onLine`; true for a reachable server even if the browser reports
|
||||
offline (UT-069).
|
||||
- [x] Every existing `isConnected` consumer still behaves correctly (banner in
|
||||
`+layout.svelte`, `MediaCard`, `favorites.ts` server-write skip — all want
|
||||
"server unreachable", which is the new semantics; `CastButton`'s local
|
||||
`isConnected` is unrelated). Full frontend suite (616 tests) green.
|
||||
- [x] Online behaviour is unchanged: with the flag on (its default, always so
|
||||
while reachable) `get_items` keeps the offline fast-path and background
|
||||
refresh (UT-067 + gate-on fall-through test).
|
||||
- [~] A download queued from a greyed offline card resolves and starts on
|
||||
reconnect (IT-017 — regression check, no code change; offlineCatalog
|
||||
resume path untouched).
|
||||
- [x] `bun run check`, `bun run test`, and `bun run test:rust` pass;
|
||||
`cd src-tauri && cargo fmt && cargo clippy` clean (no new warnings in the
|
||||
touched files).
|
||||
|
||||
## Testing
|
||||
|
||||
Rust ([offline.rs](../../src-tauri/src/repository/offline.rs) /
|
||||
[hybrid.rs](../../src-tauri/src/repository/hybrid.rs) test modules):
|
||||
|
||||
- **UT-070** — hybrid `get_items` with the gate off returns an empty offline
|
||||
result as-is and does **not** query the server. Assert via a mock online repo
|
||||
whose `get_items` bumps a call counter that must stay at zero.
|
||||
- Gate on + empty cache still falls through to the server (guard the online path).
|
||||
- UT-067 (`test_get_items_toggle_gates_synced_catalog`) must still pass untouched.
|
||||
|
||||
Frontend (vitest, `src/lib/**/*.test.ts`):
|
||||
|
||||
- **UT-069** — `isConnected` follows `isServerReachable` alone: false when
|
||||
unreachable with `navigator.onLine === true`; true when reachable with
|
||||
`navigator.onLine === false`.
|
||||
- **UT-068** — `pushCatalogVisibility` resolves `serverReachable || showCatalog`
|
||||
and pushes to the backend on a change of either input (extend the existing
|
||||
offlineCatalog tests).
|
||||
|
||||
Integration (IT-016, IT-017) are documented as pending in
|
||||
[requirements.md](../requirements.md); wire them if the e2e harness can simulate
|
||||
an unreachable-server-on-live-link state, otherwise leave them pending with a note.
|
||||
|
||||
New/changed requirement code keeps its `TRACES:` comments — see
|
||||
[CLAUDE.md](../../CLAUDE.md). The affected files already carry tags:
|
||||
`connectivity.ts` (`… | DR-079`), `hybrid.rs` (`… | DR-080`), `offline.rs`
|
||||
(`… | DR-078`). Update the getter's tag when you expose it.
|
||||
|
||||
## Notes for the implementer
|
||||
|
||||
- Read [docs/architecture/07-connectivity.md](../architecture/07-connectivity.md)
|
||||
before Fix A — it is the canonical statement of the reachability model this fix
|
||||
restores fidelity to.
|
||||
- Fix B relies on the frontend having pushed the flag before the query runs; that
|
||||
ordering already holds for the SQL gate today. No new locking.
|
||||
- Another session is active in this repo (WiFi-only downloads, account menu
|
||||
landed alongside this work). Check `git diff` before "repairing" unexpected
|
||||
changes, and expect requirement IDs around UR-052 / DR-078 to be adjacent to
|
||||
other new rows.
|
||||
@@ -0,0 +1,273 @@
|
||||
# Spec: Move search scope taxonomy behind the Rust boundary
|
||||
|
||||
**Status:** Proposed
|
||||
**Scope:** Rust + Frontend. **Revises a decision in
|
||||
[scoped-search.md](scoped-search.md).**
|
||||
**Requirements:** UR-049, UR-050 (existing) → new DRs for the boundary move
|
||||
(allocate on implementation; suggested DR-063/DR-065/DR-067 revisions plus one
|
||||
new DR for the grouped result shape — see [requirements.md](../requirements.md)).
|
||||
**UX spec:** unchanged — [ux-flows.md §6](../ux-flows.md). This is a pure
|
||||
architecture/boundary change with **no user-visible behaviour difference**.
|
||||
|
||||
## Why this spec exists
|
||||
|
||||
[scoped-search.md](scoped-search.md) shipped scoped search as "frontend only, no
|
||||
Rust changes." That was the smallest wiring change, and it worked — but it left
|
||||
**Jellyfin's item-type taxonomy encoded in the presentation layer**, which
|
||||
violates the project's core boundary rule ("Svelte frontend — presentation
|
||||
only"; all business logic in Rust — see [CLAUDE.md](../../CLAUDE.md) and
|
||||
[architecture/02-svelte-frontend.md](../architecture/02-svelte-frontend.md)).
|
||||
|
||||
The offending knowledge lives in
|
||||
[searchScope.ts](../../src/lib/utils/searchScope.ts):
|
||||
|
||||
```ts
|
||||
const SCOPE_ITEM_TYPES = {
|
||||
music: ["MusicAlbum", "MusicArtist", "Audio", "Playlist"],
|
||||
movies: ["Movie"],
|
||||
tv: ["Series", "Episode"],
|
||||
};
|
||||
const GROUP_ITEM_TYPES = {
|
||||
songs: ["Audio"], albums: ["MusicAlbum"], artists: ["MusicArtist"],
|
||||
movies: ["Movie"], tvShows: ["Series", "Episode"],
|
||||
};
|
||||
```
|
||||
|
||||
This is a **domain definition** — "what the category *Music* means in Jellyfin's
|
||||
vocabulary" — expressed twice, in the wrong layer. The concrete failure it
|
||||
creates: the day the backend starts returning a type the frontend never
|
||||
enumerated (e.g. `MusicVideo`, or Jellyfin renaming a kind), search silently
|
||||
drops it from both the query filter and the result buckets, and nothing in the
|
||||
Rust layer — the actual authority on Jellyfin's API — can correct it. Two
|
||||
sources of truth that will drift.
|
||||
|
||||
**This must be fixed while the feature is uncommitted**, before the leak ships
|
||||
baked into a released wire contract.
|
||||
|
||||
### What is *not* a leak (leave it alone)
|
||||
|
||||
Single concrete-type list pages are **not** business logic and stay as-is:
|
||||
|
||||
- `music.ts` → `["MusicAlbum"]` / `["Playlist"]`, `movies.ts` → `["Movie"]`,
|
||||
`tv.ts` → `["Series"]`
|
||||
- `GenericMediaListPage.svelte` → `[config.itemType]`
|
||||
- `ArtistDetailView`, `RelatedItemsSection`, `AddToPlaylistModal`,
|
||||
`PersonDetailView`
|
||||
|
||||
"This page shows albums" is a legitimate presentation choice expressed through a
|
||||
generic `getItems(parentId, { includeItemTypes })` API. Only the **search scope
|
||||
taxonomy** (a semantic category → many types, defined once and reused) crosses
|
||||
the line. Do **not** invent a backend enum for every list page — that is
|
||||
over-abstraction, not cleaner separation.
|
||||
|
||||
## The boundary rule after this change
|
||||
|
||||
> The frontend never names a Jellyfin item type **in connection with search.**
|
||||
> It sends an opaque `scope`, and receives results already sorted into labelled
|
||||
> groups. The frontend owns only **group order** (presentation) and
|
||||
> **rendering**.
|
||||
|
||||
## Design
|
||||
|
||||
### Rust owns scope → item-types (query side)
|
||||
|
||||
Add an opaque enum that crosses IPC, and move the expansion table into Rust:
|
||||
|
||||
```rust
|
||||
// repository/types.rs
|
||||
#[derive(specta::Type, Serialize, Deserialize, Clone, Copy, Debug)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub enum SearchScope { All, Music, Movies, Tv }
|
||||
|
||||
impl SearchScope {
|
||||
/// The Jellyfin item types this scope requests, or None for `All`
|
||||
/// (which must send NO includeItemTypes — see below).
|
||||
pub fn item_types(self) -> Option<Vec<String>> {
|
||||
match self {
|
||||
SearchScope::All => None,
|
||||
SearchScope::Music => Some(vec!["MusicAlbum", "MusicArtist", "Audio", "Playlist"]
|
||||
.into_iter().map(String::from).collect()),
|
||||
SearchScope::Movies => Some(vec!["Movie".into()]),
|
||||
SearchScope::Tv => Some(vec!["Series".into(), "Episode".into()]),
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`SearchOptions` gains `scope` and the search command resolves it into the
|
||||
existing `include_item_types` filter **inside Rust**, before dispatching to the
|
||||
online/offline paths (which already honour `include_item_types` — do not touch
|
||||
their filtering, per [scoped-search.md](scoped-search.md) §Background 2).
|
||||
|
||||
```rust
|
||||
pub struct SearchOptions {
|
||||
pub limit: Option<usize>,
|
||||
pub search_term: Option<String>,
|
||||
pub scope: Option<SearchScope>, // NEW
|
||||
// include_item_types stays for the single-type list-page callers,
|
||||
// but the SEARCH command derives it from `scope` when scope is set.
|
||||
}
|
||||
```
|
||||
|
||||
**Precedence:** if `scope` is set it wins; `include_item_types` remains for the
|
||||
non-search `getItems` callers. Document this so a future reader does not send
|
||||
both.
|
||||
|
||||
**`All` sends no filter.** Preserve the existing invariant: `All` must omit
|
||||
`includeItemTypes` entirely, not send the union of every enumerated type — types
|
||||
nobody listed (Person, folders) would otherwise be filtered out. This is why
|
||||
`item_types()` returns `Option`, and the command must skip the filter on `None`.
|
||||
|
||||
### Rust owns result bucketing (result side)
|
||||
|
||||
Results arrive **pre-grouped**. Rust classifies each returned `MediaItem` into a
|
||||
group by its type — the `GROUP_ITEM_TYPES` knowledge, moved to the authority:
|
||||
|
||||
```rust
|
||||
#[derive(specta::Type, Serialize, Deserialize, Clone, Copy, Debug)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub enum SearchGroupId { Songs, Albums, Artists, Movies, TvShows }
|
||||
|
||||
#[derive(specta::Type, Serialize, Deserialize, Clone, Debug)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct SearchGroup { pub id: SearchGroupId, pub items: Vec<MediaItem> }
|
||||
|
||||
#[derive(specta::Type, Serialize, Deserialize, Clone, Debug)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct GroupedSearchResult { pub groups: Vec<SearchGroup> }
|
||||
```
|
||||
|
||||
Rust emits **every** non-empty group it can classify, in a stable canonical
|
||||
order. It does **not** apply the user's ordering or drop out-of-scope groups —
|
||||
those are presentation and stay frontend-side (see below). Items whose type maps
|
||||
to no group are omitted from grouped output (same as today's frontend filter).
|
||||
|
||||
### 🔴 The `search-event` wrinkle — both payloads must change
|
||||
|
||||
Search returns results **twice**: the command resolves with instant local-cache
|
||||
results, then the merged cache+server union arrives later via the `search-event`
|
||||
listener (see [library.ts](../../src/lib/stores/library.ts) `search()` and
|
||||
[architecture/03-data-flow.md](../architecture/03-data-flow.md)). **Both** the
|
||||
command return value **and** the `search-event` payload must carry
|
||||
`GroupedSearchResult`. If only one is converted, the instant results group and
|
||||
the merged ones do not (or vice versa), and the UI flickers between shapes. This
|
||||
is the single largest part of the change and the easiest to half-do.
|
||||
|
||||
### What the frontend keeps (all pure presentation)
|
||||
|
||||
[searchScope.ts](../../src/lib/utils/searchScope.ts) **retains**:
|
||||
|
||||
- `SearchScope` type — now sourced from the generated bindings, mirroring the
|
||||
Rust enum (delete the hand-written union).
|
||||
- `SCOPE_LABELS`, `SEARCH_SCOPES` (chip labels / order).
|
||||
- `resolveSearchScope(pathname)` — route → initial scope. Pure, DOM-free,
|
||||
unit-tested. **Stays exactly as-is.**
|
||||
- `SearchGroupId` (from bindings), `GROUP_LABELS`.
|
||||
- `normalizeGroupOrder`, `groupsForScope`, `moveGroup`, `reorderGroups`,
|
||||
`DEFAULT_GROUP_ORDER` — group-order persistence and reordering, all
|
||||
presentation.
|
||||
|
||||
[searchScope.ts](../../src/lib/utils/searchScope.ts) **loses**:
|
||||
|
||||
- `SCOPE_ITEM_TYPES`, `GROUP_ITEM_TYPES` (moved to Rust).
|
||||
- `scopeItemTypes()`, `groupItemTypes()`.
|
||||
- The `.type`-inspecting body of `composeSearchGroups()`.
|
||||
|
||||
`composeSearchGroups()` shrinks to a **presentation composition over Rust's
|
||||
groups** — no `.type` inspection anywhere:
|
||||
|
||||
```ts
|
||||
// Take Rust's pre-bucketed groups; drop out-of-scope, sort by saved order,
|
||||
// attach labels, omit empties. No Jellyfin type vocabulary.
|
||||
composeSearchGroups(groups: SearchGroup[], scope, order): DisplayGroup[]
|
||||
```
|
||||
|
||||
`GROUP_SCOPE` (which group belongs to which scope) is a borderline case: it is
|
||||
"is Songs part of the Music scope," arguably taxonomy. But because Rust already
|
||||
filtered the query by scope, out-of-scope groups will simply be **empty** and
|
||||
drop out via the empty-omit rule — so the frontend does not strictly need
|
||||
`GROUP_SCOPE` for correctness once Rust filters. **Recommendation:** delete
|
||||
`GROUP_SCOPE` and rely on empty-omission; if kept for belt-and-suspenders, treat
|
||||
it as a display hint, not authority.
|
||||
|
||||
### Frontend call-site changes
|
||||
|
||||
- [library.ts](../../src/lib/stores/library.ts) `search(query, scope)` sends
|
||||
`{ scope }` in `SearchOptions` instead of computing `includeItemTypes`.
|
||||
Everything else (requestId bump, stale guard, 10s timeout, empty-query clear,
|
||||
event merge) is preserved.
|
||||
- [SearchResults.svelte](../../src/lib/components/search/SearchResults.svelte)
|
||||
consumes `SearchGroup[]` from the store instead of a flat `MediaItem[]` +
|
||||
client-side `composeSearchGroups(results, …)`. The store now holds grouped
|
||||
results.
|
||||
- [search/+page.svelte](../../src/routes/search/+page.svelte) is unchanged in
|
||||
behaviour; only the type it passes to `SearchResults` changes.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Any change to online/offline `include_item_types` **filtering** — it already
|
||||
works; only the *source* of the type list moves.
|
||||
- Single concrete-type list pages (see "What is not a leak").
|
||||
- Ranking within or across groups.
|
||||
- The UX / chip behaviour / persistence mechanism — all unchanged from
|
||||
[scoped-search.md](scoped-search.md).
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] No Jellyfin item-type string literal (`"MusicAlbum"`, `"Audio"`, …) remains
|
||||
in `searchScope.ts` or any search call path. Verify:
|
||||
`grep -rn '"MusicAlbum"\|"MusicArtist"\|"Audio"\|"Series"\|"Episode"\|"Movie"\|"Playlist"' src/lib/utils/searchScope.ts src/lib/stores/library.ts` returns nothing.
|
||||
- [ ] `SearchScope` and `SearchGroupId` in the frontend come from the generated
|
||||
`bindings.ts`, not hand-written unions.
|
||||
- [ ] Search behaviour is **identical** to today for the user: same scoping, same
|
||||
groups, same order, same empty/out-of-scope omission, offline included.
|
||||
- [ ] Both the command return and the `search-event` payload carry the grouped
|
||||
shape; no shape flicker between instant and merged results.
|
||||
- [ ] `All` scope still sends no `includeItemTypes` (assert in a Rust test).
|
||||
- [ ] Adding a hypothetical new type to a scope requires editing **only** Rust.
|
||||
- [ ] `cargo fmt` clean, `cargo clippy` clean, `bun run test:rust` passes.
|
||||
- [ ] `bun run check` and `bun run test` pass; `bindings.ts` regenerated and
|
||||
committed.
|
||||
|
||||
## Testing
|
||||
|
||||
**Rust** (`src-tauri`, `cargo test`):
|
||||
- `SearchScope::item_types()`: each scope's list, and `All` → `None`.
|
||||
- Search command: `scope: Music` resolves to the four music types on the query;
|
||||
`scope: All` sends no `include_item_types`.
|
||||
- Bucketing: a mixed `Vec<MediaItem>` classifies into the right `SearchGroupId`s;
|
||||
unknown types are dropped; groups come out in canonical order.
|
||||
- The `search-event` payload is the grouped shape (guard the wrinkle).
|
||||
|
||||
**Frontend** (vitest, `src/lib/**/*.test.ts`) — update existing tests:
|
||||
- `librarySearchScope.test.ts` currently asserts `includeItemTypes` on the
|
||||
outgoing options — **rewrite** to assert `scope` is sent instead.
|
||||
- `searchScope.test.ts` — drop `scopeItemTypes`/`groupItemTypes` cases; keep and
|
||||
extend `resolveSearchScope`, order normalize/move/reorder, and the new
|
||||
compose-over-groups (order + empty-omit, no type inspection).
|
||||
- `searchGroupOrder.test.ts` — unchanged.
|
||||
|
||||
## TRACES
|
||||
|
||||
Per [CLAUDE.md](../../CLAUDE.md), tag requirement-implementing code:
|
||||
- `SearchScope` enum + `item_types()` + search command scope resolution:
|
||||
`UR-049 | DR-063` (revised — resolution now Rust-side).
|
||||
- Grouped result shape + bucketing: `UR-050 | DR-067` (revised) + a new DR for
|
||||
the wire shape.
|
||||
- `library.ts` store change: `UR-049 | DR-065` (revised — sends scope not types).
|
||||
|
||||
## Notes for the implementer
|
||||
|
||||
- This spec **revises** [scoped-search.md](scoped-search.md) §Background 2 and
|
||||
§Design "Scope model / Threading scope through the store," which asserted no
|
||||
Rust change. Update that spec's status to note the boundary was moved, or add a
|
||||
banner pointing here — do not leave the two specs contradicting silently.
|
||||
- The IPC camelCase rule applies to the new enums and structs
|
||||
([CLAUDE.md](../../CLAUDE.md)): `#[serde(rename_all = "camelCase")]` on structs;
|
||||
the tagged-enum tag convention if any enum becomes tagged. Add/extend a
|
||||
`tauriIntegration`-style test if a new command is introduced.
|
||||
- Regenerate `bindings.ts` via the tauri-specta build step after changing Rust
|
||||
types; do not hand-edit it.
|
||||
- **Another Claude session may be active in these same files** (per project
|
||||
memory). `git diff` before repairing anything unexpected; these search files
|
||||
are exactly the ones a parallel session touched.
|
||||
@@ -0,0 +1,202 @@
|
||||
# Spec: Context-scoped search with filter chips and configurable group order
|
||||
|
||||
> ⚠️ **Superseded in part by
|
||||
> [scoped-search-boundary.md](scoped-search-boundary.md).** The "frontend only,
|
||||
> no Rust changes" decision below (§Background 2, §Design "Scope model" and
|
||||
> "Threading scope through the store") left Jellyfin's item-type taxonomy in the
|
||||
> presentation layer, which violates the backend/frontend boundary. The taxonomy
|
||||
> is being moved into Rust. The **user-facing behaviour and UX in this spec are
|
||||
> unchanged**; only where the scope→item-type mapping and result bucketing live
|
||||
> changes. Read the boundary spec before touching search code.
|
||||
|
||||
**Status:** Implemented (boundary revision pending — see banner above)
|
||||
**Scope:** Frontend only. No Rust changes required. *(Revised — see banner.)*
|
||||
**Requirements:** UR-049 → DR-063, DR-064, DR-065; UR-050 → DR-066, DR-067
|
||||
(see [requirements.md](../requirements.md)).
|
||||
**UX spec:** [ux-flows.md §6](../ux-flows.md) — §6.1 scope, §6.2 layout,
|
||||
§6.3 group order, §6.4 current deviations.
|
||||
|
||||
## Summary
|
||||
|
||||
Two related changes to search:
|
||||
|
||||
1. **Scope** — a search started inside a library searches *that* library.
|
||||
Started from Home, `/library`, or the search tab, it searches everything.
|
||||
The active scope shows as a chip row under the search bar, preselected from
|
||||
context and freely changeable without retyping.
|
||||
2. **Group order** — the order result groups appear in (Songs, Albums, Artists,
|
||||
Movies, TV Shows) becomes a drag-and-drop setting instead of being hardcoded.
|
||||
|
||||
## Motivation
|
||||
|
||||
Searching "office" while browsing TV currently returns music albums, because
|
||||
both search entry points call the same unscoped query. The user has already
|
||||
told us what they're looking at; ignoring that makes search feel indiscriminate
|
||||
and pushes the relevant result below unrelated media.
|
||||
|
||||
## Background: what already exists
|
||||
|
||||
Verified in code — **most of the plumbing is already there.** This is
|
||||
substantially a wiring task, not new infrastructure.
|
||||
|
||||
1. **`SearchOptions` already carries the filter.**
|
||||
[bindings.ts](../../src/lib/api/bindings.ts) —
|
||||
`SearchOptions = { limit?, includeItemTypes?, searchTerm? }`.
|
||||
|
||||
2. **Rust already honours `include_item_types` on both paths** — online
|
||||
([online.rs](../../src-tauri/src/repository/online.rs), in the `get_items`
|
||||
options mapping) and offline
|
||||
([offline.rs](../../src-tauri/src/repository/offline.rs), which builds a SQL
|
||||
type filter from it). **Do not add Rust code for filtering.**
|
||||
|
||||
3. **Per-page list search already does this correctly.**
|
||||
[GenericMediaListPage.svelte](../../src/lib/components/library/GenericMediaListPage.svelte)
|
||||
passes `includeItemTypes: [config.itemType]` to `repo.search(...)`. Use it as
|
||||
the reference for the call shape, including the `requestId` handling.
|
||||
|
||||
4. **The gap is exactly one function.**
|
||||
[library.ts](../../src/lib/stores/library.ts) — `search(query)` takes only a
|
||||
query and calls `repo.search(query, { limit: 10000 }, requestId)`, dropping
|
||||
any scope. Both callers
|
||||
([search/+page.svelte](../../src/routes/search/+page.svelte) and
|
||||
[library/+layout.svelte](../../src/routes/library/+layout.svelte)) go through
|
||||
it.
|
||||
|
||||
5. **Group order is hardcoded in markup.**
|
||||
[SearchResults.svelte](../../src/lib/components/search/SearchResults.svelte)
|
||||
categorizes into `music{tracks,albums,artists} / movies / tvShows` and
|
||||
renders three fixed sections in source order.
|
||||
|
||||
6. **Frontend preferences persist via `localStorage`**, per the existing
|
||||
`viewMode` precedent in [library.ts](../../src/lib/stores/library.ts)
|
||||
(`jellytau-view-mode`). Follow that pattern — **do not** add a Rust settings
|
||||
command for this.
|
||||
|
||||
## Design
|
||||
|
||||
### Scope model
|
||||
|
||||
One `SearchScope` type, defined once and shared:
|
||||
|
||||
| Scope | `includeItemTypes` | Chip label |
|
||||
|-------|--------------------|------------|
|
||||
| `all` | *unset* | All |
|
||||
| `music` | `MusicAlbum`, `MusicArtist`, `Audio`, `Playlist` | Music |
|
||||
| `movies` | `Movie` | Movies |
|
||||
| `tv` | `Series`, `Episode` | TV |
|
||||
|
||||
`all` must send **no** `includeItemTypes` key rather than a list of every type —
|
||||
the two are not equivalent for item types not enumerated here (Person, folders).
|
||||
|
||||
### Route → scope resolution (DR-063)
|
||||
|
||||
A pure function, unit-testable without a DOM:
|
||||
|
||||
```ts
|
||||
resolveSearchScope(pathname: string): SearchScope
|
||||
```
|
||||
|
||||
- `/library/music*` → `music`
|
||||
- `/library/movies*` → `movies`
|
||||
- `/library/tv*` → `tv`
|
||||
- `/`, `/library`, `/search`, anything else → `all`
|
||||
|
||||
Note `/library/shows/genres` exists as a route; treat `shows` as `tv`. Check the
|
||||
current route list before finalising — do not assume this table is exhaustive.
|
||||
|
||||
### Scope is a starting point, not a lock (DR-064)
|
||||
|
||||
The resolved scope sets the **initial** chip only. Once the user taps a chip,
|
||||
their choice governs until they leave the search surface. Concretely: derive the
|
||||
initial value from the route, hold it in component state, and do not re-derive
|
||||
it on every navigation — otherwise a user who widens to All snaps back to TV.
|
||||
|
||||
Changing a chip re-runs the current query at the new scope. Changing the query
|
||||
keeps the current scope.
|
||||
|
||||
### Threading scope through the store (DR-065)
|
||||
|
||||
Extend the store's search signature to accept an optional scope and pass
|
||||
`includeItemTypes` down to `repo.search`. Preserve the existing behaviour
|
||||
exactly: the `requestId` bump, the stale-response guard, the `search-event`
|
||||
listener merge, the 10s timeout, and the empty-query clear path. This is an
|
||||
additive parameter — no caller should break.
|
||||
|
||||
### Group order (DR-066, DR-067)
|
||||
|
||||
Persist an ordered array of group ids:
|
||||
|
||||
```
|
||||
["songs", "albums", "artists", "movies", "tvShows"] // shipped default
|
||||
```
|
||||
|
||||
Rendering composes scope and order as **two independent axes**, in this order:
|
||||
|
||||
1. drop groups outside the active scope,
|
||||
2. sort the remainder by the user's saved order,
|
||||
3. omit groups that came back empty.
|
||||
|
||||
Scope never rewrites the saved order — narrowing to Music and back to All must
|
||||
restore the user's full arrangement. See [ux-flows.md §6.3](../ux-flows.md) for
|
||||
the worked example.
|
||||
|
||||
Settings gets a reorderable list. **Dragging alone is not sufficient**: provide
|
||||
keyboard-operable move up/down controls with proper labels, or the setting is
|
||||
unusable with a screen reader and on any pointerless input.
|
||||
|
||||
Unknown or missing ids in the stored array must not crash rendering — treat the
|
||||
stored order as a hint, append any group it doesn't mention, and ignore ids that
|
||||
no longer exist. A user upgrading from a build with fewer groups must not lose
|
||||
the new ones.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Ranking *within* a group. Order is presentation-only.
|
||||
- Server-side search ranking or the Jellyfin query itself.
|
||||
- Scope chips on the per-page list search in `GenericMediaListPage` — that page
|
||||
is already implicitly scoped by its own `itemType`.
|
||||
- Any Rust change.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] Searching from inside Music returns no movies or TV; from inside TV, no music.
|
||||
- [ ] Searching from Home, `/library`, or the search tab returns all types.
|
||||
- [ ] The chip row renders under the search bar on both the search page and the
|
||||
in-library header search, with the context-derived chip preselected.
|
||||
- [ ] Tapping a chip re-runs the search with the query preserved; editing the
|
||||
query preserves the selected chip.
|
||||
- [ ] Tapping "All" from a context-scoped search widens results without retyping.
|
||||
- [ ] Result groups render in the user's configured order, with out-of-scope and
|
||||
empty groups omitted and relative order preserved.
|
||||
- [ ] Group order is reorderable by drag **and** by keyboard, persists across
|
||||
restarts, and ships with the documented default.
|
||||
- [ ] Offline search respects scope (the offline path already filters — verify,
|
||||
don't reimplement).
|
||||
- [ ] `bun run check` and `bun run test` pass.
|
||||
|
||||
## Testing
|
||||
|
||||
Follow the existing frontend test conventions (vitest, `src/lib/**/*.test.ts`).
|
||||
|
||||
- `resolveSearchScope` — pure unit tests over the route table, including the
|
||||
`/library/shows/genres` case and unknown routes falling back to `all`.
|
||||
- Scope → `includeItemTypes` mapping, asserting `all` omits the key entirely.
|
||||
- The compose step: scope filter + user order + empty-group omission, including
|
||||
the "narrow then widen restores order" case and a stored order containing an
|
||||
unknown id.
|
||||
- Store-level: scoped search forwards `includeItemTypes` to the repository, and
|
||||
the existing stale-`requestId` guard still discards superseded responses.
|
||||
|
||||
New requirement-implementing code needs `TRACES:` comments — see
|
||||
[CLAUDE.md](../../CLAUDE.md). Suggested tags: the scope resolver and chip row
|
||||
`UR-049 | DR-063, DR-064`, the store change `UR-049 | DR-065`, the settings list
|
||||
and ordered rendering `UR-050 | DR-066, DR-067`.
|
||||
|
||||
## Notes for the implementer
|
||||
|
||||
- Read [ux-flows.md §6](../ux-flows.md) first — it is the behavioural spec; this
|
||||
document is the implementation plan.
|
||||
- The IPC camelCase rule applies to anything new that crosses the boundary
|
||||
([CLAUDE.md](../../CLAUDE.md)) — though this change should not add commands.
|
||||
- Another session may be active in this repo. Check `git diff` before
|
||||
"repairing" unexpected changes.
|
||||
Reference in New Issue
Block a user