diff --git a/docs/requirements.md b/docs/requirements.md index 75043c8f..36d98d92 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -75,7 +75,13 @@ For a narrative overview of the system design, see | UR-062 | Opening a TV series lands the viewer **where they are in it**, not at season 1: the series page scrolls the current season into view and highlights the current episode, and the hero button opens that episode (labelled `Resume S2E4` / `Play S1E1`). "Current" means the episode in progress, else the server's Next Up for that series, else the first unwatched episode, else the first — resolved by the backend so it also works offline. A season is **never a page of its own**: every route that names a season lands on the series with that season in view, so the episodes of all seasons are always one continuous scrollable list | High | Done | | UR-063 | Each video library is **one page**, not three. Browsing (hero, Continue Watching, Next Up, Recently Added, genre rows), the full title grid, and the genre browser are tabs of `/library/tv` and `/library/movies` rather than separate routes with inconsistent names (`/library/tv/shows` vs `/library/movies/all`, `/library/shows/genres` vs `/library/movies/genres`). The old routes redirect so existing links keep working | Medium | Done | | UR-064 | Watch history can be **erased**, per series and per season, from the series page. Clearing marks every episode inside unwatched and clears resume positions, so the show returns to "never watched" and reopens on its premiere. It asks for confirmation first (it cannot be undone) and requires a connection to the server, since history cleared only locally would be undone by the next sync | Medium | Done | +| UR-065 | Search answers from a **locally indexed copy of the library**, so results appear as fast as the device can query rather than at the speed of a round trip to the server, and the same results are found with the server unreachable. A background job keeps the index current — refreshing on a schedule rather than only at app start, dropping media removed from the server, and covering everything the result groups can show (including artists and people). The server is still queried in the background so media added since the last index still turns up, merged in without reordering what is already on screen | High | Implemented | | UR-066 | The app's own chrome stays clear of the device's system chrome. On Android the bottom navigation sits above the navigation/gesture bar instead of underneath it, the header clears the status bar, and full-screen video and audio playback keep their controls inside the usable screen — clear of the gesture bar and, in landscape, of the display notch. This must hold across navigation modes (gesture and 3-button) and rotation, not only on the handsets it happened to be tested on | High | Done | +| UR-067 | Favourited media can be **found again**. A Favourites page lists everything favourited across all libraries, scoped by tabs (All / Movies / Shows / Music); the home screen carries favourite rows for movies, shows and music, hidden when a category is empty; and each library page can be filtered to favourites in place. Without this the like button writes to a store nothing reads | Medium | Done | +| UR-068 | Anything the app shows can be favourited where it is shown — from a movie, series, episode, album, artist or playlist page, and from any card in a grid or carousel — not only from the player while the item happens to be playing | Medium | Done | +| UR-069 | Favourite state agrees with the server in both directions. An item favourited in another Jellyfin client shows as favourited here without being touched, and an item favourited here while the server is unreachable reaches the server once it returns — without the user going back to the screen where they marked it | Medium | Done | +| UR-070 | Playback quality is the viewer's choice: the player offers the bitrates the server can produce for what is playing, and changing one resumes at the same point with the same audio and subtitle tracks. Because the chosen rendition can change at any moment, nothing that streams for playback is treated as a stored copy unless it happens to be byte-identical to the real file | Medium | Proposed | +| UR-071 | Media the viewer is watching can be **kept**, by a whole-file download that runs in the background independently of playback and at its own quality, so it is unaffected by bitrate changes. Where the streamed bytes already are that file (direct play), they are kept rather than fetched twice. A completed download is then played from disk rather than streamed again | Medium | Proposed | --- @@ -116,7 +122,9 @@ External system integrations and platform-specific implementations. | IR-027 | Jellyfin `/System/Info/Public` reachability probe used as an offline→online recovery detector | API | UR-043 | Done | | IR-028 | Jellyfin/LMS SyncGroups API client (list, create, join, unsync, dissolve sync groups) | API | UR-046 | Done | | IR-029 | Android `ConnectivityManager`/`NetworkCapabilities` transport probe with a `NetworkCallback` change subscription, surfaced to the frontend via the `AndroidNetworkType` JS bridge and the `jellytau-network-changed` WebView event (requires `ACCESS_NETWORK_STATE`) | Platform | UR-053 | Done (pending device verification) | +| IR-030 | Scheduled full-catalog crawl of every library (`Recursive=true`, paged) feeding the local index, driven by a Rust background task and the `ConnectivityMonitor` reconnect signal rather than by the frontend | Storage | UR-065 | Implemented | | IR-031 | Android `WindowInsets` bridge: an `OnApplyWindowInsetsListener` on the decor view reports `systemBars() | displayCutout()` in CSS pixels, pushed into the WebView as `jt-inset` CSS custom properties plus a `jellytau-insets-changed` event, and pullable via the `AndroidInsets` JS bridge | Platform | UR-066 | Done (pending device verification) | +| IR-032 | Whole-file background download of the item being played, reusing the existing resumable download worker and the Range-capable `/Videos/{id}/stream.mp4` endpoint; plus per-platform read-through caching hooks (ExoPlayer `CacheDataSource`, mpv `stream-record`) for direct-play sessions only | Storage | UR-071 | Proposed | ### 2.2 Jellyfin API Requirements @@ -156,6 +164,8 @@ API endpoints and data contracts required for Jellyfin integration. | JA-030 | Get person details and filmography | Persons | UR-036 | Done | | JA-031 | Get items by person (actor/director filmography) | Items | UR-036 | Done | | JA-032 | Get audio-only stream URL for a video item (selected audio-stream index) | MediaInfo | UR-040 | Done | +| JA-033 | Query favourite items (`Filters=IsFavorite`, recursive, scoped by item type) | Items | UR-067 | Done | +| JA-034 | Read `UserData` (favourite, played, resume position) from item responses | UserData | UR-069 | Done | ### 2.3 Development Requirements @@ -266,7 +276,28 @@ Internal architecture, components, and application logic. | DR-105 | Video library routes collapse to one per library. `/library/tv` and `/library/movies` render browse / all-titles / genres as in-page tabs driven by `?view=`, omitted for the default `browse` (the convention `searchRouteUrl` already uses for the `all` scope); `resolveLibraryView` is pure and unit-tested. The four legacy routes become redirect-only `+page.ts` loads rather than deletions, because `GenreTags` links to them and users have them in history; `resolveSearchScope` keeps its `/library/shows` branch for the same reason. The "Browse" tile grid at the bottom of both landing pages is removed — it was a second navigation affordance to the same destinations the carousels' "Show all" links already reach | UI | UR-063 | Done | | DR-106 | Erasing watch history goes through the repository, not the local cache: `clear_watch_history(item_id)` maps to Jellyfin's `DELETE /Users/{userId}/PlayedItems/{itemId}`, which clears the played flag *and* zeroes the resume position, and which the server applies recursively to a folder — so one call handles a whole series or season. `OfflineRepository` returns `RepoError::Offline` rather than clearing locally, because history diverged only on the device would be silently undone by the next sync; the button disables itself while the server is unreachable. `ClearHistoryButton` is shared by the series hero and each `SeasonSection` header, confirms before acting (there is no undo), and reloads the page on success so the recomputed current episode — the premiere, for a fully cleared series — is what the viewer sees | Repository | UR-064 | Done | | DR-107 | Seasons on the series page are collapsible, and **only the current season is expanded** on load — the one holding the episode DR-101 resolved. A show with ten seasons otherwise renders every episode of every season at once, burying the one episode the viewer came for under hundreds of rows. Expansion state is per season and pure (`initialExpandedSeasons` in `seriesNavigation.ts`): the current season, or the first season when there is no current episode, so a never-watched show still opens on season 1 rather than fully collapsed. A `?episode=` deep link expands that episode's season too. Toggling is local and not persisted — it is a reading position, not a preference | UI | UR-062 | Done | +| DR-108 | The instant (cache) leg of `repository_search` searches the **synced catalog**, not just downloads. `OfflineRepository::search` replaces its `downloaded_items` CTE with the `available_items` CTE `get_items` already uses — the same downloads branches plus a `synced_at IS NOT NULL` branch gated on the same `include_catalog_browse()` flag — so search and browse cannot diverge on what is visible. Online (flag true) search reads the whole index and answers before any HTTP request completes; offline with "Show all server media" off (flag false) it stays downloads-only, unchanged. Requires no frontend change, since the flag is already set correctly for all three states. The `include_item_types` filter is switched from string interpolation to bound parameters, as `SearchOptions` is settable from the frontend and not only from `SearchScope` | Backend | UR-065 | Implemented | +| DR-109 | Index freshness is a Rust-owned policy, not a frontend startup call. A tokio task ticks every 30 min and runs a full pass when a repository is active, the server is reachable, and `last_catalog_sync` (already persisted to `app_settings`, previously read only for a UI hint) is older than `CATALOG_INDEX_TTL` (6 h); the `ConnectivityMonitor` reconnect signal re-evaluates the same condition immediately. An `AtomicBool` prevents concurrent passes, replacing `offlineCatalog.ts`'s `syncInProgress` — the frontend trigger is removed rather than left alongside, since two triggers with one guard each is how double-crawls happen. `RepositoryManager` gains an active-handle slot so the task has something to run against. Progress is emitted as the kebab-case `catalog-index-event` | Backend | UR-065 | Implemented | +| DR-110 | Index hygiene. `save_to_cache` switches from `INSERT OR REPLACE INTO items` to `ON CONFLICT(id) DO UPDATE`: REPLACE fires no `AFTER DELETE` trigger unless `recursive_triggers` is on (it is not — only `foreign_keys` and `journal_mode` are set), so `items_ad` never ran, and because `items.id` is a `TEXT PRIMARY KEY` each replacement also took a fresh rowid and appended a second `items_fts` entry — a duplicate index per sync, invisible in results but permanently degrading `MATCH`. The upsert preserves the rowid `items_fts` keys on and fires `items_au`; migration `021_rebuild_items_fts` clears orphans on existing installs. Separately, a post-crawl sweep deletes synced-but-not-downloaded rows a successful library crawl did not return, so media removed from the server stops being searchable; it skips items with completed downloads and skips any library whose crawl errored, because `items.parent_id` is `ON DELETE CASCADE` and a partial crawl would cascade away a whole series | Storage | UR-065 | Implemented | +| DR-111 | The index covers what the result groups render: `CATALOG_ITEM_TYPES` gains `MusicArtist` and `Playlist`, and migration `022_people_fts` adds a `people_fts` virtual table over the existing `people` table (which had no FTS, and is populated incidentally by item-detail fetches) with the same trigger pattern as `items_fts`. `OfflineRepository::search` UNIONs `people_fts` matches in as `Person` items when the resolved scope admits them — i.e. `SearchScope::All`, which expands to no filter (DR-063). Without this, the Artists and People groups UR-060 mandates can only ever be filled by the server leg | Storage | UR-065, UR-060 | Implemented | | DR-112 | Safe-area insets come from **native**, not from `env()` alone. `env(safe-area-inset-*)` is 0px without `viewport-fit=cover` (missing from `app.html`, so every safe-area rule in the app was already a no-op), and even with it Android WebView maps only the *display cutout* — never the status bar or navigation bar. Since `enableEdgeToEdge()` plus `targetSdk 36` make edge-to-edge unconditional, the WebView always spans the system bars, so CSS could not learn about them by any route. `WindowInsetsBridge` reads the real insets and publishes `jt-inset` custom properties; `app.css` folds them with `env()` via `max()` into `--safe-*`, which is the only thing components may pad from. Ownership is exactly one element per edge: the app shell takes top/left/right, and BottomUi takes bottom wherever it renders (`shellReservesBottomInset` hands it back to the shell on routes with no bottom UI) so the padding sits inside BottomUi's surface box and the colour extends behind the gesture bar. The full-screen players inset their control layers only, leaving video and artwork edge-to-edge. The theme's `fitsSystemWindows=true` — which claimed the opposite and was overridden at runtime and ignored at this target SDK — is removed | UI | UR-066 | Done | +| DR-113 | `MediaItem.user_data` is populated from the server instead of being hardcoded `None`. `JellyfinItem` gains a `UserData` field (`#[serde(alias = "UserData")]` → the existing `UserData` type) and `to_media_item` maps it, so every list and detail response carries favourite/played/resume state. `UserData` is named explicitly in the `Fields=` list rather than relying on Jellyfin's default. Without this no card or detail page can render a favourite it did not itself set, and the mini player's per-track `storageGetPlaybackProgress` fetch is the only way to colour one heart | Repository | UR-069 | Done | +| DR-114 | Server favourite state is mirrored into the local `user_data` table by `OfflineRepository::save_to_cache` — the single choke point every cached server result passes through — so offline browsing and the offline Favourites page see the same favourites as the server. The upsert carries `pending_sync = 0` and is guarded by `WHERE user_data.pending_sync = 0`, which is the conflict rule: a toggle made offline is never overwritten by a stale server value before it has been pushed | Storage | UR-069 | Done | +| DR-115 | Cross-library favourites query: a `get_favorites(scope, options)` repository method plus the `repository_get_favorites` command. Online issues `Filters=IsFavorite&Recursive=true` with `IncludeItemTypes` expanded from `SearchScope::item_types()` in Rust (the frontend sends the opaque scope, never a type list — DR-063); offline reads `items ⨝ user_data (is_favorite = 1)` under the same `include_catalog_browse()` gate as browsing; hybrid races cache against server like `get_items`, including the DR-080 rule that an empty offline result is authoritative when the gate is off. A separate method rather than `get_items` because favourites span libraries and `get_items` is `ParentId`-shaped | Repository | UR-067 | Done | +| DR-116 | `GetItemsOptions.favorites_only` filters an existing library listing in place — online by appending `Filters=IsFavorite`, offline by joining `user_data` into the existing `available_items` CTE so the downloads-only gate still applies. This is what backs the per-library favourites toggle, and composes with the genre and item-type filters already there | Repository | UR-067 | Done | +| DR-117 | The Favourites page (`/library/favorites`) renders favourites across libraries with All / Movies / Shows / Music scope tabs, reusing `LibraryViewTabs` + `LibraryGrid` + `MediaCard` so card shape still follows the media (§5A.1) and a mixed All tab reads as posters, squares and thumbnails side by side. Each tab sends a `SearchScope` value and nothing else. Reached from the library overview and from "See all" on the home rows | UI | UR-067 | Done | +| DR-118 | Home carries favourite rows for movies, shows and music, loaded via `repository_get_favorites` per scope and rendered below Recently Added. A row with no items does not render at all, so a fresh install shows no empty favourite rows | UI | UR-067 | Done | +| DR-119 | `FavoriteButton` is mounted wherever a whole item is shown — movie/series/episode detail heroes, album/artist/playlist headers, and as a `MediaCard` artwork overlay — and a `favorites` store holds in-session optimistic state so un-hearting on one surface updates every other without a refetch. Resolution order is `store override ?? item.userData?.isFavorite ?? false`. On a card the heart is its own button and stops propagation, so hearting never also opens, plays, or triggers the §5B.5 long-press; it is suppressed on server-only (greyed) cards | UI | UR-068 | Done | +| DR-120 | Favourite toggles made while offline reach the server. A Rust drain, triggered by the `ConnectivityMonitor` offline→online transition, pushes every `user_data` row with `pending_sync = 1` and clears the flag on success, leaving failures pending for the next transition. It lives in Rust rather than the frontend because a frontend drain dies with the component that started it. Both the drain and the hybrid background refresh emit the kebab-case `favorites-changed` event (`{ itemIds }`) so open views update — without it a favourite marked on another client appears only on the *second* visit to a page, since the cache-first read returns local rows and the server refresh is invisible to the frontend. Supersedes the unused `syncService.queueFavorite`, which is deleted rather than left as a second queue | Backend | UR-069 | Done | +| DR-121 | Player quality selector: Rust reports the bitrates available for the current media source and owns the quality→transcode-parameter mapping (the one `get_video_download_url` already holds — playback calls into it rather than restating it, or the two tables drift). Changing quality re-negotiates the stream URL and resumes at the current position with audio/subtitle selection preserved. On Linux, video re-negotiates *within* HLS: returning `stream.mp4` is the documented cause of transcoded playback never starting. The frontend renders the list and remembers the choice; it does not decide what the choice resolves to | UI | UR-070 | Proposed | +| DR-122 | The playback path is ephemeral. Streamed bytes are never persisted unless DR-124 rules them keepable, and any in-flight capture is abandoned — partial file deleted, never promoted — the moment the viewer changes quality, because a capture spanning a rendition change is a splice of two encodings rather than a playable file | Playback | UR-070 | Proposed | +| DR-123 | The download path is independent of playback: a whole-file fetch through the existing download manager at one canonical quality (default `original`, the direct static copy) over the Range-capable `/Videos/{id}/stream.mp4`, unaffected by bitrate changes and completing into an ordinary `downloads` row so offline browsing and `refresh_queue_local_sources` pick it up unchanged. Prerequisite: downloaded video is currently never played locally — `repository_get_video_stream_url` goes straight to the online repo and the player route calls it with no local check, so a completed video download is still streamed. Without that fix nothing in this spec is observable for video | Repository | UR-071 | In Progress | +| DR-124 | Streamed bytes are kept only where they *are* the download artifact — a direct-play session. Android uses ExoPlayer `SimpleCache`/`CacheDataSource` keyed by item **and** media-source id so renditions cannot collide, sharing the existing smart-cache storage budget rather than opening a second one over the same disk; Linux audio uses mpv `stream-record`, abandoned on seek because it is documented as intended for linear streams and seeking breaks the recording. Transcoded Linux video is **not** captured: HLS segments are not a file, and assembling one needs ffmpeg, which is not a dependency and which CI may not install at job time — DR-123 covers that case instead | Playback | UR-071 | Proposed | +| DR-125 | A capture is promoted to a completed `downloads` row only when it covers the whole resource; partials stay evictable cache. A new `downloads.source_rendition` column records the negotiated quality/container/codec (`NULL` for the existing paths, which are always `original`) so a captured transcode and a real download are distinguishable rows and an "upgrade to original" remains possible. A quality change never touches a file that already exists — not a permanent download, and not a completed temporary one, both of which stay valid copies of the rendition they hold. It invalidates only an **in-flight** capture or background download of cached media, which is abandoned and restarted at the newly chosen quality, because a capture spanning a rendition change is a splice of two encodings rather than a playable file | Storage | UR-071 | Proposed | +| DR-126 | Cache eviction only reclaims the *temporary* tier. `evict_lru_async` selected every completed download ordered by `completed_at ASC` with no `download_source` filter, so hitting the 10 GB storage limit deleted the **oldest** download — typically a film saved deliberately for offline — to make room for a newly precached track. It now evicts only `COALESCE(download_source, 'user') = 'auto'` rows; `COALESCE` rather than a bare equality because rows predating migration 012 can be NULL and unknown provenance must be treated as the user's, never as disposable. Freeing less than requested is the correct outcome when only user downloads remain — the caller reports "unable to free enough space" instead of silently deleting them | Storage | UR-071 | Done | +| DR-127 | A cache entry *is* a download with a shorter life: same `downloads` row and same file handling, distinguished by `download_source = 'auto'` plus an expiry, so there is one storage model rather than a cache and a download library that can disagree. Temporary rows are reclaimed on whichever comes first — the life limit elapsing, or eviction under space pressure (DR-126). Permanent (`'user'`) rows have no expiry. A temporary row can be promoted to permanent by the user choosing to keep it, which only clears the expiry and flips the source; the bytes never move | Storage | UR-071 | Done | +| DR-128 | Audio-only playback of *downloaded* media reads the local file rather than fetching an audio-only stream. No transcode is involved or wanted: the Linux backend already runs MPV with `video: no`, so handing it the downloaded video file decodes the audio track and ignores the video, and ExoPlayer disables its video renderer equivalently. Transcoding to a separate audio artifact would cost CPU and battery, need an encoder the project does not ship, and produce a second file to keep in step — for no gain over simply not decoding the video | Playback | UR-071 | Done | +| DR-129 | A stream that stops delivering is recovered, not treated as terminal. Two failure shapes, because the streams differ. (a) *Phantom end* — the background audio-only handoff uses a progressive mp3 transcode over plain HTTP, chunked and therefore length-less, so a dropped connection reaches the player as end-of-input and ExoPlayer reports `STATE_ENDED` indistinguishably from the real end. The item's runtime is the only thing that can tell them apart: an end reported more than a tolerance short of it (comparing the *absolute* position — handoff base plus the player's relative position) is a truncation. Left unhandled, playback parked in `STATE_ENDED` and the next play intent from the lockscreen, notification or a Bluetooth reconnect seeks an ended player to position 0 — the user-visible "the episode randomly restarted". (b) *Recoverable error* — music (`/Audio/{id}/stream?Static=true`) and video (`/Videos/{id}/master.m3u8`) declare their length, so the player detects the truncation itself and raises an error; the frontend's handler stopped playback outright, turning a hiccup into silence. Both resume the current item **in place** (never via `play_item`, which would replace the queue with a single item and lose the album), the error path after a per-attempt backoff. Seekable streams are re-prepared at the URL they already have and seeked; the length-less transcode, which cannot be seeked, has `StartTimeTicks` rewritten into its existing URL so the user's audio-track selection survives and recovery needs no network round-trip. Only `Remote` sources qualify — a local file cannot fail from the network. A shared budget of consecutive attempts at the same position, refilled whenever playback progresses, stops an unreachable server from looping | Playback | UR-040, UR-004 | Done | | DR-093 | Traceability coverage gate derives its requirement denominators from `requirements.md` at run time rather than hardcoded literals: `countDefinedRequirements` counts an ID only where it leads a markdown table row (ignoring the "Traces To" column and prose) and deduplicates IDs listed both in the definition tables and in the §3 traceability matrix; `computeCoverage` reports the *intersection* of traced and defined IDs so an ID traced in code but absent from `requirements.md` is surfaced as `orphaned` instead of inflating the ratio past 100%. UT/IT test identifiers are excluded as a separate taxonomy. CI and `bun run traces:coverage` share this computation and fail on both a sub-threshold and an impossible >100% result | Tooling | - | Done | --- @@ -280,7 +311,7 @@ Internal architecture, components, and application logic. | UR-001 | IR-001, IR-002 | - | | UR-002 | IR-013 | DR-003, DR-012, DR-013, DR-014 | | UR-003 | IR-003, IR-004, IR-011 | DR-002, DR-004, DR-010 | -| UR-004 | IR-003, IR-004, IR-008, IR-011 | DR-002, DR-004, DR-006 | +| UR-004 | IR-003, IR-004, IR-008, IR-011 | DR-002, DR-004, DR-006, DR-129 | | UR-005 | - | DR-001, DR-005, DR-009 | | UR-006 | IR-005, IR-006, IR-007, IR-008 | - | | UR-007 | IR-010 | DR-007, DR-008, DR-016 | @@ -316,7 +347,7 @@ Internal architecture, components, and application logic. | UR-037 | IR-010 | DR-042 | | UR-038 | IR-010 | DR-043 | | UR-039 | - | DR-045, DR-046 | -| UR-040 | IR-025 | DR-051, DR-052 | +| UR-040 | IR-025 | DR-051, DR-052, DR-129 | | UR-041 | IR-026 | DR-053 | | UR-042 | IR-009, IR-014 | DR-054 | | UR-043 | IR-027 | DR-055 | @@ -335,12 +366,18 @@ Internal architecture, components, and application logic. | UR-056 | - | DR-085 | | UR-057 | - | DR-086 | | UR-058 | - | DR-087 | -| UR-060 | - | DR-090, DR-091 | +| UR-060 | - | DR-090, DR-091, DR-111 | | UR-061 | - | DR-092 | | UR-062 | - | DR-101, DR-102, DR-103, DR-104, DR-107 | | UR-063 | - | DR-105 | | UR-064 | - | DR-106 | +| UR-065 | IR-030 | DR-108, DR-109, DR-110, DR-111 | | UR-066 | IR-031 | DR-112 | +| UR-067 | - | DR-115, DR-116, DR-117, DR-118 | +| UR-068 | - | DR-119 | +| UR-069 | - | DR-113, DR-114, DR-120 | +| UR-070 | - | DR-121, DR-122 | +| UR-071 | IR-032 | DR-123, DR-124, DR-125, DR-126, DR-127, DR-128 | --- @@ -445,6 +482,28 @@ Internal architecture, components, and application logic. | UT-096 | `readNativeInsets` returns null with no bridge and survives a stale WebView proxy (missing or throwing `get`) instead of throwing out of layout init | IR-031, DR-112 | Done | | UT-097 | `initSafeArea` primes the document on start, re-applies on `jellytau-insets-changed` (rotation, nav-mode switch), unsubscribes on teardown, and writes nothing without a bridge so `env()` still wins on iOS/desktop | IR-031, DR-112 | Done | | UT-098 | `shellReservesBottomInset` gives the bottom inset to BottomUi wherever one renders and to the app shell only on routes without one, so the gesture bar is never ignored nor double-padded | DR-112 | Done | +| UT-099 | A Jellyfin item payload carrying `UserData.IsFavorite` maps to `MediaItem.user_data.is_favorite` | DR-113, JA-034 | Done | +| UT-100 | `OnlineRepository::get_favorites` builds `Filters=IsFavorite` + `Recursive=true` + the scope's `IncludeItemTypes`, and omits the type filter entirely for `SearchScope::All` | DR-115, JA-033 | Done | +| UT-101 | `OfflineRepository::get_favorites` returns only `is_favorite = 1` rows, honours the scope type filter, and stays downloads-only when the catalog-browse gate is off | DR-115 | Done | +| UT-102 | The `save_to_cache` favourite mirror does not overwrite a row with `pending_sync = 1` | DR-114 | Done | +| UT-103 | The reconnect drain pushes pending favourites, clears `pending_sync`, and leaves failed rows pending | DR-120 | Done | +| UT-104 | `get_items` with `favorites_only` filters online (endpoint) and offline (SQL) | DR-116 | Done | +| UT-105 | `favorites` store precedence: override beats `userData.isFavorite` beats `false` | DR-119 | Done | +| UT-106 | Un-favouriting removes an item from a favourites listing view | DR-117, DR-119 | Done | +| UT-107 | The hybrid background refresh emits `favorites-changed` only for ids whose favourite state actually flipped | DR-120 | Done | +| UT-109 | Search covers synced-but-not-downloaded items when catalog browse is on, and stays downloads-only when off | DR-108 | Done | +| UT-110 | Search item-type filter is bound, not interpolated: a quote-bearing type neither errors nor widens results | DR-108 | Done | +| UT-111 | FTS prefix queries quote each token, so apostrophes/hyphens/slashes are data; empty or punctuation-only input returns no rows rather than erroring | DR-108 | Done | +| UT-112 | Repeated catalog passes leave one `items_fts` entry per item, not one per pass | DR-110 | Done | +| UT-113 | The stale-catalog sweep removes vanished synced rows, keeps downloaded ones, keeps uncrawled types, and stays scoped to one server | DR-110 | Done | +| UT-114 | Cached people are reachable from unscoped search and excluded from scoped search | DR-111 | Done | +| UT-115 | Re-index staleness policy: never-indexed and unparseable timestamps are due, fresh ones are not, future ones are not | DR-109 | Done | +| UT-116 | `resolve_local_media_path` returns a completed download's file, and `None` for an in-progress download, a row whose file has been deleted, or an unknown item | DR-123 | Done | +| UT-118 | `resolveVideoSource` prefers a downloaded file, never marks a local file as needing transcoding, and falls back to streaming for a blank path | DR-123 | Done | +| UT-119 | The audio-only handoff picks a downloaded file over the audio-only stream URL, preserving the Jellyfin id for progress sync | DR-128 | Done | +| UT-120 | Expiry reclaim takes only expired temporary entries: derived from `completed_at`+TTL, honouring an `expires_at` override, never a user download, and disabled by a zero TTL | DR-127 | Done | +| UT-108 | LRU eviction reclaims only `'auto'` downloads and never a user's own, even when the user's is the oldest | DR-126 | Done | +| UT-117 | A background audio-only stream cut short resumes where it died instead of ending the episode; a real end still advances; the absolute position is compared against the runtime; retries at a stuck position give up. A recoverable error resumes music and video too, with growing backoff, leaving the rest of the queue intact and the seekable stream's URL untouched; local and DirectUrl sources are excluded | DR-129 | Done | ### Integration Tests diff --git a/docs/specs/catalog-index-search.md b/docs/specs/catalog-index-search.md new file mode 100644 index 00000000..1931b4db --- /dev/null +++ b/docs/specs/catalog-index-search.md @@ -0,0 +1,334 @@ +# Spec: Locally-indexed search + +**Status:** Implemented +**Requirements:** UR-065 → DR-108, DR-109, DR-110, DR-111; IR-030 +**UX spec:** [ux-flows.md §6.1](../ux-flows.md) (search surface is unchanged) +**Revises:** [scoped-search.md](scoped-search.md) and +[scoped-search-boundary.md](scoped-search-boundary.md) — scope semantics are +untouched; this changes only *which corpus* the cache leg searches. + +## Summary + +Search stops depending on a per-keystroke round trip to Jellyfin. The local +SQLite catalog — which is already synced and already FTS5-indexed — becomes the +corpus the instant leg of search reads, so results appear as fast as SQLite can +answer, online or offline. A background indexer keeps that catalog fresh on a +schedule instead of only at app start, prunes content deleted on the server, and +covers the item types search groups results by. The server query stays, demoted +to a background reconciliation that merges in late results for anything indexed +since the last pass. + +## Motivation + +The pieces are already built and simply not wired together: + +- [`sync_full_catalog`](../../src-tauri/src/commands/catalog.rs) already walks + every library `Recursive=true` and persists items with `synced_at`. +- `items_fts` (schema.rs migration 001) already indexes `name`, `overview`, + `album_name`, `album_artist`, `artists`, `series_name` with keep-in-sync + triggers. +- `repository_search` is already two-phase — synchronous cache result, then a + spawned server query merged in via the `search-event`. + +What breaks the chain is that the cache leg is hard-restricted to *downloaded* +items. `OfflineRepository::search` wraps its FTS query in a `downloaded_items` +CTE requiring `d.status = 'completed'`: + +```sql +FROM items i + JOIN items_fts fts ON fts.rowid = i.rowid + INNER JOIN downloaded_items di ON i.id = di.id + WHERE i.server_id = ? AND items_fts MATCH ? +``` + +So for a user with no downloads, phase 1 returns nothing on every query, and +every debounced keystroke falls through to a full `Recursive=true` server +request with `Limit=10000`. The populated local index is never read. + +`get_items` does not have this problem — it gates a third `synced_at IS NOT NULL` +branch on `include_catalog_browse()` (offline.rs, the "Show all server media" +toggle). The asymmetry is the bug: **offline you can already browse the whole +catalog but cannot search it.** + +Three further defects found while confirming the above: + +1. **The FTS index grows without bound.** `save_to_cache` uses + `INSERT OR REPLACE INTO items`, but `recursive_triggers` is never enabled + (`storage/mod.rs` sets only `foreign_keys` and `journal_mode`). SQLite fires + `AFTER DELETE` triggers on a REPLACE *only* with recursive triggers on — so + `items_ad` never runs, the old FTS row is orphaned, and because `items.id` is + a `TEXT PRIMARY KEY` the replacement row takes a **new rowid** and inserts a + second FTS entry. Every sync appends a duplicate index. Results stay correct + (the `INNER JOIN … ON fts.rowid = i.rowid` hides orphans, and no rowid is ever + reused because nothing is deleted) but `MATCH` degrades permanently. +2. **Server-side deletions never propagate.** There is no `DELETE FROM items` + anywhere in the codebase. The local catalog is append-only, so media removed + from the server would stay searchable forever — tolerable when the cache was + only a browse accelerator, not acceptable when it is the search corpus. +3. **The index omits types search groups by.** `CATALOG_ITEM_TYPES` is + `MusicAlbum, Movie, Series, Season, Episode, Audio, BoxSet` — no + `MusicArtist`, no `Playlist`, and People live in a separate `people` table + with no FTS at all. UR-060 mandates Artists and People result groups, so today + those can *only* come from the server. + +## Layer assignment + +| Logic / responsibility | Layer | Why it belongs there | +|------------------------|-------|----------------------| +| Which corpus search reads (downloads-only vs full synced catalog) | **Rust** | Sync/availability policy over domain data. Changes if Jellyfin's API or the offline rules change, not if the UI is redesigned. Reuses the existing `include_catalog_browse()` flag so search and browse cannot diverge again. | +| Index freshness policy — TTL, when a re-index is due, skip-while-offline | **Rust** | Explicitly named as domain policy in [SPEC-REVIEW-CHECKLIST.md](SPEC-REVIEW-CHECKLIST.md) ("reachability/sync policy"). It is currently frontend-driven in `offlineCatalog.ts`; this spec moves it. | +| Which Jellyfin item types get indexed (`CATALOG_ITEM_TYPES`) | **Rust** | Textbook domain taxonomy — a category→item-type set. Must never appear in `src/`. | +| Reconciling a crawl against local rows (what to prune) | **Rust** | Operates on domain data and depends on crawl completeness semantics. | +| FTS query construction, ranking, scope→type expansion | **Rust** | Already there (`search_rank.rs`, `SearchScope::item_types()`); unchanged by this spec. | +| Rendering a "catalog last indexed N ago" hint and any re-index button | **Frontend** | Pure presentation of a backend-supplied timestamp. | +| Debounce interval, result group order, scope chips | **Frontend** | Input handling and view preference; changes only if the UI is redesigned. | + +Borderline call, recorded: the **TTL value itself** (how many hours before a +re-index is due) could be argued as a user preference and therefore frontend. It +is placed in Rust because the frontend must not be able to decide *whether the +cache is authoritative* — that is the same class of decision as +`include_catalog_browse`, which already lives in Rust. If the TTL later becomes +user-configurable it stays a Rust-owned setting the frontend edits through a +command, not a frontend constant. Borderline defaults to Rust. + +## Design + +### 1. Search the full synced catalog (DR-108) + +`OfflineRepository::search` mirrors `get_items` exactly: rename the CTE to +`available_items` and add the same third branch, gated on the same flag. + +```rust +let catalog_branch = if include_catalog_browse() { + "UNION + + -- Synced catalog: fast online search, or the offline 'Show all + -- server media' view. Mirrors get_items; see set_include_catalog_browse. + SELECT DISTINCT i.id + FROM items i + WHERE i.synced_at IS NOT NULL" +} else { + "" +}; +``` + +No new IPC surface and no frontend change: `set_include_catalog_browse` is +already called with `true` when online or when the offline toggle is on, and +`false` only when offline with the toggle off. Search inherits the correct +behaviour in all three states, and the "search is restricted to downloads" case +survives for users who deliberately asked for downloads-only. + +Also fix, in the same function, the `type_filter` built by **string +interpolation** of `include_item_types` rather than bound parameters. It is +currently safe only because callers pass `SearchScope`-derived values, but +`SearchOptions.include_item_types` is settable directly from the frontend (as +`GenericMediaListPage` does). Bind the values. + +Phase 2 (the server query) is unchanged and still merges via `search-event`, so +content added to the server since the last index still surfaces — just late +rather than first. + +### 2. Scheduled background indexer (DR-109, IR-030) + +A Rust-owned task replaces the frontend's startup-only trigger. + +```rust +/// How long a full-catalog index stays fresh before a re-index is due. +const CATALOG_INDEX_TTL: Duration = Duration::from_secs(6 * 60 * 60); +``` + +Behaviour: + +- On app setup, spawn a tokio task that ticks every 30 min. +- Each tick: if a repository is active **and** the server is reachable **and** + `now - last_catalog_sync > CATALOG_INDEX_TTL`, run a full index pass. +- On the existing `ConnectivityMonitor` reconnect signal, evaluate the same + staleness condition immediately rather than waiting for the next tick. +- Never run two passes concurrently (the existing `syncInProgress` guard moves + into Rust as an `AtomicBool`). + +`last_catalog_sync` is already written to `app_settings` by `sync_full_catalog` +and is currently read only for a UI hint; this makes it load-bearing. + +`RepositoryManager` (`commands/repository.rs`) is a `HashMap` with no +notion of an active handle, so the task has nothing to run against. Add: + +```rust +pub struct RepositoryManager { + repositories: Arc>>>, + active: Arc>>, // set in create(), cleared in destroy() +} +``` + +Progress is reported with a **kebab-case** event (per the project convention): + +```rust +// event name: "catalog-index-event" +#[derive(specta::Type, Serialize, Clone)] +#[serde(rename_all = "camelCase")] +pub struct CatalogIndexEvent { + pub state: CatalogIndexState, // #[serde(tag = "type")] Idle | Running | Complete | Failed + pub libraries_done: usize, + pub libraries_total: usize, + pub items_indexed: usize, +} +``` + +`sync_full_catalog` stays a command so the UI can still force a pass; it and the +scheduler share one internal `run_index_pass()`. + +### 3. Index hygiene — no orphans, and deletions propagate (DR-110) + +**Orphan growth.** Replace `INSERT OR REPLACE INTO items (…)` in `save_to_cache` +with a true upsert: + +```sql +INSERT INTO items (id, server_id, …) VALUES (…) +ON CONFLICT(id) DO UPDATE SET + name = excluded.name, overview = excluded.overview, …, + synced_at = excluded.synced_at +``` + +This preserves the rowid (which `items_fts` keys on via `content_rowid`) and +fires `items_au` instead of silently orphaning a row. Preferred over +`PRAGMA recursive_triggers = ON` because it also stops the rowid churn, and the +three FTS triggers are the only triggers in the schema so nothing else depends +on REPLACE semantics. + +A new migration `021_rebuild_items_fts` clears the orphans already accumulated on +existing installs: + +```sql +INSERT INTO items_fts(items_fts) VALUES('rebuild'); +``` + +**Deletions.** After a library crawls *successfully and completely*, reconcile: +delete local rows for that library whose `id` was not seen in the crawl. Two +constraints the implementation must respect: + +- Skip any item with a completed download — the user has the file; removing the + row would orphan it. Prune only synced-but-not-downloaded rows. +- Only sweep libraries whose crawl succeeded. `sync_full_catalog` is + deliberately best-effort per library, and `items.parent_id` is + `ON DELETE CASCADE` — sweeping on a partial crawl would cascade a whole series + away because one request timed out. + +### 4. Index the types search groups by (DR-111) + +Add `MusicArtist` and `Playlist` to `CATALOG_ITEM_TYPES`. + +People need a different mechanism: they live in `people` (`id`, `server_id`, +`name`, `overview`, `primary_image_tag`, `synced_at`), populated incidentally by +item-detail fetches, with no FTS table. Migration `022_people_fts` adds one +mirroring the `items_fts` pattern: + +```sql +CREATE VIRTUAL TABLE IF NOT EXISTS people_fts USING fts5( + name, overview, content='people', content_rowid='rowid' +); +-- plus people_ai / people_ad / people_au triggers +``` + +`OfflineRepository::search` UNIONs `people_fts` matches into its result set as +`Person`-typed items when the resolved scope permits them (i.e. when +`include_item_types` is `None` — `SearchScope::All`). `search_rank.rs` already +handles `MediaKind::Person`, so ranking needs no change. + +## Out of scope + +- **Incremental indexing** (e.g. Jellyfin's `MinDateLastSaved`). A full crawl is + what makes the deletion sweep in §3 sound — it yields the authoritative id set + per library. An incremental pass cannot detect deletions, so it would need a + separate reconciliation strategy. Worth revisiting if full crawls prove too + slow on large libraries; measure first. +- **Changing search UX** — scope chips, group order, the debounce, and the + `/search` route are untouched. +- **Removing the server leg.** Phase 2 stays. +- The two dead search implementations (`storage_search_items` in + `commands/storage/mod.rs`, `offline_search` in `commands/offline.rs`) — both + registered in `lib.rs` and exported to `bindings.ts`, neither called from the + frontend. Deleting them is correct but is cleanup, not this feature; file + separately so this spec's diff stays reviewable. +- `GenericMediaListPage` passing raw `includeItemTypes` and re-implementing the + store's request-id/event protocol. A real boundary smell, tracked separately. + +## Acceptance criteria + +- [ ] With a synced catalog and **zero downloads**, typing a query returns + results from the local index before any server request completes. +- [ ] Offline with "Show all server media" **on**, search returns the full + catalog (non-downloaded entries greyed out, matching browse). +- [ ] Offline with the toggle **off**, search returns downloaded media only — + the behaviour that exists today. +- [ ] Re-running a full index pass N times does not grow `items_fts` row count + beyond the `items` row count. +- [ ] An item deleted server-side disappears from local search after one index + pass; a **downloaded** item deleted server-side does not. +- [ ] A library that fails mid-crawl prunes nothing. +- [ ] Searching an artist or actor name returns results with the server + unreachable. +- [ ] `bun run check` and `bun run test` pass. +- [ ] `cargo fmt` clean, `cargo clippy` clean, `bun run test:rust` passes. +- [ ] `bun run check:boundary` passes. +- [ ] New requirement-implementing code carries `// TRACES:` comments. +- [ ] `bindings.ts` regenerated (new `CatalogIndexEvent` type). + +## Testing + +Per CLAUDE.md, each defect gets a **failing test first**. + +Rust (`cargo test`), against an in-memory DB seeded with synced-but-not- +downloaded items: + +- `search` returns synced items when `include_catalog_browse()` is true, and + only downloaded items when false. *Fails today* — the current CTE returns + empty in the first case. +- Upserting the same item twice leaves exactly one `items_fts` row. *Fails + today.* +- The sweep removes a vanished synced item, retains a vanished downloaded item, + and no-ops for a library whose crawl errored. +- `type_filter` binds parameters — a type string containing a quote does not + alter the query. +- Staleness: a `last_catalog_sync` inside the TTL does not trigger a pass; one + outside it does; offline never does. +- `people_fts` matches surface as `Person` items under `SearchScope::All` and + are excluded under `Music`/`Movies`/`Tv`. + +Frontend (`vitest`): the catalog-index event maps to the staleness hint; no +change to the search store's request-id/stale-response handling, which stays +covered by its existing tests. + +## TRACES + +| Piece | Tag | +|---|---| +| `OfflineRepository::search` availability CTE | `// TRACES: UR-065 \| DR-108` | +| Background indexer task + scheduling | `// TRACES: UR-065 \| DR-109, IR-030` | +| `save_to_cache` upsert + FTS rebuild migration | `// TRACES: UR-065 \| DR-110` | +| Deletion reconciliation | `// TRACES: UR-065 \| DR-110` | +| `CATALOG_ITEM_TYPES` widening + `people_fts` | `// TRACES: UR-065, UR-060 \| DR-111` | + +## Notes for the implementer + +- **A parallel Claude session may be active in this repo.** Run `git diff` + before "repairing" changes you did not make (CLAUDE.md gotchas). +- The frontend's `offlineCatalog.ts` startup trigger should be **removed**, not + left alongside the Rust scheduler — two independent triggers with one + `syncInProgress` guard each is how double-crawls happen. +- `downloads` has a relaxed FK to `items` (migration 005). Verify the deletion + sweep's interaction with it before enabling the sweep, and check whether + `parent_id`'s `ON DELETE CASCADE` reaches further than intended. +- The existing 100 ms `cache_with_timeout` in `hybrid.rs` returns *empty* on + timeout rather than erroring. Once the cache leg is the primary path, that + budget may need raising — an FTS query over a large catalog on cold page cache + can exceed it, and the failure mode is a silently empty result. +- Keep `SearchScope` semantics as-is: `All => None` (no filter), deliberately + not a union, so People and folders are not filtered out (DR-063). +- Noted but deliberately not fixed here: `pushCatalogVisibility` in + `offlineCatalog.ts` derives the flag as `connected || showCatalog` — the + frontend computing an availability *policy*, even though the flag itself is + Rust-stored. DR-108 depends on that derivation being correct and it is, so + this spec leaves it alone. Once DR-109 has moved sync policy into Rust, the + derivation belongs there too, with the frontend pushing only the raw user + toggle. Folding it into this change would enlarge the diff for no behavioural + gain — but do not add *new* policy on the frontend side of that line. diff --git a/docs/specs/favorites-browsing.md b/docs/specs/favorites-browsing.md new file mode 100644 index 00000000..d694fca6 --- /dev/null +++ b/docs/specs/favorites-browsing.md @@ -0,0 +1,395 @@ +# Spec: Favourites — marking, browsing, and sync + +**Status:** Implemented +**Requirements:** UR-067, UR-068, UR-069 → DR-113 … DR-120; JA-033, JA-034 +(allocated in [requirements.md](../requirements.md); tests UT-099 … UT-107). +Note: UR-066/DR-112/IR-031 were claimed by the concurrent safe-area work while +this spec was being written, so the ids here start one higher than first drafted. +Existing: UR-017 → DR-021, JA-017, JA-018 (the toggle itself, already built). +**UX spec:** [ux-flows.md](../ux-flows.md) §3.2 (full-player favourite), §5.2 +(album detail favourite), §5B.3 (movie detail hero: *Play / Download / Favorite*) +— all three already specify favourite affordances that **do not exist in the +build**. This spec closes those, and adds a new §5C for the Favourites browse +surface. +**Supersedes / revises:** nothing. + +## Summary + +JellyTau can favourite an item but can never show you what you favourited. The +heart is mounted in exactly one place (the mini player), no query anywhere asks +Jellyfin or the local database for favourites, and favourites marked on any other +client are invisible here. This spec adds the read side (a Favourites page, home +carousels, an in-library filter), puts the heart on detail pages and media cards, +teaches the backend to ingest server-side favourite state, and drains favourite +toggles made while offline. + +## Background: what exists today + +Verified in code, 2026-08-04. The **write** path is real and mostly correct; the +**read** path does not exist at all. + +1. **Toggling works, from one place only.** + [FavoriteButton.svelte](../../src/lib/components/FavoriteButton.svelte) is + mounted solely in + [MiniPlayer.svelte:381](../../src/lib/components/player/MiniPlayer.svelte#L381). + Nothing else in `src/` renders it — so the only favouritable item in the app + is the one currently playing. + +2. **The toggle's plumbing is sound.** + [favorites.ts](../../src/lib/services/favorites.ts) writes local first + (`storage_toggle_favorite`, + [storage/mod.rs:908](../../src-tauri/src/commands/storage/mod.rs#L908) — sets + `user_data.is_favorite` + `pending_sync = 1`), then POST/DELETEs + `/Users/{uid}/FavoriteItems/{id}` + ([online.rs:1652](../../src-tauri/src/repository/online.rs#L1652)) only when + connected. Leave this design intact. + +3. **`MediaItem.user_data` is always `None` from the server.** + `JellyfinItem` has no `UserData` field, and `to_media_item` hardcodes + [`user_data: None`](../../src-tauri/src/repository/online.rs#L656) with the + comment *"User data not included in basic item responses"*. The only + populated `user_data` in the app comes from + [series_progress.rs](../../src-tauri/src/repository/series_progress.rs#L244) + and the local read in + [offline.rs:115](../../src-tauri/src/repository/offline.rs#L115). **Nothing + ingests server favourite state**, which is why the mini player has to fetch + `storageGetPlaybackProgress` per track to colour one heart + ([MiniPlayer.svelte:75-93](../../src/lib/components/player/MiniPlayer.svelte#L75-L93)). + +4. **No favourites query exists.** + [`GetItemsOptions`](../../src-tauri/src/repository/types.rs#L278) has no + favourites field; `Filters=IsFavorite` appears nowhere; no SQL selects + `is_favorite = 1`; there is no `/library/favorites` route and no favourites + carousel in [home.ts](../../src/lib/stores/home.ts). + +5. **Offline favourites are silently lossy.** Offline `mark_favorite` / + `unmark_favorite` are no-ops + ([offline.rs:1620](../../src-tauri/src/repository/offline.rs#L1620)), so an + offline toggle survives only as a local row with `pending_sync = 1` — and + **nothing ever drains that flag**. `syncService.queueFavorite` + ([syncService.ts:91](../../src/lib/services/syncService.ts#L91)) exists with + no callers. + +## Motivation + +Favouriting is a promise: the app takes the input and shows a "Added to +favorites" toast, then discards it as far as the user can tell. Three of the UX +flows already specify favourite buttons that were never built, and the one that +was built (mini player) writes to a store nothing reads. Either the feature gets +its read side or the heart should be removed — this spec takes the first option. + +## Layer assignment + +| Logic / responsibility | Layer | Why it belongs there | +|------------------------|-------|----------------------| +| Favourites **scope** → set of Jellyfin item types | Rust | Domain taxonomy. Changes when Jellyfin adds/renames a type, never when the UI is redesigned. Reuses the canonical `SearchScope::item_types()` ([types.rs:324](../../src-tauri/src/repository/types.rs#L324)) — the exact leak class of [scoped-search-boundary.md](scoped-search-boundary.md). | +| Cross-library favourites query (`Filters=IsFavorite`, `Recursive`, paging, sort field) | Rust | Query shaping against the Jellyfin API is domain logic; the endpoint's contract changes with the server, not the UI. | +| Offline favourites SQL (join `user_data`, downloaded/catalog gating) | Rust | Storage + domain. Must obey the existing catalog-browse gate (DR-080) which the frontend cannot see. | +| Deserialising Jellyfin `UserData` into `MediaItem.user_data` | Rust | Provider payload mapping. | +| Mirroring server favourite state into the local `user_data` table | Rust | Cache/sync policy. | +| Conflict rule: a local row with `pending_sync = 1` beats the server value | Rust | Business rule about which write wins; nothing to do with rendering. | +| Draining pending favourite toggles on reconnect | Rust | Sync policy, and it must run whether or not any view is mounted — a frontend-driven drain dies with the component. Consistent with *reachability from real traffic* (DR-055). | +| Which surfaces show favourites, tab order, row placement on home | Frontend | Pure presentation; changes only if the UI is redesigned. | +| Heart placement, animation, toast, haptics, empty-state copy | Frontend | Presentation. | +| In-session optimistic heart state shared across views | Frontend | View state, not persisted truth; the durable write already goes to Rust. | + +**Borderline, and the tie-breaker used:** *which* scopes appear as tabs (All / +Movies / Shows / Music) is a presentation choice — the frontend picks which +`SearchScope` values to offer. What each scope *means* is Rust's. The frontend +sends the enum value and never names an item type in connection with favourites. +Single-type pages (`itemType: "Movie"` on the Movies list page) stay as they are; +this rule targets category taxonomy, not every mention of a type. + +## Design + +### 1. Server user data reaches `MediaItem` (Rust, DR-113, JA-034) + +Jellyfin returns `UserData` on `/Users/{uid}/Items*` responses. Add the field to +`JellyfinItem` and map it in `to_media_item`, replacing the hardcoded `None`: + +```rust +// in JellyfinItem +#[serde(alias = "UserData")] +pub user_data: Option, +``` + +`JellyfinUserData` deserialises `IsFavorite`, `Played`, `PlaybackPositionTicks`, +`PlayCount`, `LastPlayedDate` into the existing +[`UserData`](../../src-tauri/src/repository/types.rs#L44) type (which already +carries `is_favorite` and already serialises camelCase, so `bindings.ts` needs no +new type — only regeneration). Add `UserData` to the `Fields=` list in `get_items` +/ `get_item` so the shape is explicit rather than relying on the default. + +Wire shape, unchanged from today's `UserData`: + +```ts +item.userData?.isFavorite // boolean | null | undefined +``` + +Delete the now-false `// User data not included in basic item responses` comment. + +### 2. Local mirror of server favourites (Rust, DR-114) + +Choke point: `save_to_cache(parent_id, &items)` in +[offline.rs](../../src-tauri/src/repository/offline.rs) — every server result +that gets cached (including via +[`cache_items_from_server`](../../src-tauri/src/repository/hybrid.rs#L124) and +the background cache refresh) passes through it. + +For each item carrying `user_data.is_favorite`, upsert: + +```sql +INSERT INTO user_data (user_id, item_id, is_favorite, synced_at, pending_sync) +VALUES (?, ?, ?, ?, 0) +ON CONFLICT(user_id, item_id) DO UPDATE SET + is_favorite = excluded.is_favorite, + synced_at = excluded.synced_at +WHERE user_data.pending_sync = 0; -- local unsynced change wins +``` + +The `WHERE` on the conflict clause is the whole conflict rule: a toggle made +offline is never overwritten by a stale server value before it has been pushed. + +### 3. Favourites queries (Rust, DR-115, DR-116, JA-033) + +**(a) In-library filter** — one new field on `GetItemsOptions`: + +```rust +#[serde(skip_serializing_if = "Option::is_none")] +pub favorites_only: Option, +``` + +- online `get_items`: append `&Filters=IsFavorite` when true. +- offline `get_items`: add `INNER JOIN user_data ud ON ud.item_id = i.id AND ud.user_id = ? AND ud.is_favorite = 1`, composed with the existing `available_items` CTE so the downloads-only gate still applies. + +Frontend sends `{ favoritesOnly: true }` (camelCase — nested struct field, needs +the existing `#[serde(rename_all = "camelCase")]` on `GetItemsOptions`, already +present). + +**(b) Cross-library favourites** — a new trait method, because favourites span +libraries and `get_items` is `ParentId`-shaped: + +```rust +/// TRACES: UR-067 | DR-115 | JA-033 +async fn get_favorites( + &self, + scope: SearchScope, + options: Option, +) -> Result; +``` + +```rust +#[tauri::command] +#[specta::specta] +pub async fn repository_get_favorites( + manager: State<'_, RepositoryManagerWrapper>, + handle: String, + scope: SearchScope, + options: Option, +) -> Result +``` + +Frontend call (command name matches the Rust fn exactly; top-level params +auto-camelCase; `SearchScope` is `#[serde(rename_all = "camelCase")]` so the wire +values are `"all" | "music" | "movies" | "tv"`): + +```ts +await commands.repositoryGetFavorites(handle, "movies", { limit: 100 }); +``` + +- **online**: `/Users/{uid}/Items?Filters=IsFavorite&Recursive=true&SortBy=SortName&SortOrder=Ascending` + `&IncludeItemTypes=…` from `scope.item_types()` (omit entirely on `None`, per that function's contract) + the standard `Fields=`. +- **offline**: `items ⨝ user_data (is_favorite = 1)`, type filter from the same `scope.item_types()`, honouring `include_catalog_browse()`. +- **hybrid**: same cache-first race as `get_items`, **including the DR-080 rule** — with the catalog-browse gate off, an empty offline result is authoritative and must not fall through to the server. Getting this wrong reproduces Defect B from [offline-downloaded-only-filter.md](offline-downloaded-only-filter.md). + +**The cache-first result arrives stale, and there is no second payload.** On a +cache hit, `hybrid::get_items` returns the local rows and refreshes the cache in +a background task whose result the frontend never sees — fine for a library +listing that changes daily, wrong for favourites, where the *point* is that +another client just changed something. Favourites is the second read path (after +search) that needs the deferred update, so the background refresh in +`get_favorites` must emit the same `favorites-changed` event as §4 when the +server's favourite set differs from what was returned: + +``` +favorites-changed → { itemIds: string[] } // union of ids whose is_favorite flipped +``` + +Both producers (background refresh, reconnect drain) emit the identical payload, +and the frontend has one handler that refreshes the `favorites` store. Without +this, a favourite marked on another client appears in JellyTau only on the +*second* visit to the page. + +### 4. Draining offline toggles (Rust, DR-120) + +On the offline→online transition already detected by `ConnectivityMonitor`, +select `user_data WHERE pending_sync = 1 AND is_favorite IS NOT NULL`, POST or +DELETE `/Users/{uid}/FavoriteItems/{id}` per row, then set `pending_sync = 0` and +`synced_at`. Failures leave the row pending for the next transition. + +Emit a kebab-case event when anything changed, so open views refresh without +polling: + +``` +favorites-changed → { itemIds: string[] } +``` + +`syncService.queueFavorite` is dead code once this lands — delete it or point it +at the backend drain; do not leave two competing queues. + +### 5. Frontend surfaces (DR-117, DR-118, DR-119) + +**Favourites page** — new route `/library/favorites`: +- Scope tabs *All / Movies / Shows / Music* via the existing `LibraryViewTabs`; each tab sends a `SearchScope` value, nothing more. +- Renders through `LibraryGrid` + `MediaCard` (tracklist for Music→tracks if the tab is later split; not in this pass). +- Entry points: a card on the library overview ([library/+page.svelte](../../src/routes/library/+page.svelte)) and "See all" on the home rows. +- Empty state per tab: "Nothing favourited yet — tap the heart on anything you like." + +**Home carousels** — `favoriteMovies`, `favoriteShows`, `favoriteMusic` added to +[home.ts](../../src/lib/stores/home.ts), each `repositoryGetFavorites(scope, { limit: 20 })`, +rendered after *Recently Added* and **only when non-empty** (no empty rows on a +fresh install). + +**In-library filter** — a favourites toggle in the header of +[GenericMediaListPage](../../src/lib/components/library/GenericMediaListPage.svelte) +and the Movies/TV landing pages, passing `favoritesOnly: true` into the existing +`repo.getItems(...)` options. Session-scoped state; not persisted (a persisted +filter that hides most of a library is a support call waiting to happen). + +**Hearts** — mount `FavoriteButton`: +- Movie / series detail hero button row, beside the download buttons ([library/[id]/+page.svelte:528-553](../../src/routes/library/%5Bid%5D/+page.svelte#L528-L553)) — closes ux-flows §5B.3. +- `EpisodeFocusView`, `ArtistDetailView`, `PlaylistDetailView`, album detail — closes ux-flows §5.2. +- `MediaCard` artwork overlay (top-right). Suppressed on `isServerOnly` cards, and must not fight the existing long-press/scroll-guard handlers ([MediaCard.svelte:60-70](../../src/lib/components/library/MediaCard.svelte#L60-L70)) — the heart is its own button and stops propagation. + +**Shared optimistic state** — a small `favorites` store (`Map` +overlay + `favorites.set(id, value)`), so un-hearting an item on the Favourites +page removes it from the grid and from any home row without a refetch, and a +heart tapped on a card is reflected on the detail page. Resolution order: + +``` +favorites store override ?? item.userData?.isFavorite ?? false +``` + +`toggleFavorite()` updates the store alongside its existing local + server +writes; the `favorites-changed` event refreshes it. This removes the mini +player's per-track `storageGetPlaybackProgress` fetch once items carry +`userData`. + +### 6. Offline behaviour + +Toggling offline keeps working exactly as now (local write + `pending_sync`), and +now actually reaches the server on reconnect (§4). The Favourites page offline +shows favourites among downloaded/cached items, subject to the existing +catalog-browse gate. The offline repo's no-op `mark_favorite`/`unmark_favorite` +stay no-ops — the local write plus the drain is the offline path. + +## Out of scope + +- Favouriting people, genres, or collections; favourite **playlists** are included only insofar as they fall under the Music scope. +- Sorting by "date favourited" — Jellyfin does not expose it. Favourites sort by name. +- A dedicated bottom-nav tab for favourites (reachable from library overview + home). +- Building a playlist or download batch from favourites. +- Reconciling favourites for items that no longer exist on the server. +- Splitting the Music tab into albums/artists/tracks sub-tabs. + +## Acceptance criteria + +- [ ] Favouriting is possible from movie, series, episode, album, artist and playlist detail pages, and from media cards in any grid. +- [ ] A favourite marked in another Jellyfin client shows a filled heart in JellyTau without toggling it here. +- [ ] `/library/favorites` lists favourites across libraries, filtered by the All/Movies/Shows/Music tabs. +- [ ] Home shows favourite rows for movies, shows and music, and shows no row when a category has none. +- [ ] Movies/TV/Music list pages can be filtered to favourites only. +- [ ] Un-hearting an item on one surface updates the others without a manual refresh. +- [ ] A favourite toggled while offline reaches the server after reconnect (verified against a real server or a fake repository). +- [ ] Offline, the Favourites page respects the "Show all server media" gate — with it off, an empty result stays empty and does not fall through to the server. +- [ ] No item-type set appears in `src/` in connection with favourites; the frontend sends `SearchScope` only. +- [ ] `bun run check` and `bun run test` pass. +- [ ] `cargo fmt` clean, `cargo clippy` clean, `bun run test:rust` passes. +- [ ] `bun run check:boundary` passes (necessary, not sufficient — see CLAUDE.md). +- [ ] New requirement-implementing code carries `// TRACES:` comments. +- [ ] `bindings.ts` regenerated from Rust, not hand-edited. + +## Testing + +**🔴 §4 (the pending-sync drain) is a bug fix — failing test first.** Write a +test that toggles a favourite with the repository offline, transitions to online, +and asserts the server call happened; watch it fail before writing the drain. + +Rust (`cd src-tauri && cargo test`): + +| Test | Covers | +|------|--------| +| UT-099 | A Jellyfin item JSON fixture with `UserData.IsFavorite: true` maps to `MediaItem.user_data.is_favorite == Some(true)` | +| UT-100 | `online::get_favorites` builds an endpoint with `Filters=IsFavorite`, `Recursive=true`, and the scope's `IncludeItemTypes`; `SearchScope::All` omits the type filter entirely | +| UT-101 | `offline::get_favorites` returns only `is_favorite = 1` rows, respects the scope type filter, and returns nothing extra when the catalog-browse gate is off | +| UT-102 | `save_to_cache` mirror does **not** overwrite a row with `pending_sync = 1` | +| UT-103 | Drain pushes pending rows, clears `pending_sync`, sets `synced_at`, and leaves failed rows pending | +| UT-104 | `get_items` with `favorites_only: true` filters both online (endpoint) and offline (SQL) | +| UT-107 | The background refresh in `hybrid::get_favorites` emits `favorites-changed` with the flipped ids, and emits nothing when the server set matches the cache | + +Frontend (`bun run test`): + +| Test | Covers | +|------|--------| +| UT-105 | `favorites` store override precedence: store value beats `userData.isFavorite` beats `false` | +| UT-106 | Un-hearting removes the item from a favourites list view (pure logic extracted to a `.ts` module, per the TrackList/episodeStrip pattern) | +| IT-0xx | `repositoryGetFavorites` param naming — add to [tauriIntegration.test.ts](../../src/lib/utils/tauriIntegration.test.ts): camelCase top-level params, scope serialised as `"movies"` etc. | + +Any component logic worth testing gets extracted into a plain `.ts` module first +(`favoritesView.ts`), rather than tested through the component. + +## TRACES + +| Piece | Tag | +|-------|-----| +| `JellyfinUserData` + `to_media_item` mapping | `// TRACES: UR-069 \| DR-113, JA-034 \| UT-099` | +| `save_to_cache` user_data mirror | `// TRACES: UR-069 \| DR-114 \| UT-102` | +| `get_favorites` (trait, online, offline, hybrid) + command | `// TRACES: UR-067 \| DR-115, JA-033 \| UT-100, UT-101` | +| `GetItemsOptions.favorites_only` handling | `// TRACES: UR-067 \| DR-116 \| UT-104` | +| `/library/favorites` route + tabs | `// TRACES: UR-067 \| DR-117` | +| Home favourite carousels | `// TRACES: UR-067 \| DR-118` | +| `FavoriteButton` mounts + `favorites` store | `// TRACES: UR-068 \| DR-119 \| UT-105, UT-106` | +| Pending-favourite drain + `favorites-changed` | `// TRACES: UR-069 \| DR-120 \| UT-103` | + +New requirement rows to add to [requirements.md](../requirements.md): + +- **UR-067** — Browse favourited media across libraries (page, home rows, in-library filter). +- **UR-068** — Mark/unmark favourites from browse and detail surfaces, not only the player. +- **UR-069** — Favourite state stays consistent with the server in both directions. +- **DR-113 … DR-120** — as tabled above. +- **JA-033** — Query favourite items (`Filters=IsFavorite`). +- **JA-034** — Read `UserData` from item responses. + +## Implementation notes (as built) + +Two things landed differently from the design above, both forced by where the +`AppHandle` lives: + +1. **The `favorites-changed` event is emitted from the command layer, not the + repository.** `HybridRepository` has no `AppHandle` — the same reason + `search-event` is emitted from `repository_search`. `repository_get_favorites` + therefore does the two-phase read itself (cache leg returned, server leg + spawned) and diffs the two id sets via `changed_favorite_ids`, which is + extracted and unit-tested (UT-107) rather than buried in the spawn. +2. **The drain hooks the existing `connectivity:reconnected` event** via + `app.listen` in `commands/favorites.rs`, rather than reaching into + `ConnectivityMonitor` (which knows nothing about repositories). It drains + through a narrow `FavoriteSink` trait so it can be tested against a recording + double instead of a forty-method `MediaRepository` mock. + +Also as built: `DatabaseService` is not object-safe (generic methods), so the +drain takes `Arc` like the rest of the storage code, and +`get_items`' endpoint construction was extracted to `build_get_items_endpoint` +so the `favorites_only` filter could be asserted without an HTTP server. + +**Not built:** the full-player heart. ux-flows §3.2 lists one among the full +player's secondary controls and it remains unbuilt — recorded as a known +deviation in ux-flows §5C.5 rather than silently dropped. + +## Notes for the implementer + +- A parallel Claude session may be active in this repo — `git diff` before "repairing" unexpected changes. +- Do **not** try to reuse `get_items` with an empty `ParentId` for cross-library favourites; that endpoint is built as `?ParentId={}` ([online.rs:731](../../src-tauri/src/repository/online.rs#L731)) and an empty value is not a reliable "all libraries" request. Use `get_favorites`. +- `SearchScope` is reused rather than a new `FavoritesScope` so there is one taxonomy expansion in the codebase, not two that can drift. If the name grates once favourites ship, rename the type across search + favourites in one commit — don't fork it. +- `SearchScope::All` returns `None` from `item_types()` **on purpose**; callers must omit `IncludeItemTypes` entirely rather than sending a union (see the doc comment at [types.rs:316](../../src-tauri/src/repository/types.rs#L316)). +- Ship order that keeps each step demonstrable: §1+§2 (state becomes visible) → §5 hearts (marking becomes possible) → §3+§5 browse surfaces (finding becomes possible) → §4 drain. +- Regenerate `bindings.ts` after the Rust types change; never hand-edit it. diff --git a/docs/specs/read-through-media-cache.md b/docs/specs/read-through-media-cache.md new file mode 100644 index 00000000..0cca4ad8 --- /dev/null +++ b/docs/specs/read-through-media-cache.md @@ -0,0 +1,219 @@ +# Spec: Two-path media — selectable playback bitrate, independent whole-file download + +**Status:** Proposed +**Requirements:** UR-070, UR-071 → DR-121, DR-122, DR-123, DR-124, DR-125; IR-032 +**UX spec:** player quality selector — needs a `ux-flows.md` section before build +**Related:** [catalog-index-search.md](catalog-index-search.md), +[downloads-as-offline-library.md](downloads-as-offline-library.md) + +## Summary + +Two things that are today tangled become explicitly separate: + +- **The playback path** streams at a bitrate the viewer can change from the + player. It is ephemeral and its rendition is volatile. +- **The download path** fetches the whole file at one canonical quality, in the + background, independently of whatever playback is doing. + +Bytes fetched for playback are kept **only** when the playback rendition happens +to be the same artifact the download path would produce — i.e. direct play. +Otherwise playback bytes are discarded and the download path does its own fetch. + +## Motivation + +The appealing version of this — "stream and download at once, switch when enough +has arrived" — breaks the moment the viewer can change bitrate. A capture taken +while the rendition changes underneath it is a splice of two encodings: not a +playable file, and not something that can be honestly recorded as a download. +Once bitrate is selectable, one stream cannot serve both jobs. + +Separating the paths also removes the thing that made the original idea +expensive: there is no mid-playback source swap to engineer, because the download +never has to take over the live session. It lands on disk and is used at the next +natural boundary — next episode, or next time the item is played. + +What exists already and is *not* this: `SmartCache` predictively downloads *other* +items, `player_preload_upcoming` warms the next one, and +`refresh_queue_local_sources` swaps queue entries to local at boundaries. All of +it concerns items you are not currently playing. + +## Layer assignment + +| Logic / responsibility | Layer | Why it belongs there | +|------------------------|-------|----------------------| +| Available bitrate options for an item | **Rust** | Derived from Jellyfin's media sources and playback-info negotiation; changes with the API. | +| Mapping a chosen bitrate to transcode parameters | **Rust** | Domain vocabulary. `get_video_download_url` already owns the quality→params mapping; playback must reuse it, not restate it. | +| Deciding whether playback bytes are keepable (direct play vs transcode) | **Rust** | Depends on the negotiated session. | +| Canonical download quality | **Rust** | Policy over domain data. | +| Cache eviction, storage budget, sparse-range bookkeeping | **Rust** | Storage policy. | +| Promotion to a `downloads` row, and what invalidates a cache entry | **Rust** | Domain state. | +| Rendering the quality selector; remembering the last choice | **Frontend** | Presentation and a view preference. The *list* comes from Rust. | +| WiFi-only / opt-in toggles | **Frontend collects, Rust enforces** | The control is UI; the gate must hold even if the UI never calls. | + +Borderline, recorded: the **default** playback bitrate could look like a user +preference (frontend). It goes to Rust because it must be reconcilable with what +the server can actually produce for a given media source — a preference the +backend has to validate is not a preference the frontend can own alone. The +frontend stores the user's *choice*; Rust decides what that choice resolves to. + +## Design + +### DR-121 — Bitrate selection in the player + +The player exposes the qualities Rust reports for the current item. Changing it +re-negotiates the stream URL at the new quality and resumes at the current +position. This is a deliberate, user-initiated interruption — a brief rebuffer is +expected and acceptable, unlike the involuntary swap the earlier design would +have needed. + +Constraints that must not be broken: + +- On Linux, video playback must keep using the HLS `master.m3u8` URL. CLAUDE.md + records that returning `stream.mp4` means transcoded playback never starts. + A quality change re-negotiates *within* HLS. +- The quality→transcode-parameter mapping already exists in + `get_video_download_url` ([online.rs:1702-1717](../../src-tauri/src/repository/online.rs#L1702-L1717)). + Playback must call into the same mapping. Two copies of that table will drift. +- Track selection (audio/subtitle) already survives a stream re-negotiation + elsewhere in the player; a quality change must preserve it too. + +### DR-122 — The playback path is ephemeral + +Playback bytes are not persisted unless DR-124 says they are keepable. No partial +capture is ever retained across a quality change: on change, any in-flight capture +for that session is abandoned and its partial file deleted. + +### DR-123 — The download path is independent + +Downloading the whole file is a separate operation through the existing download +manager, at one canonical quality (default `original`, the direct static copy), +using `/Videos/{id}/stream.mp4` — progressive and Range-capable, which is what +the resumable download worker relies on. It is unaffected by what playback is +doing, and playback is unaffected by it. + +Once complete it becomes an ordinary download row, so everything already built on +top of downloads — offline browsing, `refresh_queue_local_sources`, the Downloads +page — picks it up with no further work. + +**Prerequisite:** downloaded *video* is currently never played locally. +`repository_get_video_stream_url` goes straight to the online repo and +[player/[id]/+page.svelte:316](../../src/routes/player/[id]/+page.svelte#L316) +calls it with no local check — so a completed video download is still streamed. +This must be fixed or the whole feature is invisible for video. + +### DR-124 — Keep playback bytes only when they *are* the download + +Capture is enabled only where the played bytes and the canonical download artifact +are the same thing — a **direct-play** session. Then: + +| Path | Mechanism | +|---|---| +| Android / ExoPlayer | `SimpleCache` + `CacheDataSource`, keyed by item id **and** media-source id so renditions never collide. LRU evictor sharing the existing smart-cache budget — not a second budget over the same disk. | +| Linux audio / MPV | `stream-record`, set through the existing `set_property` plumbing. | +| Linux video (HLS transcode) | **Not captured.** Segments are not a file; assembling one needs ffmpeg, which is not a dependency and which CI is forbidden from installing at job time. The download path (DR-123) covers this case instead. | + +Two abandonment rules, both of which must delete the partial rather than promote +it: + +- **Seek during an mpv capture.** `stream-record` is documented as intended for + linear streams; seeking breaks the recording. Straight-through listening + captures, scrubbing does not. +- **Any quality change** (DR-122). + +### DR-125 — Promotion, rendition, and invalidation + +A capture is promoted to a `downloads` row (`status = 'completed'`) only when it +covers the whole resource. Partial captures stay cache and remain evictable. + +A new `downloads.source_rendition` column records the negotiated +quality/container/codec of whatever produced the bytes; `NULL` for rows fetched by +the existing paths, which are always `original`. This is what makes an "upgrade to +original" action possible later, and what stops a 720p capture and a 4K download +being indistinguishable rows. + +**Invalidation.** A quality change never touches a file that already exists — +neither a permanent download nor a completed temporary one. Both remain valid +copies of the rendition they hold, and deleting either would throw away bytes +already paid for. + +What a quality change *does* invalidate is an **in-flight** capture or background +download of cached media: it is abandoned and restarted at the newly chosen +quality, because a capture spanning a rendition change is a splice of two +encodings rather than a playable file (DR-122). + +So the rule is about *ongoing* work, not stored files. Nothing in this spec +deletes user data. + +### Gating + +Capture and background download obey the existing WiFi-only gate and storage +budget, and are off unless opted in. Enforcement is in Rust. + +## Out of scope + +- **Mid-playback switch onto a completing download.** Two independent paths make + it unnecessary; the download is used from the next boundary. +- **Backfilling the unplayed remainder of a capture.** Watch 40 minutes and you + have 40 minutes; completing it needs sparse-range bookkeeping and a resumable + tail fetch. The DR-123 download path already produces a complete file, which is + the reason this can wait. +- **Bundling ffmpeg** to make transcoded video capturable. Real option, large + packaging decision, its own proposal. +- **Routing Linux video playback through `stream.mp4`.** Regresses a documented, + hard-won fix. + +## Acceptance criteria + +- [ ] The player offers the qualities Rust reports, and changing one resumes at + the same position with audio/subtitle selection preserved. +- [ ] A quality change abandons any in-flight capture and leaves no partial file. +- [ ] A quality change never deletes a `downloads` row. +- [ ] A completed background download of a video is *played from disk* on the next + play (the DR-123 prerequisite). +- [ ] A direct-play session played start-to-finish leaves a complete local file + with no second fetch; replaying it fetches no media bytes. +- [ ] Seeking during an mpv capture abandons it; no truncated file is promoted. +- [ ] A transcoded Linux video session is never captured, and never partially + promoted. +- [ ] Promoted rows record their rendition; existing paths still record + `NULL`/`original`. +- [ ] Gates hold with the setting off *and* with the frontend never sending it. +- [ ] Eviction cannot delete bytes backing a promoted download row. +- [ ] `bun run check`, `bun run test`, `cargo fmt`, `cargo clippy`, + `bun run test:rust`, `bun run check:boundary` pass; `bindings.ts` + regenerated if Rust types changed. + +## Testing + +Rust, table-driven and pure where possible: quality→params resolution shared with +the download path; keepability (direct play vs transcode vs gate off); promotion +(complete → promoted, partial → not, seek-abandoned → not, quality-changed → not); +invalidation (evicts cache, never a download row); rendition round-trip. + +Android: instrumented — a played direct-play item yields cache entries, and a +replay issues no media network request. + +Frontend: the quality list renders from backend data with no item-type or +codec taxonomy in `src/`; the selector's remembered choice is a view preference. + +## TRACES + +| Piece | Tag | +|---|---| +| Quality selector + re-negotiation | `// TRACES: UR-070 \| DR-121` | +| Ephemeral playback / capture abandonment | `// TRACES: UR-070 \| DR-122` | +| Independent whole-file download + local video playback fix | `// TRACES: UR-071 \| DR-123, IR-032` | +| ExoPlayer cache / mpv stream-record / keepability | `// TRACES: UR-071 \| DR-124` | +| Promotion, `source_rendition`, invalidation | `// TRACES: UR-071 \| DR-125` | + +## Notes for the implementer + +- **A parallel Claude session is active in this repo.** `git diff` before + "repairing" anything you did not write. +- Do not duplicate the quality→transcode-parameter table. Call the existing one. +- Reuse the smart-cache storage budget; two budgets over one disk is how devices + fill up. +- The `downloads` FK to `items` is relaxed (migration 005) — exercise promotion + for an item that was never cached. +- Build DR-123's local-playback fix first. Without it nothing in this spec is + observable for video. diff --git a/docs/ux-flows.md b/docs/ux-flows.md index 5932a3cf..87887232 100644 --- a/docs/ux-flows.md +++ b/docs/ux-flows.md @@ -634,7 +634,7 @@ episode strip. │ │ S2E4 • 48m • ★8.1 │ │ │ │ Overview… │ │ │ │ ▓▓▓▓▓░░░░░ 32m left │ │ -│ │ [▶ Play] │ │ +│ │ [▶ Play] [⬇] [♡] │ │ │ └───────────────────────────────────────────┘ │ │ │ │ More Episodes │ ← 2. EPISODE STRIP @@ -688,7 +688,7 @@ A movie has no continuation set, so cast follows the hero directly. ### 5B.4 Series detail — section order ``` -Hero (poster, title, metadata, Resume SxEy / Download / Clear history) +Hero (poster, title, metadata, Resume SxEy / Download / Favorite / Clear history) → Crew links → Genre tags → Seasons (collapsible; only the current season expanded) @@ -757,6 +757,130 @@ opt-in. Grids and other surfaces keep tap-to-open with no long-press. --- +## 5C. Favourites + +Favouriting is a two-sided promise: the heart takes the input, and the app must +be able to give it back. This section covers both sides — where you can mark a +favourite, and where marked favourites resurface. + +See [specs/favorites-browsing.md](specs/favorites-browsing.md) for the layer +assignment and wire shapes. + +### 5C.1 The heart appears wherever an item does + +A favourite is a property of an *item*, so the affordance follows the item +rather than living on one privileged screen. Any surface that shows a whole +item shows its heart. + +| Surface | Heart position | Notes | +|---------|----------------|-------| +| Movie / Series detail hero | In the button row, after Play and Download | §5B.3, §5B.4 | +| Episode Focus View hero | Same row as Play / Download | §5B.2 | +| Album, Artist, Playlist detail | In the header button row | §5.2 | +| Media card (any grid or carousel) | Top-right overlay on the artwork | Hidden on server-only (greyed) cards | +| Mini player | Right of the track metadata | Existing behaviour, unchanged | +| Full player | Secondary controls row | §3.2 — **not yet built**, see §5C.5 | + +Rules: + +- **The heart never competes with the card.** On a media card it is its own + button and swallows the tap, so hearting an item never also opens or plays + it, and never triggers the §5B.5 long-press. +- **State is shown, not guessed.** A filled heart means the *server* considers + the item a favourite (or you just tapped it). An item favourited in Jellyfin + Web, on another device, or by another client renders filled here without + being touched in JellyTau. +- **Feedback is immediate.** The heart fills on tap and a toast confirms; + neither waits for the server round-trip. + +### 5C.2 Three ways back to what you favourited + +Favourites are not one destination — they are a lens, and the right surface +depends on whether the user is *browsing*, *deciding*, or *hunting*. + +```mermaid +flowchart TB + User[User wants their favourites] --> How{Intent} + + How -->|Passive: show me something| Home[Home carousels
Favourite Movies / Shows / Music] + How -->|Deliberate: my whole collection| Page[Favourites page
/library/favorites] + How -->|Narrowing: within this library| Filter[Favourites filter
on a library page] + + Home -->|See all| Page + Page --> Detail[Item detail page] + Filter --> Detail +``` + +**Home carousels.** Rows for favourite movies, shows and music sit below +*Recently Added*. A row with nothing in it **does not render** — a fresh install +shows no empty favourite rows. Each row ends with *See all*, landing on the +matching tab of the Favourites page. + +**The Favourites page** (`/library/favorites`) is the complete collection, +scoped by tabs: + +``` +┌─────────────────────────────────────────────────┐ +│ [←] Favourites │ +│ ┌─────┬────────┬───────┬───────┐ │ +│ │ All │ Movies │ Shows │ Music │ ← scope tabs │ +│ └─────┴────────┴───────┴───────┘ │ +│ │ +│ ┌────┐┌────┐┌────┐┌────┐┌────┐ │ +│ │ ♥ ││ ♥ ││ ♥ ││ ♥ ││ ♥ │ grid/list │ +│ └────┘└────┘└────┘└────┘└────┘ per §5A │ +└─────────────────────────────────────────────────┘ +``` + +- Cards obey §5A in full — shape follows the media, so a mixed *All* tab reads + as posters, squares and thumbnails side by side rather than one forced shape. +- Reached from a card on the library overview (`/library`) and from *See all* + on any home favourites row. +- Sorted by name. Jellyfin does not record *when* an item was favourited, so + "recently favourited" is not offerable — see §5C.5. +- Empty state, per tab: *"Nothing favourited yet — tap the heart on anything + you like."* + +**The in-library filter** is for narrowing where the user already is: a +favourites toggle in the header of the Movies, TV and Music browse pages, +filtering the current list in place. It is **session-scoped and not persisted** — +a sticky filter that silently hides most of a library reads as data loss on the +next launch. + +### 5C.3 Removing a favourite removes it everywhere, at once + +Un-hearting an item on the Favourites page removes its card from the grid +immediately; the same item disappears from the home rows and shows an empty +heart on its detail page without a manual refresh. The reverse holds for +favouriting. There is no confirmation prompt — the action is one tap to undo. + +### 5C.4 Offline + +- **Marking works offline.** The heart fills, the toast confirms, and the change + is held locally. +- **It reaches the server on reconnect**, without the user returning to the + screen where they made it. +- **Browsing offline shows favourites among media on the device**, subject to + the same "Show all server media" gate as every other browse surface (§7.2) — + with the gate off, an empty Favourites tab means *nothing favourited is + downloaded*, and the page does not quietly fall back to the server catalog. + +### 5C.5 Known deviations + +- **The full player has no heart.** §3.2 and §3.3 list a Favorite button among + the full player's secondary controls; it was never built, and this pass does + not add it. The mini player heart above it is the only in-player affordance. + *(UR-067)* +- **No "recently favourited" sort.** Jellyfin's API does not expose a favourite + timestamp, so favourites can only be ordered by name. Recording the + timestamp locally at toggle time would order *this device's* favourites only, + which is worse than a consistent name sort. +- **Music is one tab, not three.** The Music scope mixes albums, artists and + tracks in a single grid rather than offering sub-tabs. Acceptable while + favourite counts are small; revisit if the tab becomes unscannable. + +--- + ## 6. Search Flow Search is **context-scoped**: what you are looking at when you start a search diff --git a/scripts/extract-traces.test.ts b/scripts/extract-traces.test.ts index a9530864..b3701785 100644 --- a/scripts/extract-traces.test.ts +++ b/scripts/extract-traces.test.ts @@ -173,10 +173,10 @@ describe("live requirements.md", () => { ); const defined = countDefinedRequirements(md); - expect(defined.UR).toBe(65); - expect(defined.IR).toBe(30); - expect(defined.DR).toBe(105); - expect(defined.JA).toBe(32); - expect(defined.total).toBe(232); + expect(defined.UR).toBe(71); + expect(defined.IR).toBe(32); + expect(defined.DR).toBe(126); + expect(defined.JA).toBe(34); + expect(defined.total).toBe(263); }); }); diff --git a/src-tauri/src/commands/catalog.rs b/src-tauri/src/commands/catalog.rs index d9b43077..bf141a70 100644 --- a/src-tauri/src/commands/catalog.rs +++ b/src-tauri/src/commands/catalog.rs @@ -14,10 +14,12 @@ //! that were queued offline (they have `stream_url IS NULL`), mirroring the //! heal-and-pump pattern in `player_preload_upcoming`. +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; +use std::time::Duration; use log::{info, warn}; -use tauri::State; +use tauri::{Emitter, Manager, State}; use crate::commands::download::{pump_download_queue, DownloadManagerWrapper}; use crate::commands::repository::RepositoryManagerWrapper; @@ -29,16 +31,79 @@ use crate::storage::db_service::{DatabaseService, Query, QueryParam}; /// full-catalog sync. const LAST_CATALOG_SYNC_KEY: &str = "last_catalog_sync"; +/// How long an index stays fresh before a re-index is due. +/// +/// This lives in Rust rather than being a frontend constant because it decides +/// *whether the local cache is authoritative* — the same class of decision as +/// `include_catalog_browse`, and squarely the "sync policy" the spec review +/// checklist keeps out of the presentation layer. If it later becomes +/// user-configurable it stays a Rust-owned setting edited through a command. +const CATALOG_INDEX_TTL: Duration = Duration::from_secs(6 * 60 * 60); + +/// How often the scheduler wakes to *check* staleness. Far shorter than the TTL +/// because a tick is nearly free — one indexed `app_settings` lookup — and it is +/// what makes the indexer responsive to events it cannot subscribe to: signing +/// in, and coming back online. The TTL, not the tick, decides whether a crawl +/// actually happens. +const CATALOG_INDEX_TICK: Duration = Duration::from_secs(5 * 60); + +/// Delay before the first staleness check, to let sign-in complete and the +/// repository be registered. Without it the first check runs against an empty +/// repository manager and a fresh install would sit unindexed until the next +/// tick. +const CATALOG_INDEX_FIRST_CHECK: Duration = Duration::from_secs(15); + +/// Kebab-case, per the project's event convention. +pub const CATALOG_INDEX_EVENT: &str = "catalog-index-event"; + +/// Guards against two passes running at once. Replaces the frontend's +/// `syncInProgress` boolean in `offlineCatalog.ts`, which could not see a pass +/// started by the scheduler. +static INDEX_IN_PROGRESS: AtomicBool = AtomicBool::new(false); + +/// Clears [`INDEX_IN_PROGRESS`] however the pass leaves — including on the `?` +/// early return when `get_libraries` fails, which a plain store at the end of +/// the function would leak. +struct IndexPassGuard; + +impl Drop for IndexPassGuard { + fn drop(&mut self) { + INDEX_IN_PROGRESS.store(false, Ordering::SeqCst); + } +} + +/// Progress of a background index pass, for the staleness hint in the UI. +#[derive(specta::Type, Debug, Clone, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct CatalogIndexEvent { + /// `started` | `finished` | `failed` + pub state: String, + pub items_cached: usize, + pub items_pruned: usize, + pub libraries_failed: usize, + /// Present on `failed`. + pub error: Option, +} + /// Item types worth caching for offline browsing: containers the library /// landing pages render plus the playable leaves users queue for download. +/// `MusicArtist` and `Playlist` are here because search groups results by them +/// (UR-060's Artists group). Without them in the crawl, the local index can +/// never answer an artist query and those groups can only ever be filled by the +/// server leg. Keep this in step with what `prune_stale_catalog` is allowed to +/// sweep — the crawl is only authoritative for the types it asks for. +/// +/// TRACES: UR-065, UR-060 | DR-111 const CATALOG_ITEM_TYPES: &[&str] = &[ "MusicAlbum", + "MusicArtist", "Movie", "Series", "Season", "Episode", "Audio", "BoxSet", + "Playlist", ]; #[derive(specta::Type, Debug, Clone, serde::Serialize, serde::Deserialize)] @@ -48,6 +113,9 @@ pub struct CatalogSyncResult { pub items_cached: usize, /// Libraries that failed to sync (e.g. server hiccup); best-effort. pub libraries_failed: usize, + /// Entries removed because the server no longer has them. Always 0 when any + /// library failed, since a partial crawl cannot prove an item is gone. + pub items_pruned: usize, } #[derive(specta::Type, Debug, Clone, serde::Serialize, serde::Deserialize)] @@ -71,8 +139,6 @@ pub async fn sync_full_catalog( db: State<'_, DatabaseWrapper>, handle: String, ) -> Result { - use crate::repository::MediaRepository; - let repo = repository.0.get(&handle).ok_or("Repository not found")?; let db_service = { @@ -80,6 +146,27 @@ pub async fn sync_full_catalog( Arc::new(database.service()) }; + run_index_pass(repo, db_service).await +} + +/// One full-catalog indexing pass, shared by the [`sync_full_catalog`] command +/// and the background scheduler (DR-109) so there is exactly one implementation +/// and one concurrency guard. +/// +/// TRACES: UR-065 | DR-109, DR-110 +pub(crate) async fn run_index_pass( + repo: Arc, + db_service: Arc, +) -> Result { + use crate::repository::MediaRepository; + + // One pass at a time. The command and the scheduler can both land here, and + // two concurrent crawls would double the server load and race on the sweep. + if INDEX_IN_PROGRESS.swap(true, Ordering::SeqCst) { + return Err("A catalog index pass is already running".to_string()); + } + let _guard = IndexPassGuard; + let libraries = repo.get_libraries().await.map_err(|e| e.to_string())?; info!( "[Catalog] Full sync starting across {} libraries", @@ -88,6 +175,10 @@ pub async fn sync_full_catalog( let include_types: Vec = CATALOG_ITEM_TYPES.iter().map(|s| s.to_string()).collect(); + // Taken before the crawl: every row the crawl writes gets a `synced_at` + // newer than this, so anything still older afterwards is gone server-side. + let pass_started_at = chrono::Utc::now().to_rfc3339(); + let mut items_cached = 0usize; let mut libraries_failed = 0usize; @@ -118,6 +209,35 @@ pub async fn sync_full_catalog( } } + // Propagate server-side deletions — but only after a *complete* crawl. + // `sync_full_catalog` is best-effort per library, and `items.parent_id` is + // ON DELETE CASCADE, so sweeping when a library failed to fetch could + // cascade a whole series away because one request timed out. + let mut items_pruned = 0usize; + if libraries_failed == 0 && !libraries.is_empty() { + match repo + .prune_stale_catalog(&pass_started_at, &include_types) + .await + { + Ok(removed) => { + items_pruned = removed; + if removed > 0 { + info!( + "[Catalog] Pruned {} entries no longer on the server", + removed + ); + } + } + Err(e) => warn!("[Catalog] Prune of stale catalog entries failed: {:?}", e), + } + } else if libraries_failed > 0 { + info!( + "[Catalog] Skipping stale-entry prune: {} librar{} failed to sync, so the crawl is not authoritative", + libraries_failed, + if libraries_failed == 1 { "y" } else { "ies" } + ); + } + // Record the sync time so callers can skip re-syncing too eagerly. let now = chrono::Utc::now().to_rfc3339(); let upsert = Query::with_params( @@ -133,16 +253,169 @@ pub async fn sync_full_catalog( } info!( - "[Catalog] Full sync complete: {} items cached, {} libraries failed", - items_cached, libraries_failed + "[Catalog] Full sync complete: {} items cached, {} pruned, {} libraries failed", + items_cached, items_pruned, libraries_failed ); Ok(CatalogSyncResult { items_cached, libraries_failed, + items_pruned, }) } +/// Whether an index pass is due, given when one last completed. +/// +/// Pure so the policy is unit-testable without a clock, a server, or a database. +/// `None` (never indexed) and an unparseable stored value both mean "due" — a +/// corrupt timestamp should trigger a re-index, not silently freeze the catalog. +/// +/// TRACES: UR-065 | DR-109 | UT-115 +pub(crate) fn index_is_due( + last_synced_at: Option<&str>, + now: chrono::DateTime, + ttl: Duration, +) -> bool { + let Some(raw) = last_synced_at else { + return true; + }; + let Ok(last) = chrono::DateTime::parse_from_rfc3339(raw) else { + return true; + }; + now.signed_duration_since(last.with_timezone(&chrono::Utc)) + .to_std() + .map(|elapsed| elapsed >= ttl) + // Negative elapsed => the stored stamp is in the future (clock skew). + // Not due; a future stamp will age into due-ness on its own. + .unwrap_or(false) +} + +/// Read the last-sync timestamp straight from `app_settings`. +async fn read_last_sync( + db_service: &Arc, +) -> Option { + db_service + .query_optional( + Query::with_params( + "SELECT value FROM app_settings WHERE key = ?", + vec![QueryParam::String(LAST_CATALOG_SYNC_KEY.to_string())], + ), + |row| row.get(0), + ) + .await + .ok() + .flatten() +} + +/// Start the background catalog indexer. +/// +/// Replaces the frontend's startup-only `syncCatalog()` call: index freshness is +/// sync policy and belongs in Rust (see the layer assignment in +/// docs/specs/catalog-index-search.md). Ticks every [`CATALOG_INDEX_TICK`] and +/// runs a pass when a repository exists, the server is reachable, and the index +/// is older than [`CATALOG_INDEX_TTL`]. +/// +/// TRACES: UR-065 | DR-109, IR-030 +pub fn spawn_catalog_indexer(app: tauri::AppHandle) { + tauri::async_runtime::spawn(async move { + // Check shortly after launch, then on every tick — not tick-then-check, + // which would leave a fresh install unindexed for a full tick. + tokio::time::sleep(CATALOG_INDEX_FIRST_CHECK).await; + + loop { + if let Err(e) = maybe_run_scheduled_pass(&app).await { + // Never fatal — a failed pass leaves the existing index in place + // and we retry on the next tick. + warn!("[Catalog] Scheduled index pass skipped: {}", e); + } + + tokio::time::sleep(CATALOG_INDEX_TICK).await; + } + }); +} + +/// One scheduler tick: check the preconditions, then index if due. +async fn maybe_run_scheduled_pass(app: &tauri::AppHandle) -> Result<(), String> { + if INDEX_IN_PROGRESS.load(Ordering::SeqCst) { + return Ok(()); + } + + let db_service = { + let db = app.state::(); + let database = db.0.lock().map_err(|e| e.to_string())?; + Arc::new(database.service()) + }; + + if !index_is_due( + read_last_sync(&db_service).await.as_deref(), + chrono::Utc::now(), + CATALOG_INDEX_TTL, + ) { + return Ok(()); + } + + // Offline: leave the index alone. The crawl would fail every library and, + // more importantly, a partial crawl must never reach the deletion sweep. + { + let monitor = app.state::(); + let monitor = monitor.0.lock().await; + if !monitor.get_status().await.is_server_reachable { + return Ok(()); + } + } + + let repo = { + let manager = app.state::(); + let handles = manager.0.handles(); + let Some(handle) = handles.first() else { + // Not signed in yet. + return Ok(()); + }; + manager.0.get(handle).ok_or("Repository not found")? + }; + + info!("[Catalog] Index is stale; starting a scheduled pass"); + let _ = app.emit( + CATALOG_INDEX_EVENT, + CatalogIndexEvent { + state: "started".to_string(), + items_cached: 0, + items_pruned: 0, + libraries_failed: 0, + error: None, + }, + ); + + match run_index_pass(repo, db_service).await { + Ok(result) => { + let _ = app.emit( + CATALOG_INDEX_EVENT, + CatalogIndexEvent { + state: "finished".to_string(), + items_cached: result.items_cached, + items_pruned: result.items_pruned, + libraries_failed: result.libraries_failed, + error: None, + }, + ); + Ok(()) + } + Err(e) => { + let _ = app.emit( + CATALOG_INDEX_EVENT, + CatalogIndexEvent { + state: "failed".to_string(), + items_cached: 0, + items_pruned: 0, + libraries_failed: 0, + error: Some(e.clone()), + }, + ); + Err(e) + } + } +} + /// Report the last-synced timestamp so the UI can show a hint / decide whether /// to trigger a fresh sync. #[tauri::command] @@ -378,6 +651,43 @@ mod tests { use rusqlite::Connection; use std::sync::Mutex; + /// The re-index policy. Pure, so it is testable without a clock, a server or + /// a database — which is the reason it was factored out of the scheduler. + /// + /// TRACES: UR-065 | DR-109 | UT-115 + #[test] + fn test_index_is_due() { + let ttl = Duration::from_secs(6 * 60 * 60); + let now = chrono::DateTime::parse_from_rfc3339("2026-08-04T12:00:00+00:00") + .unwrap() + .with_timezone(&chrono::Utc); + + // Never indexed => due. This is the first-run case. + assert!(index_is_due(None, now, ttl)); + + // Indexed 7 hours ago => past the 6h TTL => due. + assert!(index_is_due(Some("2026-08-04T05:00:00+00:00"), now, ttl)); + + // Indexed 1 hour ago => fresh => not due. This is what stops the + // scheduler re-crawling every tick. + assert!(!index_is_due(Some("2026-08-04T11:00:00+00:00"), now, ttl)); + + // Exactly at the TTL boundary counts as due. + assert!(index_is_due(Some("2026-08-04T06:00:00+00:00"), now, ttl)); + + // A corrupt stored value must trigger a re-index, not freeze the + // catalog forever behind an unparseable timestamp. + assert!(index_is_due(Some("not-a-timestamp"), now, ttl)); + assert!(index_is_due(Some(""), now, ttl)); + + // A timestamp in the future (clock skew, or a restored backup) is not + // due — it ages into due-ness rather than causing a crawl every tick. + assert!(!index_is_due(Some("2026-08-05T00:00:00+00:00"), now, ttl)); + + // Offsets other than UTC are compared as instants, not as strings. + assert!(!index_is_due(Some("2026-08-04T13:30:00+02:00"), now, ttl)); + } + fn test_db() -> Arc { let conn = Connection::open_in_memory().unwrap(); conn.execute_batch( diff --git a/src-tauri/src/commands/download/mod.rs b/src-tauri/src/commands/download/mod.rs index 9d9686d8..16e065a7 100644 --- a/src-tauri/src/commands/download/mod.rs +++ b/src-tauri/src/commands/download/mod.rs @@ -270,6 +270,19 @@ pub async fn download_item( if !can_download { warn!("Storage limit reached. Attempting to free space..."); + // Reclaim expired temporary entries first: they are dead weight, so + // freeing them may avoid evicting cache that is still within its + // life. Best-effort — a failure here just means eviction does more. + // TRACES: UR-071 | DR-127 + match cache_arc + .reclaim_expired_async(&db_service, &user_id, &chrono::Utc::now().to_rfc3339()) + .await + { + Ok(n) if n > 0 => info!("Reclaimed {} expired cache entries", n), + Ok(_) => {} + Err(e) => warn!("Expired-entry reclaim failed: {}", e), + } + // Try to evict LRU items to make space match cache_arc .evict_lru_async(&db_service, &user_id, size as u64) diff --git a/src-tauri/src/commands/favorites.rs b/src-tauri/src/commands/favorites.rs new file mode 100644 index 00000000..eaeba618 --- /dev/null +++ b/src-tauri/src/commands/favorites.rs @@ -0,0 +1,367 @@ +//! Pushing favourite toggles made while the server was unreachable. +//! +//! Favouriting works offline: `storage_toggle_favorite` writes the local +//! `user_data` row and sets `pending_sync = 1`. Until DR-120 nothing ever +//! cleared that flag — the offline `mark_favorite`/`unmark_favorite` are no-ops +//! and `syncService.queueFavorite` had no callers — so an offline toggle was +//! silently lost. +//! +//! The drain lives in Rust, not the frontend, because it must run whether or +//! not any view is mounted; a drain started by a component dies with it. +//! +//! TRACES: UR-069 | DR-120 | UT-103 + +use std::sync::Arc; + +use async_trait::async_trait; +use log::{debug, info, warn}; +use tauri::{Emitter, Listener, Manager}; + +use crate::repository::types::RepoError; +use crate::repository::MediaRepository; +use crate::storage::db_service::{DatabaseService, Query, QueryParam, RusqliteService}; + +/// The subset of the repository the drain needs. +/// +/// Narrow on purpose: a test double for `MediaRepository` would be forty +/// unimplemented methods, which is how a drain ends up untested. +#[async_trait] +pub trait FavoriteSink: Send + Sync { + async fn push_favorite(&self, item_id: &str, is_favorite: bool) -> Result<(), RepoError>; +} + +#[async_trait] +impl FavoriteSink for T { + async fn push_favorite(&self, item_id: &str, is_favorite: bool) -> Result<(), RepoError> { + if is_favorite { + self.mark_favorite(item_id).await + } else { + self.unmark_favorite(item_id).await + } + } +} + +/// A local favourite change still waiting to reach the server. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PendingFavorite { + pub item_id: String, + pub is_favorite: bool, +} + +/// Read every favourite change this user has pending. +async fn read_pending( + db: &Arc, + user_id: &str, +) -> Result, String> { + db.query_many( + Query::with_params( + "SELECT item_id, is_favorite FROM user_data \ + WHERE user_id = ? AND pending_sync = 1 AND is_favorite IS NOT NULL", + vec![QueryParam::String(user_id.to_string())], + ), + |row| { + Ok(PendingFavorite { + item_id: row.get::<_, String>(0)?, + is_favorite: row.get::<_, Option>(1)?.unwrap_or(0) != 0, + }) + }, + ) + .await +} + +/// Push pending favourite changes to the server and clear their flags. +/// +/// Returns the ids that reached the server, for the `favorites-changed` event. +/// A row whose push fails keeps `pending_sync = 1` and is retried on the next +/// reconnect rather than being dropped. +/// +/// TRACES: UR-069 | DR-120 | UT-103 +pub async fn drain_pending_favorites( + db: &Arc, + sink: &dyn FavoriteSink, + user_id: &str, +) -> Result, String> { + let pending = read_pending(db, user_id).await?; + if pending.is_empty() { + return Ok(Vec::new()); + } + + info!( + "[Favorites] Pushing {} favourite change(s) queued while offline", + pending.len() + ); + + let mut pushed = Vec::new(); + for change in pending { + match sink + .push_favorite(&change.item_id, change.is_favorite) + .await + { + Ok(()) => { + let cleared = db + .execute(Query::with_params( + "UPDATE user_data SET pending_sync = 0, synced_at = ? \ + WHERE user_id = ? AND item_id = ?", + vec![ + QueryParam::String(chrono::Utc::now().to_rfc3339()), + QueryParam::String(user_id.to_string()), + QueryParam::String(change.item_id.clone()), + ], + )) + .await; + + match cleared { + Ok(_) => pushed.push(change.item_id), + // The server took it; failing to clear the flag only means + // we push it again next time, which is harmless. + Err(e) => warn!( + "[Favorites] Pushed {} but could not clear pending_sync: {}", + change.item_id, e + ), + } + } + Err(e) => { + // Still pending — retried on the next reconnect. + debug!( + "[Favorites] Deferring {}, server rejected the push: {:?}", + change.item_id, e + ); + } + } + } + + Ok(pushed) +} + +/// Drain on every offline→online transition. +/// +/// Hooks the `connectivity:reconnected` event the `ConnectivityMonitor` +/// already emits, rather than polling — reachability is derived from real +/// traffic (DR-055) and this just reacts to it. +/// +/// TRACES: UR-069 | DR-120 +pub fn spawn_favorites_drain(app: tauri::AppHandle) { + let handle = app.clone(); + app.listen("connectivity:reconnected", move |_event| { + let app = handle.clone(); + tauri::async_runtime::spawn(async move { + if let Err(e) = run_drain(&app).await { + warn!("[Favorites] Drain skipped: {}", e); + } + }); + }); +} + +async fn run_drain(app: &tauri::AppHandle) -> Result<(), String> { + let db_service: Arc = { + let db = app.state::(); + let database = db.0.lock().map_err(|e| e.to_string())?; + Arc::new(database.service()) + }; + + let (repo, user_id) = { + let manager = app.state::(); + let handles = manager.0.handles(); + let Some(handle) = handles.first() else { + // Not signed in — nothing to push on behalf of. + return Ok(()); + }; + let repo = manager.0.get(handle).ok_or("Repository not found")?; + let user_id = repo.user_id().to_string(); + (repo, user_id) + }; + + let pushed = drain_pending_favorites(&db_service, repo.as_ref(), &user_id).await?; + + if !pushed.is_empty() { + let event = crate::commands::repository::FavoritesChangedEvent { item_ids: pushed }; + if let Err(e) = app.emit(crate::commands::repository::FAVORITES_CHANGED_EVENT, &event) { + warn!("[Favorites] Failed to emit change event: {}", e); + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use rusqlite::Connection; + use std::sync::Mutex; + + /// Records what the server was asked to do, and can be told to fail. + struct RecordingSink { + calls: Mutex>, + fail_for: Option, + } + + impl RecordingSink { + fn new() -> Self { + Self { + calls: Mutex::new(Vec::new()), + fail_for: None, + } + } + + fn failing_for(item_id: &str) -> Self { + Self { + calls: Mutex::new(Vec::new()), + fail_for: Some(item_id.to_string()), + } + } + + fn calls(&self) -> Vec<(String, bool)> { + let mut calls = self.calls.lock().unwrap().clone(); + calls.sort(); + calls + } + } + + #[async_trait] + impl FavoriteSink for RecordingSink { + async fn push_favorite(&self, item_id: &str, is_favorite: bool) -> Result<(), RepoError> { + if self.fail_for.as_deref() == Some(item_id) { + return Err(RepoError::Offline); + } + self.calls + .lock() + .unwrap() + .push((item_id.to_string(), is_favorite)); + Ok(()) + } + } + + fn test_db() -> Arc { + let conn = Connection::open_in_memory().unwrap(); + conn.execute_batch( + r#" + CREATE TABLE user_data ( + user_id TEXT NOT NULL, + item_id TEXT NOT NULL, + is_favorite INTEGER, + synced_at TEXT, + pending_sync INTEGER DEFAULT 0, + PRIMARY KEY (user_id, item_id) + ); + "#, + ) + .unwrap(); + Arc::new(RusqliteService::new(Arc::new(Mutex::new(conn)))) + } + + async fn seed(db: &Arc, rows: &[(&str, &str, i32, i32)]) { + for (user, item, fav, pending) in rows { + db.execute(Query::with_params( + "INSERT INTO user_data (user_id, item_id, is_favorite, pending_sync) \ + VALUES (?, ?, ?, ?)", + vec![ + QueryParam::String(user.to_string()), + QueryParam::String(item.to_string()), + QueryParam::Int(*fav), + QueryParam::Int(*pending), + ], + )) + .await + .unwrap(); + } + } + + async fn pending_flag(db: &Arc, item_id: &str) -> Option { + db.query_optional( + Query::with_params( + "SELECT pending_sync FROM user_data WHERE item_id = ?", + vec![QueryParam::String(item_id.to_string())], + ), + |row| row.get::<_, Option>(0), + ) + .await + .unwrap() + .flatten() + } + + /// UT-103 — the core of the bug: a favourite toggled while offline reaches + /// the server on reconnect, and stops being pending. + /// + /// TRACES: UR-069 | DR-120 | UT-103 + #[tokio::test] + async fn test_drain_pushes_pending_favorites_and_clears_the_flag() { + let db = test_db(); + seed( + &db, + &[ + ("u1", "marked-offline", 1, 1), + ("u1", "unmarked-offline", 0, 1), + // Already synced — must not be pushed again. + ("u1", "already-synced", 1, 0), + ], + ) + .await; + + let sink = RecordingSink::new(); + let pushed = drain_pending_favorites(&db, &sink, "u1").await.unwrap(); + + assert_eq!( + sink.calls(), + vec![ + ("marked-offline".to_string(), true), + ("unmarked-offline".to_string(), false), + ], + "both pending changes push, with their direction preserved" + ); + assert_eq!(pushed.len(), 2); + assert_eq!(pending_flag(&db, "marked-offline").await, Some(0)); + assert_eq!(pending_flag(&db, "unmarked-offline").await, Some(0)); + } + + /// A push that fails keeps its row pending, so the change is retried rather + /// than dropped on the floor. + /// + /// TRACES: UR-069 | DR-120 | UT-103 + #[tokio::test] + async fn test_drain_leaves_failed_pushes_pending() { + let db = test_db(); + seed(&db, &[("u1", "ok", 1, 1), ("u1", "boom", 1, 1)]).await; + + let sink = RecordingSink::failing_for("boom"); + let pushed = drain_pending_favorites(&db, &sink, "u1").await.unwrap(); + + assert_eq!(pushed, vec!["ok".to_string()]); + assert_eq!(pending_flag(&db, "ok").await, Some(0)); + assert_eq!( + pending_flag(&db, "boom").await, + Some(1), + "a failed push must stay queued for the next reconnect" + ); + } + + /// Another user's queued changes are not pushed with this user's token. + /// + /// TRACES: UR-069 | DR-120 | UT-103 + #[tokio::test] + async fn test_drain_only_touches_the_given_user() { + let db = test_db(); + seed(&db, &[("u1", "mine", 1, 1), ("u2", "theirs", 1, 1)]).await; + + let sink = RecordingSink::new(); + let pushed = drain_pending_favorites(&db, &sink, "u1").await.unwrap(); + + assert_eq!(pushed, vec!["mine".to_string()]); + assert_eq!(pending_flag(&db, "theirs").await, Some(1)); + } + + /// Nothing pending means no server calls at all — a reconnect must not + /// generate traffic just because it happened. + /// + /// TRACES: UR-069 | DR-120 | UT-103 + #[tokio::test] + async fn test_drain_is_a_noop_when_nothing_is_pending() { + let db = test_db(); + seed(&db, &[("u1", "synced", 1, 0)]).await; + + let sink = RecordingSink::new(); + let pushed = drain_pending_favorites(&db, &sink, "u1").await.unwrap(); + + assert!(pushed.is_empty()); + assert!(sink.calls().is_empty()); + } +} diff --git a/src-tauri/src/commands/mod.rs b/src-tauri/src/commands/mod.rs index c5ffa6fe..005c2685 100644 --- a/src-tauri/src/commands/mod.rs +++ b/src-tauri/src/commands/mod.rs @@ -7,6 +7,7 @@ pub mod connectivity; pub mod conversions; pub mod device; pub mod download; +pub mod favorites; pub mod offline; pub mod playback_mode; pub mod playback_reporting; diff --git a/src-tauri/src/commands/player/mod.rs b/src-tauri/src/commands/player/mod.rs index efc40db2..dcf3ce0c 100644 --- a/src-tauri/src/commands/player/mod.rs +++ b/src-tauri/src/commands/player/mod.rs @@ -379,16 +379,49 @@ pub(super) async fn create_media_item( }) } -/// Check if an item has a completed download -pub(super) async fn check_for_local_download( - db: &DatabaseWrapper, +/// Pick the source for an audio-only handoff. +/// +/// A downloaded file wins over the audio-only stream URL. No transcode or audio +/// extraction is involved or wanted: the native backends already play a video +/// container without decoding its video — the Linux MPV backend is configured +/// with `video: no`, and ExoPlayer simply has no surface to render to when the +/// item is `MediaType::Audio`. Producing a separate audio-only file would cost +/// CPU and battery, need an encoder the project does not ship, and leave a +/// second artifact to keep in step with the first. +/// +/// TRACES: UR-071 | DR-128 | UT-119 +pub(super) fn background_audio_source( + local_path: Option, + stream_url: String, + item_id: &str, +) -> MediaSource { + match local_path { + Some(path) => MediaSource::Local { + file_path: PathBuf::from(path), + jellyfin_item_id: Some(item_id.to_string()), + }, + None => MediaSource::Remote { + stream_url, + jellyfin_item_id: item_id.to_string(), + }, + } +} + +/// Resolve the on-disk file backing a completed download, if there is one. +/// +/// A `downloads` row is not proof of a file: it can outlive the bytes (manual +/// deletion, a cleared cache directory, a restored database). Every caller wants +/// "can I play this from disk right now", so existence is checked here rather +/// than trusted from the row. +/// +/// Split out from [`check_for_local_download`] so the resolution is testable +/// without a `DatabaseWrapper`, and reusable by the video path. +/// +/// TRACES: UR-071 | DR-123 | UT-116 +pub(super) async fn resolve_local_media_path( + db_service: &Arc, item_id: &str, ) -> Result, String> { - let db_service = { - let database = db.0.lock().map_err(|e| e.to_string())?; - Arc::new(database.service()) - }; - let query = Query::with_params( "SELECT file_path FROM downloads WHERE item_id = ? AND status = 'completed' LIMIT 1", vec![QueryParam::String(item_id.to_string())], @@ -399,22 +432,58 @@ pub(super) async fn check_for_local_download( .await .map_err(|e| e.to_string())?; - // Verify the file actually exists on disk - if let Some(ref file_path) = path { - if std::path::Path::new(file_path).exists() { - Ok(path) - } else { + match path { + Some(ref file_path) if std::path::Path::new(file_path).exists() => Ok(path), + Some(file_path) => { warn!( "[Player] Download entry exists in DB but file not found: {}", file_path ); Ok(None) } - } else { - Ok(None) + None => Ok(None), } } +/// Check if an item has a completed download +pub(super) async fn check_for_local_download( + db: &DatabaseWrapper, + item_id: &str, +) -> Result, String> { + let db_service = { + let database = db.0.lock().map_err(|e| e.to_string())?; + Arc::new(database.service()) + }; + + resolve_local_media_path(&db_service, item_id).await +} + +/// The on-disk path for a downloaded item, for playback surfaces that resolve +/// their own source rather than going through the queue. +/// +/// The video player is the reason this exists: audio has preferred local files +/// since queue construction, but video asks the repository for a stream URL and +/// never consults `downloads`, so a downloaded film was still streamed — costing +/// bandwidth that had already been spent and failing outright when offline. +/// +/// Returns `None` when nothing is downloaded *or* the file is missing, so the +/// caller falls back to streaming. +/// +/// TRACES: UR-071 | DR-123 | UT-116 +#[tauri::command] +#[specta::specta] +pub async fn player_local_media_path( + db: State<'_, DatabaseWrapper>, + item_id: String, +) -> Result, String> { + let db_service = { + let database = db.0.lock().map_err(|e| e.to_string())?; + Arc::new(database.service()) + }; + + resolve_local_media_path(&db_service, &item_id).await +} + /// Re-point queued streaming items at completed local downloads. /// /// Sources are resolved once when the queue is built, so downloads that finish @@ -574,6 +643,7 @@ pub async fn player_play_item( pub async fn player_enter_background_audio( player: State<'_, PlayerStateWrapper>, session: State<'_, MediaSessionManagerWrapper>, + db: State<'_, DatabaseWrapper>, item: PlayItemRequest, position_seconds: f64, ) -> Result { @@ -582,6 +652,19 @@ pub async fn player_enter_background_audio( item.title, position_seconds ); + // Prefer the downloaded file over the audio-only stream URL the frontend + // resolved. Handing the native backend a local video container yields + // audio-only playback for free — no transcode, no second artifact. + // TRACES: UR-071 | DR-128 + let local_path = check_for_local_download(&db, &item.id).await?; + if local_path.is_some() { + info!( + "player_enter_background_audio: using downloaded file for {}", + item.id + ); + } + let source = background_audio_source(local_path, item.stream_url, &item.id); + // Build an AUDIO media item pointing at the audio-only stream. We do not use // create_media_item() because that hardcodes MediaType::Video; background // audio must be Audio so no video decode is started. @@ -605,10 +688,7 @@ pub async fn player_enter_background_audio( duration: item.duration_seconds, artwork_url: None, media_type: MediaType::Audio, - source: MediaSource::Remote { - stream_url: item.stream_url, - jellyfin_item_id: item.id.clone(), - }, + source, video_codec: None, needs_transcoding: false, video_width: None, @@ -2379,6 +2459,128 @@ pub async fn player_disable_jellyfin(player: State<'_, PlayerStateWrapper>) -> R #[cfg(test)] mod tests { + /// The audio-only handoff must play a downloaded file when there is one, + /// rather than fetching an audio-only stream for media already on disk. + /// + /// TRACES: UR-071 | DR-128 | UT-119 + #[test] + fn test_background_audio_source_prefers_local_file() { + use super::background_audio_source; + use crate::player::MediaSource; + use std::path::PathBuf; + + let local = background_audio_source( + Some("/downloads/ep1.mkv".to_string()), + "https://server/audio-only".to_string(), + "ep-1", + ); + match local { + MediaSource::Local { + file_path, + jellyfin_item_id, + } => { + assert_eq!(file_path, PathBuf::from("/downloads/ep1.mkv")); + // The Jellyfin id must survive so progress still syncs back. + assert_eq!(jellyfin_item_id.as_deref(), Some("ep-1")); + } + other => panic!("expected a local source, got {:?}", other), + } + + let remote = background_audio_source(None, "https://server/audio-only".to_string(), "ep-1"); + match remote { + MediaSource::Remote { + stream_url, + jellyfin_item_id, + } => { + assert_eq!(stream_url, "https://server/audio-only"); + assert_eq!(jellyfin_item_id, "ep-1"); + } + other => panic!("expected a remote source, got {:?}", other), + } + } + + /// A downloaded item must resolve to its file, and a `downloads` row whose + /// file has gone must resolve to `None` so the caller falls back to + /// streaming instead of handing the player a path that cannot be opened. + /// + /// TRACES: UR-071 | DR-123 | UT-116 + #[tokio::test] + async fn test_resolve_local_media_path() { + use super::resolve_local_media_path; + use crate::storage::db_service::{DatabaseService, Query, RusqliteService}; + use rusqlite::Connection; + use std::sync::{Arc, Mutex}; + + let conn = Connection::open_in_memory().unwrap(); + conn.execute( + "CREATE TABLE downloads (id INTEGER PRIMARY KEY, item_id TEXT, status TEXT, file_path TEXT)", + [], + ) + .unwrap(); + let db_service = Arc::new(RusqliteService::new(Arc::new(Mutex::new(conn)))); + + // A real file on disk, so the existence check passes. + let present = std::env::temp_dir().join("jellytau-resolve-local-test.mp4"); + std::fs::write(&present, b"x").unwrap(); + let present_str = present.to_string_lossy().to_string(); + + for (item, status, path) in [ + ("downloaded", "completed", present_str.as_str()), + ("still-going", "downloading", present_str.as_str()), + ( + "file-gone", + "completed", + "/nonexistent/jellytau/missing.mp4", + ), + ] { + db_service + .execute(Query::with_params( + "INSERT INTO downloads (item_id, status, file_path) VALUES (?, ?, ?)", + vec![ + crate::storage::db_service::QueryParam::String(item.to_string()), + crate::storage::db_service::QueryParam::String(status.to_string()), + crate::storage::db_service::QueryParam::String(path.to_string()), + ], + )) + .await + .unwrap(); + } + + assert_eq!( + resolve_local_media_path(&db_service, "downloaded") + .await + .unwrap() + .as_deref(), + Some(present_str.as_str()), + "a completed download with its file present must resolve" + ); + + assert_eq!( + resolve_local_media_path(&db_service, "still-going") + .await + .unwrap(), + None, + "an in-progress download is not playable from disk" + ); + + assert_eq!( + resolve_local_media_path(&db_service, "file-gone") + .await + .unwrap(), + None, + "a row whose file has gone must fall back to streaming, not hand over a dead path" + ); + + assert_eq!( + resolve_local_media_path(&db_service, "never-heard-of-it") + .await + .unwrap(), + None + ); + + let _ = std::fs::remove_file(&present); + } + /// Queue items enqueued as Remote must flip to Local once a completed /// download exists on disk — this is what makes preloaded tracks (and /// offline playback after a connection drop) actually use the cache. diff --git a/src-tauri/src/commands/player/timers.rs b/src-tauri/src/commands/player/timers.rs index ca56ac64..5bef84c4 100644 --- a/src-tauri/src/commands/player/timers.rs +++ b/src-tauri/src/commands/player/timers.rs @@ -142,7 +142,7 @@ pub async fn player_play_next_episode( /// - Frontend when audio track ends via backend event - no itemId/repositoryHandle needed /// - Android JNI callback also triggers this logic directly /// -/// TRACES: UR-023, UR-026, UR-040 | DR-047, DR-052 +/// TRACES: UR-023, UR-026, UR-040 | DR-047, DR-052, DR-129 #[tauri::command] #[specta::specta] pub async fn player_on_playback_ended( @@ -257,6 +257,23 @@ pub async fn player_on_playback_ended( .await; } } + AutoplayDecision::ResumeStream { position } => { + // The stream was cut short by the network, not by the media ending. + // Re-open it where it died — no queue clearing, no PlaybackEnded, and + // above all no leaving the player parked in ExoPlayer's STATE_ENDED, + // where the next play intent restarts the item from 0:00. + log::info!( + "[Autoplay] Decision: Resume truncated stream at {:.1}s", + position + ); + let controller = controller_arc.lock().await; + if let Err(e) = controller.resume_stream_at(position).await { + log::error!("[Autoplay] Failed to resume truncated stream: {}", e); + if let Some(emitter) = controller.event_emitter() { + emitter.emit(PlayerStatusEvent::PlaybackEnded); + } + } + } } Ok(()) diff --git a/src-tauri/src/commands/repository.rs b/src-tauri/src/commands/repository.rs index c293c4a5..cea15dfd 100644 --- a/src-tauri/src/commands/repository.rs +++ b/src-tauri/src/commands/repository.rs @@ -41,6 +41,19 @@ impl RepositoryManager { repos.get(handle).cloned() } + /// Handles of every live repository. + /// + /// The background catalog indexer (DR-109) runs outside any command, so it + /// has no handle passed in and needs to discover one. In practice there is a + /// single signed-in repository; returning all of them avoids inventing an + /// "active" concept the rest of the code does not have. + /// + /// TRACES: UR-065 | DR-109 + pub fn handles(&self) -> Vec { + let repos = self.repositories.lock_safe(); + repos.keys().cloned().collect() + } + pub fn destroy(&self, handle: &str) { let mut repos = self.repositories.lock_safe(); repos.remove(handle); @@ -780,6 +793,108 @@ pub async fn repository_mark_favorite( .map_err(|e| format!("{:?}", e)) } +/// Tauri event announcing that favourite state changed behind the UI's back — +/// either because the server disagreed with the cache on a background refresh, +/// or because pending offline toggles were pushed on reconnect. +/// +/// TRACES: UR-069 | DR-120 +pub const FAVORITES_CHANGED_EVENT: &str = "favorites-changed"; + +/// Payload for [`FAVORITES_CHANGED_EVENT`] — the ids whose favourite state +/// actually flipped, so the frontend refreshes those rather than everything. +/// +/// TRACES: UR-069 | DR-120 | UT-107 +#[derive(specta::Type, Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FavoritesChangedEvent { + pub item_ids: Vec, +} + +/// Ids whose favourite state differs between what we showed and what the server +/// has — favourited elsewhere since the cache was written, or un-favourited +/// elsewhere. +/// +/// Pulled out of the command so the "emit nothing when nothing changed" rule is +/// testable: an unchanged set must leave a quiet page quiet rather than +/// triggering a refetch on every visit. +/// +/// TRACES: UR-069 | DR-120 | UT-107 +fn changed_favorite_ids( + cached: &std::collections::HashSet, + server: &std::collections::HashSet, +) -> Vec { + let mut changed: Vec = server.symmetric_difference(cached).cloned().collect(); + // Deterministic order so the event payload does not depend on hash seeding. + changed.sort(); + changed +} + +/// Everything the viewer has favourited, across libraries, narrowed by scope. +/// +/// Two-phase like `repository_search`: the local answer returns immediately and +/// a background server pass emits `favorites-changed` when the server's set +/// differs. Without the second phase a favourite marked in another client shows +/// up only on the *second* visit to the page, since the cache-first read hands +/// back local rows and the refresh is invisible to the frontend. +/// +/// TRACES: UR-067 | DR-115, DR-120, JA-033 | UT-107 +#[tauri::command] +#[specta::specta] +pub async fn repository_get_favorites( + app: AppHandle, + manager: State<'_, RepositoryManagerWrapper>, + handle: String, + scope: SearchScope, + options: Option, +) -> Result { + let repo = manager.0.get(&handle).ok_or("Repository not found")?; + + let cache_result = repo + .get_favorites_cache_only(scope, options.clone()) + .await + .unwrap_or_else(|e| { + debug!("[Favorites] Cache miss/timeout: {:?}", e); + SearchResult { + items: Vec::new(), + total_record_count: 0, + } + }); + + // With "Show all server media" off the local answer is authoritative + // (DR-080) — don't go behind the user's back to the server. + if !crate::repository::offline::include_catalog_browse() { + return Ok(cache_result); + } + + let repo_bg = repo.clone(); + let cached_ids: std::collections::HashSet = + cache_result.items.iter().map(|i| i.id.clone()).collect(); + tauri::async_runtime::spawn(async move { + match repo_bg.get_favorites_server_only(scope, options).await { + Ok(server_result) => { + let server_ids: std::collections::HashSet = + server_result.items.iter().map(|i| i.id.clone()).collect(); + let changed = changed_favorite_ids(&cached_ids, &server_ids); + + if !changed.is_empty() { + let event = FavoritesChangedEvent { item_ids: changed }; + if let Err(e) = app.emit(FAVORITES_CHANGED_EVENT, &event) { + error!("[Favorites] Failed to emit change event: {}", e); + } + } + } + Err(e) => { + warn!( + "[Favorites] Server refresh failed, keeping cached favourites: {:?}", + e + ); + } + } + }); + + Ok(cache_result) +} + /// Unmark an item as favorite #[tauri::command] #[specta::specta] @@ -853,6 +968,44 @@ mod tests { assert!(manager.get("any-handle").is_none()); } + fn ids(values: &[&str]) -> std::collections::HashSet { + values.iter().map(|v| v.to_string()).collect() + } + + /// UT-107 — the background refresh reports only what actually changed. + /// + /// TRACES: UR-069 | DR-120 | UT-107 + #[test] + fn test_changed_favorite_ids_reports_both_directions() { + // Favourited in another client since we cached. + assert_eq!( + changed_favorite_ids(&ids(&["a"]), &ids(&["a", "b"])), + vec!["b".to_string()] + ); + + // Un-favourited in another client. + assert_eq!( + changed_favorite_ids(&ids(&["a", "b"]), &ids(&["a"])), + vec!["b".to_string()] + ); + + // Both at once, in a stable order. + assert_eq!( + changed_favorite_ids(&ids(&["a", "b"]), &ids(&["b", "c"])), + vec!["a".to_string(), "c".to_string()] + ); + } + + /// An unchanged set emits nothing — otherwise every visit to the page would + /// fire an event and trigger a pointless refetch. + /// + /// TRACES: UR-069 | DR-120 | UT-107 + #[test] + fn test_changed_favorite_ids_is_empty_when_nothing_moved() { + assert!(changed_favorite_ids(&ids(&["a", "b"]), &ids(&["b", "a"])).is_empty()); + assert!(changed_favorite_ids(&ids(&[]), &ids(&[])).is_empty()); + } + #[test] fn test_repository_manager_wrapper_structure() { let manager = RepositoryManager::new(); diff --git a/src-tauri/src/commands/storage/people.rs b/src-tauri/src/commands/storage/people.rs index 11ba5b4f..9e5ad405 100644 --- a/src-tauri/src/commands/storage/people.rs +++ b/src-tauri/src/commands/storage/people.rs @@ -47,10 +47,25 @@ pub async fn storage_save_person( }; let query = Query::with_params( - "INSERT OR REPLACE INTO people ( + // A real UPSERT, not INSERT OR REPLACE — `people` is now backed by the + // `people_fts` index (migration 022), and REPLACE would orphan an index + // entry on every re-cache: it fires no AFTER DELETE trigger without + // `recursive_triggers`, and reassigns the rowid that `content_rowid` + // refers to. Same defect as DR-110 fixed for `items`. + // + // TRACES: UR-065 | DR-110, DR-111 + "INSERT INTO people ( id, server_id, name, overview, primary_image_tag, premiere_date, end_date, synced_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)", + ) VALUES (?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) + ON CONFLICT(id) DO UPDATE SET + server_id = excluded.server_id, + name = excluded.name, + overview = excluded.overview, + primary_image_tag = excluded.primary_image_tag, + premiere_date = excluded.premiere_date, + end_date = excluded.end_date, + synced_at = CURRENT_TIMESTAMP", vec![ QueryParam::String(person.id), QueryParam::String(person.server_id), diff --git a/src-tauri/src/download/cache.rs b/src-tauri/src/download/cache.rs index 25c3963a..905344f1 100644 --- a/src-tauri/src/download/cache.rs +++ b/src-tauri/src/download/cache.rs @@ -26,6 +26,12 @@ pub struct CacheConfig { pub storage_limit: u64, /// Only cache on WiFi pub wifi_only: bool, + /// How long a temporary (`download_source = 'auto'`) download lives before + /// it is reclaimed, in hours. 0 disables expiry, leaving space pressure as + /// the only reclaim trigger. + /// + /// TRACES: UR-071 | DR-127 + pub temporary_ttl_hours: u64, } impl Default for CacheConfig { @@ -37,6 +43,10 @@ impl Default for CacheConfig { album_affinity_threshold: 3, storage_limit: 10 * 1024 * 1024 * 1024, // 10GB wifi_only: false, // Allow preloading on any connection by default + // A week: long enough that re-watching over a weekend still hits + // disk, short enough that a one-off play does not hold space + // indefinitely. + temporary_ttl_hours: 24 * 7, } } } @@ -183,7 +193,92 @@ impl SmartCache { current_size + new_size <= storage_limit } - /// Evict least recently used items to make space (async version) + /// Reclaim temporary downloads whose life limit has passed. + /// + /// The time-based half of the temporary tier (DR-127); [`evict_lru_async`] + /// is the space-pressure half. A row is reclaimed by whichever fires first. + /// + /// Scoped to `download_source = 'auto'` for the same reason eviction is: a + /// `'user'` row is someone's own download and has no expiry. `COALESCE` + /// guards rows predating migration 012, whose source is NULL and whose + /// provenance must therefore be treated as the user's. + /// + /// Expiry is normally *derived* — `completed_at` plus the configured TTL — + /// rather than stamped at completion. That means a TTL change applies to + /// entries already on disk instead of only to future ones, and entries + /// predating the column expire without a backfill. `expires_at` is honoured + /// as a per-row override when something sets it. + /// + /// `now` is passed in rather than read from the clock so the policy is + /// testable without sleeping. Both sides go through SQLite's `datetime()` + /// because `completed_at` is written as `CURRENT_TIMESTAMP` + /// (`YYYY-MM-DD HH:MM:SS`) while callers pass RFC-3339 (`…T…+00:00`) — a raw + /// string comparison between the two formats is wrong, since `' ' < 'T'`. + /// + /// Returns the number of entries reclaimed. + /// + /// TRACES: UR-071 | DR-127 | UT-120 + pub async fn reclaim_expired_async( + &self, + db_service: &Arc, + user_id: &str, + now: &str, + ) -> Result { + let ttl_hours = { + let config = self.config.lock().map_err(|e| e.to_string())?; + config.temporary_ttl_hours + }; + // 0 disables time-based reclaim; space pressure remains the only trigger. + if ttl_hours == 0 { + return Ok(0); + } + + let expired: Vec<(i64, String)> = db_service + .query_many( + Query::with_params( + "SELECT id, file_path FROM downloads + WHERE user_id = ? + AND COALESCE(download_source, 'user') = 'auto' + AND status = 'completed' + AND datetime( + COALESCE(expires_at, datetime(completed_at, '+' || ? || ' hours')) + ) < datetime(?)", + vec![ + QueryParam::String(user_id.to_string()), + QueryParam::String(ttl_hours.to_string()), + QueryParam::String(now.to_string()), + ], + ), + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .await + .map_err(|e| e.to_string())?; + + let mut reclaimed = 0usize; + for (id, file_path) in expired { + // Best-effort on the file: a missing one still needs its row gone, + // or the sweep retries it forever. + let _ = std::fs::remove_file(&file_path); + db_service + .execute(Query::with_params( + "DELETE FROM downloads WHERE id = ?", + vec![QueryParam::Int64(id)], + )) + .await + .map_err(|e| e.to_string())?; + reclaimed += 1; + } + + if reclaimed > 0 { + info!("[SmartCache] Reclaimed {} expired entries", reclaimed); + } + Ok(reclaimed) + } + + /// Evict least recently used items to make space (async version). + /// + /// The space-pressure half of the temporary tier; [`reclaim_expired_async`] + /// is the time-based half. pub async fn evict_lru_async( &self, db_service: &Arc, @@ -207,10 +302,26 @@ impl SmartCache { let to_free = (current_size + space_needed) - limit; let mut freed: u64 = 0; - // Get downloads ordered by last access (oldest first) + // Only the *temporary* tier is evictable. `download_source = 'auto'` is + // precache — the cache put it there, the cache may reclaim it. A 'user' + // row is a download someone explicitly asked for; deleting it to make + // room for a predictive fetch is data loss, and because the old query + // ordered purely by `completed_at ASC` it took the oldest — typically + // exactly the film saved for a flight. + // + // COALESCE, not `= 'auto'` alone: migration 012 added the column with a + // 'user' default, but rows predating it can be NULL, and an unknown + // provenance must be treated as the user's, never as disposable. + // + // Freeing less than requested is the correct outcome when only user + // downloads remain — the caller surfaces "unable to free enough space" + // rather than silently deleting them. + // + // TRACES: UR-071 | DR-126 | UT-108 let query = Query::with_params( "SELECT id, file_size, file_path FROM downloads WHERE user_id = ? AND status = 'completed' + AND COALESCE(download_source, 'user') = 'auto' ORDER BY completed_at ASC", vec![QueryParam::String(user_id.to_string())], ); @@ -304,6 +415,205 @@ mod tests { assert!(cache.should_precache_queue()); } + /// Expiry reclaims only temporary entries that are actually past their life + /// limit — never a user's download (which has no expiry), and never a + /// temporary entry still within its life. + /// + /// TRACES: UR-071 | DR-127 | UT-120 + #[tokio::test] + async fn test_reclaim_expired_only_takes_expired_temporary_entries() { + use crate::storage::db_service::{DatabaseService, RusqliteService}; + use rusqlite::Connection; + use std::sync::{Arc, Mutex}; + + let conn = Connection::open_in_memory().unwrap(); + conn.execute( + "CREATE TABLE downloads ( + id INTEGER PRIMARY KEY, + user_id TEXT, + status TEXT, + file_size INTEGER, + file_path TEXT, + completed_at TEXT, + download_source TEXT DEFAULT 'user', + expires_at TEXT + )", + [], + ) + .unwrap(); + + // `completed_at` is in SQLite's CURRENT_TIMESTAMP format (space, not + // 'T'), deliberately: the query has to compare it against an RFC-3339 + // "now" and must not do so as raw strings. + for (path, source, completed, expires) in [ + // Completed long ago, no override => derived expiry has passed. + ("/tmp/jt-expired.mp4", "auto", "2026-01-01 00:00:00", None), + // Completed yesterday => still inside the 7-day default TTL. + ("/tmp/jt-fresh.mp4", "auto", "2026-05-31 00:00:00", None), + // Old, but an explicit override keeps it alive. + ( + "/tmp/jt-override.mp4", + "auto", + "2026-01-01 00:00:00", + Some("2026-12-01T00:00:00+00:00"), + ), + // A user download must never carry an expiry, but assert the sweep + // ignores it even if one were somehow set. + ( + "/tmp/jt-user.mp4", + "user", + "2026-01-01 00:00:00", + Some("2026-01-01T00:00:00+00:00"), + ), + ( + "/tmp/jt-user-noexp.mp4", + "user", + "2026-01-01 00:00:00", + None, + ), + ] { + conn.execute( + "INSERT INTO downloads (user_id, status, file_size, file_path, download_source, completed_at, expires_at) + VALUES ('user1', 'completed', 10, ?1, ?2, ?3, ?4)", + rusqlite::params![path, source, completed, expires], + ) + .unwrap(); + } + + let conn_arc = Arc::new(Mutex::new(conn)); + let db_service = Arc::new(RusqliteService::new(conn_arc.clone())); + let cache = SmartCache::new(CacheConfig::default()); + + let reclaimed = cache + .reclaim_expired_async(&db_service, "user1", "2026-06-01T00:00:00+00:00") + .await + .unwrap(); + assert_eq!( + reclaimed, 1, + "only the expired temporary entry is reclaimed" + ); + + let surviving: Vec = { + let guard = conn_arc.lock_safe(); + let mut stmt = guard + .prepare("SELECT file_path FROM downloads ORDER BY id") + .unwrap(); + let rows = stmt + .query_map([], |row| row.get::<_, String>(0)) + .unwrap() + .map(|r| r.unwrap()) + .collect(); + rows + }; + assert_eq!( + surviving, + vec![ + "/tmp/jt-fresh.mp4".to_string(), + "/tmp/jt-override.mp4".to_string(), + "/tmp/jt-user.mp4".to_string(), + "/tmp/jt-user-noexp.mp4".to_string(), + ], + "entries within their life, those with a later override, and every user download must survive" + ); + + // TTL of 0 disables time-based reclaim entirely. + let cache_no_ttl = SmartCache::new(CacheConfig { + temporary_ttl_hours: 0, + ..Default::default() + }); + assert_eq!( + cache_no_ttl + .reclaim_expired_async(&db_service, "user1", "2027-01-01T00:00:00+00:00") + .await + .unwrap(), + 0, + "a zero TTL leaves space pressure as the only reclaim trigger" + ); + } + + /// Eviction must only reclaim *temporary* (`download_source = 'auto'`) + /// downloads — the precache tier. A download the user explicitly asked for + /// is their file: it may be deleted by them, never by the cache making room + /// for a predictive fetch. + /// + /// Before the fix, `evict_lru_async` selected every completed row ordered by + /// `completed_at ASC` with no source filter, so hitting the storage limit + /// deleted the *oldest* download — typically the film someone downloaded for + /// a flight — in favour of a newer auto-precached track. + /// + /// TRACES: UR-071 | DR-126 | UT-108 + #[tokio::test] + async fn test_evict_lru_never_deletes_user_downloads() { + use crate::storage::db_service::RusqliteService; + use rusqlite::Connection; + use std::sync::{Arc, Mutex}; + + let conn = Connection::open_in_memory().unwrap(); + conn.execute( + "CREATE TABLE downloads ( + id INTEGER PRIMARY KEY, + user_id TEXT, + status TEXT, + file_size INTEGER, + file_path TEXT, + completed_at TEXT, + download_source TEXT DEFAULT 'user' + )", + [], + ) + .unwrap(); + + // The user's own download is the OLDEST, so a purely time-ordered + // eviction would take it first. + conn.execute( + "INSERT INTO downloads (user_id, status, file_size, file_path, completed_at, download_source) + VALUES ('user1', 'completed', 600, '/tmp/jellytau-test-user.mp4', '2026-01-01', 'user')", + [], + ) + .unwrap(); + // A newer, auto-precached item. + conn.execute( + "INSERT INTO downloads (user_id, status, file_size, file_path, completed_at, download_source) + VALUES ('user1', 'completed', 600, '/tmp/jellytau-test-auto.mp4', '2026-06-01', 'auto')", + [], + ) + .unwrap(); + + let conn_arc = Arc::new(Mutex::new(conn)); + let db_service = Arc::new(RusqliteService::new(conn_arc.clone())); + + let cache = SmartCache::new(CacheConfig { + storage_limit: 1000, + ..Default::default() + }); + + // 1200 bytes held against a 1000 limit: eviction must free something. + let freed = cache + .evict_lru_async(&db_service, "user1", 0) + .await + .unwrap(); + assert!(freed > 0, "eviction should have reclaimed the auto entry"); + + let surviving: Vec = { + let guard = conn_arc.lock_safe(); + let mut stmt = guard + .prepare("SELECT download_source FROM downloads ORDER BY id") + .unwrap(); + let rows = stmt + .query_map([], |row| row.get::<_, String>(0)) + .unwrap() + .map(|r| r.unwrap()) + .collect(); + rows + }; + + assert_eq!( + surviving, + vec!["user".to_string()], + "the user's own download must survive; only the 'auto' entry is evictable" + ); + } + #[tokio::test] async fn test_storage_limit_check() { use crate::storage::db_service::RusqliteService; diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 6e443ecd..f9b4ff6a 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -128,6 +128,8 @@ use commands::{ player_get_sleep_timer, player_get_status, player_get_video_settings, + // Preload commands + player_local_media_path, player_move_in_queue, player_next, player_on_playback_ended, @@ -138,7 +140,6 @@ use commands::{ player_play_next_episode, player_play_queue, player_play_tracks, - // Preload commands player_preload_upcoming, player_previous, player_remove_from_queue, @@ -187,6 +188,7 @@ use commands::{ repository_get_download_disk_usage, repository_get_downloaded_items, repository_get_downloaded_libraries, + repository_get_favorites, repository_get_genres, repository_get_image_url, repository_get_item, @@ -700,6 +702,7 @@ fn specta_builder() -> Builder { player_report_position, player_report_media_loaded, // Preload commands + player_local_media_path, player_preload_upcoming, player_set_cache_config, player_get_cache_config, @@ -893,6 +896,7 @@ fn specta_builder() -> Builder { repository_get_image_url, repository_mark_favorite, repository_unmark_favorite, + repository_get_favorites, repository_get_person, repository_get_items_by_person, repository_get_similar_items, @@ -1288,6 +1292,21 @@ pub fn run() { let playback_reporter_wrapper = PlaybackReporterWrapper(playback_reporter.clone()); app.manage(playback_reporter_wrapper); + // Keep the local search index fresh. Ownership of *when* to re-index + // sits here rather than in the frontend: it is sync policy over + // domain data, and a startup-only trigger left a long session + // searching a stale catalog. + // TRACES: UR-065 | DR-109, IR-030 + info!("[INIT] Starting background catalog indexer..."); + commands::catalog::spawn_catalog_indexer(app.handle().clone()); + + // Push favourite toggles made while the server was unreachable, on + // every reconnect. In Rust rather than the frontend so it runs + // whether or not the screen that made the change is still mounted. + // TRACES: UR-069 | DR-120 + info!("[INIT] Starting favourites drain..."); + commands::favorites::spawn_favorites_drain(app.handle().clone()); + info!("[INIT] Application setup completed successfully"); Ok(()) }) diff --git a/src-tauri/src/player/android/mod.rs b/src-tauri/src/player/android/mod.rs index 3a637381..fd023a42 100644 --- a/src-tauri/src/player/android/mod.rs +++ b/src-tauri/src/player/android/mod.rs @@ -930,6 +930,25 @@ pub extern "system" fn Java_com_dtourolle_jellytau_player_JellyTauPlayer_nativeO .await; } } + Ok(AutoplayDecision::ResumeStream { position }) => { + // ExoPlayer reported ENDED because the progressive transcode's + // connection dropped, not because the episode finished. This + // is the arm that matters while backgrounded: it needs no + // frontend echo, so the stream re-opens even with the webview + // suspended — and playback never parks in STATE_ENDED, where + // the next lockscreen/Bluetooth play restarts the item at 0:00. + log::info!( + "[Autoplay] Decision: Resume truncated stream at {:.1}s", + position + ); + let ctrl = controller.lock().await; + if let Err(e) = ctrl.resume_stream_at(position).await { + log::error!("[Autoplay] Failed to resume truncated stream: {}", e); + if let Some(emitter) = EVENT_EMITTER.get() { + emitter.emit(PlayerStatusEvent::PlaybackEnded); + } + } + } Err(e) => { log::error!("[Autoplay] Decision failed: {}", e); // Emit PlaybackEnded event on error @@ -974,11 +993,58 @@ pub extern "system" fn Java_com_dtourolle_jellytau_player_JellyTauPlayer_nativeO .get_string(&message) .map(|s| s.into()) .unwrap_or_else(|_| "Unknown error".to_string()); + let recoverable = recoverable != 0; + + // A background audio-only handoff is an mp3 the device was already decoding, + // so a recoverable failure part-way through is the network. Surfacing it as a + // player error stops playback for good (the frontend's handler calls + // player_stop); re-opening the stream where it died is the "buffer and + // resume" this actually is. Everything else keeps reporting the error. + if recoverable { + if let Some(controller) = PLAYER_CONTROLLER.get() { + let controller = controller.clone(); + let message_str = message_str.clone(); + tauri::async_runtime::spawn(async move { + let resume = controller.lock().await.recoverable_error_resume(); + let Some((position, delay_secs)) = resume else { + if let Some(emitter) = EVENT_EMITTER.get() { + emitter.emit(PlayerStatusEvent::Error { + message: message_str, + recoverable: true, + }); + } + return; + }; + + log::warn!( + "[ExoPlayer] Recoverable stream error ({}) — re-opening at {:.1}s in {}s", + message_str, + position, + delay_secs + ); + // Give a brief outage time to clear before asking the server for + // the stream again; retrying instantly just burns the budget. + tokio::time::sleep(std::time::Duration::from_secs(delay_secs)).await; + + let ctrl = controller.lock().await; + if let Err(e) = ctrl.resume_stream_at(position).await { + log::error!("[ExoPlayer] Failed to resume after error: {}", e); + if let Some(emitter) = EVENT_EMITTER.get() { + emitter.emit(PlayerStatusEvent::Error { + message: message_str, + recoverable: true, + }); + } + } + }); + return; + } + } if let Some(emitter) = EVENT_EMITTER.get() { emitter.emit(PlayerStatusEvent::Error { message: message_str, - recoverable: recoverable != 0, + recoverable, }); } } diff --git a/src-tauri/src/player/autoplay.rs b/src-tauri/src/player/autoplay.rs index 881b36f4..aa59a8d0 100644 --- a/src-tauri/src/player/autoplay.rs +++ b/src-tauri/src/player/autoplay.rs @@ -11,6 +11,10 @@ pub enum AutoplayDecision { Stop, /// Advance to next track in queue (for audio/movies) AdvanceToNext, + /// The stream ended well short of the item's runtime — the connection + /// dropped, not the media. Re-open the same stream at `position` instead of + /// running any end-of-item logic (UR-040). + ResumeStream { position: f64 }, /// Show next episode popup with countdown ShowNextEpisodePopup { current_episode: MediaItem, diff --git a/src-tauri/src/player/mod.rs b/src-tauri/src/player/mod.rs index 7037766b..bf68952f 100644 --- a/src-tauri/src/player/mod.rs +++ b/src-tauri/src/player/mod.rs @@ -11,6 +11,7 @@ pub mod seek; pub mod session; pub mod sleep_timer; pub mod state; +pub mod stream_end; #[cfg(test)] mod mpv_backend_test; @@ -54,6 +55,16 @@ pub use android::{ set_remote_volume_handler, MediaCommandHandler, RemoteVolumeHandler, }; +/// Seconds added per attempt before retrying a stream that failed with an error. +/// +/// Attempt 1 waits this long, attempt 2 twice as long, and so on — a spread that +/// covers roughly a quarter-minute of outage across the retry budget without +/// leaving the user staring at a dead notification when the network is truly gone. +/// Only *read* by the Android error callback (`#[cfg(android)]`), but compiled +/// and unit-tested on the host, hence `allow(dead_code)` off-Android. +#[cfg_attr(not(target_os = "android"), allow(dead_code))] +const RESUME_BACKOFF_STEP_SECS: u64 = 2; + /// Metadata for the lockscreen / media notification. /// /// Used to drive the Android MediaSession from Rust in remote (cast) mode, where @@ -168,6 +179,15 @@ pub struct PlayerController { // TRACES: UR-040 | DR-052 background_audio_base: Arc>, + // Budget for re-opening a stream that ended short of the item's runtime. + // + // A resume re-requests the same URL, so a server that is genuinely gone would + // otherwise end → resume → end without limit. The tracker only bounds retries + // that make no progress; a resume that plays on refills it. + // + // TRACES: UR-040 | DR-129 + stream_resume: Arc>, + // Last state reported by a webview-rendered HTML5