fix(deezer): only disk-cache an album cover when the album lacks local art (#493)

CLAUDE.md

@@ -112,7 +112,7 @@ Playlist sort dropdown (custom / title / artist / album / recently added / durat
 
 ### Integrations ([`docs/features/integrations.md`](docs/features/integrations.md))
 
-Deezer enrichment (pictures, covers, fans — cached 30 days in `metadata_artist` / `metadata_album` in `app.db`, hashes point into shared `metadata_artwork/<blake3>.jpg` so artwork renders offline; **Web Radio now-playing artwork** is the one Deezer path that is NOT disk-cached — [`fetch_radio_artwork`](src-tauri/crates/app/src/commands/deezer.rs) resolves an album cover URL from the ICY `Artist - Title` via `search_track` and returns the remote CDN link directly because a radio now-playing line is ephemeral; `PlayerContext` swaps it into `currentTrack.artwork_path` over the station favicon, token + `isRadioTrack`-guarded so a stale fetch or a library track that started meanwhile is never clobbered) · artist bio source selector (Settings → Integrations: **Last.fm** default vs **TheAudioDB** multi-language via [`metadata::theaudiodb`](src-tauri/crates/core/src/metadata/theaudiodb.rs), `app_setting['metadata.bio_source']` + `['metadata.bio_language']`; `enrich_artist_deezer` branches on it and stores `bio_source`/`bio_language` in `metadata_artist` so a switch invalidates the cached bio) · **wide artist fanart** (issue #482 — the same TheAudioDB `search.php` response carries `strArtistFanart*` / `strArtistWideThumb` / `strArtistBanner`, so `TheAudioDbClient::artist_info` returns bio **and** `fanart_url` from ONE lookup; `enrich_artist_deezer` calls it **regardless of `metadata.bio_source`** because Last.fm has no equivalent image and gating it would leave Last.fm users with no artist hero at all, cached in `metadata_artist.background_{url,hash}` + `background_fetched_at` — the "we already looked" marker without which a NULL hash is indistinguishable from "never queried" and every fanart-less artist would re-hit a rate-limited API per page visit; stamped when the API was *reached*, left NULL on a transport error so a blip retries instead of caching for the 30-day TTL) · Last.fm (bios, similar artists with Deezer picture backfill — Last.fm's `artist.getSimilar` returns generic star placeholders for every image since their artist-image API was killed in 2019, so [`similar::enrich_with_deezer_pictures`](src-tauri/crates/app/src/commands/similar.rs) joins the result against `app.metadata_artist` and fans out parallel Deezer `search_artist` calls for any cache miss before responding; scrobbler) · **per-artist offline overrides** (issue #323: `artist.custom_bio` + library-scoped `artist_similar_custom` table, both per-profile; [`commands/artist_overrides.rs`](src-tauri/crates/app/src/commands/artist_overrides.rs) write/read commands, edited from Artist Detail → "Edit info" [`ArtistMetadataEditorModal`](src/components/common/ArtistMetadataEditorModal.tsx); `enrich_artist_deezer` swaps the custom bio onto the returned payload and `get_similar_artists` short-circuits to the curated list before any cache/network, so both work offline and survive enrichment passes) · Discord RPC · Native OS track-change toast notifications ([`notifications.rs`](src-tauri/crates/app/src/notifications.rs) — `tauri-plugin-notification` bridge to Windows Action Center / macOS Notification Center / libnotify, opt-in `app_setting['notifications.track_change']` default OFF) · DLNA / UPnP MediaServer ([`docs/features/dlna.md`](docs/features/dlna.md)) · MPD protocol server ([`docs/features/mpd.md`](docs/features/mpd.md) — opt-in LAN control surface for existing MPD clients; control + queue mutation only, library browsing deliberately out of scope for v1) · **offline Web Radio catalogue** ([`commands/web_radio_catalogue.rs`](src-tauri/crates/app/src/commands/web_radio_catalogue.rs)) — the `web-radio` WASM plugin queries radio-browser live and can't host SQLite, so the offline catalogue is a NATIVE side path: `download_radio_catalogue` snapshots the ~35k-station directory into an app.db `radio_station` table + contentless FTS5 index (user-triggered from Settings → Data), and `resolve_radio_catalogue` answers the SAME opaque query tokens as the plugin (`top` / `tag:x` / `country:xx` / free text) returning the SAME `PluginTrack` shape. [`WebRadioView`](src/components/views/WebRadioView.tsx) routes browse/search through it when `offline_mode` is on OR the `radio.catalogue.local_first` setting is enabled with a catalogue present; the stream url rides inside the track id (`url:<stream>`) so `plugin_stream_url` + playback stay network-free regardless.
+Deezer enrichment (pictures, covers, fans — cached 30 days in `metadata_artist` / `metadata_album` in `app.db`, hashes point into shared `metadata_artwork/<blake3>.jpg` so artwork renders offline; **album covers are only disk-cached when the local album lacks its own art** (`album.artwork_id IS NULL`, issue #493) — the album page's auto-enrich (label/release-date only) and Discord (remote `cover_url`) never show the downloaded file, so caching it for albums the user already has covers for just bloated the shared cache; **Web Radio now-playing artwork** is the one Deezer path that is NOT disk-cached — [`fetch_radio_artwork`](src-tauri/crates/app/src/commands/deezer.rs) resolves an album cover URL from the ICY `Artist - Title` via `search_track` and returns the remote CDN link directly because a radio now-playing line is ephemeral; `PlayerContext` swaps it into `currentTrack.artwork_path` over the station favicon, token + `isRadioTrack`-guarded so a stale fetch or a library track that started meanwhile is never clobbered) · artist bio source selector (Settings → Integrations: **Last.fm** default vs **TheAudioDB** multi-language via [`metadata::theaudiodb`](src-tauri/crates/core/src/metadata/theaudiodb.rs), `app_setting['metadata.bio_source']` + `['metadata.bio_language']`; `enrich_artist_deezer` branches on it and stores `bio_source`/`bio_language` in `metadata_artist` so a switch invalidates the cached bio) · **wide artist fanart** (issue #482 — the same TheAudioDB `search.php` response carries `strArtistFanart*` / `strArtistWideThumb` / `strArtistBanner`, so `TheAudioDbClient::artist_info` returns bio **and** `fanart_url` from ONE lookup; `enrich_artist_deezer` calls it **regardless of `metadata.bio_source`** because Last.fm has no equivalent image and gating it would leave Last.fm users with no artist hero at all, cached in `metadata_artist.background_{url,hash}` + `background_fetched_at` — the "we already looked" marker without which a NULL hash is indistinguishable from "never queried" and every fanart-less artist would re-hit a rate-limited API per page visit; stamped when the API was *reached*, left NULL on a transport error so a blip retries instead of caching for the 30-day TTL) · Last.fm (bios, similar artists with Deezer picture backfill — Last.fm's `artist.getSimilar` returns generic star placeholders for every image since their artist-image API was killed in 2019, so [`similar::enrich_with_deezer_pictures`](src-tauri/crates/app/src/commands/similar.rs) joins the result against `app.metadata_artist` and fans out parallel Deezer `search_artist` calls for any cache miss before responding; scrobbler) · **per-artist offline overrides** (issue #323: `artist.custom_bio` + library-scoped `artist_similar_custom` table, both per-profile; [`commands/artist_overrides.rs`](src-tauri/crates/app/src/commands/artist_overrides.rs) write/read commands, edited from Artist Detail → "Edit info" [`ArtistMetadataEditorModal`](src/components/common/ArtistMetadataEditorModal.tsx); `enrich_artist_deezer` swaps the custom bio onto the returned payload and `get_similar_artists` short-circuits to the curated list before any cache/network, so both work offline and survive enrichment passes) · Discord RPC · Native OS track-change toast notifications ([`notifications.rs`](src-tauri/crates/app/src/notifications.rs) — `tauri-plugin-notification` bridge to Windows Action Center / macOS Notification Center / libnotify, opt-in `app_setting['notifications.track_change']` default OFF) · DLNA / UPnP MediaServer ([`docs/features/dlna.md`](docs/features/dlna.md)) · MPD protocol server ([`docs/features/mpd.md`](docs/features/mpd.md) — opt-in LAN control surface for existing MPD clients; control + queue mutation only, library browsing deliberately out of scope for v1) · **offline Web Radio catalogue** ([`commands/web_radio_catalogue.rs`](src-tauri/crates/app/src/commands/web_radio_catalogue.rs)) — the `web-radio` WASM plugin queries radio-browser live and can't host SQLite, so the offline catalogue is a NATIVE side path: `download_radio_catalogue` snapshots the ~35k-station directory into an app.db `radio_station` table + contentless FTS5 index (user-triggered from Settings → Data), and `resolve_radio_catalogue` answers the SAME opaque query tokens as the plugin (`top` / `tag:x` / `country:xx` / free text) returning the SAME `PluginTrack` shape. [`WebRadioView`](src/components/views/WebRadioView.tsx) routes browse/search through it when `offline_mode` is on OR the `radio.catalogue.local_first` setting is enabled with a catalogue present; the stream url rides inside the track id (`url:<stream>`) so `plugin_stream_url` + playback stay network-free regardless.
 
 ### Preferences & maintenance
 

docs/features/integrations.md

@@ -14,6 +14,8 @@ A single global toggle — Settings → Intégrations → "Mode hors-ligne" —
 - Album covers (`enrich_album_deezer`, `search_albums_deezer`, `set_album_artwork_from_deezer`, `batch_fetch_missing_album_covers`)
 - Label / fan-count metadata
 
+> **Album cover disk-caching is gated on the album lacking local art (issue #493).** `enrich_album_inner` fires automatically on every album-page open (which only reads `label` + `release_date`) and from the Discord presence (which reads the remote `cover_url`); neither displays the downloaded file, and the album grid / detail header render the *local* artwork. So the cover image is written to the shared `metadata_artwork` cache **only when `album.artwork_id IS NULL`** — otherwise the cache filled with never-shown Deezer covers for albums the user already had covers for. The remote `cover_url` still rides through for Discord + the cache row, and the deliberate `batch_fetch_missing_album_covers` (which iterates only `artwork_id IS NULL` albums) is unaffected.
+
 Results are cached in the `deezer_artist` / `deezer_album` tables of the **shared** `app.db` (one cache across every profile) with a 30-day `expires_at` TTL. Cache-first: zero network round-trips when the row is fresh. Failures are non-fatal — the UI degrades to local-only artwork and an empty enrichment payload.
 
 **Auto-enrichment on play.** [`PlayerProvider`](../../src/contexts/PlayerContext.tsx) fires `enrich_artist_deezer(currentTrack.artist_id)` (fire-and-forget) on every track-change. Cache hits are ~10 ms so the duplicate call done by `NowPlayingPanel` when it renders is harmless; the point is to populate the cache for views the user _isn't_ looking at right now (e.g. the artist grid in `LibraryView`) so a tile gets its picture as soon as the user plays one of that artist's tracks, regardless of whether the Now Playing panel is open.

src-tauri/crates/app/src/commands/deezer.rs

@@ -87,24 +87,37 @@ pub async fn enrich_album_deezer(
     enrich_album_inner(&pool, &artwork_dir, album_id).await
 }
 
+/// Whether a fresh cached `metadata_album` row is a usable hit **for the needs
+/// of the album being enriched**. A row with no `cover_hash` still counts as
+/// complete when the local album has its own artwork (it will never need the
+/// Deezer cover). But for an art-less album a cover-less row is *incomplete* —
+/// it must trigger a re-fetch instead of serving a permanent miss. Without this
+/// the #493 download-skip would poison the shared cache: a fresh cover-less row
+/// (art removed, a different profile sharing the cache, or a prior failed
+/// download) would block the cover from ever being fetched until the TTL lapsed.
+fn metadata_album_cache_complete(cover_hash: Option<&str>, has_local_art: bool) -> bool {
+    cover_hash.is_some() || has_local_art
+}
+
 pub(crate) async fn enrich_album_inner(
     pool: &SqlitePool,
     artwork_dir: &Path,
     album_id: i64,
 ) -> AppResult<DeezerAlbumEnrichment> {
     let now = now_ms();
 
-    // 1. Read the local album + its existing deezer_id.
-    let local: Option<(String, Option<String>, Option<i64>)> = sqlx::query_as(
-        "SELECT al.title, ar.name, al.deezer_id
+    // 1. Read the local album + its existing deezer_id + whether it already
+    //    has local artwork (issue #493 — see the cover-download guard below).
+    let local: Option<(String, Option<String>, Option<i64>, Option<i64>)> = sqlx::query_as(
+        "SELECT al.title, ar.name, al.deezer_id, al.artwork_id
            FROM album al LEFT JOIN artist ar ON ar.id = al.artist_id
           WHERE al.id = ?",
     )
     .bind(album_id)
     .fetch_optional(pool)
     .await?;
 
-    let Some((album_title, artist_name, existing_deezer_id)) = local else {
+    let Some((album_title, artist_name, existing_deezer_id, local_artwork_id)) = local else {
         return Ok(DeezerAlbumEnrichment::empty());
     };
 
@@ -125,7 +138,15 @@ pub(crate) async fn enrich_album_inner(
         .await?;
 
         if let Some((label, release_date, cover_url, cover_hash, expires_at)) = cached {
-            if expires_at > now {
+            // A fresh row is a usable hit only when it's also complete for this
+            // album's needs — otherwise a cover-less row for an art-less album
+            // would block a re-fetch until the TTL lapsed (issue #493).
+            let usable = expires_at > now
+                && metadata_album_cache_complete(
+                    cover_hash.as_deref(),
+                    local_artwork_id.is_some(),
+                );
+            if usable {
                 let cover_path = cover_hash
                     .as_deref()
                     .and_then(|h| metadata_artwork::existing_path(artwork_dir, h));
@@ -187,10 +208,21 @@ pub(crate) async fn enrich_album_inner(
 
     let cover_url = hit.cover_xl.clone().or_else(|| hit.cover_big.clone());
 
-    // 4. Download artwork into the shared cache (best-effort).
-    let cover_hash = match cover_url.as_deref() {
-        Some(url) => metadata_artwork::download_and_cache(url, artwork_dir).await,
-        None => None,
+    // 4. Download artwork into the shared cache (best-effort) — but ONLY for an
+    //    album that has NO local cover of its own (issue #493). This function is
+    //    fired automatically every time an album page opens (which only reads
+    //    `label` + `release_date`) and by the Discord presence (which reads the
+    //    remote `cover_url`); neither uses the downloaded file, and the album
+    //    grid / detail header render the LOCAL artwork. Without this guard the
+    //    shared `metadata_artwork` cache filled up with Deezer covers for albums
+    //    the user already has artwork for — never displayed. The deliberate
+    //    paths still work: `batch_fetch_missing_album_covers` only iterates
+    //    `artwork_id IS NULL` albums, and a genuinely cover-less album still
+    //    gets its fallback. `cover_url` always rides through for Discord + the
+    //    cache row regardless.
+    let cover_hash = match (local_artwork_id.is_none(), cover_url.as_deref()) {
+        (true, Some(url)) => metadata_artwork::download_and_cache(url, artwork_dir).await,
+        _ => None,
     };
     let cover_path = cover_hash
         .as_deref()
@@ -210,7 +242,13 @@ pub(crate) async fn enrich_album_inner(
            title = excluded.title,
            release_date = excluded.release_date,
            cover_url = excluded.cover_url,
-           cover_hash = excluded.cover_hash,
+           -- Only overwrite the cached hash on a NEW successful download.
+           -- `excluded.cover_hash` is NULL when the download was skipped
+           -- (art-having album, #493) or failed transiently — in both cases
+           -- keep whatever cover was already cached rather than dropping a good
+           -- one over a network blip. The #493 cleanup is what deliberately
+           -- clears art-having albums' covers, not this best-effort upsert.
+           cover_hash = COALESCE(excluded.cover_hash, cover_hash),
            label = excluded.label,
            fetched_at = excluded.fetched_at,
            expires_at = excluded.expires_at",
@@ -1172,3 +1210,23 @@ async fn download_image_bytes(url: &str) -> AppResult<Vec<u8>> {
     }
     Ok(bytes)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::metadata_album_cache_complete;
+
+    #[test]
+    fn album_cache_completeness_gates_refetch() {
+        // Art-less album (`has_local_art = false`): only a cached cover makes
+        // the row usable. A cover-less fresh row must NOT be served — it has to
+        // re-fetch (art was removed, another profile shares the cache, or a
+        // prior download failed), which is the #493 regression this guards.
+        assert!(metadata_album_cache_complete(Some("hash"), false));
+        assert!(!metadata_album_cache_complete(None, false));
+
+        // Art-having album never needs the Deezer cover, so a cover-less row is
+        // a complete hit — no wasteful re-download every time the page opens.
+        assert!(metadata_album_cache_complete(None, true));
+        assert!(metadata_album_cache_complete(Some("hash"), true));
+    }
+}