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).
310 lines
17 KiB
Markdown
310 lines
17 KiB
Markdown
# 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.
|