From b16fc340b4d71e0a641dc2b8a85794b4f83d18f5 Mon Sep 17 00:00:00 2001 From: KeelrLeek <156356974+KeelrLeek@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:13:29 -0500 Subject: [PATCH] provider art: a duplicate MusicBrainz entry is not a namesake TheAudioDB records are held to a cross-provider identity test because a shared name is not evidence -- namesakes are the bug it protects against. But it demanded that TheAudioDB's MBID EQUAL the MB artist Deezer links to, and treated any other id as a different act. MusicBrainz carries duplicate entries for the same artist more often than that assumes. Roosevelt has two -- 84825fb6 and d7eecee2, both "Roosevelt", Person, DE, "German producer & DJ from Cologne". Deezer links the first, TheAudioDB references the second, so his logo was discarded as belonging to someone else. The rescue keeps names untrusted, which is the entire point of the gate: when the ids disagree it asks whether TheAudioDB's OWN MusicBrainz entry links back to the Deezer artist being resolved. d7eecee2 lists https://www.deezer.com/artist/85901 among its url relations -- two MB entries pointing at one Deezer id are the same act whatever their ids say. It only runs when the cheap checks fail and it rides the shared cached artist lookup, so the hot path is unchanged. A linked MBID still outranks TheAudioDB's own strDeezerID, which can be stale (`linked_mbid_outranks_stale_audiodb_deezer_id` covers that and still passes) -- the doc comment's "either ... or" describes a precedence the code deliberately does not have. Manifest recipe bumped to v5, or artists cached under v4 keep serving the logo-less result this build would now fill. Safe now that a stale-recipe manifest survives as a floor rather than being deleted on read. Roosevelt: 0 logos -> 1 (his TheAudioDB logo). Verified unchanged: Fishmans 5, King Gizzard 7, Wednesday 4, Pond 1, Ross from Friends 1, Camping in Alaska 0 (genuinely has none). --- crates/metadata/src/lib.rs | 114 ++++++++++++++++++++++++++++++------- src-tauri/src/commands.rs | 7 ++- 2 files changed, 98 insertions(+), 23 deletions(-) diff --git a/crates/metadata/src/lib.rs b/crates/metadata/src/lib.rs index 8f400d0..d63a272 100644 --- a/crates/metadata/src/lib.rs +++ b/crates/metadata/src/lib.rs @@ -1005,6 +1005,53 @@ impl MetadataClient { self.cached_json(url, true).await } + /// The identity gate, plus a rescue for MusicBrainz DUPLICATES. + /// + /// MusicBrainz carries more than one entry for the same artist more often + /// than the strict-equality gate assumes, and TheAudioDB may reference the + /// other one. Roosevelt has two — 84825fb6 and d7eecee2, both "Roosevelt", + /// Person, DE, "German producer & DJ from Cologne" — and because TheAudioDB + /// pointed at the entry Deezer does not link, his logo was thrown away as a + /// namesake's. + /// + /// Names stay untrusted, which is the whole point of the gate: the rescue + /// asks whether TheAudioDB's OWN MusicBrainz entry links back to the Deezer + /// artist we are resolving. Two MB entries pointing at one Deezer id are the + /// same act whatever their ids say. Only reached when the cheap checks fail, + /// and it rides the shared cached artist lookup. + async fn audiodb_identity_ok( + &self, + record: &serde_json::Value, + deezer_id: u64, + linked_mbid: Option<&str>, + ) -> bool { + if audiodb_artist_matches_identity(record, deezer_id, linked_mbid) { + return true; + } + let Some(record_mbid) = audiodb_record_mbid(record) else { + return false; + }; + let Some(body) = self + .musicbrainz_json(&artist_detail_url(record_mbid)) + .await + else { + return false; + }; + let needle = format!("deezer.com/artist/{deezer_id}"); + body.get("relations") + .and_then(|value| value.as_array()) + .is_some_and(|relations| { + relations.iter().any(|relation| { + relation + .pointer("/url/resource") + .and_then(|value| value.as_str()) + .is_some_and(|url| { + url.trim_end_matches('/').to_lowercase().ends_with(&needle) + }) + }) + }) + } + async fn cached_json( &self, url: &str, @@ -1253,7 +1300,10 @@ impl MetadataClient { // strDeezerID). The lookup is shared with the Fanart branch, so // this closes a namesake hole without another hot-path request. let linked_mbid = self.deezer_linked_mbid(deezer_id).await; - if !audiodb_artist_matches_identity(&record, deezer_id, linked_mbid.as_deref()) { + if !self + .audiodb_identity_ok(&record, deezer_id, linked_mbid.as_deref()) + .await + { return None; } if let Some(banner) = audiodb_banner_urls(&record).into_iter().next() { @@ -1403,9 +1453,14 @@ impl MetadataClient { // loop back to the requested Deezer identity. In particular, never use // an unlinked TADB logo as the provisional logo before Fanart's curated // result lands. - if tadb.as_ref().is_some_and(|record| { - !audiodb_artist_matches_identity(record, deezer_id, linked_mbid.as_deref()) - }) { + let tadb_identified = match tadb.as_ref() { + Some(record) => { + self.audiodb_identity_ok(record, deezer_id, linked_mbid.as_deref()) + .await + } + None => false, + }; + if !tadb_identified { tadb = None; } @@ -2411,18 +2466,21 @@ impl MetadataClient { self.deezer_linked_mbid(artist_id), self.audiodb_artist(artist), ); - record - .filter(|record| { - audiodb_artist_matches_identity(record, artist_id, linked_mbid.as_deref()) - }) - .and_then(|record| { - record - .get("idArtist") - .and_then(|id| id.as_str()) - .map(str::trim) - .filter(|id| !id.is_empty()) - .map(String::from) - }) + let identified = match record.as_ref() { + Some(record) => { + self.audiodb_identity_ok(record, artist_id, linked_mbid.as_deref()) + .await + } + None => false, + }; + record.filter(|_| identified).and_then(|record| { + record + .get("idArtist") + .and_then(|id| id.as_str()) + .map(str::trim) + .filter(|id| !id.is_empty()) + .map(String::from) + }) } else { None }; @@ -3834,7 +3892,10 @@ impl MetadataClient { let record = self.audiodb_artist(artist).await?; if let Some(artist_id) = artist_id { let linked_mbid = self.deezer_linked_mbid(artist_id).await; - if !audiodb_artist_matches_identity(&record, artist_id, linked_mbid.as_deref()) { + if !self + .audiodb_identity_ok(&record, artist_id, linked_mbid.as_deref()) + .await + { return None; } } @@ -6901,17 +6962,26 @@ fn audiodb_artist_matches_identity( deezer_id: u64, linked_mbid: Option<&str>, ) -> bool { + // A linked MBID deliberately outranks TheAudioDB's own strDeezerID, which + // can be stale -- see `linked_mbid_outranks_stale_audiodb_deezer_id`. The + // MusicBrainz-duplicate rescue lives in `audiodb_identity_ok`, which asks a + // question this cheap check cannot: whether TheAudioDB's MB entry is a + // second entry for the SAME act. if let Some(linked_mbid) = linked_mbid { - return record - .get("strMusicBrainzID") - .and_then(|value| value.as_str()) - .map(str::trim) - .filter(|value| !value.is_empty()) + return audiodb_record_mbid(record) .is_some_and(|value| value.eq_ignore_ascii_case(linked_mbid)); } audiodb_deezer_id(record) == Some(deezer_id) } +fn audiodb_record_mbid(record: &serde_json::Value) -> Option<&str> { + record + .get("strMusicBrainzID") + .and_then(|value| value.as_str()) + .map(str::trim) + .filter(|value| !value.is_empty()) +} + #[cfg(test)] fn deezer_search_allows_bing( body: &serde_json::Value, diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 0403b54..8578530 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -761,7 +761,12 @@ const ARTIST_HERO_CACHE_MAX_ENTRY_BYTES: u64 = 41 * 1024 * 1024; // whenever hero *selection* changes, not merely its rendering. Keeping the same // 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"; +// v5: TheAudioDB records referencing a DUPLICATE MusicBrainz entry for the same +// artist are no longer rejected as namesakes, so artists cached under v4 are +// holding a logo-less manifest this build would now fill. Safe to bump: a +// stale-recipe manifest is kept as a floor and only replaced by a better +// lookup, so nobody goes blank while the new one resolves. +const ARTIST_ART_MANIFEST_RECIPE: &str = "artist-art-tier-v5-mb-duplicate-identity"; // 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