fix(musicbrainz): identity-based artist matching, never an unrelated first result (#768)
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:"<lucene-escaped name>"`, 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
This commit is contained in:
7 files changed
+2555
-207
No files matched your search
@@ -634,6 +634,25 @@ pub async fn load_artist(
|
||||
runtime: &Arc<AppRuntime<LoggingAdapter>>,
|
||||
artist_id: &str,
|
||||
) -> Result<ArtistViewData, String> {
|
||||
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<String>,
|
||||
}
|
||||
|
||||
pub(crate) async fn load_artist_with_evidence(
|
||||
runtime: &Arc<AppRuntime<LoggingAdapter>>,
|
||||
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<String, String> {
|
||||
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<String>,
|
||||
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<AppRuntime<LoggingAdapter>>,
|
||||
evidence: &SourceEvidence,
|
||||
artist_name: &str,
|
||||
) -> Result<Option<MbMetadata>, 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)
|
||||
|
||||
Reference in new issue
Block a user