From 7e30ad769d1f83c0cac96aea93be6073fdfe3c2f Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Fri, 11 Sep 2026 09:46:02 -0600 Subject: [PATCH] fix(musicbrainz): identity-based artist matching, never an unrelated first result (#768) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Swim Deep's Artist page showed Deep Purple's founding year and members. Two defects combined: `search_artist` emitted `artist:Swim Deep` unquoted (the second word leaks into the global query; Deep Purple ranks first at 100, Swim Deep second at 88), and `resolve_artist` fell back unconditionally to the first result when no exact name scored >= 90. Name-keyed caches then made such guesses durable and shareable across homonyms. New `qbz-integrations::musicbrainz::identity` — pure decisions plus orchestration over injected fetcher/store/catalog boundaries: - query `artist:""`, URL-encoded once; - containment rule: exactly one exact-name candidate with score >= 90, else None — the first-result fallback no longer exists; - source direction (Qobuz artist id -> MBID): the page's own Popular-Track ISRCs (max 3, guests excluded) vote among the exact-name candidates using every credit of every recording that carries the ISRC; ties, conflicts and failures fail closed; - scene direction (MBID + catalog scope -> Qobuz id): a unique exact-name Qobuz artist is provisional; several homonyms are split by their own top-track ISRCs (max 2 each); no evidence omits the row instead of taking max(albums_count); - four `*_v2` cache tables keyed by the stable identifier + matcher version, carrying evidence kind, expiring at read time (verified 30 d, provisional 7 d, search windows 7 d, ISRC credits 30 d); legacy name-keyed rows are neither read nor written; scene cache key bumped to v2 with the real account territory (no scope -> no cache), rows re-derived on replay. Core: `catalog_scope()`, `musicbrainz_resolve_source_artist`, store/catalog adapters; `musicbrainz_resolve_artist` and the musician resolver go through the containment rule. Artist page passes {qobuz id, own ISRCs} to core and keeps its generation guard; QML, playback, auth and reco crates untouched. Tests: qbz-integrations 59 (query encoding, #768 ordering fixture, ISRC voting incl. collaborations/dupes/conflicts, cache keys/TTL/version/legacy, bounded fan-out with request counting), qbz-core 40, qbz-qt 782. cc --- crates/qbz-core/src/core.rs | 445 +++-- .../qbz-integrations/src/musicbrainz/cache.rs | 496 +++++ .../src/musicbrainz/client.rs | 110 +- .../src/musicbrainz/identity.rs | 1631 +++++++++++++++++ .../src/musicbrainz/location.rs | 5 +- .../qbz-integrations/src/musicbrainz/mod.rs | 1 + crates/qbz-qt/src/artist_qt.rs | 74 +- 7 files changed, 2555 insertions(+), 207 deletions(-) create mode 100644 crates/qbz-integrations/src/musicbrainz/identity.rs diff --git a/crates/qbz-core/src/core.rs b/crates/qbz-core/src/core.rs index 5b53e6272..5f4d2b338 100644 --- a/crates/qbz-core/src/core.rs +++ b/crates/qbz-core/src/core.rs @@ -6,7 +6,11 @@ use std::sync::Arc; use tokio::sync::RwLock; -use qbz_integrations::musicbrainz::cache::{MusicBrainzCache, QobuzArtistMatch}; +use qbz_integrations::musicbrainz::cache::MusicBrainzCache; +use qbz_integrations::musicbrainz::identity::{ + self, ArtistCandidate, BoxFuture, FetchError, IsrcCredits, SceneIdentityRow, + SceneQobuzCandidate, SourceIdentityRow, +}; use qbz_integrations::musicbrainz::genre::{extract_affinity_seeds, genre_summary, is_broad_genre}; use qbz_integrations::musicbrainz::location::compute_affinity_score; use qbz_integrations::musicbrainz::{ @@ -2378,41 +2382,67 @@ impl QbzCore { Arc::clone(&self.musicbrainz) } - /// Resolve an artist name to a MusicBrainz id. Returns `None` if no - /// confident match is found. + /// The authenticated Qobuz catalog territory (ISO 3166-1 alpha-2), the + /// scope of every MBID → Qobuz-id identity row. `None` before login — + /// callers then display but persist nothing (an invented constant would + /// let one territory's duplicate poison another's). + pub async fn catalog_scope(&self) -> Option { + let client = self.client.read().await; + let client = client.as_ref()?; + client + .session() + .await + .and_then(|s| s.country_code) + .map(|c| c.trim().to_uppercase()) + .filter(|c| !c.is_empty()) + } + + /// Resolve an artist NAME to a MusicBrainz id under the containment rule + /// (`identity::select_exact_name`): exactly one result in the quoted-name + /// window with the same name and score >= 90, else `None`. Used where + /// only a name exists (playlist suggestions, local artist images). The + /// search WINDOW is cached as evidence and re-gated on every hit; the + /// legacy name-keyed `resolved_artists` table is neither read nor written + /// — a name alone is not an identity (#768). pub async fn musicbrainz_resolve_artist( &self, name: &str, ) -> Result, CoreError> { - // Cache-first (piece 3): a resolved artist is stable, so every - // ArtistPage open re-searched `artist?query=` live otherwise. Same - // shape as `musicbrainz_get_artist_metadata` (cache-first, write-after). - if let Ok(guard) = self.musicbrainz_cache.lock() { - if let Some(cache) = guard.as_ref() { - if let Ok(Some(cached)) = cache.get_artist(name) { - return Ok(Some(cached)); - } - } - } - - let resolved = self - .musicbrainz - .resolve_artist(name) + let store = CoreIdentityStore(&self.musicbrainz_cache); + identity::resolve_artist_by_name(Some(&store), self.musicbrainz.as_ref(), name) .await - .map_err(|e| CoreError::Internal(e.to_string()))?; + .map_err(|e| CoreError::Internal(e.to_string())) + } - // Write-after, but NEVER cache a negative: a "not found" pinned for 30 - // days is worse than re-searching (handoff policy). Expiry stays with - // the existing `cleanup(ttl_days)`; no new TTL here. - if let Some(ref artist) = resolved { - if let Ok(guard) = self.musicbrainz_cache.lock() { - if let Some(cache) = guard.as_ref() { - let _ = cache.put_artist(artist); - } - } + /// Resolve the SOURCE artist of an Artist Page — a Qobuz artist id, its + /// name and the ISRCs of its own Popular Tracks — to a MusicBrainz id + /// (`identity::resolve_source_artist`). Verified mappings are cached by + /// Qobuz artist id (Qobuz artist ids are catalog-global: territory + /// decides availability, not which artist an id names), bounded to + /// `MAX_SOURCE_ISRCS` MusicBrainz lookups through the shared client. + pub async fn musicbrainz_resolve_source_artist( + &self, + qobuz_artist_id: u64, + name: &str, + isrcs: &[String], + ) -> identity::SourceResolution { + if !self.musicbrainz.is_enabled().await { + return identity::SourceResolution { + identity: identity::SourceIdentity::Unavailable, + resolved: None, + requests: 0, + cache: "disabled", + }; } - - Ok(resolved) + let store = CoreIdentityStore(&self.musicbrainz_cache); + identity::resolve_source_artist( + Some(&store), + self.musicbrainz.as_ref(), + qobuz_artist_id, + name, + isrcs, + ) + .await } /// Generate playlist "Suggested Songs" via the artist_vectors engine. @@ -2960,8 +2990,8 @@ impl QbzCore { /// account switch. The post-filter on read covers newly-BLOCKED artists; /// this covers the two cases it cannot: un-blocking (the validation loop /// had already substituted a different same-name Qobuz artist for that - /// MBID) and a change of account territory (which decides what validates - /// at all and is not yet part of the cache key). + /// MBID). The account territory is part of the v2 key, so a territory + /// change no longer needs this. pub fn invalidate_scene_cache(&self) { if let Ok(guard) = self.musicbrainz_cache.lock() { if let Some(cache) = guard.as_ref() { @@ -3069,29 +3099,54 @@ impl QbzCore { // pre-resolution identity saves up to five MusicBrainz round-trips on // every hit. First page only, matching Tauri — paginated pages are // cheap to recompute and would multiply the key space by the offset. - let cache_key = location::build_scene_cache_key_v1(&location::SceneCacheKey { - source_mbid, - area_id: area_id.unwrap_or(area_name), - country, - seeds: &source_seeds, - page_size: limit, - catalog_scope: None, + // The catalog territory is part of the key (v2) — which same-name + // Qobuz artist validates depends on it. Without a scope (not logged + // in) the scene is computed but neither read from nor written to the + // materialised cache, rather than keyed on an invented constant. + let scope = self.catalog_scope().await; + let store = CoreIdentityStore(&self.musicbrainz_cache); + let catalog = SceneQobuz { + core: self, + fatal: std::sync::Mutex::new(None), + }; + let cache_key = scope.as_deref().map(|scope| { + location::build_scene_cache_key_v1(&location::SceneCacheKey { + source_mbid, + area_id: area_id.unwrap_or(area_name), + country, + seeds: &source_seeds, + page_size: limit, + catalog_scope: Some(scope), + }) }); + if cache_key.is_none() { + log::info!("[scene] no catalog scope (not logged in): scene cache bypassed"); + } - if offset == 0 { + if let (0, Some(cache_key)) = (offset, cache_key.as_deref()) { let mut cached = None; if let Ok(guard) = self.musicbrainz_cache.lock() { if let Some(cache) = guard.as_ref() { - cached = cache.get_scene_cache(&cache_key).ok().flatten(); + cached = cache.get_scene_cache(cache_key).ok().flatten(); } } if let Some(mut hit) = cached { let removed = apply_scene_blacklist(&mut hit, blacklist); + // The materialised scene is NOT an identity authority: every + // row's Qobuz projection is re-derived from the identifier- + // keyed identity cache (verified rows live 30 days, a + // provisional unique-name row 7 days, then one cheap Qobuz + // re-search). A provisional match therefore never silently + // becomes a 30-day association because the page was cached. + let dropped = self + .revalidate_scene_rows(&mut hit, &store, &catalog, scope.as_deref(), blacklist) + .await?; log::info!( - "[scene] cache hit for {} ({} artists, {} blacklisted out)", + "[scene] cache hit for {} ({} artists, {} blacklisted out, {} dropped on revalidation)", area_name, hit.artists.len(), - removed + removed, + dropped ); return Ok(hit); } @@ -3303,112 +3358,45 @@ impl QbzCore { ); } - let name_normalized = MusicBrainzCache::normalize_name(mb_name); - - // Read-through validation cache. Only the Qobuz projection is - // stored (id/name/image/album count) — those four are a pure - // function of the name. The candidate is rebuilt around them from - // THIS scene's mbid, score and genres, which is exactly what made - // the Tauri-era whole-LocationCandidate cache unreplayable. - let mut cached_match = None; - if let Ok(guard) = self.musicbrainz_cache.lock() { - if let Some(cache) = guard.as_ref() { - cached_match = cache.get_qobuz_artist_match(&name_normalized).ok().flatten(); - } - } - // A hit on a since-blacklisted artist is treated as a MISS, not as - // "no match": re-running the search lets the next-best same-name - // Qobuz artist take the slot, exactly as a cold run would. - let cached_match = - cached_match.filter(|m| !(m.qobuz_id > 0 && blacklist(m.qobuz_id as u64))); - - // RE-ASSERT THE ACCEPTANCE GATE ON REPLAY. The cache KEY is - // `MusicBrainzCache::normalize_name`, which also strips `'`, `"`, - // `.` and `,`; the cold path below accepts a result only when - // `normalize_artist_name` matches, and that one merely - // trims/lowercases/collapses whitespace. The key is therefore - // strictly COARSER than the predicate, so two MusicBrainz - // candidates that the cold path would treat as different names - // share one cache slot — "Guns N' Roses" and "Guns N Roses", - // "Eve." and "Eve". Whichever validated first would then own the - // row for BOTH, for the full 30-day TTL, and a replay that skipped - // the gate is the only way an artist the cold path would have - // REJECTED can end up attributed to a candidate. - // - // Checked here rather than by narrowing the key, so it still holds - // if either normaliser changes independently later. - let cached_match = cached_match - .filter(|m| normalize_artist_name(&m.name) == normalize_artist_name(mb_name)); - - if let Some(hit) = cached_match { - validated.push(LocationCandidate { + // Identity, not popularity (#768): `identity::resolve_scene_candidate` + // keys its cache on THIS candidate's MBID + the catalog scope, + // takes a unique exact-name Qobuz artist as a provisional match, + // and splits several same-name Qobuz artists with their own + // top-track ISRCs — never with `albums_count` alone. No evidence, + // no row. + match identity::resolve_scene_candidate( + Some(&store), + self.musicbrainz.as_ref(), + &catalog, + mbid, + mb_name, + scope.as_deref(), + blacklist, + ) + .await + { + Ok(Some(m)) => validated.push(LocationCandidate { mbid: mbid.clone(), mb_name: mb_name.clone(), - qobuz_id: Some(hit.qobuz_id), - qobuz_name: Some(hit.name), - qobuz_image: hit.image, + qobuz_id: Some(m.qobuz_id as i64), + qobuz_name: Some(m.name), + qobuz_image: m.image, score: *score, genres: candidate_genres.clone(), - qobuz_albums_count: hit.albums_count, - }); - continue; - } - - let results = match self.search_artists(mb_name, 5, 0, None).await { - Ok(results) => results, + qobuz_albums_count: m.albums_count, + }), + Ok(None) => {} Err(e) => { // Terminal classes end the page here rather than burning // up to 99 more searches that will fail the same way. - if let Some(fatal) = scene_fatal_qobuz(&e) { + if let Some(fatal) = catalog.take_fatal() { log::warn!("[scene] Qobuz validation aborted: {e}"); return Err(fatal); } log::warn!("[scene] Qobuz validation failed for {mb_name:?}: {e}"); validation_failures += 1; last_validation_error = e.to_string(); - continue; } - }; - let mb_norm = normalize_artist_name(mb_name); - // The blacklist filters INSIDE the match, not after it: when the - // top same-name artist is blocked, the next-best one takes the - // slot instead of the row disappearing. That is Tauri's rule. - let best = results - .items - .iter() - .filter(|a| normalize_artist_name(&a.name) == mb_norm && !blacklist(a.id)) - .max_by_key(|a| a.albums_count.unwrap_or(0)); - if let Some(qobuz_artist) = best { - let image_url = qobuz_artist - .image - .as_ref() - .and_then(|img| img.small.as_ref().or(img.thumbnail.as_ref()).cloned()); - // Positive results only. Tauri's negative cache is disabled on - // purpose: "not on Qobuz" stuck for 30 days is how a newly - // added artist stays invisible. - if let Ok(guard) = self.musicbrainz_cache.lock() { - if let Some(cache) = guard.as_ref() { - let _ = cache.set_qobuz_artist_match( - &name_normalized, - &QobuzArtistMatch { - qobuz_id: qobuz_artist.id as i64, - name: qobuz_artist.name.clone(), - image: image_url.clone(), - albums_count: qobuz_artist.albums_count, - }, - ); - } - } - validated.push(LocationCandidate { - mbid: mbid.clone(), - mb_name: mb_name.clone(), - qobuz_id: Some(qobuz_artist.id as i64), - qobuz_name: Some(qobuz_artist.name.clone()), - qobuz_image: image_url, - score: *score, - genres: candidate_genres.clone(), - qobuz_albums_count: qobuz_artist.albums_count, - }); } } @@ -3443,16 +3431,201 @@ impl QbzCore { // First page only, non-empty only — Tauri's rule. The stored rows are // already blacklist-aware; the post-filter on read catches anything // blocked afterwards, and `invalidate_scene_cache` covers un-blocking. - if offset == 0 && !response.artists.is_empty() { + if let (0, false, Some(cache_key)) = + (offset, response.artists.is_empty(), cache_key.as_deref()) + { if let Ok(guard) = self.musicbrainz_cache.lock() { if let Some(cache) = guard.as_ref() { - let _ = cache.set_scene_cache(&cache_key, &response); + let _ = cache.set_scene_cache(cache_key, &response); } } } Ok(response) } + + /// Re-derive every cached scene row's Qobuz projection from the + /// identifier-keyed identity cache (cheap on a live row; a cheap Qobuz + /// re-search on an expired provisional one). Rows that no longer resolve + /// are dropped. Returns the number dropped. + async fn revalidate_scene_rows( + &self, + hit: &mut LocationDiscoveryResponse, + store: &CoreIdentityStore<'_>, + catalog: &SceneQobuz<'_, A>, + scope: Option<&str>, + blacklist: &(dyn Fn(u64) -> bool + Send + Sync), + ) -> Result { + let before = hit.artists.len(); + let mut kept: Vec = Vec::with_capacity(before); + for mut row in std::mem::take(&mut hit.artists) { + match identity::resolve_scene_candidate( + Some(store), + self.musicbrainz.as_ref(), + catalog, + &row.mbid, + &row.mb_name, + scope, + blacklist, + ) + .await + { + Ok(Some(m)) => { + row.qobuz_id = Some(m.qobuz_id as i64); + row.qobuz_name = Some(m.name); + row.qobuz_image = m.image; + row.qobuz_albums_count = m.albums_count; + kept.push(row); + } + Ok(None) => {} + Err(e) => { + if let Some(fatal) = catalog.take_fatal() { + return Err(fatal); + } + // Transient: keep the cached projection for this row. + log::warn!("[scene] revalidation of {:?} failed, keeping cached row: {e}", row.mb_name); + kept.push(row); + } + } + } + let dropped = before.saturating_sub(kept.len()); + hit.artists = kept; + Ok(dropped) + } +} + +// ----- #768 identity boundaries ---------------------------------------------- + +/// `identity::IdentityStore` over the core's optional SQLite cache. Every +/// failure is logged and reads as a miss; without a cache the matcher simply +/// runs uncached. +struct CoreIdentityStore<'a>(&'a std::sync::Mutex>); + +impl CoreIdentityStore<'_> { + fn with( + &self, + what: &str, + f: impl FnOnce(&MusicBrainzCache) -> Result, + ) -> Option { + let guard = self.0.lock().ok()?; + let cache = guard.as_ref()?; + match f(cache) { + Ok(v) => Some(v), + Err(e) => { + log::warn!("[mb-identity] cache {what}: {e}"); + None + } + } + } +} + +impl identity::IdentityStore for CoreIdentityStore<'_> { + fn artist_query(&self, key: &str) -> Option> { + self.with("artist_query", |c| c.get_artist_query(key)).flatten() + } + fn put_artist_query(&self, key: &str, window: &[ArtistCandidate]) { + self.with("put_artist_query", |c| c.set_artist_query(key, window)); + } + fn isrc_credits(&self, isrc: &str) -> Option { + self.with("isrc_credits", |c| c.get_isrc_credits(isrc)).flatten() + } + fn put_isrc_credits(&self, credits: &IsrcCredits) { + self.with("put_isrc_credits", |c| c.set_isrc_credits(credits)); + } + fn source_identity(&self, qobuz_artist_id: u64) -> Option { + self.with("source_identity", |c| c.get_source_identity(qobuz_artist_id)) + .flatten() + } + fn put_source_identity(&self, row: &SourceIdentityRow) { + self.with("put_source_identity", |c| c.set_source_identity(row)); + } + fn scene_identity(&self, mbid: &str, scope: &str) -> Option { + self.with("scene_identity", |c| c.get_scene_identity(mbid, scope)) + .flatten() + } + fn put_scene_identity(&self, row: &SceneIdentityRow) { + self.with("put_scene_identity", |c| c.set_scene_identity(row)); + } +} + +/// `identity::SceneCatalog` over the core's Qobuz client. A Qobuz failure +/// that will refuse every further call (auth/region, `scene_fatal_qobuz`) +/// is parked in `fatal` so the validation loop can end the page instead of +/// burning the remaining searches. +struct SceneQobuz<'a, A: FrontendAdapter + Send + Sync + 'static> { + core: &'a QbzCore, + fatal: std::sync::Mutex>, +} + +impl SceneQobuz<'_, A> { + fn park(&self, e: &CoreError) -> FetchError { + if let Some(fatal) = scene_fatal_qobuz(e) { + if let Ok(mut slot) = self.fatal.lock() { + *slot = Some(fatal); + } + } + FetchError::Unavailable(e.to_string()) + } + + fn take_fatal(&self) -> Option { + self.fatal.lock().ok().and_then(|mut slot| slot.take()) + } +} + +impl identity::SceneCatalog for SceneQobuz<'_, A> { + /// The bounded Qobuz search (five results, unchanged) filtered to exact + /// normalised names. The blacklist is applied by the matcher. + fn exact_name_candidates<'a>( + &'a self, + name: &'a str, + ) -> BoxFuture<'a, Result, FetchError>> { + Box::pin(async move { + let results = self + .core + .search_artists(name, 5, 0, None) + .await + .map_err(|e| self.park(&e))?; + let norm = normalize_artist_name(name); + Ok(results + .items + .iter() + .filter(|a| normalize_artist_name(&a.name) == norm) + .map(|a| SceneQobuzCandidate { + qobuz_id: a.id, + name: a.name.clone(), + image: a + .image + .as_ref() + .and_then(|img| img.small.as_ref().or(img.thumbnail.as_ref()).cloned()), + albums_count: a.albums_count, + }) + .collect()) + }) + } + + /// One `/artist/page` per AMBIGUOUS candidate, its own top-track ISRCs + /// only (guest appearances excluded), capped by the matcher. + fn candidate_isrcs<'a>( + &'a self, + qobuz_id: u64, + cap: usize, + ) -> BoxFuture<'a, Result, FetchError>> { + Box::pin(async move { + let page = self + .core + .get_artist_page(qobuz_id, None) + .await + .map_err(|e| self.park(&e))?; + let tracks = page.top_tracks.unwrap_or_default(); + Ok(identity::pick_source_isrcs( + qobuz_id, + tracks + .iter() + .map(|t| (t.isrc.as_deref(), t.artist.as_ref().map(|a| a.id))), + cap, + )) + }) + } } // ----- Musician resolution + appearances ----------------------------------- diff --git a/crates/qbz-integrations/src/musicbrainz/cache.rs b/crates/qbz-integrations/src/musicbrainz/cache.rs index 82e761e12..8cef4ba56 100644 --- a/crates/qbz-integrations/src/musicbrainz/cache.rs +++ b/crates/qbz-integrations/src/musicbrainz/cache.rs @@ -7,6 +7,10 @@ use rusqlite::{params, Connection, OptionalExtension}; use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; +use super::identity::{ + ArtistCandidate, EvidenceKind, IsrcCredits, SceneIdentityRow, SourceIdentityRow, + MATCHER_VERSION, +}; use super::models::{ ArtistMetadata, ArtistRelationships, ArtistType, LocationDiscoveryResponse, MatchConfidence, ResolvedArtist, ResolvedTrack, @@ -28,6 +32,15 @@ const SCENE_TTL_SECS: i64 = 30 * 24 * 60 * 60; const QOBUZ_VALIDATION_TTL_SECS: i64 = 30 * 24 * 60 * 60; /// TTL for the typed Qobuz artist-match cache (30 days, same as above) const QOBUZ_ARTIST_MATCH_TTL_SECS: i64 = 30 * 24 * 60 * 60; +/// TTL of a cached exact-name search WINDOW (evidence, re-gated on every hit) +const ARTIST_QUERY_TTL_SECS: i64 = 7 * 24 * 60 * 60; +/// TTL of a cached ISRC → recording-credits set +const ISRC_CREDITS_TTL_SECS: i64 = 30 * 24 * 60 * 60; +/// TTL of an ISRC-verified identity row (source or scene) +const VERIFIED_IDENTITY_TTL_SECS: i64 = 30 * 24 * 60 * 60; +/// TTL of a provisional (unique-exact-name) scene identity row — a name alone +/// is not identity, so it may not outlive a week +const PROVISIONAL_IDENTITY_TTL_SECS: i64 = 7 * 24 * 60 * 60; /// The Qobuz half of a scene validation, and NOTHING else. /// @@ -81,6 +94,15 @@ impl MusicBrainzCache { Ok(cache) } + /// An in-memory cache with the full schema — tests and dry runs. + pub fn open_in_memory() -> Result { + let conn = Connection::open_in_memory() + .map_err(|e| format!("Failed to open in-memory MusicBrainz cache: {}", e))?; + let cache = Self { conn }; + cache.init_schema()?; + Ok(cache) + } + fn init_schema(&self) -> Result<(), String> { self.conn .execute_batch( @@ -183,6 +205,44 @@ impl MusicBrainzCache { cached_at INTEGER NOT NULL DEFAULT (strftime('%s', 'now')) ); + -- #768 identity matcher (identity.rs). Versioned, identifier- + -- keyed, provenance-carrying; TTL is applied at READ time. + -- Old name-keyed rows (resolved_artists, mb_qobuz_artist_match) + -- stay on disk, inert: nothing on the corrected path reads them. + CREATE TABLE IF NOT EXISTS mb_artist_query_v2 ( + query_key TEXT PRIMARY KEY, + data TEXT NOT NULL, + fetched_at INTEGER NOT NULL + ); + CREATE TABLE IF NOT EXISTS mb_isrc_credits_v2 ( + isrc TEXT PRIMARY KEY, + data TEXT NOT NULL, + fetched_at INTEGER NOT NULL + ); + CREATE TABLE IF NOT EXISTS mb_source_identity_v2 ( + qobuz_artist_id INTEGER NOT NULL, + matcher_version INTEGER NOT NULL, + mbid TEXT NOT NULL, + evidence_kind TEXT NOT NULL, + evidence_count INTEGER NOT NULL, + source_name TEXT NOT NULL, + fetched_at INTEGER NOT NULL, + PRIMARY KEY (qobuz_artist_id, matcher_version) + ); + CREATE TABLE IF NOT EXISTS mb_scene_identity_v2 ( + mbid TEXT NOT NULL, + scope TEXT NOT NULL, + matcher_version INTEGER NOT NULL, + qobuz_artist_id INTEGER NOT NULL, + evidence_kind TEXT NOT NULL, + evidence_count INTEGER NOT NULL, + qobuz_name TEXT NOT NULL, + image TEXT, + albums_count INTEGER, + fetched_at INTEGER NOT NULL, + PRIMARY KEY (mbid, scope, matcher_version) + ); + CREATE TABLE IF NOT EXISTS cache_stats ( key TEXT PRIMARY KEY, value INTEGER NOT NULL DEFAULT 0 @@ -715,6 +775,242 @@ impl MusicBrainzCache { Ok(()) } + // ============ #768 identity matcher (identity.rs) ============ + // + // Every getter enforces TTL and matcher version in the query itself and + // treats a malformed row as a miss. The `*_at` variants exist so tests can + // plant an old row without waiting for the clock. + + fn kind_ttl(kind: EvidenceKind) -> i64 { + match kind { + EvidenceKind::VerifiedIsrc => VERIFIED_IDENTITY_TTL_SECS, + EvidenceKind::UniqueExactName => PROVISIONAL_IDENTITY_TTL_SECS, + } + } + + /// Cached exact-name search window for `query_key` (evidence only — + /// the caller re-runs the acceptance rule on it). + pub fn get_artist_query(&self, query_key: &str) -> Result>, String> { + let min_fetched_at = Self::current_timestamp() - ARTIST_QUERY_TTL_SECS; + let result: Option = self + .conn + .query_row( + "SELECT data FROM mb_artist_query_v2 WHERE query_key = ? AND fetched_at > ?", + params![query_key, min_fetched_at], + |row| row.get(0), + ) + .optional() + .map_err(|e| format!("Failed to query artist-query cache: {}", e))?; + Ok(result.and_then(|data| serde_json::from_str(&data).ok())) + } + + pub fn set_artist_query(&self, query_key: &str, window: &[ArtistCandidate]) -> Result<(), String> { + self.set_artist_query_at(query_key, window, Self::current_timestamp()) + } + + pub fn set_artist_query_at( + &self, + query_key: &str, + window: &[ArtistCandidate], + fetched_at: i64, + ) -> Result<(), String> { + let json = serde_json::to_string(window) + .map_err(|e| format!("Failed to serialize artist query: {}", e))?; + self.conn + .execute( + "INSERT OR REPLACE INTO mb_artist_query_v2 (query_key, data, fetched_at) VALUES (?, ?, ?)", + params![query_key, json, fetched_at], + ) + .map_err(|e| format!("Failed to cache artist query: {}", e))?; + Ok(()) + } + + /// Cached complete recording/credit set for a normalised ISRC. + pub fn get_isrc_credits(&self, isrc: &str) -> Result, String> { + let min_fetched_at = Self::current_timestamp() - ISRC_CREDITS_TTL_SECS; + let result: Option = self + .conn + .query_row( + "SELECT data FROM mb_isrc_credits_v2 WHERE isrc = ? AND fetched_at > ?", + params![isrc, min_fetched_at], + |row| row.get(0), + ) + .optional() + .map_err(|e| format!("Failed to query isrc-credits cache: {}", e))?; + Ok(result + .and_then(|data| serde_json::from_str::(&data).ok()) + .filter(|c| c.isrc == isrc)) + } + + pub fn set_isrc_credits(&self, credits: &IsrcCredits) -> Result<(), String> { + self.set_isrc_credits_at(credits, Self::current_timestamp()) + } + + pub fn set_isrc_credits_at(&self, credits: &IsrcCredits, fetched_at: i64) -> Result<(), String> { + let json = serde_json::to_string(credits) + .map_err(|e| format!("Failed to serialize isrc credits: {}", e))?; + self.conn + .execute( + "INSERT OR REPLACE INTO mb_isrc_credits_v2 (isrc, data, fetched_at) VALUES (?, ?, ?)", + params![credits.isrc, json, fetched_at], + ) + .map_err(|e| format!("Failed to cache isrc credits: {}", e))?; + Ok(()) + } + + /// The VERIFIED source identity for a Qobuz artist id under the current + /// matcher version, if it has not expired. Provisional rows are never + /// stored here (identity.rs only writes `VerifiedIsrc`), and a row whose + /// kind or MBID is malformed is a miss. + pub fn get_source_identity(&self, qobuz_artist_id: u64) -> Result, String> { + let min_fetched_at = Self::current_timestamp() - VERIFIED_IDENTITY_TTL_SECS; + let result: Option<(String, String, i64, String)> = self + .conn + .query_row( + "SELECT mbid, evidence_kind, evidence_count, source_name + FROM mb_source_identity_v2 + WHERE qobuz_artist_id = ? AND matcher_version = ? AND fetched_at > ?", + params![qobuz_artist_id as i64, MATCHER_VERSION as i64, min_fetched_at], + |row| Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?)), + ) + .optional() + .map_err(|e| format!("Failed to query source identity: {}", e))?; + Ok(result.and_then(|(mbid, kind, count, source_name)| { + let evidence_kind = EvidenceKind::parse(&kind)?; + if evidence_kind != EvidenceKind::VerifiedIsrc || mbid.trim().is_empty() { + return None; + } + Some(SourceIdentityRow { + qobuz_artist_id, + mbid, + evidence_kind, + evidence_count: u32::try_from(count).unwrap_or(0), + source_name, + }) + })) + } + + pub fn set_source_identity(&self, row: &SourceIdentityRow) -> Result<(), String> { + self.set_source_identity_at(row, Self::current_timestamp()) + } + + pub fn set_source_identity_at(&self, row: &SourceIdentityRow, fetched_at: i64) -> Result<(), String> { + self.conn + .execute( + "INSERT OR REPLACE INTO mb_source_identity_v2 + (qobuz_artist_id, matcher_version, mbid, evidence_kind, evidence_count, source_name, fetched_at) + VALUES (?, ?, ?, ?, ?, ?, ?)", + params![ + row.qobuz_artist_id as i64, + MATCHER_VERSION as i64, + row.mbid, + row.evidence_kind.as_str(), + row.evidence_count as i64, + row.source_name, + fetched_at + ], + ) + .map_err(|e| format!("Failed to cache source identity: {}", e))?; + Ok(()) + } + + /// The scene identity for (MBID, catalog scope) under the current matcher + /// version, with the TTL of ITS OWN evidence kind (verified 30 d, + /// provisional 7 d). Malformed rows are a miss. + pub fn get_scene_identity(&self, mbid: &str, scope: &str) -> Result, String> { + let now = Self::current_timestamp(); + let result: Option<(i64, String, i64, String, Option, Option, i64)> = self + .conn + .query_row( + "SELECT qobuz_artist_id, evidence_kind, evidence_count, qobuz_name, image, albums_count, fetched_at + FROM mb_scene_identity_v2 + WHERE mbid = ? AND scope = ? AND matcher_version = ?", + params![mbid, scope, MATCHER_VERSION as i64], + |row| { + Ok(( + row.get(0)?, + row.get(1)?, + row.get(2)?, + row.get(3)?, + row.get(4)?, + row.get(5)?, + row.get(6)?, + )) + }, + ) + .optional() + .map_err(|e| format!("Failed to query scene identity: {}", e))?; + Ok(result.and_then( + |(qobuz_id, kind, count, qobuz_name, image, albums_count, fetched_at)| { + let evidence_kind = EvidenceKind::parse(&kind)?; + if fetched_at <= now - Self::kind_ttl(evidence_kind) { + return None; + } + let qobuz_id = u64::try_from(qobuz_id).ok().filter(|id| *id > 0)?; + Some(SceneIdentityRow { + mbid: mbid.to_string(), + scope: scope.to_string(), + qobuz_id, + evidence_kind, + evidence_count: u32::try_from(count).unwrap_or(0), + qobuz_name, + image, + albums_count: albums_count.and_then(|n| u32::try_from(n).ok()), + }) + }, + )) + } + + pub fn set_scene_identity(&self, row: &SceneIdentityRow) -> Result<(), String> { + self.set_scene_identity_at(row, Self::current_timestamp()) + } + + pub fn set_scene_identity_at(&self, row: &SceneIdentityRow, fetched_at: i64) -> Result<(), String> { + self.conn + .execute( + "INSERT OR REPLACE INTO mb_scene_identity_v2 + (mbid, scope, matcher_version, qobuz_artist_id, evidence_kind, evidence_count, qobuz_name, image, albums_count, fetched_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + params![ + row.mbid, + row.scope, + MATCHER_VERSION as i64, + row.qobuz_id as i64, + row.evidence_kind.as_str(), + row.evidence_count as i64, + row.qobuz_name, + row.image, + row.albums_count.map(|n| n as i64), + fetched_at + ], + ) + .map_err(|e| format!("Failed to cache scene identity: {}", e))?; + Ok(()) + } + + /// Plant a row under an ARBITRARY matcher version (tests: an old matcher's + /// row must not satisfy the current lookup). + #[cfg(test)] + fn set_source_identity_versioned(&self, row: &SourceIdentityRow, version: i64) -> Result<(), String> { + self.conn + .execute( + "INSERT OR REPLACE INTO mb_source_identity_v2 + (qobuz_artist_id, matcher_version, mbid, evidence_kind, evidence_count, source_name, fetched_at) + VALUES (?, ?, ?, ?, ?, ?, ?)", + params![ + row.qobuz_artist_id as i64, + version, + row.mbid, + row.evidence_kind.as_str(), + row.evidence_count as i64, + row.source_name, + Self::current_timestamp() + ], + ) + .map_err(|e| e.to_string())?; + Ok(()) + } + // ============ Maintenance ============ /// Clear expired entries from all tables @@ -731,6 +1027,12 @@ impl MusicBrainzCache { ("mb_scene_cache", SCENE_TTL_SECS), ("mb_qobuz_validation", QOBUZ_VALIDATION_TTL_SECS), ("mb_qobuz_artist_match", QOBUZ_ARTIST_MATCH_TTL_SECS), + ("mb_artist_query_v2", ARTIST_QUERY_TTL_SECS), + ("mb_isrc_credits_v2", ISRC_CREDITS_TTL_SECS), + ("mb_source_identity_v2", VERIFIED_IDENTITY_TTL_SECS), + // The longer of the two kinds; the read path applies the + // provisional one itself. + ("mb_scene_identity_v2", VERIFIED_IDENTITY_TTL_SECS), ]; for (table, ttl) in &tables_and_ttls { @@ -767,6 +1069,10 @@ impl MusicBrainzCache { DELETE FROM mb_qobuz_artist_match; DELETE FROM resolved_tracks; DELETE FROM resolved_artists; + DELETE FROM mb_artist_query_v2; + DELETE FROM mb_isrc_credits_v2; + DELETE FROM mb_source_identity_v2; + DELETE FROM mb_scene_identity_v2; UPDATE cache_stats SET value = 0; ", ) @@ -825,3 +1131,193 @@ impl MusicBrainzCache { ); } } + +#[cfg(test)] +mod identity_cache_tests { + use super::*; + + fn cand(mbid: &str, name: &str) -> ArtistCandidate { + ArtistCandidate { + mbid: mbid.into(), + name: name.into(), + score: Some(100), + sort_name: None, + artist_type: None, + country: None, + disambiguation: None, + } + } + + fn source_row(qobuz: u64, mbid: &str, kind: EvidenceKind) -> SourceIdentityRow { + SourceIdentityRow { + qobuz_artist_id: qobuz, + mbid: mbid.into(), + evidence_kind: kind, + evidence_count: 1, + source_name: "Eve".into(), + } + } + + fn scene_row(mbid: &str, scope: &str, qobuz: u64, kind: EvidenceKind) -> SceneIdentityRow { + SceneIdentityRow { + mbid: mbid.into(), + scope: scope.into(), + qobuz_id: qobuz, + evidence_kind: kind, + evidence_count: 1, + qobuz_name: "Eve".into(), + image: None, + albums_count: Some(3), + } + } + + #[test] + fn two_qobuz_ids_with_the_same_name_do_not_share_a_source_mapping() { + let c = MusicBrainzCache::open_in_memory().unwrap(); + c.set_source_identity(&source_row(7, "mb-a", EvidenceKind::VerifiedIsrc)).unwrap(); + assert_eq!(c.get_source_identity(7).unwrap().unwrap().mbid, "mb-a"); + assert!(c.get_source_identity(8).unwrap().is_none()); + } + + #[test] + fn two_mbids_with_the_same_name_do_not_share_a_scene_mapping() { + let c = MusicBrainzCache::open_in_memory().unwrap(); + c.set_scene_identity(&scene_row("mb-a", "FR", 7, EvidenceKind::VerifiedIsrc)).unwrap(); + assert_eq!(c.get_scene_identity("mb-a", "FR").unwrap().unwrap().qobuz_id, 7); + assert!(c.get_scene_identity("mb-b", "FR").unwrap().is_none()); + assert!(c.get_scene_identity("mb-a", "US").unwrap().is_none(), "scope is part of the key"); + } + + #[test] + fn legacy_name_keyed_rows_cannot_satisfy_a_v2_lookup() { + let c = MusicBrainzCache::open_in_memory().unwrap(); + // The pre-fix tables still accept writes; the v2 getters never read them. + c.put_artist(&ResolvedArtist { + mbid: "deep-purple".into(), + name: "Deep Purple".into(), + sort_name: None, + artist_type: ArtistType::Group, + country: None, + disambiguation: None, + confidence: MatchConfidence::Exact, + }) + .unwrap(); + c.set_qobuz_artist_match( + "swim deep", + &QobuzArtistMatch { + qobuz_id: 42, + name: "Swim Deep".into(), + image: None, + albums_count: Some(3), + }, + ) + .unwrap(); + assert!(c.get_source_identity(42).unwrap().is_none()); + assert!(c.get_scene_identity("85e67b0f-afd2-46c2-88b2-c3ba9b0883f2", "FR").unwrap().is_none()); + assert!(c.get_artist_query(&super::super::identity::artist_query_key("Swim Deep")).unwrap().is_none()); + } + + #[test] + fn a_provisional_kind_never_reads_back_as_a_verified_source_identity() { + let c = MusicBrainzCache::open_in_memory().unwrap(); + c.set_source_identity(&source_row(7, "mb-a", EvidenceKind::UniqueExactName)).unwrap(); + assert!(c.get_source_identity(7).unwrap().is_none()); + } + + #[test] + fn expired_rows_miss_at_read_time_without_any_cleanup_call() { + let c = MusicBrainzCache::open_in_memory().unwrap(); + let now = MusicBrainzCache::current_timestamp(); + c.set_source_identity_at( + &source_row(7, "mb-a", EvidenceKind::VerifiedIsrc), + now - VERIFIED_IDENTITY_TTL_SECS - 1, + ) + .unwrap(); + assert!(c.get_source_identity(7).unwrap().is_none()); + + // Scene: provisional expires after 7 days, verified after 30. + c.set_scene_identity_at( + &scene_row("mb-p", "FR", 1, EvidenceKind::UniqueExactName), + now - PROVISIONAL_IDENTITY_TTL_SECS - 1, + ) + .unwrap(); + c.set_scene_identity_at( + &scene_row("mb-v", "FR", 2, EvidenceKind::VerifiedIsrc), + now - PROVISIONAL_IDENTITY_TTL_SECS - 1, + ) + .unwrap(); + assert!(c.get_scene_identity("mb-p", "FR").unwrap().is_none()); + assert_eq!(c.get_scene_identity("mb-v", "FR").unwrap().unwrap().qobuz_id, 2); + + c.set_artist_query_at("k", &[cand("a", "Eve")], now - ARTIST_QUERY_TTL_SECS - 1) + .unwrap(); + assert!(c.get_artist_query("k").unwrap().is_none()); + c.set_isrc_credits_at( + &IsrcCredits { + isrc: "GBAAA0000001".into(), + recordings: vec![], + }, + now - ISRC_CREDITS_TTL_SECS - 1, + ) + .unwrap(); + assert!(c.get_isrc_credits("GBAAA0000001").unwrap().is_none()); + } + + #[test] + fn a_previous_matcher_version_row_is_a_miss() { + let c = MusicBrainzCache::open_in_memory().unwrap(); + c.set_source_identity_versioned( + &source_row(7, "mb-a", EvidenceKind::VerifiedIsrc), + MATCHER_VERSION as i64 - 1, + ) + .unwrap(); + assert!(c.get_source_identity(7).unwrap().is_none()); + } + + #[test] + fn malformed_provenance_is_a_miss() { + let c = MusicBrainzCache::open_in_memory().unwrap(); + c.conn + .execute( + "INSERT INTO mb_source_identity_v2 (qobuz_artist_id, matcher_version, mbid, evidence_kind, evidence_count, source_name, fetched_at) VALUES (7, ?, '', 'verified_isrc', 1, 'Eve', ?)", + params![MATCHER_VERSION as i64, MusicBrainzCache::current_timestamp()], + ) + .unwrap(); + assert!(c.get_source_identity(7).unwrap().is_none(), "empty mbid"); + c.conn + .execute( + "INSERT INTO mb_scene_identity_v2 (mbid, scope, matcher_version, qobuz_artist_id, evidence_kind, evidence_count, qobuz_name, image, albums_count, fetched_at) VALUES ('m', 'FR', ?, 9, 'guessed', 1, 'Eve', NULL, NULL, ?)", + params![MATCHER_VERSION as i64, MusicBrainzCache::current_timestamp()], + ) + .unwrap(); + assert!(c.get_scene_identity("m", "FR").unwrap().is_none(), "unknown kind"); + } + + #[test] + fn search_window_and_isrc_credits_round_trip() { + let c = MusicBrainzCache::open_in_memory().unwrap(); + let window = vec![cand("a", "Eve"), cand("b", "Eve")]; + c.set_artist_query("k", &window).unwrap(); + assert_eq!(c.get_artist_query("k").unwrap().unwrap(), window); + let credits = IsrcCredits { + isrc: "GBAAA0000001".into(), + recordings: vec![super::super::identity::RecordingCredits { + recording_mbid: "r".into(), + credit_mbids: vec!["a".into(), "b".into()], + }], + }; + c.set_isrc_credits(&credits).unwrap(); + assert_eq!(c.get_isrc_credits("GBAAA0000001").unwrap().unwrap(), credits); + } + + #[test] + fn clear_all_and_cleanup_cover_the_v2_tables() { + let c = MusicBrainzCache::open_in_memory().unwrap(); + c.set_source_identity(&source_row(7, "mb-a", EvidenceKind::VerifiedIsrc)).unwrap(); + c.clear_all().unwrap(); + assert!(c.get_source_identity(7).unwrap().is_none()); + let old = MusicBrainzCache::current_timestamp() - VERIFIED_IDENTITY_TTL_SECS - 1; + c.set_source_identity_at(&source_row(7, "mb-a", EvidenceKind::VerifiedIsrc), old).unwrap(); + assert_eq!(c.cleanup_expired().unwrap(), 1); + } +} diff --git a/crates/qbz-integrations/src/musicbrainz/client.rs b/crates/qbz-integrations/src/musicbrainz/client.rs index 7c25d5403..9884dc5ee 100644 --- a/crates/qbz-integrations/src/musicbrainz/client.rs +++ b/crates/qbz-integrations/src/musicbrainz/client.rs @@ -8,6 +8,7 @@ use std::sync::{Arc, OnceLock}; use std::time::{Duration, Instant}; use tokio::sync::Mutex; +use super::identity; use super::models::*; use crate::error::{IntegrationError, IntegrationResult}; @@ -196,12 +197,11 @@ impl MusicBrainzClient { } + // Exact quoted field expression, URL-encoded exactly once (#768: + // `artist:Swim Deep` let "Deep" leak into the global query and ranked + // Deep Purple first at score 100). let base = self.base_url().await; - let encoded_name = urlencoding::encode(name); - let url = format!( - "{}/artist?query=artist:{}&limit={}&fmt=json", - base, encoded_name, limit - ); + let url = identity::artist_search_url(base, name, limit); let response = self.send_with_retry(&url).await?; response.json().await.map_err(Into::into) @@ -255,40 +255,29 @@ impl MusicBrainzClient { Ok(None) } - /// Resolve an artist to get MusicBrainz ID - /// - /// Prefers exact name matches over highest score to avoid disambiguation - /// issues (e.g., multiple artists named "The Warning"). + /// The quoted-name search window as cacheable candidates. + pub async fn search_artist_candidates( + &self, + name: &str, + ) -> IntegrationResult> { + let response = self + .search_artist(name, identity::NAME_SEARCH_LIMIT) + .await?; + Ok(response.artists.iter().map(identity::ArtistCandidate::from).collect()) + } + + /// Resolve an artist NAME to a MusicBrainz id under the containment rule + /// (`identity::select_exact_name`): exactly one result in the quoted-name + /// window with the same trimmed, case-insensitive name and score >= 90. + /// Zero or several acceptable candidates resolve to `None`. There is no + /// first-result fallback any more — that fallback is how #768 handed + /// Swim Deep the biography of Deep Purple. pub async fn resolve_artist(&self, name: &str) -> IntegrationResult> { - let response = self.search_artist(name, 10).await?; - - if response.artists.is_empty() { - return Ok(None); - } - - // Prefer exact name match (case-insensitive) - let target = name.trim().to_lowercase(); - let best = response - .artists - .iter() - .find(|a| a.name.trim().to_lowercase() == target && a.score.unwrap_or(0) >= 90) - .or_else(|| response.artists.first()); - - if let Some(artist) = best { - let confidence = MatchConfidence::from_score(artist.score); - - return Ok(Some(ResolvedArtist { - mbid: artist.id.clone(), - name: artist.name.clone(), - sort_name: artist.sort_name.clone(), - artist_type: ArtistType::from(artist.artist_type.as_deref()), - country: artist.country.clone(), - disambiguation: artist.disambiguation.clone(), - confidence, - })); - } - - Ok(None) + let window = self.search_artist_candidates(name).await?; + Ok(match identity::select_exact_name(name, &window) { + identity::ExactNameSelection::Unique(c) => Some(c.to_resolved()), + _ => None, + }) } // ============ Extended API Methods ============ @@ -867,24 +856,33 @@ impl MusicBrainzClient { /// Escape special characters in Lucene queries fn escape_query(s: &str) -> String { - s.replace('\\', "\\\\") - .replace('"', "\\\"") - .replace(':', "\\:") - .replace('(', "\\(") - .replace(')', "\\)") - .replace('[', "\\[") - .replace(']', "\\]") - .replace('{', "\\{") - .replace('}', "\\}") - .replace('^', "\\^") - .replace('~', "\\~") - .replace('*', "\\*") - .replace('?', "\\?") - .replace('!', "\\!") - .replace('+', "\\+") - .replace('-', "\\-") - .replace('&', "\\&") - .replace('|', "\\|") + identity::lucene_escape(s) + } +} + +impl identity::IdentityFetcher for MusicBrainzClient { + fn artist_candidates<'a>( + &'a self, + name: &'a str, + ) -> identity::BoxFuture<'a, Result, identity::FetchError>> { + Box::pin(async move { + self.search_artist_candidates(name) + .await + .map_err(|e| identity::FetchError::Unavailable(e.to_string())) + }) + } + + fn isrc_credits<'a>( + &'a self, + isrc: &'a str, + ) -> identity::BoxFuture<'a, Result> { + Box::pin(async move { + let response = self + .search_recording_by_isrc(isrc) + .await + .map_err(|e| identity::FetchError::Unavailable(e.to_string()))?; + Ok(identity::IsrcCredits::from_search(isrc, &response)) + }) } } diff --git a/crates/qbz-integrations/src/musicbrainz/identity.rs b/crates/qbz-integrations/src/musicbrainz/identity.rs new file mode 100644 index 000000000..bb173347b --- /dev/null +++ b/crates/qbz-integrations/src/musicbrainz/identity.rs @@ -0,0 +1,1631 @@ +//! Artist identity matching — the #768 contract. +//! +//! Two directions, deliberately NOT one name-keyed helper: +//! +//! 1. **Source artist** (the Qobuz artist whose page is open) → MusicBrainz +//! MBID. Evidence: the exact quoted-name search window plus the ISRCs of +//! the page's own Popular Tracks, resolved to MusicBrainz recording +//! artist credits. +//! 2. **Scene candidate** (a MusicBrainz MBID discovered by tag/area) → Qobuz +//! artist id. Evidence: exact-name Qobuz candidates plus, only when there +//! are several, the ISRCs of each candidate's own top tracks. +//! +//! Everything in this module is either a pure decision function or an +//! orchestration over injected [`IdentityFetcher`] / [`IdentityStore`] / +//! [`SceneCatalog`] implementations, so every rule is unit-testable with no +//! network and no SQLite. The MusicBrainz client implements the fetcher, the +//! SQLite cache implements the store, and `QbzCore` implements the catalog. +//! +//! Invariants (from the guardrails, `qbz-nix-docs/research/2026-09-11-768-…`): +//! - score, result order, `albums_count`, top-track order and string +//! similarity are NEVER identity; they only decide what to inspect; +//! - an unrelated first result is structurally impossible: the only way out +//! of a name search is [`select_exact_name`], which returns a candidate +//! only when EXACTLY ONE result has the requested name and score >= 90; +//! - every artist credit of every recording that really carries the ISRC is +//! inspected; one ISRC is one vote no matter how many rows repeat it; +//! - ties, conflicts and failures fail closed (no identity, no cache write); +//! - durable identity rows are keyed by the stable identifier on the known +//! side (Qobuz artist id → MBID; MBID + catalog scope → Qobuz id), carry +//! their evidence kind and matcher version, and expire at read time. + +use std::collections::{BTreeMap, BTreeSet}; +use std::future::Future; +use std::pin::Pin; + +use serde::{Deserialize, Serialize}; + +use super::models::{ + ArtistResult, ArtistType, MatchConfidence, RecordingSearchResponse, ResolvedArtist, +}; + +/// Bumped whenever the acceptance/voting semantics change, so rows written by +/// an older matcher can never satisfy a newer lookup. +pub const MATCHER_VERSION: u32 = 2; +/// Minimum MusicBrainz search score for an exact-name candidate. +pub const EXACT_NAME_MIN_SCORE: i32 = 90; +/// Size of the quoted-name search window. +pub const NAME_SEARCH_LIMIT: u32 = 10; +/// Distinct Popular-Track ISRCs inspected for one source artist. +pub const MAX_SOURCE_ISRCS: usize = 3; +/// Distinct top-track ISRCs inspected per ambiguous scene candidate. +pub const MAX_SCENE_ISRCS_PER_CANDIDATE: usize = 2; + +pub type BoxFuture<'a, T> = Pin + Send + 'a>>; + +// ============ Query construction ============ + +/// Escape Lucene special characters in a single term. +pub fn lucene_escape(s: &str) -> String { + s.replace('\\', "\\\\") + .replace('"', "\\\"") + .replace(':', "\\:") + .replace('(', "\\(") + .replace(')', "\\)") + .replace('[', "\\[") + .replace(']', "\\]") + .replace('{', "\\{") + .replace('}', "\\}") + .replace('^', "\\^") + .replace('~', "\\~") + .replace('*', "\\*") + .replace('?', "\\?") + .replace('!', "\\!") + .replace('+', "\\+") + .replace('-', "\\-") + .replace('&', "\\&") + .replace('|', "\\|") +} + +/// The complete fielded expression for an exact artist-name search: +/// `artist:""`. The name is escaped, then quoted, then the +/// CALLER URL-encodes the whole expression exactly once +/// ([`artist_search_url`]). Unquoted, a multi-word name lets its second word +/// leak into the global query — `artist:Swim Deep` ranks Deep Purple first +/// (#768). +pub fn artist_query_expression(name: &str) -> String { + format!("artist:\"{}\"", lucene_escape(name.trim())) +} + +/// Full search URL for the quoted-name query against `base`. +pub fn artist_search_url(base: &str, name: &str, limit: u32) -> String { + format!( + "{}/artist?query={}&limit={}&fmt=json", + base, + urlencoding::encode(&artist_query_expression(name)), + limit + ) +} + +/// Key of the cached search window for `name` — the exact expression plus +/// the window size and matcher version, so a coarser normalisation can never +/// share a slot between two different queries. +pub fn artist_query_key(name: &str) -> String { + format!( + "v{}|{}|{}", + MATCHER_VERSION, + NAME_SEARCH_LIMIT, + artist_query_expression(name) + ) +} + +/// ISRC normalised once at the boundary: trimmed, hyphens/spaces dropped, +/// upper-cased. `None` when nothing usable remains. +pub fn normalize_isrc(raw: &str) -> Option { + let cleaned: String = raw + .chars() + .filter(|c| c.is_ascii_alphanumeric()) + .map(|c| c.to_ascii_uppercase()) + .collect(); + if cleaned.is_empty() { + None + } else { + Some(cleaned) + } +} + +// ============ Name candidates ============ + +/// One row of a MusicBrainz artist search window, in the shape that is +/// cached. Deliberately a value type separate from the wire `ArtistResult`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ArtistCandidate { + pub mbid: String, + pub name: String, + pub score: Option, + #[serde(default)] + pub sort_name: Option, + #[serde(default)] + pub artist_type: Option, + #[serde(default)] + pub country: Option, + #[serde(default)] + pub disambiguation: Option, +} + +impl From<&ArtistResult> for ArtistCandidate { + fn from(a: &ArtistResult) -> Self { + Self { + mbid: a.id.clone(), + name: a.name.clone(), + score: a.score, + sort_name: a.sort_name.clone(), + artist_type: a.artist_type.clone(), + country: a.country.clone(), + disambiguation: a.disambiguation.clone(), + } + } +} + +impl ArtistCandidate { + /// The legacy `ResolvedArtist` shape. `confidence` is derived from the + /// search score and is METADATA of a name match; it never means + /// "ISRC verified". + pub fn to_resolved(&self) -> ResolvedArtist { + ResolvedArtist { + mbid: self.mbid.clone(), + name: self.name.clone(), + sort_name: self.sort_name.clone(), + artist_type: ArtistType::from(self.artist_type.as_deref()), + country: self.country.clone(), + disambiguation: self.disambiguation.clone(), + confidence: MatchConfidence::from_score(self.score), + } + } +} + +fn same_name(a: &str, b: &str) -> bool { + a.trim().eq_ignore_ascii_case(b.trim()) || a.trim().to_lowercase() == b.trim().to_lowercase() +} + +/// Every candidate in the window whose trimmed, case-insensitive name equals +/// the requested one and whose score is at least [`EXACT_NAME_MIN_SCORE`]. +/// No punctuation stripping, no fuzzy distance, no aliases. +pub fn exact_name_candidates<'a>(name: &str, window: &'a [ArtistCandidate]) -> Vec<&'a ArtistCandidate> { + window + .iter() + .filter(|c| same_name(&c.name, name) && c.score.unwrap_or(0) >= EXACT_NAME_MIN_SCORE) + .collect() +} + +/// Containment rule: exactly one acceptable exact-name candidate, or nothing. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ExactNameSelection<'a> { + Unique(&'a ArtistCandidate), + /// Several acceptable same-name candidates — a homonym the name cannot + /// split. Carries the count for logs. + Ambiguous(usize), + None, +} + +pub fn select_exact_name<'a>(name: &str, window: &'a [ArtistCandidate]) -> ExactNameSelection<'a> { + let exact = exact_name_candidates(name, window); + match exact.len() { + 0 => ExactNameSelection::None, + 1 => ExactNameSelection::Unique(exact[0]), + n => ExactNameSelection::Ambiguous(n), + } +} + +// ============ ISRC evidence ============ + +/// One MusicBrainz recording that really carries the queried ISRC, with +/// EVERY artist credit on it. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RecordingCredits { + pub recording_mbid: String, + pub credit_mbids: Vec, +} + +/// The complete matching-recording set for one normalised ISRC. This — not a +/// first-result pick — is what gets cached and voted on. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct IsrcCredits { + pub isrc: String, + pub recordings: Vec, +} + +impl IsrcCredits { + /// Keep only the recordings whose own `isrcs` list contains the queried + /// ISRC (case-insensitively). A search hit that merely SCORES against + /// the ISRC contributes nothing. + pub fn from_search(isrc: &str, response: &RecordingSearchResponse) -> Self { + let wanted = normalize_isrc(isrc).unwrap_or_default(); + let recordings = response + .recordings + .iter() + .filter(|r| { + r.isrcs + .as_ref() + .map(|list| { + list.iter() + .any(|i| normalize_isrc(i).as_deref() == Some(wanted.as_str())) + }) + .unwrap_or(false) + }) + .map(|r| RecordingCredits { + recording_mbid: r.id.clone(), + credit_mbids: r + .artist_credit + .as_ref() + .map(|ac| ac.iter().map(|c| c.artist.id.clone()).collect()) + .unwrap_or_default(), + }) + .collect(); + Self { + isrc: wanted, + recordings, + } + } + + /// Union of every credit MBID across every matching recording. + pub fn credited(&self) -> BTreeSet<&str> { + self.recordings + .iter() + .flat_map(|r| r.credit_mbids.iter().map(String::as_str)) + .collect() + } +} + +/// The page's own Popular-Track ISRCs, in server order, deduplicated after +/// normalisation and capped. Tracks credited to another artist id (guest +/// appearances) are excluded — they cannot establish the page identity. +pub fn pick_source_isrcs<'a, I>(source_artist_id: u64, tracks: I, cap: usize) -> Vec +where + I: IntoIterator, Option)>, +{ + let mut seen = BTreeSet::new(); + let mut out = Vec::new(); + for (isrc, artist_id) in tracks { + if out.len() >= cap { + break; + } + if artist_id != Some(source_artist_id) { + continue; + } + let Some(norm) = isrc.and_then(normalize_isrc) else { + continue; + }; + if seen.insert(norm.clone()) { + out.push(norm); + } + } + out +} + +// ============ Decisions ============ + +/// Provenance of a durable identity row. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum EvidenceKind { + /// One uniquely intersecting ISRC, or a unique winner over >= 2 ISRCs. + VerifiedIsrc, + /// Exactly one acceptable result in the quoted-name window; provisional. + UniqueExactName, +} + +impl EvidenceKind { + pub fn as_str(self) -> &'static str { + match self { + Self::VerifiedIsrc => "verified_isrc", + Self::UniqueExactName => "unique_exact_name", + } + } + + pub fn parse(s: &str) -> Option { + match s { + "verified_isrc" => Some(Self::VerifiedIsrc), + "unique_exact_name" => Some(Self::UniqueExactName), + _ => None, + } + } +} + +/// Evidence state of a source-artist resolution. Only `VerifiedIsrc` may be +/// written to the durable identifier-keyed cache. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SourceIdentity { + VerifiedIsrc { mbid: String, isrc_votes: usize }, + UniqueExactName { mbid: String, score: Option }, + Ambiguous, + NotFound, + Unavailable, +} + +impl SourceIdentity { + /// The MBID a caller may DISPLAY (verified or provisional). `None` for + /// every other state. + pub fn displayable_mbid(&self) -> Option<&str> { + match self { + Self::VerifiedIsrc { mbid, .. } | Self::UniqueExactName { mbid, .. } => Some(mbid), + _ => None, + } + } + + pub fn label(&self) -> &'static str { + match self { + Self::VerifiedIsrc { .. } => "verified_isrc", + Self::UniqueExactName { .. } => "unique_exact_name", + Self::Ambiguous => "ambiguous", + Self::NotFound => "not_found", + Self::Unavailable => "unavailable", + } + } +} + +/// Vote among the exact-name candidates with the ISRC evidence. +/// +/// - one ISRC votes for a candidate only when its credits intersect the +/// candidate set in EXACTLY ONE MBID (a collaboration credited to two +/// homonyms is not a vote); +/// - votes for two different MBIDs are a conflict → `Ambiguous`; +/// - one MBID with >= 1 unique vote wins, unless another candidate appears on +/// MORE distinct ISRCs than it (contradictory evidence → `Ambiguous`); +/// - with no unique vote at all, a candidate that appears on >= 2 distinct +/// ISRCs, strictly more than any other, wins; +/// - with no ISRC touching the candidates, the name rule applies: one +/// candidate is `UniqueExactName`, several are `Ambiguous`. +pub fn decide_source_identity( + candidates: &[&ArtistCandidate], + evidence: &[IsrcCredits], +) -> SourceIdentity { + if candidates.is_empty() { + return SourceIdentity::NotFound; + } + let set: BTreeSet<&str> = candidates.iter().map(|c| c.mbid.as_str()).collect(); + + let mut unique_votes: BTreeMap<&str, usize> = BTreeMap::new(); + let mut tally: BTreeMap<&str, usize> = BTreeMap::new(); + let mut seen_isrcs: BTreeSet<&str> = BTreeSet::new(); + for ev in evidence { + // One ISRC, one vote — a repeated evidence row for the same ISRC + // counts once. + if !seen_isrcs.insert(ev.isrc.as_str()) { + continue; + } + let credited = ev.credited(); + let intersection: Vec<&str> = set + .iter() + .copied() + .filter(|m| credited.contains(m)) + .collect(); + for m in &intersection { + *tally.entry(m).or_insert(0) += 1; + } + if intersection.len() == 1 { + *unique_votes.entry(intersection[0]).or_insert(0) += 1; + } + } + + match unique_votes.len() { + 0 => {} + 1 => { + let (winner, votes) = unique_votes.iter().next().map(|(m, v)| (*m, *v)).unwrap(); + let winner_tally = tally.get(winner).copied().unwrap_or(0); + let contradicted = tally + .iter() + .any(|(m, count)| *m != winner && *count > winner_tally); + if contradicted { + return SourceIdentity::Ambiguous; + } + return SourceIdentity::VerifiedIsrc { + mbid: winner.to_string(), + isrc_votes: votes, + }; + } + _ => return SourceIdentity::Ambiguous, + } + + if !tally.is_empty() { + let mut ranked: Vec<(&str, usize)> = tally.iter().map(|(m, c)| (*m, *c)).collect(); + ranked.sort_by(|a, b| b.1.cmp(&a.1).then_with(|| a.0.cmp(b.0))); + let (top, top_count) = ranked[0]; + let second = ranked.get(1).map(|r| r.1).unwrap_or(0); + if top_count >= 2 && top_count > second { + return SourceIdentity::VerifiedIsrc { + mbid: top.to_string(), + isrc_votes: top_count, + }; + } + return SourceIdentity::Ambiguous; + } + + if candidates.len() == 1 { + SourceIdentity::UniqueExactName { + mbid: candidates[0].mbid.clone(), + score: candidates[0].score, + } + } else { + SourceIdentity::Ambiguous + } +} + +/// A Qobuz artist that carries the scene candidate's exact name. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SceneQobuzCandidate { + pub qobuz_id: u64, + pub name: String, + pub image: Option, + pub albums_count: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SceneDecision { + Verified { qobuz_id: u64, isrc_votes: usize }, + Unresolved, +} + +/// For a known MBID, pick the Qobuz duplicate whose top-track ISRCs are +/// credited to that MBID. Several verified duplicates fall back to +/// `albums_count` as an OPERATIONAL tie-break (they are the same artist); +/// a tie there, or no verified duplicate at all, is `Unresolved`. +pub fn decide_scene_candidate( + expected_mbid: &str, + evidence: &[(u64, Option, Vec)], +) -> SceneDecision { + let mut verified: Vec<(u64, Option, usize)> = Vec::new(); + for (qobuz_id, albums_count, credits) in evidence { + let mut voting_isrcs: BTreeSet<&str> = BTreeSet::new(); + for ev in credits { + if ev.credited().contains(expected_mbid) { + voting_isrcs.insert(ev.isrc.as_str()); + } + } + if !voting_isrcs.is_empty() { + verified.push((*qobuz_id, *albums_count, voting_isrcs.len())); + } + } + match verified.len() { + 0 => SceneDecision::Unresolved, + 1 => SceneDecision::Verified { + qobuz_id: verified[0].0, + isrc_votes: verified[0].2, + }, + _ => { + verified.sort_by(|a, b| b.1.unwrap_or(0).cmp(&a.1.unwrap_or(0))); + if verified[0].1.unwrap_or(0) > verified[1].1.unwrap_or(0) { + SceneDecision::Verified { + qobuz_id: verified[0].0, + isrc_votes: verified[0].2, + } + } else { + SceneDecision::Unresolved + } + } + } +} + +// ============ Injected boundaries ============ + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum FetchError { + /// Disabled integration, rate limit after retries, transport or server + /// failure. Never negative-cached. + Unavailable(String), +} + +impl std::fmt::Display for FetchError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Unavailable(s) => write!(f, "unavailable: {s}"), + } + } +} + +/// MusicBrainz reads. The core's shared client implements this; tests use +/// fixtures and count calls. +pub trait IdentityFetcher: Sync { + /// The quoted-name search window for `name` (already limited). + fn artist_candidates<'a>( + &'a self, + name: &'a str, + ) -> BoxFuture<'a, Result, FetchError>>; + /// Every recording carrying `isrc` (normalised) with all its credits. + fn isrc_credits<'a>(&'a self, isrc: &'a str) -> BoxFuture<'a, Result>; +} + +/// Durable source-identity row: Qobuz artist id → MBID. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SourceIdentityRow { + pub qobuz_artist_id: u64, + pub mbid: String, + pub evidence_kind: EvidenceKind, + pub evidence_count: u32, + pub source_name: String, +} + +/// Durable scene-identity row: MBID + catalog scope → Qobuz artist id. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SceneIdentityRow { + pub mbid: String, + pub scope: String, + pub qobuz_id: u64, + pub evidence_kind: EvidenceKind, + pub evidence_count: u32, + pub qobuz_name: String, + pub image: Option, + pub albums_count: Option, +} + +/// Cache boundary. Every getter applies TTL + matcher version itself and +/// treats a malformed row as a miss; failures are logged by the +/// implementation and surface as `None`. +pub trait IdentityStore: Sync { + fn artist_query(&self, key: &str) -> Option>; + fn put_artist_query(&self, key: &str, window: &[ArtistCandidate]); + fn isrc_credits(&self, isrc: &str) -> Option; + fn put_isrc_credits(&self, credits: &IsrcCredits); + /// Only `VerifiedIsrc` rows are ever returned for a source lookup. + fn source_identity(&self, qobuz_artist_id: u64) -> Option; + fn put_source_identity(&self, row: &SourceIdentityRow); + fn scene_identity(&self, mbid: &str, scope: &str) -> Option; + fn put_scene_identity(&self, row: &SceneIdentityRow); +} + +/// Qobuz reads the scene direction needs. `QbzCore` implements this. +pub trait SceneCatalog: Sync { + /// Exact-name Qobuz artists for `name`, from the bounded search. + fn exact_name_candidates<'a>( + &'a self, + name: &'a str, + ) -> BoxFuture<'a, Result, FetchError>>; + /// Normalised, deduplicated top-track ISRCs credited to `qobuz_id` + /// itself, capped by the caller's bound. + fn candidate_isrcs<'a>( + &'a self, + qobuz_id: u64, + cap: usize, + ) -> BoxFuture<'a, Result, FetchError>>; +} + +// ============ Orchestration ============ + +/// Cache-first search window for `name`; the acceptance rule runs again on +/// every hit (a cached window is evidence, not an identity). +async fn candidate_window( + store: Option<&dyn IdentityStore>, + fetcher: &dyn IdentityFetcher, + name: &str, + requests: &mut usize, +) -> Result, FetchError> { + let key = artist_query_key(name); + if let Some(window) = store.and_then(|s| s.artist_query(&key)) { + return Ok(window); + } + *requests += 1; + let window = fetcher.artist_candidates(name).await?; + if let Some(s) = store { + s.put_artist_query(&key, &window); + } + Ok(window) +} + +async fn credits_for( + store: Option<&dyn IdentityStore>, + fetcher: &dyn IdentityFetcher, + isrc: &str, + requests: &mut usize, +) -> Result { + if let Some(hit) = store.and_then(|s| s.isrc_credits(isrc)) { + return Ok(hit); + } + *requests += 1; + let credits = fetcher.isrc_credits(isrc).await?; + if let Some(s) = store { + s.put_isrc_credits(&credits); + } + Ok(credits) +} + +/// The generic name-only resolver (playlist suggestions, musician lookups, +/// local artist images): containment rule, nothing more. Never writes a +/// durable identity — a name alone is not one. +pub async fn resolve_artist_by_name( + store: Option<&dyn IdentityStore>, + fetcher: &dyn IdentityFetcher, + name: &str, +) -> Result, FetchError> { + let mut requests = 0; + let window = candidate_window(store, fetcher, name, &mut requests).await?; + Ok(match select_exact_name(name, &window) { + ExactNameSelection::Unique(c) => Some(c.to_resolved()), + ExactNameSelection::Ambiguous(n) => { + log::info!("[mb-identity] name {name:?}: {n} exact-name candidates, unresolved"); + None + } + ExactNameSelection::None => None, + }) +} + +/// Outcome of a source-artist resolution, with the numbers a log line needs. +#[derive(Debug, Clone)] +pub struct SourceResolution { + pub identity: SourceIdentity, + /// The candidate behind a displayable identity (legacy shape for the + /// Artist Page), `None` otherwise. + pub resolved: Option, + /// MusicBrainz requests actually issued. + pub requests: usize, + pub cache: &'static str, +} + +/// Direction 1: Qobuz artist id (+ name + own-track ISRCs) → MBID. +pub async fn resolve_source_artist( + store: Option<&dyn IdentityStore>, + fetcher: &dyn IdentityFetcher, + qobuz_artist_id: u64, + name: &str, + isrcs: &[String], +) -> SourceResolution { + // Verified identifier-keyed hit: zero requests. + if let Some(row) = store.and_then(|s| s.source_identity(qobuz_artist_id)) { + if row.evidence_kind == EvidenceKind::VerifiedIsrc && !row.mbid.is_empty() { + return SourceResolution { + identity: SourceIdentity::VerifiedIsrc { + mbid: row.mbid.clone(), + isrc_votes: row.evidence_count as usize, + }, + resolved: Some(ResolvedArtist { + mbid: row.mbid, + name: row.source_name, + sort_name: None, + artist_type: ArtistType::Other, + country: None, + disambiguation: None, + confidence: MatchConfidence::Exact, + }), + requests: 0, + cache: "hit", + }; + } + } + + let mut requests = 0; + let window = match candidate_window(store, fetcher, name, &mut requests).await { + Ok(w) => w, + Err(e) => { + log::warn!("[mb-identity] source {qobuz_artist_id} {name:?}: search failed: {e}"); + return SourceResolution { + identity: SourceIdentity::Unavailable, + resolved: None, + requests, + cache: "miss", + }; + } + }; + let exact = exact_name_candidates(name, &window); + if exact.is_empty() { + return SourceResolution { + identity: SourceIdentity::NotFound, + resolved: None, + requests, + cache: "miss", + }; + } + + // Distinct ISRCs only, bounded, sequential through the shared client. + let mut distinct: Vec = Vec::new(); + for raw in isrcs { + if distinct.len() >= MAX_SOURCE_ISRCS { + break; + } + if let Some(norm) = normalize_isrc(raw) { + if !distinct.contains(&norm) { + distinct.push(norm); + } + } + } + let mut evidence: Vec = Vec::new(); + let mut failures = 0usize; + for isrc in &distinct { + match credits_for(store, fetcher, isrc, &mut requests).await { + Ok(c) => evidence.push(c), + Err(e) => { + failures += 1; + log::warn!("[mb-identity] source {qobuz_artist_id}: isrc {isrc} failed: {e}"); + } + } + } + + let identity = decide_source_identity(&exact, &evidence); + let identity = match identity { + // Evidence was requested but none arrived and the name alone cannot + // decide: that is a transient failure, not a verdict. + SourceIdentity::Ambiguous if failures > 0 && evidence.is_empty() => { + SourceIdentity::Unavailable + } + other => other, + }; + + let resolved = identity.displayable_mbid().and_then(|mbid| { + exact + .iter() + .find(|c| c.mbid == mbid) + .map(|c| c.to_resolved()) + }); + + if let (SourceIdentity::VerifiedIsrc { mbid, isrc_votes }, Some(s)) = (&identity, store) { + s.put_source_identity(&SourceIdentityRow { + qobuz_artist_id, + mbid: mbid.clone(), + evidence_kind: EvidenceKind::VerifiedIsrc, + evidence_count: *isrc_votes as u32, + source_name: name.to_string(), + }); + } + + log::info!( + "[mb-identity] source qobuz={qobuz_artist_id} name={name:?}: {} (exact-name candidates {}, distinct isrcs {}, evidence {}, failures {}, mb requests {})", + identity.label(), + exact.len(), + distinct.len(), + evidence.len(), + failures, + requests + ); + + SourceResolution { + identity, + resolved, + requests, + cache: "miss", + } +} + +/// A scene candidate's Qobuz projection, with its provenance. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SceneMatch { + pub qobuz_id: u64, + pub name: String, + pub image: Option, + pub albums_count: Option, + pub kind: EvidenceKind, + /// MusicBrainz requests actually issued. + pub requests: usize, +} + +/// Direction 2: MBID (+ name + catalog scope) → Qobuz artist id. +/// +/// `scope` is the authenticated catalog territory; `None` means the row can +/// be displayed but nothing is persisted (an invented constant would poison +/// the identifier-keyed cache across territories). +pub async fn resolve_scene_candidate( + store: Option<&dyn IdentityStore>, + fetcher: &dyn IdentityFetcher, + catalog: &dyn SceneCatalog, + expected_mbid: &str, + name: &str, + scope: Option<&str>, + blacklist: &(dyn Fn(u64) -> bool + Sync), +) -> Result, FetchError> { + if let (Some(s), Some(scope)) = (store, scope) { + if let Some(row) = s.scene_identity(expected_mbid, scope) { + // A since-blacklisted hit is a MISS, not "no match": the next + // same-name candidate takes the slot on the fresh path below. + if row.qobuz_id > 0 && !blacklist(row.qobuz_id) { + return Ok(Some(SceneMatch { + qobuz_id: row.qobuz_id, + name: row.qobuz_name, + image: row.image, + albums_count: row.albums_count, + kind: row.evidence_kind, + requests: 0, + })); + } + } + } + + let candidates: Vec = catalog + .exact_name_candidates(name) + .await? + .into_iter() + .filter(|c| !blacklist(c.qobuz_id)) + .collect(); + + let persist = |kind: EvidenceKind, count: usize, c: &SceneQobuzCandidate| { + if let (Some(s), Some(scope)) = (store, scope) { + s.put_scene_identity(&SceneIdentityRow { + mbid: expected_mbid.to_string(), + scope: scope.to_string(), + qobuz_id: c.qobuz_id, + evidence_kind: kind, + evidence_count: count as u32, + qobuz_name: c.name.clone(), + image: c.image.clone(), + albums_count: c.albums_count, + }); + } + }; + + match candidates.len() { + 0 => Ok(None), + 1 => { + // Unique exact name: the cheap path, provisional, never labelled + // verified. No ISRC fan-out for a unique name. + let c = &candidates[0]; + persist(EvidenceKind::UniqueExactName, 0, c); + Ok(Some(SceneMatch { + qobuz_id: c.qobuz_id, + name: c.name.clone(), + image: c.image.clone(), + albums_count: c.albums_count, + kind: EvidenceKind::UniqueExactName, + requests: 0, + })) + } + n => { + let mut requests = 0usize; + let mut evidence: Vec<(u64, Option, Vec)> = Vec::new(); + for c in &candidates { + let isrcs = match catalog + .candidate_isrcs(c.qobuz_id, MAX_SCENE_ISRCS_PER_CANDIDATE) + .await + { + Ok(list) => list, + Err(e) => { + log::warn!( + "[mb-identity] scene {expected_mbid}: qobuz {} isrcs failed: {e}", + c.qobuz_id + ); + Vec::new() + } + }; + let mut credits = Vec::new(); + for isrc in isrcs.iter().take(MAX_SCENE_ISRCS_PER_CANDIDATE) { + match credits_for(store, fetcher, isrc, &mut requests).await { + Ok(cr) => credits.push(cr), + Err(e) => log::warn!( + "[mb-identity] scene {expected_mbid}: isrc {isrc} failed: {e}" + ), + } + } + evidence.push((c.qobuz_id, c.albums_count, credits)); + } + let decision = decide_scene_candidate(expected_mbid, &evidence); + log::info!( + "[mb-identity] scene mbid={expected_mbid} name={name:?}: {n} exact-name qobuz candidates, decision {:?}, mb requests {requests}", + decision + ); + match decision { + SceneDecision::Verified { + qobuz_id, + isrc_votes, + } => { + let c = candidates + .iter() + .find(|c| c.qobuz_id == qobuz_id) + .expect("winner comes from the candidate list"); + persist(EvidenceKind::VerifiedIsrc, isrc_votes, c); + Ok(Some(SceneMatch { + qobuz_id, + name: c.name.clone(), + image: c.image.clone(), + albums_count: c.albums_count, + kind: EvidenceKind::VerifiedIsrc, + requests, + })) + } + // Never the most prolific unverified homonym. + SceneDecision::Unresolved => Ok(None), + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Mutex; + + fn cand(mbid: &str, name: &str, score: i32) -> ArtistCandidate { + ArtistCandidate { + mbid: mbid.into(), + name: name.into(), + score: Some(score), + sort_name: None, + artist_type: None, + country: None, + disambiguation: None, + } + } + + fn credits(isrc: &str, recordings: &[&[&str]]) -> IsrcCredits { + IsrcCredits { + isrc: normalize_isrc(isrc).unwrap(), + recordings: recordings + .iter() + .enumerate() + .map(|(i, c)| RecordingCredits { + recording_mbid: format!("rec-{i}"), + credit_mbids: c.iter().map(|s| s.to_string()).collect(), + }) + .collect(), + } + } + + const SWIM_DEEP: &str = "85e67b0f-afd2-46c2-88b2-c3ba9b0883f2"; + const DEEP_PURPLE: &str = "79491354-3d83-40e3-9d8e-7592d58d790a"; + + /// The #768 window, as observed live on 2026-09-11 for the UNQUOTED + /// query: Deep Purple first at 100, Swim Deep second at 88. + fn swim_deep_window() -> Vec { + vec![ + cand(DEEP_PURPLE, "Deep Purple", 100), + cand(SWIM_DEEP, "Swim Deep", 88), + cand("x-deep-blue", "Deep Blue Something", 70), + ] + } + + // ----- query construction ------------------------------------------- + + #[test] + fn query_expression_quotes_the_whole_name() { + assert_eq!(artist_query_expression("Swim Deep"), "artist:\"Swim Deep\""); + assert_eq!(artist_query_expression(" Swim Deep "), "artist:\"Swim Deep\""); + } + + #[test] + fn search_url_encodes_the_expression_exactly_once() { + let url = artist_search_url("https://mb/ws/2", "Swim Deep", 10); + assert_eq!( + url, + "https://mb/ws/2/artist?query=artist%3A%22Swim%20Deep%22&limit=10&fmt=json" + ); + } + + #[test] + fn lucene_specials_are_escaped_before_url_encoding_not_double_encoded() { + let expr = artist_query_expression("AC/DC: \"Live\" (50%)"); + assert_eq!(expr, "artist:\"AC/DC\\: \\\"Live\\\" \\(50%\\)\""); + let url = artist_search_url("b", "AC/DC: \"Live\" (50%)", 10); + // Backslash -> %5C, quote -> %22, percent -> %25, each once. + assert!(url.contains("%5C%3A"), "{url}"); + assert!(url.contains("%5C%22Live%5C%22"), "{url}"); + assert!(url.contains("50%25%5C%29"), "{url}"); + assert!(!url.contains("%2522"), "double-encoded percent: {url}"); + assert!(!url.contains("%255C"), "double-encoded backslash: {url}"); + } + + #[test] + fn query_key_carries_version_limit_and_exact_expression() { + assert_eq!(artist_query_key("Eve."), "v2|10|artist:\"Eve.\""); + assert_ne!(artist_query_key("Eve."), artist_query_key("Eve")); + } + + #[test] + fn isrc_normalisation() { + assert_eq!(normalize_isrc(" gb-aaa-20-12345 ").as_deref(), Some("GBAAA2012345")); + assert_eq!(normalize_isrc(" "), None); + } + + // ----- containment ---------------------------------------------------- + + #[test] + fn swim_deep_window_never_resolves_to_deep_purple() { + let window = swim_deep_window(); + // Score 88 < 90: the containment rule yields nothing — and NOT the + // unrelated first result. + assert_eq!(select_exact_name("Swim Deep", &window), ExactNameSelection::None); + assert_eq!(decide_source_identity(&[], &[]), SourceIdentity::NotFound); + } + + #[test] + fn quoted_window_resolves_swim_deep_uniquely() { + let window = vec![cand(SWIM_DEEP, "Swim Deep", 100)]; + match select_exact_name("swim deep", &window) { + ExactNameSelection::Unique(c) => assert_eq!(c.mbid, SWIM_DEEP), + other => panic!("{other:?}"), + } + } + + #[test] + fn low_score_unrelated_first_result_is_never_accepted() { + let window = vec![cand("x", "Something Else", 100), cand("y", "Swim Deep", 60)]; + assert_eq!(select_exact_name("Swim Deep", &window), ExactNameSelection::None); + } + + #[test] + fn several_exact_names_are_ambiguous_not_best_of_ties() { + let window = vec![cand("a", "Eve", 100), cand("b", "Eve", 95)]; + assert_eq!(select_exact_name("Eve", &window), ExactNameSelection::Ambiguous(2)); + assert!(matches!( + decide_source_identity(&exact_name_candidates("Eve", &window), &[]), + SourceIdentity::Ambiguous + )); + } + + // ----- ISRC decisions ------------------------------------------------- + + #[test] + fn one_isrc_with_one_exact_candidate_in_credits_resolves() { + let window = vec![cand("a", "Eve", 100), cand("b", "Eve", 95)]; + let ev = vec![credits("GBAAA0000001", &[&["a", "guest-z"]])]; + assert_eq!( + decide_source_identity(&exact_name_candidates("Eve", &window), &ev), + SourceIdentity::VerifiedIsrc { + mbid: "a".into(), + isrc_votes: 1 + } + ); + } + + #[test] + fn collaboration_credited_to_two_homonyms_stays_unresolved() { + let window = vec![cand("a", "Eve", 100), cand("b", "Eve", 95)]; + let ev = vec![credits("GBAAA0000001", &[&["a", "b"]])]; + assert_eq!( + decide_source_identity(&exact_name_candidates("Eve", &window), &ev), + SourceIdentity::Ambiguous + ); + } + + #[test] + fn collaboration_does_not_select_its_first_credit() { + // The expected artist is the SECOND credit; the first credit is a + // homonym-free stranger who is not a name candidate at all. + let window = vec![cand("a", "Eve", 100), cand("b", "Eve", 95)]; + let ev = vec![credits("GBAAA0000001", &[&["stranger", "b"]])]; + assert_eq!( + decide_source_identity(&exact_name_candidates("Eve", &window), &ev), + SourceIdentity::VerifiedIsrc { + mbid: "b".into(), + isrc_votes: 1 + } + ); + } + + #[test] + fn two_distinct_isrcs_agreeing_resolve_over_collaborations() { + // No single ISRC is unique, but "a" appears on two distinct ISRCs + // and every other candidate on one. + let window = vec![cand("a", "Eve", 100), cand("b", "Eve", 95), cand("c", "Eve", 92)]; + let ev = vec![ + credits("GBAAA0000001", &[&["a", "b"]]), + credits("GBAAA0000002", &[&["a", "c"]]), + ]; + assert_eq!( + decide_source_identity(&exact_name_candidates("Eve", &window), &ev), + SourceIdentity::VerifiedIsrc { + mbid: "a".into(), + isrc_votes: 2 + } + ); + } + + #[test] + fn duplicate_copies_of_one_isrc_count_once() { + let window = vec![cand("a", "Eve", 100), cand("b", "Eve", 95), cand("c", "Eve", 92)]; + // The same ISRC twice (two recording rows AND two evidence rows). + let ev = vec![ + credits("GBAAA0000001", &[&["a", "b"], &["a", "b"]]), + credits("gbaaa0000001", &[&["a", "b"]]), + ]; + assert_eq!( + decide_source_identity(&exact_name_candidates("Eve", &window), &ev), + SourceIdentity::Ambiguous + ); + } + + #[test] + fn conflicting_unique_votes_fail_closed() { + let window = vec![cand("a", "Eve", 100), cand("b", "Eve", 95)]; + let ev = vec![ + credits("GBAAA0000001", &[&["a"]]), + credits("GBAAA0000002", &[&["b"]]), + ]; + assert_eq!( + decide_source_identity(&exact_name_candidates("Eve", &window), &ev), + SourceIdentity::Ambiguous + ); + } + + #[test] + fn a_unique_vote_contradicted_by_a_more_credited_rival_fails_closed() { + let window = vec![cand("a", "Eve", 100), cand("b", "Eve", 95), cand("c", "Eve", 92)]; + let ev = vec![ + credits("GBAAA0000001", &[&["a"]]), + credits("GBAAA0000002", &[&["b", "c"]]), + credits("GBAAA0000003", &[&["b", "c"]]), + ]; + assert_eq!( + decide_source_identity(&exact_name_candidates("Eve", &window), &ev), + SourceIdentity::Ambiguous + ); + } + + #[test] + fn missing_isrc_evidence_falls_back_to_the_name_rule_only() { + let one = vec![cand("a", "Eve", 100)]; + assert_eq!( + decide_source_identity(&exact_name_candidates("Eve", &one), &[]), + SourceIdentity::UniqueExactName { + mbid: "a".into(), + score: Some(100) + } + ); + // An ISRC credited to nobody in the window is no evidence either way. + let ev = vec![credits("GBAAA0000001", &[&["stranger"]])]; + assert_eq!( + decide_source_identity(&exact_name_candidates("Eve", &one), &ev), + SourceIdentity::UniqueExactName { + mbid: "a".into(), + score: Some(100) + } + ); + } + + #[test] + fn recording_without_the_queried_isrc_contributes_no_vote() { + let response: RecordingSearchResponse = serde_json::from_value(serde_json::json!({ + "count": 2, "offset": 0, + "recordings": [ + { "id": "r1", "score": 100, "isrcs": ["GBAAA0000009"], + "artist-credit": [ { "artist": { "id": "stranger", "name": "S" } } ] }, + { "id": "r2", "score": 100, "isrcs": ["gbaaa0000001"], + "artist-credit": [ { "artist": { "id": "a", "name": "Eve" } } ] } + ] + })) + .unwrap(); + let ev = IsrcCredits::from_search("GBAAA0000001", &response); + assert_eq!(ev.isrc, "GBAAA0000001"); + assert_eq!(ev.recordings.len(), 1, "only the recording that carries the ISRC"); + assert_eq!(ev.recordings[0].recording_mbid, "r2"); + assert!(ev.credited().contains("a")); + assert!(!ev.credited().contains("stranger")); + } + + #[test] + fn guest_top_tracks_are_excluded_and_isrcs_deduped_and_capped() { + let tracks = vec![ + (Some("GBAAA0000001"), Some(7)), + (Some("GBAAA0000002"), Some(99)), // guest appearance + (Some("gb-aaa-00-00001"), Some(7)), // same ISRC, other spelling + (None, Some(7)), + (Some("GBAAA0000003"), Some(7)), + (Some("GBAAA0000004"), Some(7)), + (Some("GBAAA0000005"), Some(7)), + ]; + let picked = pick_source_isrcs(7, tracks, MAX_SOURCE_ISRCS); + assert_eq!(picked, vec!["GBAAA0000001", "GBAAA0000003", "GBAAA0000004"]); + } + + // ----- scene decisions ------------------------------------------------ + + #[test] + fn scene_picks_the_duplicate_whose_isrcs_credit_the_expected_mbid() { + let ev = vec![ + (1, Some(40), vec![credits("A1", &[&["other"]])]), + (2, Some(3), vec![credits("B1", &[&["x", "mb-expected"]])]), + ]; + assert_eq!( + decide_scene_candidate("mb-expected", &ev), + SceneDecision::Verified { + qobuz_id: 2, + isrc_votes: 1 + } + ); + } + + #[test] + fn scene_with_no_verified_duplicate_is_unresolved_not_max_albums() { + let ev = vec![ + (1, Some(40), vec![credits("A1", &[&["other"]])]), + (2, Some(3), vec![]), + ]; + assert_eq!(decide_scene_candidate("mb-expected", &ev), SceneDecision::Unresolved); + } + + #[test] + fn scene_two_verified_duplicates_use_albums_count_then_tie_fails() { + let ev = vec![ + (1, Some(40), vec![credits("A1", &[&["mb-expected"]])]), + (2, Some(3), vec![credits("B1", &[&["mb-expected"]])]), + ]; + assert_eq!( + decide_scene_candidate("mb-expected", &ev), + SceneDecision::Verified { + qobuz_id: 1, + isrc_votes: 1 + } + ); + let tie = vec![ + (1, Some(3), vec![credits("A1", &[&["mb-expected"]])]), + (2, Some(3), vec![credits("B1", &[&["mb-expected"]])]), + ]; + assert_eq!(decide_scene_candidate("mb-expected", &tie), SceneDecision::Unresolved); + } + + // ----- orchestration with fakes (request counting) --------------------- + + #[derive(Default)] + struct FakeFetcher { + windows: Mutex>>, + isrcs: Mutex>, + fail_isrcs: bool, + fail_search: bool, + name_requests: Mutex, + isrc_requests: Mutex, + } + + impl FakeFetcher { + fn with_window(self, name: &str, window: Vec) -> Self { + self.windows.lock().unwrap().insert(name.to_string(), window); + self + } + fn with_isrc(self, c: IsrcCredits) -> Self { + self.isrcs.lock().unwrap().insert(c.isrc.clone(), c); + self + } + fn counts(&self) -> (usize, usize) { + (*self.name_requests.lock().unwrap(), *self.isrc_requests.lock().unwrap()) + } + } + + impl IdentityFetcher for FakeFetcher { + fn artist_candidates<'a>( + &'a self, + name: &'a str, + ) -> BoxFuture<'a, Result, FetchError>> { + Box::pin(async move { + *self.name_requests.lock().unwrap() += 1; + if self.fail_search { + return Err(FetchError::Unavailable("503".into())); + } + Ok(self.windows.lock().unwrap().get(name).cloned().unwrap_or_default()) + }) + } + fn isrc_credits<'a>(&'a self, isrc: &'a str) -> BoxFuture<'a, Result> { + Box::pin(async move { + *self.isrc_requests.lock().unwrap() += 1; + if self.fail_isrcs { + return Err(FetchError::Unavailable("503".into())); + } + Ok(self + .isrcs + .lock() + .unwrap() + .get(isrc) + .cloned() + .unwrap_or(IsrcCredits { + isrc: isrc.to_string(), + recordings: vec![], + })) + }) + } + } + + #[derive(Default)] + struct MemStore { + queries: Mutex>>, + isrcs: Mutex>, + sources: Mutex>, + scenes: Mutex>, + } + + impl IdentityStore for MemStore { + fn artist_query(&self, key: &str) -> Option> { + self.queries.lock().unwrap().get(key).cloned() + } + fn put_artist_query(&self, key: &str, window: &[ArtistCandidate]) { + self.queries.lock().unwrap().insert(key.into(), window.to_vec()); + } + fn isrc_credits(&self, isrc: &str) -> Option { + self.isrcs.lock().unwrap().get(isrc).cloned() + } + fn put_isrc_credits(&self, credits: &IsrcCredits) { + self.isrcs.lock().unwrap().insert(credits.isrc.clone(), credits.clone()); + } + fn source_identity(&self, qobuz_artist_id: u64) -> Option { + self.sources + .lock() + .unwrap() + .get(&qobuz_artist_id) + .filter(|r| r.evidence_kind == EvidenceKind::VerifiedIsrc) + .cloned() + } + fn put_source_identity(&self, row: &SourceIdentityRow) { + self.sources.lock().unwrap().insert(row.qobuz_artist_id, row.clone()); + } + fn scene_identity(&self, mbid: &str, scope: &str) -> Option { + self.scenes.lock().unwrap().get(&(mbid.into(), scope.into())).cloned() + } + fn put_scene_identity(&self, row: &SceneIdentityRow) { + self.scenes + .lock() + .unwrap() + .insert((row.mbid.clone(), row.scope.clone()), row.clone()); + } + } + + fn rt() -> tokio::runtime::Runtime { + tokio::runtime::Builder::new_current_thread().build().unwrap() + } + + #[test] + fn source_resolution_is_bounded_to_the_distinct_isrc_cap() { + let fetcher = FakeFetcher::default() + .with_window("Eve", vec![cand("a", "Eve", 100), cand("b", "Eve", 95)]) + .with_isrc(credits("I1", &[&["a", "b"]])) + .with_isrc(credits("I2", &[&["a", "b"]])) + .with_isrc(credits("I3", &[&["a", "b"]])); + let store = MemStore::default(); + let isrcs: Vec = ["I1", "i1", "I2", "I3", "I4", "I5"] + .iter() + .map(|s| s.to_string()) + .collect(); + let out = rt().block_on(resolve_source_artist(Some(&store), &fetcher, 7, "Eve", &isrcs)); + assert_eq!(fetcher.counts(), (1, MAX_SOURCE_ISRCS)); + assert_eq!(out.requests, 1 + MAX_SOURCE_ISRCS); + assert_eq!(out.identity, SourceIdentity::Ambiguous); + assert!(store.sources.lock().unwrap().is_empty(), "ambiguous writes no identity"); + } + + #[test] + fn verified_source_is_cached_by_qobuz_id_and_replays_with_zero_requests() { + let fetcher = FakeFetcher::default() + .with_window("Eve", vec![cand("a", "Eve", 100), cand("b", "Eve", 95)]) + .with_isrc(credits("I1", &[&["a"]])); + let store = MemStore::default(); + let isrcs = vec!["I1".to_string()]; + let first = rt().block_on(resolve_source_artist(Some(&store), &fetcher, 7, "Eve", &isrcs)); + assert!(matches!(first.identity, SourceIdentity::VerifiedIsrc { .. })); + assert_eq!(first.resolved.as_ref().map(|r| r.mbid.as_str()), Some("a")); + + // Same name, OTHER Qobuz id: no shared mapping. + assert!(store.source_identity(8).is_none()); + + let again = rt().block_on(resolve_source_artist(Some(&store), &fetcher, 7, "Eve", &isrcs)); + assert_eq!(again.cache, "hit"); + assert_eq!(again.requests, 0); + assert_eq!(fetcher.counts(), (1, 1), "second resolve issued nothing"); + } + + #[test] + fn containment_only_result_writes_no_source_identity() { + let fetcher = FakeFetcher::default().with_window("Eve", vec![cand("a", "Eve", 100)]); + let store = MemStore::default(); + let out = rt().block_on(resolve_source_artist(Some(&store), &fetcher, 7, "Eve", &[])); + assert!(matches!(out.identity, SourceIdentity::UniqueExactName { .. })); + assert!(store.sources.lock().unwrap().is_empty()); + // ...but the search WINDOW is cached as evidence and re-gated. + assert!(store.artist_query(&artist_query_key("Eve")).is_some()); + let again = rt().block_on(resolve_source_artist(Some(&store), &fetcher, 7, "Eve", &[])); + assert_eq!(again.requests, 0); + } + + #[test] + fn search_failure_is_unavailable_and_cached_nowhere() { + let fetcher = FakeFetcher { + fail_search: true, + ..Default::default() + }; + let store = MemStore::default(); + let out = rt().block_on(resolve_source_artist(Some(&store), &fetcher, 7, "Eve", &[])); + assert_eq!(out.identity, SourceIdentity::Unavailable); + assert!(store.queries.lock().unwrap().is_empty()); + assert!(store.sources.lock().unwrap().is_empty()); + } + + #[test] + fn isrc_failure_with_ambiguous_name_is_unavailable_not_negative() { + let fetcher = FakeFetcher { + fail_isrcs: true, + ..Default::default() + } + .with_window("Eve", vec![cand("a", "Eve", 100), cand("b", "Eve", 95)]); + let store = MemStore::default(); + let out = rt().block_on(resolve_source_artist( + Some(&store), + &fetcher, + 7, + "Eve", + &["I1".to_string()], + )); + assert_eq!(out.identity, SourceIdentity::Unavailable); + assert!(store.sources.lock().unwrap().is_empty()); + assert!(store.isrcs.lock().unwrap().is_empty()); + } + + #[test] + fn name_resolver_never_returns_an_unrelated_first_result() { + let fetcher = FakeFetcher::default().with_window("Swim Deep", swim_deep_window()); + let out = rt() + .block_on(resolve_artist_by_name(None, &fetcher, "Swim Deep")) + .unwrap(); + assert!(out.is_none()); + } + + #[derive(Default)] + struct FakeCatalog { + by_name: std::collections::HashMap>, + isrcs: std::collections::HashMap>, + page_requests: Mutex, + } + + impl SceneCatalog for FakeCatalog { + fn exact_name_candidates<'a>( + &'a self, + name: &'a str, + ) -> BoxFuture<'a, Result, FetchError>> { + Box::pin(async move { Ok(self.by_name.get(name).cloned().unwrap_or_default()) }) + } + fn candidate_isrcs<'a>( + &'a self, + qobuz_id: u64, + cap: usize, + ) -> BoxFuture<'a, Result, FetchError>> { + Box::pin(async move { + *self.page_requests.lock().unwrap() += 1; + Ok(self + .isrcs + .get(&qobuz_id) + .cloned() + .unwrap_or_default() + .into_iter() + .take(cap) + .collect()) + }) + } + } + + fn qc(id: u64, name: &str, albums: u32) -> SceneQobuzCandidate { + SceneQobuzCandidate { + qobuz_id: id, + name: name.into(), + image: None, + albums_count: Some(albums), + } + } + + #[test] + fn unique_scene_name_performs_no_isrc_fan_out_and_is_provisional() { + let mut catalog = FakeCatalog::default(); + catalog.by_name.insert("Eve".into(), vec![qc(1, "Eve", 4)]); + let fetcher = FakeFetcher::default(); + let store = MemStore::default(); + let out = rt() + .block_on(resolve_scene_candidate( + Some(&store), + &fetcher, + &catalog, + "mb-1", + "Eve", + Some("FR"), + &|_| false, + )) + .unwrap() + .unwrap(); + assert_eq!(out.qobuz_id, 1); + assert_eq!(out.kind, EvidenceKind::UniqueExactName); + assert_eq!(fetcher.counts(), (0, 0)); + assert_eq!(*catalog.page_requests.lock().unwrap(), 0); + let row = store.scene_identity("mb-1", "FR").unwrap(); + assert_eq!(row.evidence_kind, EvidenceKind::UniqueExactName); + // Same name, other MBID: no shared mapping. + assert!(store.scene_identity("mb-2", "FR").is_none()); + // Other scope: no shared mapping either. + assert!(store.scene_identity("mb-1", "US").is_none()); + } + + #[test] + fn ambiguous_scene_name_is_bounded_and_picks_the_verified_duplicate() { + let mut catalog = FakeCatalog::default(); + catalog + .by_name + .insert("Eve".into(), vec![qc(1, "Eve", 40), qc(2, "Eve", 3)]); + catalog.isrcs.insert(1, vec!["A1".into(), "A2".into(), "A3".into()]); + catalog.isrcs.insert(2, vec!["B1".into(), "B2".into(), "B3".into()]); + let fetcher = FakeFetcher::default() + .with_isrc(credits("A1", &[&["other"]])) + .with_isrc(credits("A2", &[&["other"]])) + .with_isrc(credits("B1", &[&["mb-1"]])) + .with_isrc(credits("B2", &[&["mb-1"]])); + let store = MemStore::default(); + let out = rt() + .block_on(resolve_scene_candidate( + Some(&store), + &fetcher, + &catalog, + "mb-1", + "Eve", + Some("FR"), + &|_| false, + )) + .unwrap() + .unwrap(); + assert_eq!(out.qobuz_id, 2, "the smaller catalog wins because it is the verified one"); + assert_eq!(out.kind, EvidenceKind::VerifiedIsrc); + assert_eq!(fetcher.counts(), (0, 2 * MAX_SCENE_ISRCS_PER_CANDIDATE)); + assert_eq!(*catalog.page_requests.lock().unwrap(), 2); + assert_eq!( + store.scene_identity("mb-1", "FR").unwrap().evidence_kind, + EvidenceKind::VerifiedIsrc + ); + // Replay: zero requests. + let again = rt() + .block_on(resolve_scene_candidate( + Some(&store), + &fetcher, + &catalog, + "mb-1", + "Eve", + Some("FR"), + &|_| false, + )) + .unwrap() + .unwrap(); + assert_eq!(again.requests, 0); + assert_eq!(fetcher.counts(), (0, 2 * MAX_SCENE_ISRCS_PER_CANDIDATE)); + } + + #[test] + fn ambiguous_scene_name_without_evidence_omits_the_row() { + let mut catalog = FakeCatalog::default(); + catalog + .by_name + .insert("Eve".into(), vec![qc(1, "Eve", 40), qc(2, "Eve", 3)]); + let fetcher = FakeFetcher::default(); + let store = MemStore::default(); + let out = rt() + .block_on(resolve_scene_candidate( + Some(&store), + &fetcher, + &catalog, + "mb-1", + "Eve", + Some("FR"), + &|_| false, + )) + .unwrap(); + assert!(out.is_none(), "never the most prolific unverified homonym"); + assert!(store.scenes.lock().unwrap().is_empty()); + } + + #[test] + fn blacklisted_scene_hit_is_a_miss_and_unknown_scope_persists_nothing() { + let mut catalog = FakeCatalog::default(); + catalog.by_name.insert("Eve".into(), vec![qc(1, "Eve", 4), qc(9, "Eve", 1)]); + let fetcher = FakeFetcher::default(); + let store = MemStore::default(); + store.put_scene_identity(&SceneIdentityRow { + mbid: "mb-1".into(), + scope: "FR".into(), + qobuz_id: 9, + evidence_kind: EvidenceKind::VerifiedIsrc, + evidence_count: 1, + qobuz_name: "Eve".into(), + image: None, + albums_count: Some(1), + }); + let blocked = |id: u64| id == 9; + // Hit on 9 is blacklisted -> miss -> fresh path: 1 remains unique. + let out = rt() + .block_on(resolve_scene_candidate( + Some(&store), + &fetcher, + &catalog, + "mb-1", + "Eve", + Some("FR"), + &blocked, + )) + .unwrap() + .unwrap(); + assert_eq!(out.qobuz_id, 1); + + let store2 = MemStore::default(); + let _ = rt() + .block_on(resolve_scene_candidate( + Some(&store2), + &fetcher, + &catalog, + "mb-1", + "Eve", + None, + &|_| false, + )) + .unwrap(); + assert!(store2.scenes.lock().unwrap().is_empty(), "no scope, no durable row"); + } +} diff --git a/crates/qbz-integrations/src/musicbrainz/location.rs b/crates/qbz-integrations/src/musicbrainz/location.rs index 708f05411..0986b2d02 100644 --- a/crates/qbz-integrations/src/musicbrainz/location.rs +++ b/crates/qbz-integrations/src/musicbrainz/location.rs @@ -266,7 +266,10 @@ pub fn build_scene_cache_key(area_id: &str, seeds: &AffinitySeeds) -> String { /// the per-genre limit, the `LocationCandidate` shape. Bumping is how old /// entries get abandoned; there is no migration and none is wanted, since a /// stale scene is indistinguishable from a fresh one on the wire. -pub const SCENE_CACHE_KEY_VERSION: &str = "v1"; +/// v2 (2026-09-11, #768): the catalog scope became a REAL key component and +/// rows are re-derived from the identifier-keyed identity cache on replay; +/// v1 entries (name-validated by `max(albums_count)`) are orphaned on purpose. +pub const SCENE_CACHE_KEY_VERSION: &str = "v2"; /// Everything a scene discovery response actually depends on. /// diff --git a/crates/qbz-integrations/src/musicbrainz/mod.rs b/crates/qbz-integrations/src/musicbrainz/mod.rs index 4fdd20379..e87085f0a 100644 --- a/crates/qbz-integrations/src/musicbrainz/mod.rs +++ b/crates/qbz-integrations/src/musicbrainz/mod.rs @@ -26,6 +26,7 @@ //! ``` mod client; +pub mod identity; mod models; #[cfg(feature = "cache")] diff --git a/crates/qbz-qt/src/artist_qt.rs b/crates/qbz-qt/src/artist_qt.rs index 4bbf08ca4..377cb2f08 100644 --- a/crates/qbz-qt/src/artist_qt.rs +++ b/crates/qbz-qt/src/artist_qt.rs @@ -634,6 +634,25 @@ pub async fn load_artist( runtime: &Arc>, artist_id: &str, ) -> Result { + load_artist_with_evidence(runtime, artist_id) + .await + .map(|(data, _)| data) +} + +/// The raw identity evidence the Artist Page hands the MusicBrainz source +/// resolver (#768): the page's Qobuz id and the ISRCs of its OWN Popular +/// Tracks. Collected off the raw `/artist/page` response before the lossy +/// `map_artist`, so no field is added to the rendered document and no second +/// Qobuz request is made. Never reaches QML. +pub(crate) struct SourceEvidence { + pub qobuz_id: u64, + pub isrcs: Vec, +} + +pub(crate) async fn load_artist_with_evidence( + runtime: &Arc>, + artist_id: &str, +) -> Result<(ArtistViewData, SourceEvidence), String> { let id: u64 = artist_id .parse() .map_err(|_| format!("invalid artist id: {artist_id}"))?; @@ -642,7 +661,19 @@ pub async fn load_artist( .get_artist_page(id, None) .await .map_err(|e| e.to_string())?; - Ok(map_artist(page)) + let isrcs = qbz_integrations::musicbrainz::identity::pick_source_isrcs( + page.id, + page.top_tracks + .iter() + .flatten() + .map(|t| (t.isrc.as_deref(), t.artist.as_ref().map(|a| a.id))), + qbz_integrations::musicbrainz::identity::MAX_SOURCE_ISRCS, + ); + let evidence = SourceEvidence { + qobuz_id: page.id, + isrcs, + }; + Ok((map_artist(page), evidence)) } fn map_artist(page: PageArtistResponse) -> ArtistViewData { @@ -1340,7 +1371,7 @@ pub async fn load_artist_view( artist_id: &str, ) -> Result { let t = Instant::now(); - let mut data = load_artist(runtime, artist_id).await?; + let (mut data, evidence) = load_artist_with_evidence(runtime, artist_id).await?; // The embedded artist/page buckets use release-date order. A persisted // popularity choice must fetch the server ranking before the first paint. for section in &mut data.release_sections { @@ -1411,6 +1442,7 @@ pub async fn load_artist_view( generation, artist_id.to_string(), data.name.clone(), + evidence, similar_names, mb_on, ); @@ -1598,6 +1630,7 @@ fn spawn_enrichment( generation: u64, artist_id: String, artist_name: String, + evidence: SourceEvidence, similar_names: Vec, mb_on: bool, ) { @@ -1619,7 +1652,7 @@ fn spawn_enrichment( return; } crate::spawn(async move { - let meta = match load_mb_metadata(&runtime, &artist_name).await { + let meta = match load_mb_metadata(&runtime, &evidence, &artist_name).await { Ok(Some(meta)) => meta, Ok(None) => { publish_mb_unavailable(generation); @@ -1735,27 +1768,40 @@ struct MbMetadata { origin: MbOriginJson, } -/// Resolve the artist name to an MBID, then fetch its metadata. `Ok(None)` = -/// MB disabled or no confident match — the caller hides every MB section. +/// Resolve the SOURCE artist (Qobuz id + name + own Popular-Track ISRCs) to +/// an MBID, then fetch its metadata. `Ok(None)` = MB disabled, unavailable, +/// or no identity strong enough to show — the caller hides every MB section. +/// Missing Network data beats another band's biography (#768). Identity and +/// voting live in core/`identity`; this only passes evidence and renders. async fn load_mb_metadata( runtime: &Arc>, + evidence: &SourceEvidence, artist_name: &str, ) -> Result, String> { if !runtime.core().musicbrainz_is_enabled().await { return Ok(None); } - let resolved = runtime + let resolution = runtime .core() - .musicbrainz_resolve_artist(artist_name) - .await - .map_err(|e| e.to_string())?; - let Some(resolved) = resolved else { + .musicbrainz_resolve_source_artist(evidence.qobuz_id, artist_name, &evidence.isrcs) + .await; + log::info!( + "[qbz-qt] artist {} {:?}: MB identity {} ({} isrcs offered, {} MB requests, cache {})", + evidence.qobuz_id, + artist_name, + resolution.identity.label(), + evidence.isrcs.len(), + resolution.requests, + resolution.cache + ); + let Some(mbid) = resolution + .identity + .displayable_mbid() + .map(str::to_string) + .filter(|m| !m.is_empty()) + else { return Ok(None); }; - let mbid = resolved.mbid; - if mbid.is_empty() { - return Ok(None); - } let meta = runtime .core() .musicbrainz_get_artist_metadata(&mbid)