feat(settings): rework settings page; remove unused SkeletonLoader/StorageManagement

Settings page refactor plus supporting docs (requirements, ux-flows, traceability) and the frontend-domain-model spec with implementation-status banner. Removes SkeletonLoader and StorageManagement components (no remaining references).
2026-07-23 22:18:37 +02:00
parent f89b241ad6
commit 9b1c9b3c91
7 changed files with 817 additions and 738 deletions
@@ -0,0 +1,309 @@
+# Spec: Remove Jellyfin-specific models from the frontend
+
+> **Implementation status (branch `frontend-domain-model`, worktree
+> `../JellyTau-domain-model`):** Catalog surface **done**. The frontend's *item
+> classification* and *time units* no longer speak Jellyfin:
+> - `domain/` module is the single source of truth; `MediaKind` enum + isolated
+>   `from_jellyfin` mapping. The model gained real distinctions the flat
+>   `item_type` had hidden: `LiveChannel` / `ChannelItem` / `Channel`.
+> - Every catalog `item.type === "..."` → `item.kind` (0 remaining in `src/`).
+> - Catalog ticks → milliseconds (`durationMs`, `playbackPositionMs`);
+>   `formatDuration` takes ms; progress bars are unit-consistent.
+> - User-facing type badge → `kindLabel()`.
+> - Old Jellyfin-named fields remain **dual-carried** on the wire so nothing broke.
+>
+> **Deferred (tracked, not done):**
+> - `primaryImageTag` → `imageId` rename (naming-only; ~40 sites across catalog +
+>   `PlayerMediaItem`/`MergedMediaItem`, the latter needing a Rust `image_id`
+>   round-trip). Catalog `MediaItem` already has `imageId`.
+> - Player/session/reporting tick math (`Queue`, `SessionCard`, `RemoteControls`,
+>   `playbackReporting`, `playerEvents`) — crosses storage/Jellyfin *command
+>   signatures* in ticks; needs those commands to accept ms (phase 4).
+> - `stream.type` (`mediaStreams[].type`) — Jellyfin stream vocabulary (phase 4).
+> - Delete `playbackUnits.ts` / `jellyfinFieldMapping.ts` once their last
+>   consumers migrate; drop the dual-carried fields once nothing reads them.
+
+**Status:** Partially implemented (catalog surface); see banner.
+**Requirements:** Architectural (boundary integrity — CLAUDE.md core principles).
+Allocate new DRs on acceptance; suggested: DR for the domain `MediaItem`/`MediaKind`
+type, DR for tick/image-tag hoisting, DR for the phased frontend migration
+(see [requirements.md](../requirements.md)). Relates to UR-007, UR-008, UR-034.
+**UX spec:** n/a — zero user-visible behaviour change. This is a pure
+architecture/boundary migration.
+**Supersedes / revises:** none. Extends the boundary work started in
+[scoped-search-boundary.md](scoped-search-boundary.md) from *taxonomy* to the
+*whole media model*.
+
+## Summary
+
+The frontend currently consumes Jellyfin's data model directly: `MediaItem` is a
+Jellyfin DTO (`runTimeTicks`, `primaryImageTag`, `parentIndexNumber`, a
+stringly-typed `type: string` carrying Jellyfin's item vocabulary), mirrored via
+specta into **36+ frontend files**, with **127 `item.type === "…"` string
+comparisons across 23 files** and two frontend utility modules
+(`playbackUnits.ts`, `jellyfinFieldMapping.ts`) doing Jellyfin-specific unit and
+field conversion in the presentation layer.
+
+This spec defines a **provider-neutral domain model**, owned by Rust, that the
+Jellyfin repository maps *into*. The frontend consumes only that model. When done,
+no Jellyfin vocabulary — item-type strings, ticks, image tags, Jellyfin field
+names — remains in `src/`.
+
+## Motivation
+
+Two concrete problems, one strategic:
+
+1. **Boundary violation at scale.** Per CLAUDE.md, the frontend is
+   presentation-only and Rust owns the domain. Today the *domain model itself* is
+   Jellyfin's wire shape, propagated unchanged across IPC. The frontend knows what
+   a "tick" is, what `primaryImageTag` means, and that `"Audio"` is a track. That
+   is domain knowledge in the wrong layer, 36 files deep.
+2. **Fragility.** `type: string` is unchecked: a typo (`"Epis0de"`) or a Jellyfin
+   rename fails silently at runtime with no compiler help, across 127 sites. Tick
+   math (`* 10_000_000`) duplicated frontend-side is a class of bug the backend
+   should have already resolved.
+3. **Strategic (the reason we chose the ambitious target):** a neutral domain
+   model is the precondition for **ever supporting a non-Jellyfin backend** (Plex,
+   local files, Subsonic). As long as the UI speaks Jellyfin, that door is welded
+   shut.
+
+## Layer assignment
+
+| Logic / responsibility | Layer | Why it belongs there |
+|------------------------|-------|----------------------|
+| Definition of the media domain model (`MediaItem`, `MediaKind`) | **Rust** | The canonical shape the whole app reasons about; must not be a provider's wire format. |
+| Jellyfin DTO → domain mapping (ticks→ms, image tag→url/id, `"Audio"`→`Track`, `PremiereDate`→`releaseDate`) | **Rust**, in the Jellyfin repository | Provider-specific translation; changes if Jellyfin changes; is the definition of "how Jellyfin maps to our domain." |
+| Tick arithmetic (`playbackUnits.ts`) | **Rust** | A Jellyfin unit. The frontend should never see ticks; it receives `durationMs`/`positionMs`. |
+| Sort-field mapping (`jellyfinFieldMapping.ts`, `title→SortName`) | **Rust** | Maps neutral sort keys to Jellyfin query fields — provider vocabulary. Frontend sends a neutral `SortKey`. |
+| `MediaKind` classification (is this a track / album / episode?) | **Rust** | Derived from Jellyfin's `item_type`; the frontend receives the already-classified kind. |
+| Choosing which kind renders as a card vs a list row; grid/list toggle; group order | **Frontend** | Pure presentation over the neutral `kind`. Changes only if the UI is redesigned. |
+| Navigation decisions (`kind === Track && albumId` → go to album) | **Frontend** | Presentation/routing over neutral fields. |
+
+**Borderline calls, resolved:**
+
+- *`MergedMediaItem`* (the lightweight now-playing projection) is already
+  half-neutral (`title`, `artist`, `duration`) — it becomes a straightforward
+  subset of the new domain model, not a special case.
+- *Context discriminators* `"album"`, `"playlist"`, `"remote"` (in `TrackList`,
+  playback context, sessions) are **already domain-neutral** — they are *our*
+  vocabulary, not Jellyfin's. They stay as-is; do not confuse them with
+  `item_type`. Only the Jellyfin item-type strings move.
+- *`mediaStreams[].type === "Audio"/"Subtitle"/"Video"`* (track selection in
+  VideoPlayer) is Jellyfin stream vocabulary too, but is lower-risk and
+  self-contained — deferred to a late phase, not phase 1.
+
+## Design
+
+### Single canonical model, one location, isolated mappings
+
+The domain model is defined **once**, in a dedicated top-level Rust module
+`src-tauri/src/domain/`, and is the single source of truth shared across the
+whole app:
+
+```
+src-tauri/src/domain/
+  media.rs         canonical MediaItem, MediaKind, and the other media types
+  from_jellyfin.rs Jellyfin DTO -> domain mapping, ISOLATED here
+  mod.rs           re-exports
+        |  tauri-specta (export_typescript_bindings test)
+        v
+  src/lib/api/bindings.ts   generated MediaItem/MediaKind — the frontend copy
+```
+
+- **One definition.** `domain::MediaItem` is *the* model. Rust (repositories,
+  player, downloads) uses it directly. The frontend uses the generated `bindings.ts`
+  projection of it. There is no second hand-written copy in either language, so it
+  cannot drift — "shared between frontend and backend" is realized by generation,
+  not duplication.
+- **Mappings live beside the model, never in consumers.** All provider translation
+  (`JellyfinItem` → `domain::MediaItem`, ticks→ms, image-tag→id, item-type→`MediaKind`)
+  lives in `domain/from_jellyfin.rs`. It is the *only* place Jellyfin vocabulary
+  touches the domain type. Adding a second provider later means a new
+  `from_<provider>.rs` beside it — the model and every consumer stay untouched.
+- **`domain` is a top-level module** (not under `repository/`) because `MediaItem`
+  is used by `player/`, `download/`, and `playback_mode/` too — it is not
+  repository-specific.
+- The existing `JellyfinItem` DTO + `to_media_item()` in
+  [online.rs](../../src-tauri/src/repository/online.rs) is the seam that already
+  exists; it **moves** into `domain/from_jellyfin.rs` and is enriched to do real
+  translation instead of copying `item_type` through.
+
+### The domain model (Rust)
+
+```rust
+// src-tauri/src/domain/media.rs — provider-neutral. NO Jellyfin vocabulary.
+#[derive(specta::Type, Serialize, Deserialize, Clone, Copy, Debug, PartialEq, Eq)]
+#[serde(rename_all = "camelCase")]
+pub enum MediaKind {
+    Track, Album, Artist, Playlist,          // music
+    Movie, Series, Season, Episode,          // video
+    Person,                                  // cast/crew
+    Channel, Folder,                         // containers/live
+}
+
+#[derive(specta::Type, Serialize, Deserialize, Clone, Debug)]
+#[serde(rename_all = "camelCase")]
+pub struct MediaItem {
+    pub id: String,
+    pub name: String,
+    pub kind: MediaKind,          // was: type: String
+    pub is_folder: bool,
+    pub server_id: String,
+
+    // Times in milliseconds — NEVER ticks.
+    pub duration_ms: Option<i64>,        // was: run_time_ticks
+
+    // Image as a resolved identifier the frontend turns into a URL via the
+    // existing image command — no raw Jellyfin tag semantics leak.
+    pub image_id: Option<String>,        // was: primary_image_tag
+    pub backdrop_image_ids: Option<Vec<String>>,
+
+    pub overview: Option<String>,
+    pub genres: Option<Vec<String>>,
+    pub production_year: Option<i32>,
+    pub release_date: Option<String>,    // was: premiere_date (ISO-8601)
+    pub community_rating: Option<f64>,
+    pub official_rating: Option<String>,
+
+    // Relationships — already neutral, kept.
+    pub album_id: Option<String>, pub album_name: Option<String>,
+    pub album_artist: Option<String>, pub artists: Option<Vec<String>>,
+    pub artist_items: Option<Vec<ArtistItem>>,
+    pub series_id: Option<String>, pub series_name: Option<String>,
+    pub season_id: Option<String>, pub season_name: Option<String>,
+
+    // Ordinal position — rename off Jellyfin's index vocabulary.
+    pub track_number: Option<i32>,       // was: index_number
+    pub disc_number: Option<i32>,        // was: parent_index_number
+
+    pub user_data: Option<UserData>,
+    pub media_streams: Option<Vec<MediaStream>>,
+    pub media_sources: Option<Vec<MediaSource>>,
+    pub people: Option<Vec<Person>>,
+}
+```
+
+The existing `JellyfinItem` DTO (already defined in `online.rs`, deserialized
+from the Jellyfin JSON) **moves into `domain/from_jellyfin.rs`** and stays
+private to that module. Its `to_media_item()` — today a near-passthrough that
+copies `item_type` straight across — is enriched into the single, tested place
+that:
+
+- classifies `item_type: String` → `MediaKind` (including the edge cases found in
+  the audit: `"ChannelFolderItem"` → `Channel`/`Folder` by `is_folder`,
+  `"TvChannel"` → `Channel`, `"Composer"/"Director"/"Writer"` → `Person`,
+  `"Video"` → `Movie` or a video leaf). Unknown strings map to `Folder` or a new
+  `Other` variant — **decide at implementation; must not panic.**
+- converts `run_time_ticks` → `duration_ms` (`ticks / 10_000`).
+- maps `PremiereDate` → `release_date`, image tags → image ids.
+
+`SortKey` enum + its Jellyfin field mapping (`jellyfinFieldMapping.ts` contents)
+moves into the Jellyfin repository; the command takes a neutral `SortKey`.
+
+### 🔴 The `search-event` / dual-payload rule applies again
+
+Every path that returns `MediaItem` — command returns **and** the `search-event`
+and any other event payloads — emits the new domain shape. Both sides of a
+twice-delivered result must match (same rule as
+[scoped-search-boundary.md](scoped-search-boundary.md)). Grep for `MediaItem` in
+event definitions before declaring a phase done.
+
+### Frontend after
+
+- `MediaItem`/`MediaKind` come from generated `bindings.ts`.
+- `item.type === "Audio"` → `item.kind === "track"` (127 sites, mechanical).
+- `runTimeTicks` usages → `durationMs`; **delete `playbackUnits.ts`** (ticks no
+  longer cross the boundary; keep only any purely-display seconds↔clock helpers if
+  they exist, which are not Jellyfin-specific).
+- `primaryImageTag` → `imageId` through the existing image-URL command.
+- **Delete `jellyfinFieldMapping.ts`**; sort options send a neutral `SortKey`.
+- Assert with the boundary tripwire + a new grep (see acceptance).
+
+## Phased migration
+
+This is too large and too collision-prone for one change. Phases are independently
+shippable, each keeps all tests green, and each is a reviewable PR:
+
+1. **Establish the `domain/` module + enriched mapping, tests — no frontend
+   change yet.** Create `src-tauri/src/domain/{media,from_jellyfin,mod}.rs`. Move
+   `JellyfinItem`/`to_media_item` in. Add `MediaKind` and the neutral fields to
+   `domain::MediaItem` as *additive, defaulted* fields, and populate them in the
+   mapping, while **keeping the old Jellyfin-named fields too** (dual-carry). The
+   wire shape is a superset of today's, so the frontend still compiles and
+   behaves identically. Lands the authority + full mapping unit coverage first,
+   with zero blast radius on the 52 construction sites (they set the old fields;
+   new ones default).
+2. **Flip the wire shape.** Commands + events emit the new `MediaItem`.
+   Regenerate `bindings.ts`. Frontend breaks to compile errors — fix them
+   mechanically (`type`→`kind`, values `"Audio"`→`"track"`, `runTimeTicks`→
+   `durationMs`, `primaryImageTag`→`imageId`). This is the big mechanical PR;
+   `bun run check` is the driver.
+3. **Delete the frontend conversion helpers** (`playbackUnits.ts` ticks,
+   `jellyfinFieldMapping.ts`) and route sorting through the neutral `SortKey`.
+4. **Stream vocabulary** (`mediaStreams[].type`) and any remaining stragglers;
+   tighten the boundary check to forbid Jellyfin item-type strings in `src/`
+   outside tests.
+
+Ship 1 → 2 → 3 → 4 as separate PRs. Do **not** attempt all four at once.
+
+## Out of scope
+
+- Actually adding a second backend (Plex/Subsonic). This spec only *unblocks* it.
+- Changing any user-visible behaviour, layout, or copy.
+- The player-internal `PlayerMediaItem` / `MediaSessionType` shapes, except where
+  they carry the fields being renamed — align them in phase 2 only if the compiler
+  demands it.
+- Context discriminators (`"album"`, `"playlist"`, `"remote"`) — already neutral.
+
+## Acceptance criteria
+
+- [ ] No Jellyfin item-type string (`"Audio"`, `"MusicAlbum"`, `"Series"`, …) is
+      compared against `.type`/`.kind` anywhere in `src/` (outside tests). Verify:
+      `grep -rIn '\.kind === "\(Audio\|MusicAlbum\|MusicArtist\|Series\|Episode\|Movie\|Playlist\)"' src/` returns nothing.
+- [ ] No `Ticks`, `runTimeTicks`, `primaryImageTag`, `PremiereDate`, or Jellyfin
+      sort-field name (`SortName`, `RunTimeTicks`, …) appears in `src/` outside
+      tests. `playbackUnits.ts` (ticks) and `jellyfinFieldMapping.ts` are deleted.
+- [ ] `MediaItem`/`MediaKind`/`SortKey` in the frontend come from `bindings.ts`.
+- [ ] The `From<JellyfinMediaDto>` mapping is total and never panics on an unknown
+      item type (Rust test with a garbage type string).
+- [ ] Behaviour is identical: same library/search/home rendering, same sorting,
+      same navigation, offline included.
+- [ ] Both command returns and event payloads carry the new shape (no flicker).
+- [ ] `bun run check`, `bun run test`, `bun run check:boundary` pass;
+      `cargo fmt`/`cargo clippy`/`bun run test:rust` pass; `bindings.ts` regenerated.
+
+## Testing
+
+**Rust** (`cargo test`): the `From<JellyfinMediaDto> for MediaItem` mapping is the
+critical surface —
+- every known `item_type` → correct `MediaKind` (table test over all 20 values
+  found in the audit, incl. `ChannelFolderItem`, `TvChannel`, `Composer`);
+- unknown type string → safe fallback, no panic;
+- `run_time_ticks` → `duration_ms` (10_000 divisor), boundary/None cases;
+- `SortKey` → Jellyfin field mapping (port `jellyfinFieldMapping.ts`'s cases).
+
+**Frontend** (vitest): update the many tests asserting `.type`/`runTimeTicks`;
+they become `.kind`/`durationMs`. `jellyfinFieldMapping`/`playbackUnits` tests are
+deleted with their modules. Add a compose/render test proving `kind`-based
+branching matches the old `type`-based branching for a representative mix.
+
+## TRACES
+
+Per [CLAUDE.md](../../CLAUDE.md): the domain type + mapping
+`UR-007, UR-008 | <new DR>`; the tick/field hoist `<new DR>`; frontend migration
+phases share the DRs of the capability each touches (don't invent per-file DRs).
+
+## Notes for the implementer
+
+- **This is the highest-collision change in the repo's history** — it touches 36+
+  frontend files and the core Rust types. A parallel Claude session in any media
+  file will conflict. Strongly prefer a dedicated worktree per phase, and
+  `git diff` before repairing anything (CLAUDE.md gotchas / project memory).
+- Phase 1 deliberately maps *back* to the old shape so it can land safely ahead of
+  the disruptive flip. Resist the urge to skip it.
+- IPC camelCase rules apply to the new enums/structs
+  ([04-type-sync-and-threading.md](../architecture/04-type-sync-and-threading.md)):
+  `#[serde(rename_all = "camelCase")]`; tagged-enum tag convention; regenerate
+  `bindings.ts`, never hand-edit.
+- Reviewed against [SPEC-REVIEW-CHECKLIST.md](SPEC-REVIEW-CHECKLIST.md) — the
+  Layer assignment table above is the load-bearing section.