diff --git a/crates/metadata/src/lib.rs b/crates/metadata/src/lib.rs index e5d9c10..5000d04 100644 --- a/crates/metadata/src/lib.rs +++ b/crates/metadata/src/lib.rs @@ -566,6 +566,10 @@ const HERO_SCORE_CONCURRENCY: usize = 4; /// every press photo across the probe fixtures measures 1.33 or wider. const HERO_MIN_ASPECT: f64 = 1.15; const HERO_SEARCH_WAVE_CONCURRENCY: usize = 3; +/// A Bing image page with at most this many cards is a degraded answer, not a +/// small result set. Bing's genuine pages run 12-100 cards; the degraded shape +/// is one card of something unrelated. +const HERO_DEGRADED_PAGE_CARDS: usize = 2; const HERO_SCORE_CACHE_MAX_ENTRIES: usize = 128; const HERO_SCORE_RECIPE: &str = "bing-identity-visual-v1"; @@ -1819,6 +1823,16 @@ impl MetadataClient { // crowding out better solo/DJ portraits for non-band artists. let queries = [ format!("{artist} band photo"), + // The BARE name, no decoration at all. Our decorated queries can + // collapse a rare artist onto a popular entity that shares a word: + // every "Hana Vu ... photo" phrasing returns the Japanese girl group + // HANA (525 of 525 cards, and quoting does not help), while the bare + // "Hana Vu" returns her real press photos -- 16 of 17 pass the + // identity gate as Strong (NME, Pitchfork, LA Times, Nylon, Dork). + // Safe to widen the funnel here because the identity gate is the + // thing that filters: it rejected ~3,000 cards in the audit that + // found this, with no observed false rejects. + artist.to_string(), format!( "\"{artist}\" musician portrait photo -album -cover -vinyl -poster -flyer -tickets" ), @@ -1883,18 +1897,30 @@ impl MetadataClient { .map(|url| async move { // Bing answers the SAME url with wildly different pages: // measured back to back, one card on one read and 25-80 on - // the next. A card-less page is a degraded answer, not an + // the next. A near-empty page is a degraded answer, not an // empty result set, so drop it and ask once more rather than // caching it as this artist's verdict for the next quarter // hour. + // + // The emptiness test used to be `is_empty()`, which let the + // commonest degraded shape straight through: a ONE-card page + // carrying something unrelated. Measured across one audit, + // 15 of 30 waves for MSPAINT and Billy Cobb, 6 of 15 for + // DJ Sabrina and 9 of 15 for TAGABOW came back with exactly + // one junk card -- each one enough to blank that artist for + // a full CATALOG_TTL. Keep whichever read is richer so a + // thin retry can never lose a good first answer. let mut parsed = match self.browser_html(&url).await { Some(page) => parse_bing_image_results(&page), None => Vec::new(), }; - if parsed.is_empty() { + if parsed.len() <= HERO_DEGRADED_PAGE_CARDS { self.forget_html(&url).await; if let Some(page) = self.browser_html(&url).await { - parsed = parse_bing_image_results(&page); + let retried = parse_bing_image_results(&page); + if retried.len() > parsed.len() { + parsed = retried; + } } } parsed diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index dc225c3..a9d4b7e 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -762,7 +762,14 @@ const ARTIST_HERO_CACHE_MAX_ENTRY_BYTES: u64 = 41 * 1024 * 1024; // file path is safe: a recipe mismatch forces a fresh lookup and the successful // result atomically replaces the stale manifest. const ARTIST_ART_MANIFEST_RECIPE: &str = "artist-art-tier-v4-wide-only"; -const ARTIST_ART_MANIFEST_TTL: std::time::Duration = std::time::Duration::from_secs(24 * 60 * 60); +// A resolved hero is expensive to find and does not go stale the way metadata +// does -- a press photo is still the artist's press photo a month later. The +// old 24h expiry meant every artist was re-discovered daily through a tier that +// fails per-name, so a single bad day blanked artists that had perfectly good +// heroes: 26 manifests on disk (Crazy P, MSPAINT, Sword II, Hermeth, Billy Cobb, +// Pond, Weezer, Daft Punk...) went to 3. Match the cover-resolution horizon. +const ARTIST_ART_MANIFEST_TTL: std::time::Duration = + std::time::Duration::from_secs(30 * 24 * 60 * 60); const ARTIST_ART_MANIFEST_MAX_BYTES: u64 = 64 * 1024; // Identity is part of the key because two Deezer artists can share the same // display name (and, occasionally, the same release title/reference artwork). @@ -955,37 +962,93 @@ fn artist_art_manifest_path( Ok(dir.join(format!("{artist_id}.json"))) } -async fn load_artist_art_manifest( +/// A persisted manifest that passed structural validation, plus whether it can +/// be served without asking the network again. +struct StoredArtistArt { + art: metadata::CatalogArtistArt, + /// Fresh enough AND written by the current selection recipe. + current: bool, +} + +/// Read a persisted manifest without destroying it. +/// +/// This deliberately does NOT delete stale or older-recipe manifests. It used +/// to, and that is what blanked artists: the file was removed on the way in, +/// then discovery failed, and an artist that had a perfectly good hero five +/// minutes earlier had nothing at all. A known hero is only ever replaced by a +/// better one now -- see `catalog_artist_art`. Only genuinely unusable files +/// (oversized or unparseable) are removed. +async fn read_artist_art_manifest( app: &tauri::AppHandle, artist_id: u64, -) -> Option { +) -> Option { let path = artist_art_manifest_path(app, artist_id).ok()?; let metadata = tokio::fs::metadata(&path).await.ok()?; + if metadata.len() > ARTIST_ART_MANIFEST_MAX_BYTES { + let _ = tokio::fs::remove_file(path).await; + return None; + } let fresh = metadata .modified() .ok() .and_then(|modified| modified.elapsed().ok()) .is_some_and(|age| age < ARTIST_ART_MANIFEST_TTL); - if !fresh || metadata.len() > ARTIST_ART_MANIFEST_MAX_BYTES { + let bytes = tokio::fs::read(&path).await.ok()?; + let Ok(manifest) = serde_json::from_slice::(&bytes) else { let _ = tokio::fs::remove_file(path).await; return None; - } - let bytes = tokio::fs::read(&path).await.ok()?; - let manifest = serde_json::from_slice::(&bytes).ok()?; + }; let art = manifest.art; let valid_urls = art .banners .iter() .chain(art.logos.iter()) .all(|url| url.starts_with("https://")); - (manifest.recipe == ARTIST_ART_MANIFEST_RECIPE - && manifest.artist_id == artist_id - && art.artist.id == artist_id - && !art.banner_custom - && art.banners.len() <= 8 - && art.logos.len() <= 8 - && valid_urls) - .then_some(art) + // Identity and shape must hold whatever the recipe was; a manifest for the + // wrong artist is never usable, however recent. + if manifest.artist_id != artist_id + || art.artist.id != artist_id + || art.banner_custom + || art.banners.len() > 8 + || art.logos.len() > 8 + || !valid_urls + { + return None; + } + let current = fresh && manifest.recipe == ARTIST_ART_MANIFEST_RECIPE; + Some(StoredArtistArt { art, current }) +} + +/// The strict read: only a manifest that is fresh and matches the current +/// recipe. Callers that must not show an out-of-recipe hero use this. +async fn load_artist_art_manifest( + app: &tauri::AppHandle, + artist_id: u64, +) -> Option { + read_artist_art_manifest(app, artist_id) + .await + .and_then(|stored| stored.current.then_some(stored.art)) +} + +/// Persist a resolved manifest off the request path. Re-stamps whatever is +/// written with the current recipe, which is also how an older-recipe manifest +/// that survived as a floor gets migrated forward. +fn store_art_manifest_in_background( + app: &tauri::AppHandle, + artist_id: u64, + art: &metadata::CatalogArtistArt, +) { + let store_app = app.clone(); + let store_art = art.clone(); + tauri::async_runtime::spawn(async move { + let result = tokio::task::spawn_blocking(move || { + store_artist_art_manifest(&store_app, artist_id, &store_art) + }) + .await; + if let Ok(Err(error)) = result { + log::warn!("could not cache artist artwork manifest: {error}"); + } + }); } fn store_artist_art_manifest( @@ -1373,26 +1436,48 @@ pub async fn catalog_artist_art( // The selected provider/Bing URL tier is stable for the same period as the // metadata cache. Persisting that tiny manifest means an app restart can // address the already-known hero immediately instead of rediscovering Bing. - let mut art = if let Some(cached) = load_artist_art_manifest(&app, artist_id).await { - cached - } else { - let resolved = state - .metadata - .catalog_artist_art(artist_id) - .await - .ok_or_else(|| "artist artwork lookup failed".to_string())?; - let store_app = app.clone(); - let store_art = resolved.clone(); - tauri::async_runtime::spawn(async move { - let result = tokio::task::spawn_blocking(move || { - store_artist_art_manifest(&store_app, artist_id, &store_art) - }) - .await; - if let Ok(Err(error)) = result { - log::warn!("could not cache artist artwork manifest: {error}"); + let stored = read_artist_art_manifest(&app, artist_id).await; + let mut art = match stored { + // Fresh, current recipe: serve it, no network. + Some(stored) if stored.current => stored.art, + // Either nothing on disk, or something stale/older-recipe. Try to do + // better, but keep what we have as a floor: the discovery tier fails + // per-artist-name (Bing collapses rare names onto popular entities and + // intermittently serves other queries' results), so a failed lookup is + // a normal event, not a reason to show an artist with no hero. + other => { + let resolved = state.metadata.catalog_artist_art(artist_id).await; + // Only banners we already had count as a floor. Note that + // `catalog_artist_art` returns Some as soon as the Deezer *profile* + // resolves, so "found nothing" arrives as Some with an empty banner + // list -- an emptiness check, not a None check, is what detects a + // failed hero discovery. + let previous = other + .map(|stored| stored.art) + .filter(|art| !art.banners.is_empty()); + match (resolved, previous) { + (Some(mut resolved), Some(previous)) if resolved.banners.is_empty() => { + log::info!( + "artist {artist_id}: hero discovery found nothing, keeping {} \ + previously resolved banner(s)", + previous.banners.len() + ); + resolved.banner = previous.banner; + resolved.banners = previous.banners; + if resolved.logos.is_empty() { + resolved.logos = previous.logos; + } + store_art_manifest_in_background(&app, artist_id, &resolved); + resolved + } + (Some(resolved), _) => { + store_art_manifest_in_background(&app, artist_id, &resolved); + resolved + } + (None, Some(previous)) => previous, + (None, None) => return Err("artist artwork lookup failed".to_string()), } - }); - resolved + } }; if let Some(custom) = load_artist_hero_override(&app, artist_id)? { art.banner = Some(custom.data_url.clone());