feat(reco): identity guardrail replaces the genre blocklist; rank by score
- INCOMPATIBLE_GENRES / INCOMPATIBLE_TITLE_KEYWORDS removed: a global
blocklist papered over an identity bug (name search returning a
homonym) and punished every user's taste for it.
- artist_guardrail: ArtistLookup seam (mockable), id before text
(qobuz:{id} nodes resolve by id; MBIDs hit the mb_qobuz_artist_match
cache before search_artists), validation relative to the playlist
seeds (shared level-1 genre root or a shared MusicBrainz tag), homonym
tie-break by seed neighbourhood / label, reject when no signal exists.
- suggestions: stable sort by similarity_score desc, dedup keeping the
higher score, THEN truncate (the shuffle threw the score away and the
dedup kept the first hit while claiming the best).
- Eight network-free tests cover the rules; rand is now unused here.
cx
This commit is contained in:
4 files changed
+1072
-397
No files matched your search
@@ -0,0 +1,852 @@
|
||||
//! Identity-first Qobuz artist resolution for playlist recommendations.
|
||||
//!
|
||||
//! Network access lives behind [`ArtistLookup`] so the acceptance rules can be
|
||||
//! exercised without Qobuz or MusicBrainz sessions in unit tests.
|
||||
|
||||
use std::collections::HashSet;
|
||||
use std::future::Future;
|
||||
use std::pin::Pin;
|
||||
use std::sync::Arc;
|
||||
|
||||
use qbz_integrations::musicbrainz::cache::{MusicBrainzCache, QobuzArtistMatch};
|
||||
use qbz_integrations::MusicBrainzClient;
|
||||
use qbz_models::Artist;
|
||||
use qbz_qobuz::QobuzClient;
|
||||
use tokio::sync::RwLock;
|
||||
|
||||
/// Boxed future used by the object-safe lookup trait.
|
||||
pub type ArtistLookupFuture<'a, T> = Pin<Box<dyn Future<Output = T> + Send + 'a>>;
|
||||
|
||||
/// The identity and seed-relative facts needed by the recommendation guardrail.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ArtistFacts {
|
||||
/// Qobuz artist ID.
|
||||
pub id: u64,
|
||||
/// Catalog artist name.
|
||||
pub name: String,
|
||||
/// Number of releases advertised by the artist endpoint.
|
||||
pub albums_count: u32,
|
||||
/// Best available Qobuz artist image.
|
||||
pub image: Option<String>,
|
||||
/// Level-1 genre ancestor IDs derived from a bounded album sample.
|
||||
pub genre_roots: Vec<u64>,
|
||||
/// Label IDs derived from the same album sample.
|
||||
pub label_ids: Vec<u64>,
|
||||
}
|
||||
|
||||
/// Network/cache seam for artist identity validation.
|
||||
pub trait ArtistLookup: Send + Sync {
|
||||
/// Resolve an asserted Qobuz identity without text search.
|
||||
fn artist_by_id(&self, id: u64) -> ArtistLookupFuture<'_, Option<ArtistFacts>>;
|
||||
/// Return exact-name candidates for a text fallback.
|
||||
fn search_artists(&self, name: &str) -> ArtistLookupFuture<'_, Vec<ArtistFacts>>;
|
||||
/// Return the bounded Qobuz neighbourhood for a seed artist.
|
||||
fn similar_artist_ids(&self, id: u64) -> ArtistLookupFuture<'_, Vec<u64>>;
|
||||
/// Read a confirmed MusicBrainz-name to Qobuz-ID match.
|
||||
fn cached_match(&self, name_normalized: &str) -> ArtistLookupFuture<'_, Option<u64>>;
|
||||
/// Persist a seed-validated cold-search match.
|
||||
fn cache_match(&self, name_normalized: &str, artist: &ArtistFacts);
|
||||
/// Fetch normalized MusicBrainz tags for a real MBID.
|
||||
fn mb_tags(&self, mbid: &str) -> ArtistLookupFuture<'_, Vec<String>>;
|
||||
}
|
||||
|
||||
/// Production lookup backed by the existing shared clients and optional cache.
|
||||
pub(crate) struct ProductionArtistLookup {
|
||||
qobuz_client: Arc<RwLock<Option<QobuzClient>>>,
|
||||
mb_cache: Arc<std::sync::Mutex<Option<MusicBrainzCache>>>,
|
||||
mb_client: Arc<MusicBrainzClient>,
|
||||
}
|
||||
|
||||
impl ProductionArtistLookup {
|
||||
pub(crate) fn new(
|
||||
qobuz_client: Arc<RwLock<Option<QobuzClient>>>,
|
||||
mb_cache: Arc<std::sync::Mutex<Option<MusicBrainzCache>>>,
|
||||
mb_client: Arc<MusicBrainzClient>,
|
||||
) -> Self {
|
||||
Self {
|
||||
qobuz_client,
|
||||
mb_cache,
|
||||
mb_client,
|
||||
}
|
||||
}
|
||||
|
||||
async fn enrich_artist(client: &QobuzClient, basic: Artist) -> ArtistFacts {
|
||||
let detailed = client
|
||||
.get_artist_with_pagination_and_locale(basic.id, true, Some(5), None, Some("en"))
|
||||
.await;
|
||||
|
||||
let detailed = match detailed {
|
||||
Ok(artist) => Some(artist),
|
||||
Err(error) => {
|
||||
log::warn!(
|
||||
"[SuggestionsEngine] Failed to load seed facts for Qobuz artist {}: {}",
|
||||
basic.id,
|
||||
error
|
||||
);
|
||||
None
|
||||
}
|
||||
};
|
||||
|
||||
let albums = detailed
|
||||
.as_ref()
|
||||
.and_then(|artist| artist.albums.as_ref())
|
||||
.map(|albums| albums.items.as_slice())
|
||||
.unwrap_or_default();
|
||||
let mut genre_roots = Vec::new();
|
||||
let mut label_ids = Vec::new();
|
||||
|
||||
for album in albums {
|
||||
if let Some(root) = album.genre.as_ref().and_then(|genre| {
|
||||
genre.path.as_ref().and_then(|path| match path.as_slice() {
|
||||
[_, level_one, ..] => Some(*level_one),
|
||||
[only] => Some(*only),
|
||||
[] => None,
|
||||
})
|
||||
}) {
|
||||
if !genre_roots.contains(&root) {
|
||||
genre_roots.push(root);
|
||||
}
|
||||
}
|
||||
if let Some(label) = &album.label {
|
||||
if !label_ids.contains(&label.id) {
|
||||
label_ids.push(label.id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let name = detailed
|
||||
.as_ref()
|
||||
.map(|artist| artist.name.as_str())
|
||||
.filter(|name| !name.is_empty())
|
||||
.unwrap_or(&basic.name)
|
||||
.to_string();
|
||||
let albums_count = detailed
|
||||
.as_ref()
|
||||
.and_then(|artist| artist.albums_count)
|
||||
.or_else(|| {
|
||||
detailed
|
||||
.as_ref()
|
||||
.and_then(|artist| artist.albums.as_ref())
|
||||
.map(|albums| albums.total)
|
||||
})
|
||||
.or(basic.albums_count)
|
||||
.unwrap_or(0);
|
||||
let image = detailed
|
||||
.as_ref()
|
||||
.and_then(|artist| artist.image.as_ref())
|
||||
.or(basic.image.as_ref())
|
||||
.and_then(|image| image.best().cloned());
|
||||
|
||||
ArtistFacts {
|
||||
id: basic.id,
|
||||
name,
|
||||
albums_count,
|
||||
image,
|
||||
genre_roots,
|
||||
label_ids,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ArtistLookup for ProductionArtistLookup {
|
||||
fn artist_by_id(&self, id: u64) -> ArtistLookupFuture<'_, Option<ArtistFacts>> {
|
||||
Box::pin(async move {
|
||||
let guard = self.qobuz_client.read().await;
|
||||
let client = guard.as_ref()?;
|
||||
|
||||
// A synthetic qobuz:{id} node is an identity assertion. Resolve
|
||||
// that ID directly before enriching it; never turn it into text.
|
||||
match client.get_artist_basic(id).await {
|
||||
Ok(artist) => Some(Self::enrich_artist(client, artist).await),
|
||||
Err(error) => {
|
||||
log::warn!(
|
||||
"[SuggestionsEngine] Failed to resolve Qobuz artist ID {}: {}",
|
||||
id,
|
||||
error
|
||||
);
|
||||
None
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn search_artists(&self, name: &str) -> ArtistLookupFuture<'_, Vec<ArtistFacts>> {
|
||||
let name = name.to_string();
|
||||
Box::pin(async move {
|
||||
let guard = self.qobuz_client.read().await;
|
||||
let Some(client) = guard.as_ref() else {
|
||||
return Vec::new();
|
||||
};
|
||||
let results = match client.search_artists(&name, 10, 0, None).await {
|
||||
Ok(results) => results,
|
||||
Err(error) => {
|
||||
log::warn!(
|
||||
"[SuggestionsEngine] Artist search failed for '{}': {}",
|
||||
name,
|
||||
error
|
||||
);
|
||||
return Vec::new();
|
||||
}
|
||||
};
|
||||
|
||||
let normalized = normalize_name(&name);
|
||||
let the_variant = format!("the {normalized}");
|
||||
let relevant = results.items.into_iter().filter(|artist| {
|
||||
let candidate = normalize_name(&artist.name);
|
||||
candidate == normalized || candidate == the_variant
|
||||
});
|
||||
let mut facts = Vec::new();
|
||||
for artist in relevant {
|
||||
facts.push(Self::enrich_artist(client, artist).await);
|
||||
}
|
||||
facts
|
||||
})
|
||||
}
|
||||
|
||||
fn similar_artist_ids(&self, id: u64) -> ArtistLookupFuture<'_, Vec<u64>> {
|
||||
Box::pin(async move {
|
||||
let guard = self.qobuz_client.read().await;
|
||||
let Some(client) = guard.as_ref() else {
|
||||
return Vec::new();
|
||||
};
|
||||
match client.get_similar_artists(id, 20, 0).await {
|
||||
Ok(page) => page.items.into_iter().map(|artist| artist.id).collect(),
|
||||
Err(error) => {
|
||||
log::warn!(
|
||||
"[SuggestionsEngine] Failed to load similar artists for Qobuz ID {}: {}",
|
||||
id,
|
||||
error
|
||||
);
|
||||
Vec::new()
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn cached_match(&self, name_normalized: &str) -> ArtistLookupFuture<'_, Option<u64>> {
|
||||
let name_normalized = name_normalized.to_string();
|
||||
Box::pin(async move {
|
||||
let guard = self.mb_cache.lock().ok()?;
|
||||
let cache = guard.as_ref()?;
|
||||
let cached = cache.get_qobuz_artist_match(&name_normalized).ok()??;
|
||||
u64::try_from(cached.qobuz_id).ok().filter(|id| *id > 0)
|
||||
})
|
||||
}
|
||||
|
||||
fn cache_match(&self, name_normalized: &str, artist: &ArtistFacts) {
|
||||
let Ok(guard) = self.mb_cache.lock() else {
|
||||
return;
|
||||
};
|
||||
let Some(cache) = guard.as_ref() else {
|
||||
return;
|
||||
};
|
||||
let Ok(qobuz_id) = i64::try_from(artist.id) else {
|
||||
return;
|
||||
};
|
||||
if let Err(error) = cache.set_qobuz_artist_match(
|
||||
name_normalized,
|
||||
&QobuzArtistMatch {
|
||||
qobuz_id,
|
||||
name: artist.name.clone(),
|
||||
image: artist.image.clone(),
|
||||
albums_count: Some(artist.albums_count),
|
||||
},
|
||||
) {
|
||||
log::warn!(
|
||||
"[SuggestionsEngine] Failed to cache Qobuz artist match '{}': {}",
|
||||
artist.name,
|
||||
error
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn mb_tags(&self, mbid: &str) -> ArtistLookupFuture<'_, Vec<String>> {
|
||||
let mbid = mbid.to_string();
|
||||
Box::pin(async move {
|
||||
match self.mb_client.get_artist_tags(&mbid).await {
|
||||
Ok(tags) => tags,
|
||||
Err(error) => {
|
||||
log::debug!(
|
||||
"[SuggestionsEngine] MusicBrainz tags unavailable for {}: {}",
|
||||
mbid,
|
||||
error
|
||||
);
|
||||
Vec::new()
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub(crate) struct ResolvedSeed {
|
||||
pub(crate) node_id: String,
|
||||
pub(crate) requested_name: String,
|
||||
pub(crate) facts: ArtistFacts,
|
||||
}
|
||||
|
||||
/// Qobuz and MusicBrainz context collected once from the playlist seeds.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub struct SeedContext {
|
||||
seeds: Vec<ResolvedSeed>,
|
||||
genre_roots: HashSet<u64>,
|
||||
label_ids: HashSet<u64>,
|
||||
tags: HashSet<String>,
|
||||
neighbourhood_ids: Vec<u64>,
|
||||
neighbourhood_set: HashSet<u64>,
|
||||
}
|
||||
|
||||
impl SeedContext {
|
||||
pub(crate) fn resolved_seed(
|
||||
&self,
|
||||
node_id: &str,
|
||||
requested_name: &str,
|
||||
) -> Option<&ArtistFacts> {
|
||||
self.seeds
|
||||
.iter()
|
||||
.find(|seed| {
|
||||
seed.node_id == node_id
|
||||
&& normalize_name(&seed.requested_name) == normalize_name(requested_name)
|
||||
})
|
||||
.map(|seed| &seed.facts)
|
||||
}
|
||||
|
||||
pub(crate) fn neighbourhood_ids(&self) -> &[u64] {
|
||||
&self.neighbourhood_ids
|
||||
}
|
||||
|
||||
async fn refresh_qobuz_context(&mut self, lookup: &dyn ArtistLookup) {
|
||||
self.genre_roots.clear();
|
||||
self.label_ids.clear();
|
||||
self.neighbourhood_ids.clear();
|
||||
self.neighbourhood_set.clear();
|
||||
|
||||
let mut seen_seed_ids = HashSet::new();
|
||||
for seed in &self.seeds {
|
||||
self.genre_roots
|
||||
.extend(seed.facts.genre_roots.iter().copied());
|
||||
self.label_ids.extend(seed.facts.label_ids.iter().copied());
|
||||
if !seen_seed_ids.insert(seed.facts.id) {
|
||||
continue;
|
||||
}
|
||||
for similar_id in lookup.similar_artist_ids(seed.facts.id).await {
|
||||
if self.neighbourhood_set.insert(similar_id) {
|
||||
self.neighbourhood_ids.push(similar_id);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Build the seed-relative validation context once per suggestion request.
|
||||
pub async fn resolve_seed_context(
|
||||
lookup: &dyn ArtistLookup,
|
||||
playlist_artists: &[(String, String)],
|
||||
) -> SeedContext {
|
||||
let mut context = SeedContext::default();
|
||||
let mut ambiguous = Vec::new();
|
||||
|
||||
for (node_id, requested_name) in playlist_artists {
|
||||
let qobuz_id = parse_qobuz_id(node_id);
|
||||
let mut seed_has_tags = false;
|
||||
if qobuz_id.is_none() {
|
||||
for tag in lookup.mb_tags(node_id).await {
|
||||
let tag = normalize_tag(&tag);
|
||||
if !tag.is_empty() {
|
||||
seed_has_tags = true;
|
||||
context.tags.insert(tag);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(qobuz_id) = qobuz_id {
|
||||
if let Some(facts) = lookup.artist_by_id(qobuz_id).await {
|
||||
if has_expected_identity(&facts, requested_name)
|
||||
&& seed_has_identity_signal(&facts, seed_has_tags)
|
||||
{
|
||||
context.seeds.push(ResolvedSeed {
|
||||
node_id: node_id.clone(),
|
||||
requested_name: requested_name.clone(),
|
||||
facts,
|
||||
});
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
let cache_key = MusicBrainzCache::normalize_name(requested_name);
|
||||
if let Some(qobuz_id) = lookup.cached_match(&cache_key).await {
|
||||
if let Some(facts) = lookup.artist_by_id(qobuz_id).await {
|
||||
if has_expected_identity(&facts, requested_name)
|
||||
&& seed_has_identity_signal(&facts, seed_has_tags)
|
||||
{
|
||||
context.seeds.push(ResolvedSeed {
|
||||
node_id: node_id.clone(),
|
||||
requested_name: requested_name.clone(),
|
||||
facts,
|
||||
});
|
||||
}
|
||||
}
|
||||
// A cache hit is authoritative enough to avoid a text fallback;
|
||||
// a bad/stale row is rejected instead of silently changing IDs.
|
||||
continue;
|
||||
}
|
||||
|
||||
let search = named_search_candidates(lookup, requested_name).await;
|
||||
if search.candidates.len() == 1
|
||||
&& !search.homonymous
|
||||
&& seed_has_identity_signal(&search.candidates[0], seed_has_tags)
|
||||
{
|
||||
context.seeds.push(ResolvedSeed {
|
||||
node_id: node_id.clone(),
|
||||
requested_name: requested_name.clone(),
|
||||
facts: search.candidates.into_iter().next().expect("one candidate"),
|
||||
});
|
||||
} else if !search.candidates.is_empty() {
|
||||
ambiguous.push((node_id.clone(), requested_name.clone(), search.candidates));
|
||||
}
|
||||
}
|
||||
|
||||
// Definite seed identities establish the neighbourhood used to resolve any
|
||||
// same-name seed ambiguity. With no such evidence, ambiguity stays rejected.
|
||||
if !ambiguous.is_empty() {
|
||||
context.refresh_qobuz_context(lookup).await;
|
||||
}
|
||||
for (node_id, requested_name, candidates) in ambiguous {
|
||||
let mut validated = Vec::new();
|
||||
for candidate in candidates {
|
||||
if validate_candidate(lookup, &candidate, Some(&node_id), &context).await {
|
||||
validated.push(candidate);
|
||||
}
|
||||
}
|
||||
if let Some(facts) = select_homonym(validated, &context) {
|
||||
context.seeds.push(ResolvedSeed {
|
||||
node_id,
|
||||
requested_name,
|
||||
facts,
|
||||
});
|
||||
}
|
||||
}
|
||||
context.refresh_qobuz_context(lookup).await;
|
||||
context
|
||||
}
|
||||
|
||||
/// Resolve a related-artist node, then apply the seed-relative guardrail.
|
||||
pub async fn resolve_candidate(
|
||||
lookup: &dyn ArtistLookup,
|
||||
node_id: &str,
|
||||
requested_name: &str,
|
||||
seed_context: &SeedContext,
|
||||
) -> Option<ArtistFacts> {
|
||||
if let Some(qobuz_id) = parse_qobuz_id(node_id) {
|
||||
let facts = lookup.artist_by_id(qobuz_id).await?;
|
||||
if !has_expected_identity(&facts, requested_name) {
|
||||
return None;
|
||||
}
|
||||
return validate_candidate(lookup, &facts, None, seed_context)
|
||||
.await
|
||||
.then_some(facts);
|
||||
}
|
||||
|
||||
let cache_key = MusicBrainzCache::normalize_name(requested_name);
|
||||
if let Some(qobuz_id) = lookup.cached_match(&cache_key).await {
|
||||
let facts = lookup.artist_by_id(qobuz_id).await?;
|
||||
if !has_expected_identity(&facts, requested_name) {
|
||||
return None;
|
||||
}
|
||||
return validate_candidate(lookup, &facts, Some(node_id), seed_context)
|
||||
.await
|
||||
.then_some(facts);
|
||||
}
|
||||
|
||||
let search = named_search_candidates(lookup, requested_name).await;
|
||||
let mut validated = Vec::new();
|
||||
for candidate in search.candidates {
|
||||
if validate_candidate(lookup, &candidate, Some(node_id), seed_context).await {
|
||||
validated.push(candidate);
|
||||
}
|
||||
}
|
||||
let facts = if search.homonymous {
|
||||
select_homonym(validated, seed_context)?
|
||||
} else {
|
||||
validated.into_iter().next()?
|
||||
};
|
||||
|
||||
// Only seed-relative validation turns a cold text hit into a confirmed
|
||||
// reusable identity match.
|
||||
lookup.cache_match(&cache_key, &facts);
|
||||
Some(facts)
|
||||
}
|
||||
|
||||
/// Accept only candidates sharing a Qobuz genre root or MusicBrainz tag with a seed.
|
||||
pub async fn validate_candidate(
|
||||
lookup: &dyn ArtistLookup,
|
||||
candidate: &ArtistFacts,
|
||||
candidate_mbid: Option<&str>,
|
||||
seed_context: &SeedContext,
|
||||
) -> bool {
|
||||
if candidate.albums_count == 0 {
|
||||
return false;
|
||||
}
|
||||
|
||||
if candidate
|
||||
.genre_roots
|
||||
.iter()
|
||||
.any(|root| seed_context.genre_roots.contains(root))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
let candidate_tags = if let Some(mbid) = candidate_mbid {
|
||||
lookup
|
||||
.mb_tags(mbid)
|
||||
.await
|
||||
.into_iter()
|
||||
.map(|tag| normalize_tag(&tag))
|
||||
.filter(|tag| !tag.is_empty())
|
||||
.collect::<HashSet<_>>()
|
||||
} else {
|
||||
HashSet::new()
|
||||
};
|
||||
if candidate_tags
|
||||
.iter()
|
||||
.any(|tag| seed_context.tags.contains(tag))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
if candidate.genre_roots.is_empty() && candidate_tags.is_empty() {
|
||||
log::debug!(
|
||||
"[SuggestionsEngine] Rejecting '{}' because identity context is unavailable",
|
||||
candidate.name
|
||||
);
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
struct NamedSearchCandidates {
|
||||
candidates: Vec<ArtistFacts>,
|
||||
homonymous: bool,
|
||||
}
|
||||
|
||||
async fn named_search_candidates(
|
||||
lookup: &dyn ArtistLookup,
|
||||
requested_name: &str,
|
||||
) -> NamedSearchCandidates {
|
||||
let normalized = normalize_name(requested_name);
|
||||
let mut results = lookup.search_artists(requested_name).await;
|
||||
if results.is_empty() && requested_name != normalized {
|
||||
results = lookup.search_artists(&normalized).await;
|
||||
}
|
||||
|
||||
let mut exact = matching_candidates(&results, &normalized);
|
||||
if exact.candidates.is_empty() {
|
||||
exact = matching_candidates(&results, &format!("the {normalized}"));
|
||||
}
|
||||
exact
|
||||
}
|
||||
|
||||
fn matching_candidates(results: &[ArtistFacts], normalized_name: &str) -> NamedSearchCandidates {
|
||||
let mut seen = HashSet::new();
|
||||
let matches = results
|
||||
.iter()
|
||||
.filter(|artist| normalize_name(&artist.name) == normalized_name)
|
||||
.filter(|artist| seen.insert(artist.id))
|
||||
.cloned()
|
||||
.collect::<Vec<_>>();
|
||||
NamedSearchCandidates {
|
||||
homonymous: matches.len() > 1,
|
||||
candidates: matches
|
||||
.into_iter()
|
||||
.filter(|artist| artist.albums_count > 0)
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
fn select_homonym(candidates: Vec<ArtistFacts>, context: &SeedContext) -> Option<ArtistFacts> {
|
||||
candidates.into_iter().find(|candidate| {
|
||||
candidate.albums_count > 0
|
||||
&& (context.neighbourhood_set.contains(&candidate.id)
|
||||
|| candidate
|
||||
.label_ids
|
||||
.iter()
|
||||
.any(|label| context.label_ids.contains(label)))
|
||||
})
|
||||
}
|
||||
|
||||
fn has_expected_identity(artist: &ArtistFacts, requested_name: &str) -> bool {
|
||||
if artist.albums_count == 0 {
|
||||
return false;
|
||||
}
|
||||
let requested = normalize_name(requested_name);
|
||||
let actual = normalize_name(&artist.name);
|
||||
actual == requested || actual == format!("the {requested}")
|
||||
}
|
||||
|
||||
fn seed_has_identity_signal(artist: &ArtistFacts, has_tags: bool) -> bool {
|
||||
!artist.genre_roots.is_empty() || has_tags
|
||||
}
|
||||
|
||||
fn parse_qobuz_id(node_id: &str) -> Option<u64> {
|
||||
node_id.strip_prefix("qobuz:")?.parse().ok()
|
||||
}
|
||||
|
||||
/// Normalize a name for identity comparisons (accent-insensitive, as before).
|
||||
pub(crate) fn normalize_name(name: &str) -> String {
|
||||
name.to_lowercase()
|
||||
.replace('á', "a")
|
||||
.replace('é', "e")
|
||||
.replace('í', "i")
|
||||
.replace('ó', "o")
|
||||
.replace('ú', "u")
|
||||
.replace('à', "a")
|
||||
.replace('è', "e")
|
||||
.replace('ì', "i")
|
||||
.replace('ò', "o")
|
||||
.replace('ù', "u")
|
||||
.replace('ä', "a")
|
||||
.replace('ë', "e")
|
||||
.replace('ï', "i")
|
||||
.replace('ö', "o")
|
||||
.replace('ü', "u")
|
||||
.replace('ñ', "n")
|
||||
.replace('ç', "c")
|
||||
}
|
||||
|
||||
fn normalize_tag(tag: &str) -> String {
|
||||
tag.trim()
|
||||
.to_lowercase()
|
||||
.split_whitespace()
|
||||
.collect::<Vec<_>>()
|
||||
.join(" ")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
|
||||
#[derive(Default)]
|
||||
struct FakeLookup {
|
||||
artists: HashMap<u64, ArtistFacts>,
|
||||
searches: HashMap<String, Vec<ArtistFacts>>,
|
||||
similar: HashMap<u64, Vec<u64>>,
|
||||
cached: std::sync::Mutex<HashMap<String, u64>>,
|
||||
tags: HashMap<String, Vec<String>>,
|
||||
artist_by_id_calls: AtomicUsize,
|
||||
search_calls: AtomicUsize,
|
||||
}
|
||||
|
||||
impl ArtistLookup for FakeLookup {
|
||||
fn artist_by_id(&self, id: u64) -> ArtistLookupFuture<'_, Option<ArtistFacts>> {
|
||||
self.artist_by_id_calls.fetch_add(1, Ordering::SeqCst);
|
||||
Box::pin(async move { self.artists.get(&id).cloned() })
|
||||
}
|
||||
|
||||
fn search_artists(&self, name: &str) -> ArtistLookupFuture<'_, Vec<ArtistFacts>> {
|
||||
self.search_calls.fetch_add(1, Ordering::SeqCst);
|
||||
let name = name.to_string();
|
||||
Box::pin(async move { self.searches.get(&name).cloned().unwrap_or_default() })
|
||||
}
|
||||
|
||||
fn similar_artist_ids(&self, id: u64) -> ArtistLookupFuture<'_, Vec<u64>> {
|
||||
Box::pin(async move { self.similar.get(&id).cloned().unwrap_or_default() })
|
||||
}
|
||||
|
||||
fn cached_match(&self, name_normalized: &str) -> ArtistLookupFuture<'_, Option<u64>> {
|
||||
let hit = self
|
||||
.cached
|
||||
.lock()
|
||||
.ok()
|
||||
.and_then(|cache| cache.get(name_normalized).copied());
|
||||
Box::pin(async move { hit })
|
||||
}
|
||||
|
||||
fn cache_match(&self, name_normalized: &str, artist: &ArtistFacts) {
|
||||
if let Ok(mut cache) = self.cached.lock() {
|
||||
cache.insert(name_normalized.to_string(), artist.id);
|
||||
}
|
||||
}
|
||||
|
||||
fn mb_tags(&self, mbid: &str) -> ArtistLookupFuture<'_, Vec<String>> {
|
||||
let mbid = mbid.to_string();
|
||||
Box::pin(async move { self.tags.get(&mbid).cloned().unwrap_or_default() })
|
||||
}
|
||||
}
|
||||
|
||||
fn facts(id: u64, name: &str, roots: &[u64], labels: &[u64]) -> ArtistFacts {
|
||||
ArtistFacts {
|
||||
id,
|
||||
name: name.to_string(),
|
||||
albums_count: 1,
|
||||
image: None,
|
||||
genre_roots: roots.to_vec(),
|
||||
label_ids: labels.to_vec(),
|
||||
}
|
||||
}
|
||||
|
||||
fn context(root: u64) -> SeedContext {
|
||||
SeedContext {
|
||||
genre_roots: HashSet::from([root]),
|
||||
..SeedContext::default()
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn qobuz_node_resolves_by_id_without_text_search() {
|
||||
let artist = facts(42, "Direct Artist", &[7], &[]);
|
||||
let lookup = FakeLookup {
|
||||
artists: HashMap::from([(42, artist.clone())]),
|
||||
..FakeLookup::default()
|
||||
};
|
||||
|
||||
let resolved = resolve_candidate(&lookup, "qobuz:42", "Direct Artist", &context(7)).await;
|
||||
|
||||
assert_eq!(resolved, Some(artist));
|
||||
assert_eq!(lookup.artist_by_id_calls.load(Ordering::SeqCst), 1);
|
||||
assert_eq!(lookup.search_calls.load(Ordering::SeqCst), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mbid_uses_cache_before_search_and_searches_only_on_miss() {
|
||||
let cached_artist = facts(10, "Cached Artist", &[7], &[]);
|
||||
let cached_lookup = FakeLookup {
|
||||
artists: HashMap::from([(10, cached_artist.clone())]),
|
||||
cached: std::sync::Mutex::new(HashMap::from([(
|
||||
MusicBrainzCache::normalize_name("Cached Artist"),
|
||||
10,
|
||||
)])),
|
||||
..FakeLookup::default()
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
resolve_candidate(&cached_lookup, "real-mbid", "Cached Artist", &context(7)).await,
|
||||
Some(cached_artist)
|
||||
);
|
||||
assert_eq!(cached_lookup.search_calls.load(Ordering::SeqCst), 0);
|
||||
|
||||
let searched_artist = facts(20, "Searched Artist", &[7], &[]);
|
||||
let miss_lookup = FakeLookup {
|
||||
searches: HashMap::from([(
|
||||
"Searched Artist".to_string(),
|
||||
vec![searched_artist.clone()],
|
||||
)]),
|
||||
..FakeLookup::default()
|
||||
};
|
||||
assert_eq!(
|
||||
resolve_candidate(&miss_lookup, "another-mbid", "Searched Artist", &context(7)).await,
|
||||
Some(searched_artist)
|
||||
);
|
||||
assert_eq!(miss_lookup.search_calls.load(Ordering::SeqCst), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn homonym_requires_seed_neighbourhood_or_label_evidence() {
|
||||
let wrong = facts(10, "Same Name", &[7], &[]);
|
||||
let neighbour = facts(11, "Same Name", &[7], &[]);
|
||||
let lookup = FakeLookup {
|
||||
searches: HashMap::from([("Same Name".to_string(), vec![wrong, neighbour.clone()])]),
|
||||
similar: HashMap::from([(1, vec![11])]),
|
||||
..FakeLookup::default()
|
||||
};
|
||||
let mut seed_context = SeedContext {
|
||||
seeds: vec![ResolvedSeed {
|
||||
node_id: "seed-mbid".to_string(),
|
||||
requested_name: "Seed Artist".to_string(),
|
||||
facts: facts(1, "Seed Artist", &[7], &[]),
|
||||
}],
|
||||
..SeedContext::default()
|
||||
};
|
||||
seed_context.refresh_qobuz_context(&lookup).await;
|
||||
|
||||
assert_eq!(
|
||||
resolve_candidate(&lookup, "candidate-mbid", "Same Name", &seed_context).await,
|
||||
Some(neighbour)
|
||||
);
|
||||
|
||||
let discarded_lookup = FakeLookup {
|
||||
searches: lookup.searches.clone(),
|
||||
..FakeLookup::default()
|
||||
};
|
||||
assert_eq!(
|
||||
resolve_candidate(
|
||||
&discarded_lookup,
|
||||
"candidate-mbid",
|
||||
"Same Name",
|
||||
&context(7),
|
||||
)
|
||||
.await,
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn homonym_validation_precedes_neighbourhood_tie_breaking() {
|
||||
let invalid_neighbour = facts(11, "Same Name", &[8], &[]);
|
||||
let valid_label_match = facts(12, "Same Name", &[7], &[50]);
|
||||
let lookup = FakeLookup {
|
||||
searches: HashMap::from([(
|
||||
"Same Name".to_string(),
|
||||
vec![invalid_neighbour, valid_label_match.clone()],
|
||||
)]),
|
||||
..FakeLookup::default()
|
||||
};
|
||||
let mut seed_context = context(7);
|
||||
seed_context.neighbourhood_set.insert(11);
|
||||
seed_context.label_ids.insert(50);
|
||||
|
||||
assert_eq!(
|
||||
resolve_candidate(&lookup, "candidate-mbid", "Same Name", &seed_context).await,
|
||||
Some(valid_label_match)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn candidate_needs_shared_genre_root_or_musicbrainz_tag() {
|
||||
let lookup = FakeLookup::default();
|
||||
assert!(
|
||||
validate_candidate(
|
||||
&lookup,
|
||||
&facts(10, "Shared Root", &[7], &[]),
|
||||
None,
|
||||
&context(7),
|
||||
)
|
||||
.await
|
||||
);
|
||||
assert!(
|
||||
!validate_candidate(
|
||||
&lookup,
|
||||
&facts(11, "Different Root", &[8], &[]),
|
||||
None,
|
||||
&context(7),
|
||||
)
|
||||
.await
|
||||
);
|
||||
assert!(
|
||||
!validate_candidate(
|
||||
&lookup,
|
||||
&facts(12, "No Signals", &[], &[]),
|
||||
None,
|
||||
&context(7),
|
||||
)
|
||||
.await
|
||||
);
|
||||
|
||||
let tagged_lookup = FakeLookup {
|
||||
tags: HashMap::from([("candidate-mbid".to_string(), vec!["Metal".to_string()])]),
|
||||
..FakeLookup::default()
|
||||
};
|
||||
let mut tagged_context = context(7);
|
||||
tagged_context.tags.insert("metal".to_string());
|
||||
assert!(
|
||||
validate_candidate(
|
||||
&tagged_lookup,
|
||||
&facts(13, "Tagged Artist", &[8], &[]),
|
||||
Some("candidate-mbid"),
|
||||
&tagged_context,
|
||||
)
|
||||
.await
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -15,7 +15,9 @@ use std::sync::Arc;
|
||||
use tokio::sync::{Mutex, RwLock};
|
||||
|
||||
use qbz_integrations::musicbrainz::cache::MusicBrainzCache;
|
||||
use qbz_integrations::musicbrainz::{ArtistFullResponse, ArtistRelationships, Period, RelatedArtist};
|
||||
use qbz_integrations::musicbrainz::{
|
||||
ArtistFullResponse, ArtistRelationships, Period, RelatedArtist,
|
||||
};
|
||||
use qbz_integrations::MusicBrainzClient;
|
||||
use qbz_qobuz::QobuzClient;
|
||||
|
||||
@@ -70,6 +72,16 @@ impl ArtistVectorBuilder {
|
||||
}
|
||||
}
|
||||
|
||||
/// Shared MusicBrainz client used by identity validation in suggestions.
|
||||
pub(crate) fn musicbrainz_client(&self) -> Arc<MusicBrainzClient> {
|
||||
self.mb_client.clone()
|
||||
}
|
||||
|
||||
/// Optional persistent match cache used before any name-based lookup.
|
||||
pub(crate) fn musicbrainz_cache(&self) -> Arc<std::sync::Mutex<Option<MusicBrainzCache>>> {
|
||||
self.mb_cache.clone()
|
||||
}
|
||||
|
||||
/// Build a vector for an artist, fetching data from all sources.
|
||||
///
|
||||
/// 1. Fetches MusicBrainz relationships (members, groups, collaborators)
|
||||
|
||||
@@ -4,16 +4,22 @@
|
||||
//! crate so the Slint frontend (and any headless caller) can produce a
|
||||
//! playlist's "Suggested Songs" without a `tauri::State` dependency.
|
||||
//!
|
||||
//! Modules are ported 1:1 from the Tauri source. The dead cosine-similarity /
|
||||
//! `find_nearest` ranking path is dropped (production ranks by summed
|
||||
//! relationship weight via the vector store) per the epic's decision D3.
|
||||
//! The vector modules originated in the Tauri implementation. The dead
|
||||
//! cosine-similarity / `find_nearest` path is dropped (production ranks by
|
||||
//! summed relationship weight), and artist resolution uses seed-relative
|
||||
//! identity evidence rather than a global genre blocklist.
|
||||
|
||||
mod artist_guardrail;
|
||||
mod builder;
|
||||
mod sparse_vector;
|
||||
mod store;
|
||||
mod suggestions;
|
||||
mod weights;
|
||||
|
||||
pub use artist_guardrail::{
|
||||
resolve_candidate, resolve_seed_context, validate_candidate, ArtistFacts, ArtistLookup,
|
||||
ArtistLookupFuture, SeedContext,
|
||||
};
|
||||
pub use builder::{ArtistVectorBuilder, BuildResult};
|
||||
pub use sparse_vector::SparseVector;
|
||||
pub use store::{ArtistVectorStore, SimilarArtist, VECTOR_TTL_SECS};
|
||||
|
||||
+198
-393
@@ -8,15 +8,12 @@
|
||||
//! 4. Search Qobuz for top tracks by those artists
|
||||
//! 5. Return suggested tracks with optional reasons
|
||||
//!
|
||||
//! Ported 1:1 from the Tauri `artist_vectors::suggestions`. Only the API client
|
||||
//! types are swapped (`crate::api::{QobuzClient, Track}` →
|
||||
//! `qbz_qobuz::QobuzClient` / `qbz_models::Track`); the `Arc<tokio::Mutex/RwLock>`
|
||||
//! ownership is kept (the store/cache hold `!Sync` rusqlite connections, so each
|
||||
//! guard is dropped before every `.await`, exactly like `builder.rs`). Step 3
|
||||
//! The `Arc<tokio::Mutex/RwLock>` ownership from the original Tauri engine is
|
||||
//! kept because the store/cache contain `!Sync` rusqlite connections. Step 3
|
||||
//! ranks candidates by summed relationship weight via
|
||||
//! `store.get_all_related_artists`, NOT cosine similarity (epic decision D3); the
|
||||
//! Step-2 `compute_playlist_vector` is kept because production still uses it as
|
||||
//! the empty-vector gate (it only sums + normalizes — it never `find_nearest`s).
|
||||
//! `store.get_all_related_artists`, NOT cosine similarity (epic decision D3).
|
||||
//! Step 2 remains only as the empty-vector gate, while Qobuz identity resolution
|
||||
//! is ID/cache-first and validated against context collected from playlist seeds.
|
||||
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
@@ -27,6 +24,10 @@ use serde::{Deserialize, Serialize};
|
||||
use qbz_models::Track;
|
||||
use qbz_qobuz::QobuzClient;
|
||||
|
||||
use crate::artist_guardrail::{
|
||||
normalize_name, resolve_candidate, resolve_seed_context, validate_candidate, ArtistLookup,
|
||||
ProductionArtistLookup, SeedContext,
|
||||
};
|
||||
use crate::builder::ArtistVectorBuilder;
|
||||
use crate::sparse_vector::SparseVector;
|
||||
use crate::store::ArtistVectorStore;
|
||||
@@ -110,6 +111,8 @@ pub struct SuggestionsEngine {
|
||||
builder: Arc<ArtistVectorBuilder>,
|
||||
/// Qobuz client for track search
|
||||
qobuz_client: Arc<RwLock<Option<QobuzClient>>>,
|
||||
/// Identity-first artist lookup, replaceable with an in-memory fake in tests.
|
||||
artist_lookup: Arc<dyn ArtistLookup>,
|
||||
/// Configuration
|
||||
config: SuggestionConfig,
|
||||
}
|
||||
@@ -122,14 +125,26 @@ impl SuggestionsEngine {
|
||||
qobuz_client: Arc<RwLock<Option<QobuzClient>>>,
|
||||
config: SuggestionConfig,
|
||||
) -> Self {
|
||||
let artist_lookup = Arc::new(ProductionArtistLookup::new(
|
||||
qobuz_client.clone(),
|
||||
builder.musicbrainz_cache(),
|
||||
builder.musicbrainz_client(),
|
||||
));
|
||||
Self {
|
||||
store,
|
||||
builder,
|
||||
qobuz_client,
|
||||
artist_lookup,
|
||||
config,
|
||||
}
|
||||
}
|
||||
|
||||
/// Replace production artist lookups, primarily for deterministic tests.
|
||||
pub fn with_artist_lookup(mut self, artist_lookup: Arc<dyn ArtistLookup>) -> Self {
|
||||
self.artist_lookup = artist_lookup;
|
||||
self
|
||||
}
|
||||
|
||||
/// Generate suggestions for a playlist
|
||||
///
|
||||
/// # Arguments
|
||||
@@ -208,6 +223,11 @@ impl SuggestionsEngine {
|
||||
});
|
||||
}
|
||||
|
||||
// Resolve playlist identity once. Every related candidate is validated
|
||||
// against these seed genre/tag/neighbourhood facts, never a global list.
|
||||
let seed_context =
|
||||
resolve_seed_context(self.artist_lookup.as_ref(), playlist_artists).await;
|
||||
|
||||
// 3. Find related artists (using direct relationships, not vector similarity)
|
||||
log::debug!("[SuggestionsEngine] Step 3: Finding related artists");
|
||||
let step3_start = Instant::now();
|
||||
@@ -256,7 +276,13 @@ impl SuggestionsEngine {
|
||||
playlist_artist_limit
|
||||
);
|
||||
let tracks = self
|
||||
.search_artist_tracks_with_limit(mbid, Some(name), 1.0, playlist_artist_limit)
|
||||
.search_artist_tracks_with_limit(
|
||||
mbid,
|
||||
Some(name),
|
||||
1.0,
|
||||
playlist_artist_limit,
|
||||
&seed_context,
|
||||
)
|
||||
.await;
|
||||
log::info!(
|
||||
"[SuggestionsEngine] Step 4a: Found {} tracks for '{}'",
|
||||
@@ -308,7 +334,12 @@ impl SuggestionsEngine {
|
||||
|
||||
// Search Qobuz for tracks by this related artist
|
||||
let tracks = self
|
||||
.search_artist_tracks(&artist.mbid, artist.name.as_deref(), artist.similarity)
|
||||
.search_artist_tracks(
|
||||
&artist.mbid,
|
||||
artist.name.as_deref(),
|
||||
artist.similarity,
|
||||
&seed_context,
|
||||
)
|
||||
.await;
|
||||
|
||||
for mut track in tracks {
|
||||
@@ -356,83 +387,41 @@ impl SuggestionsEngine {
|
||||
);
|
||||
let step4c_start = Instant::now();
|
||||
|
||||
// Get client for Qobuz API calls
|
||||
let guard__ = self.qobuz_client.read().await;
|
||||
let client = guard__
|
||||
.as_ref()
|
||||
.ok_or_else(|| "No active session - please log in".to_string())?;
|
||||
|
||||
// For each playlist artist, get their Qobuz similar artists
|
||||
let mut qobuz_similar_ids: HashSet<u64> = HashSet::new();
|
||||
for &similar_id in seed_context.neighbourhood_ids() {
|
||||
if !qobuz_similar_ids.insert(similar_id) {
|
||||
continue;
|
||||
}
|
||||
let Some(similar_artist) = self.artist_lookup.artist_by_id(similar_id).await else {
|
||||
continue;
|
||||
};
|
||||
if !validate_candidate(
|
||||
self.artist_lookup.as_ref(),
|
||||
&similar_artist,
|
||||
None,
|
||||
&seed_context,
|
||||
)
|
||||
.await
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
for (_mbid, name) in playlist_artists {
|
||||
// First, find the Qobuz artist ID for this playlist artist
|
||||
if let Some((qobuz_id, _)) = self.validate_qobuz_artist(&client, name).await {
|
||||
// Get similar artists from Qobuz (up to 10 per playlist artist)
|
||||
match client.get_similar_artists(qobuz_id, 10, 0).await {
|
||||
Ok(similar_page) => {
|
||||
for similar_artist in similar_page.items {
|
||||
// Skip if we've already processed this artist
|
||||
if qobuz_similar_ids.contains(&similar_artist.id) {
|
||||
continue;
|
||||
}
|
||||
qobuz_similar_ids.insert(similar_artist.id);
|
||||
if !source_artists.contains(&similar_artist.name) {
|
||||
source_artists.push(similar_artist.name.clone());
|
||||
}
|
||||
|
||||
// Check genre compatibility
|
||||
if self
|
||||
.has_incompatible_genre(
|
||||
&client,
|
||||
similar_artist.id,
|
||||
&similar_artist.name,
|
||||
)
|
||||
.await
|
||||
{
|
||||
log::debug!(
|
||||
"[SuggestionsEngine] Skipping Qobuz similar '{}' - incompatible genre",
|
||||
similar_artist.name
|
||||
);
|
||||
continue;
|
||||
}
|
||||
let tracks = self
|
||||
.search_artist_tracks_by_qobuz_id(similar_artist.id, &similar_artist.name, 0.8)
|
||||
.await;
|
||||
|
||||
if !source_artists.contains(&similar_artist.name) {
|
||||
source_artists.push(similar_artist.name.clone());
|
||||
}
|
||||
|
||||
// Search tracks for this similar artist (use empty MBID since we have Qobuz ID)
|
||||
let tracks = self
|
||||
.search_artist_tracks_by_qobuz_id(
|
||||
&client,
|
||||
similar_artist.id,
|
||||
&similar_artist.name,
|
||||
0.8, // High similarity since Qobuz says they're similar
|
||||
)
|
||||
.await;
|
||||
|
||||
for mut track in tracks {
|
||||
if exclude_track_ids.contains(&track.track_id) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if include_reasons {
|
||||
track.reason = Some(format!("Similar to {} (Qobuz)", name));
|
||||
}
|
||||
|
||||
all_tracks.push(track);
|
||||
}
|
||||
|
||||
// Stop if we have enough
|
||||
if all_tracks.len() >= self.config.max_pool_size {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
log::warn!(
|
||||
"[SuggestionsEngine] Failed to get Qobuz similar artists for '{}': {}",
|
||||
name, e
|
||||
);
|
||||
}
|
||||
for mut track in tracks {
|
||||
if exclude_track_ids.contains(&track.track_id) {
|
||||
continue;
|
||||
}
|
||||
if include_reasons {
|
||||
track.reason = Some("Similar to your playlist (Qobuz)".to_string());
|
||||
}
|
||||
all_tracks.push(track);
|
||||
}
|
||||
|
||||
if all_tracks.len() >= self.config.max_pool_size {
|
||||
@@ -448,24 +437,7 @@ impl SuggestionsEngine {
|
||||
);
|
||||
}
|
||||
|
||||
// 5. Deduplicate by title+artist (keeps highest similarity version)
|
||||
let mut seen_titles: HashSet<String> = HashSet::new();
|
||||
all_tracks.retain(|track| {
|
||||
let key = format!(
|
||||
"{}|{}",
|
||||
track.title.to_lowercase(),
|
||||
track.artist_name.to_lowercase()
|
||||
);
|
||||
seen_titles.insert(key)
|
||||
});
|
||||
|
||||
// 6. Shuffle tracks for variety (so same artist doesn't dominate)
|
||||
use rand::seq::SliceRandom;
|
||||
let mut rng = rand::rng();
|
||||
all_tracks.shuffle(&mut rng);
|
||||
|
||||
// 7. Limit pool size
|
||||
all_tracks.truncate(self.config.max_pool_size);
|
||||
all_tracks = rank_dedup_and_truncate(all_tracks, self.config.max_pool_size);
|
||||
|
||||
Ok(SuggestionResult {
|
||||
tracks: all_tracks,
|
||||
@@ -502,26 +474,31 @@ impl SuggestionsEngine {
|
||||
artist_mbid: &str,
|
||||
artist_name: Option<&str>,
|
||||
similarity: f32,
|
||||
seed_context: &SeedContext,
|
||||
) -> Vec<SuggestedTrack> {
|
||||
self.search_artist_tracks_with_limit(
|
||||
artist_mbid,
|
||||
artist_name,
|
||||
similarity,
|
||||
self.config.tracks_per_artist,
|
||||
seed_context,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Search Qobuz for tracks by Qobuz artist ID (more reliable when we already validated the artist)
|
||||
/// Takes client reference to avoid deadlock when caller already holds the lock
|
||||
async fn search_artist_tracks_by_qobuz_id(
|
||||
&self,
|
||||
client: &QobuzClient,
|
||||
qobuz_artist_id: u64,
|
||||
artist_name: &str,
|
||||
similarity: f32,
|
||||
) -> Vec<SuggestedTrack> {
|
||||
let limit = self.config.tracks_per_artist;
|
||||
let guard = self.qobuz_client.read().await;
|
||||
let Some(client) = guard.as_ref() else {
|
||||
log::warn!("[SuggestionsEngine] No active Qobuz session; skipping");
|
||||
return Vec::new();
|
||||
};
|
||||
|
||||
// Search by artist name but verify tracks belong to this specific Qobuz artist ID
|
||||
match client
|
||||
@@ -605,15 +582,15 @@ impl SuggestionsEngine {
|
||||
|
||||
/// Search Qobuz for tracks by an artist with custom limit
|
||||
///
|
||||
/// First validates that the artist EXISTS in Qobuz (has a dedicated artist page).
|
||||
/// This prevents false matches for session musicians who don't have their own catalog
|
||||
/// (e.g., "Martin Lopez" drummer returning tracks from unrelated "Martin Lopez" artists).
|
||||
/// Resolves the artist by ID/cache before any text fallback, then requires
|
||||
/// seed-relative genre or tag evidence before searching for tracks.
|
||||
async fn search_artist_tracks_with_limit(
|
||||
&self,
|
||||
artist_mbid: &str,
|
||||
artist_name: Option<&str>,
|
||||
similarity: f32,
|
||||
limit: usize,
|
||||
seed_context: &SeedContext,
|
||||
) -> Vec<SuggestedTrack> {
|
||||
let search_query = match artist_name {
|
||||
Some(name) => name.to_string(),
|
||||
@@ -630,25 +607,38 @@ impl SuggestionsEngine {
|
||||
}
|
||||
};
|
||||
|
||||
let guard__ = self.qobuz_client.read().await;
|
||||
let Some(client) = guard__.as_ref() else {
|
||||
log::warn!("[SuggestionsEngine] No active Qobuz session; skipping");
|
||||
return Vec::new();
|
||||
};
|
||||
|
||||
// Step 1: Validate artist exists in Qobuz with their own catalog
|
||||
// This prevents searching for session musicians who don't have artist pages
|
||||
let validated_artist = self.validate_qobuz_artist(&client, &search_query).await;
|
||||
|
||||
if validated_artist.is_none() {
|
||||
// Resolve identity before taking the client lock: the production lookup
|
||||
// uses the same lock for its direct-ID/cache-first checks.
|
||||
let validated_artist =
|
||||
if let Some(seed_artist) = seed_context.resolved_seed(artist_mbid, &search_query) {
|
||||
let candidate_mbid = (!artist_mbid.starts_with("qobuz:")).then_some(artist_mbid);
|
||||
validate_candidate(
|
||||
self.artist_lookup.as_ref(),
|
||||
seed_artist,
|
||||
candidate_mbid,
|
||||
seed_context,
|
||||
)
|
||||
.await
|
||||
.then(|| seed_artist.clone())
|
||||
} else {
|
||||
resolve_candidate(
|
||||
self.artist_lookup.as_ref(),
|
||||
artist_mbid,
|
||||
&search_query,
|
||||
seed_context,
|
||||
)
|
||||
.await
|
||||
};
|
||||
let Some(validated_artist) = validated_artist else {
|
||||
log::info!(
|
||||
"[SuggestionsEngine] Skipping '{}' - no Qobuz artist page found or incompatible genre",
|
||||
"[SuggestionsEngine] Skipping '{}' - identity could not be validated against playlist seeds",
|
||||
search_query
|
||||
);
|
||||
return Vec::new();
|
||||
}
|
||||
};
|
||||
|
||||
let (qobuz_artist_id, qobuz_artist_name) = validated_artist.unwrap();
|
||||
let qobuz_artist_id = validated_artist.id;
|
||||
let qobuz_artist_name = validated_artist.name;
|
||||
log::info!(
|
||||
"[SuggestionsEngine] Validated '{}' -> Qobuz artist '{}' (ID: {})",
|
||||
search_query,
|
||||
@@ -656,6 +646,12 @@ impl SuggestionsEngine {
|
||||
qobuz_artist_id
|
||||
);
|
||||
|
||||
let guard = self.qobuz_client.read().await;
|
||||
let Some(client) = guard.as_ref() else {
|
||||
log::warn!("[SuggestionsEngine] No active Qobuz session; skipping");
|
||||
return Vec::new();
|
||||
};
|
||||
|
||||
// Step 2: Search for tracks by artist name
|
||||
// Fetch many more since search results include tracks where the artist appears,
|
||||
// not just tracks BY the artist. We filter down to exact matches.
|
||||
@@ -698,260 +694,6 @@ impl SuggestionsEngine {
|
||||
}
|
||||
}
|
||||
|
||||
/// Validate that an artist exists in Qobuz with their own catalog AND compatible genre
|
||||
///
|
||||
/// Returns Some((artist_id, artist_name)) if found, None otherwise.
|
||||
/// This prevents false matches for:
|
||||
/// - Session musicians without their own page (e.g., "Martin Lopez" drummer)
|
||||
/// - Names that match different artists (e.g., Latin "Martin Mendez" vs bassist)
|
||||
/// - Artists with incompatible genres (bachata/merengue artist vs metal drummer)
|
||||
async fn validate_qobuz_artist(
|
||||
&self,
|
||||
client: &QobuzClient,
|
||||
name: &str,
|
||||
) -> Option<(u64, String)> {
|
||||
// Normalize name for comparison (removes accents: å→a, é→e, etc.)
|
||||
let name_normalized = normalize_name(name);
|
||||
|
||||
// Search Qobuz for artist - try original name first
|
||||
let mut results = match client.search_artists(name, 10, 0, None).await {
|
||||
Ok(r) => r,
|
||||
Err(e) => {
|
||||
log::warn!(
|
||||
"[SuggestionsEngine] Artist search failed for '{}': {}",
|
||||
name,
|
||||
e
|
||||
);
|
||||
return None;
|
||||
}
|
||||
};
|
||||
|
||||
// If no results and name has accents, also try normalized name
|
||||
// e.g., "Mikael Åkerfeldt" -> "Mikael Akerfeldt"
|
||||
if results.items.is_empty() && name != name_normalized {
|
||||
log::debug!(
|
||||
"[SuggestionsEngine] No results for '{}', trying normalized '{}'",
|
||||
name,
|
||||
name_normalized
|
||||
);
|
||||
if let Ok(r) = client.search_artists(&name_normalized, 10, 0, None).await {
|
||||
results = r;
|
||||
}
|
||||
}
|
||||
|
||||
// Look for exact name match (comparing normalized versions)
|
||||
let mut candidate: Option<(u64, String)> = None;
|
||||
|
||||
for artist in &results.items {
|
||||
let artist_normalized = normalize_name(&artist.name);
|
||||
|
||||
// Exact match (after accent normalization)
|
||||
// This allows "Mikael Åkerfeldt" to match "Mikael Akerfeldt"
|
||||
if artist_normalized == name_normalized && artist.albums_count.unwrap_or(0) > 0 {
|
||||
candidate = Some((artist.id, artist.name.clone()));
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// Also try "The X" variant (e.g., "Beatles" -> "The Beatles")
|
||||
if candidate.is_none() {
|
||||
let the_name_normalized = format!("the {}", name_normalized);
|
||||
for artist in &results.items {
|
||||
let artist_normalized = normalize_name(&artist.name);
|
||||
if artist_normalized == the_name_normalized && artist.albums_count.unwrap_or(0) > 0
|
||||
{
|
||||
candidate = Some((artist.id, artist.name.clone()));
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// If we found a candidate, verify their genre is compatible
|
||||
if let Some((artist_id, artist_name)) = candidate {
|
||||
if self
|
||||
.has_incompatible_genre(client, artist_id, &artist_name)
|
||||
.await
|
||||
{
|
||||
log::info!(
|
||||
"[SuggestionsEngine] Rejecting '{}' (ID: {}) - incompatible genre detected",
|
||||
artist_name,
|
||||
artist_id
|
||||
);
|
||||
return None;
|
||||
}
|
||||
return Some((artist_id, artist_name));
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
/// Check if an artist has incompatible genres (bachata, merengue, k-pop, etc.)
|
||||
///
|
||||
/// Fetches a few albums and checks their genres against a blocklist.
|
||||
/// Returns true if incompatible, false if compatible or unknown.
|
||||
async fn has_incompatible_genre(
|
||||
&self,
|
||||
client: &QobuzClient,
|
||||
artist_id: u64,
|
||||
artist_name: &str,
|
||||
) -> bool {
|
||||
// Incompatible genre keywords - these would never appear in a rock/metal context
|
||||
// NOTE: We force English locale when fetching, so only English names needed
|
||||
const INCOMPATIBLE_GENRES: &[&str] = &[
|
||||
// Latin/Tropical
|
||||
"bachata",
|
||||
"merengue",
|
||||
"reggaeton",
|
||||
"salsa",
|
||||
"cumbia",
|
||||
"vallenato",
|
||||
"latin pop",
|
||||
"latin music",
|
||||
"tropical",
|
||||
"urbano",
|
||||
"regional mexican",
|
||||
"latin", // Generic Latin parent genre
|
||||
// Asian pop
|
||||
"k-pop",
|
||||
"kpop",
|
||||
"j-pop",
|
||||
"jpop",
|
||||
"mandopop",
|
||||
"cantopop",
|
||||
"c-pop",
|
||||
// European folk/schlager
|
||||
"schlager",
|
||||
"chanson",
|
||||
"french chanson",
|
||||
"volksmusik",
|
||||
// Religious
|
||||
"gospel",
|
||||
"christian",
|
||||
"worship",
|
||||
"religious",
|
||||
"spiritual",
|
||||
// Children/Family
|
||||
"children",
|
||||
"nursery",
|
||||
"lullaby",
|
||||
"kids",
|
||||
// Electronic/Dance (club-oriented)
|
||||
"trance",
|
||||
"techno",
|
||||
"house",
|
||||
"edm",
|
||||
"dubstep",
|
||||
"drum and bass",
|
||||
"hardstyle",
|
||||
"eurodance",
|
||||
"hands up",
|
||||
"happy hardcore",
|
||||
"dance",
|
||||
// Spoken word/Non-music
|
||||
"audiobook",
|
||||
"spoken word",
|
||||
"podcast",
|
||||
"meditation",
|
||||
"asmr",
|
||||
"relaxation",
|
||||
"sleep",
|
||||
"nature sounds",
|
||||
"white noise",
|
||||
"comedy",
|
||||
"stand-up",
|
||||
// Country (usually incompatible with metal)
|
||||
"country",
|
||||
"bluegrass",
|
||||
"americana",
|
||||
// New age/Wellness
|
||||
"new age",
|
||||
"healing",
|
||||
"spa",
|
||||
"yoga",
|
||||
"mindfulness",
|
||||
"wellness",
|
||||
];
|
||||
|
||||
// Fetch artist with a few albums (use English locale for consistent genre names)
|
||||
match client
|
||||
.get_artist_with_pagination_and_locale(artist_id, true, Some(5), None, Some("en"))
|
||||
.await
|
||||
{
|
||||
Ok(artist) => {
|
||||
if let Some(albums) = &artist.albums {
|
||||
for album in &albums.items {
|
||||
if let Some(genre) = &album.genre {
|
||||
let genre_lower = genre.name.to_lowercase();
|
||||
|
||||
// Check if genre matches any incompatible keyword
|
||||
for incompatible in INCOMPATIBLE_GENRES {
|
||||
if genre_lower.contains(incompatible) {
|
||||
log::debug!(
|
||||
"[SuggestionsEngine] Artist '{}' has incompatible genre: '{}' (album: {})",
|
||||
artist_name, genre.name, album.title
|
||||
);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Also check album title for genre hints (e.g., "Latino Bachata Amor")
|
||||
let title_lower = album.title.to_lowercase();
|
||||
for incompatible in INCOMPATIBLE_GENRES {
|
||||
if title_lower.contains(incompatible) {
|
||||
log::debug!(
|
||||
"[SuggestionsEngine] Artist '{}' has incompatible album title: '{}'",
|
||||
artist_name, album.title
|
||||
);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
// Additional title-based checks for non-music content
|
||||
const INCOMPATIBLE_TITLE_KEYWORDS: &[&str] = &[
|
||||
"audiobook",
|
||||
"hörbuch",
|
||||
"hörspiel",
|
||||
"gelesen von",
|
||||
"read by",
|
||||
"narrated by",
|
||||
"lesung",
|
||||
"märchen",
|
||||
"fairy tale",
|
||||
"meditation",
|
||||
"relaxation",
|
||||
"sleep music",
|
||||
"yoga music",
|
||||
"trance mix",
|
||||
"club mix",
|
||||
"dance mix",
|
||||
"dj mix",
|
||||
];
|
||||
for keyword in INCOMPATIBLE_TITLE_KEYWORDS {
|
||||
if title_lower.contains(keyword) {
|
||||
log::debug!(
|
||||
"[SuggestionsEngine] Artist '{}' has incompatible album title keyword '{}': '{}'",
|
||||
artist_name, keyword, album.title
|
||||
);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
Err(e) => {
|
||||
log::warn!(
|
||||
"[SuggestionsEngine] Failed to fetch albums for genre check ({}): {}",
|
||||
artist_name,
|
||||
e
|
||||
);
|
||||
// On error, don't block - let it through
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Convert a Track to a SuggestedTrack
|
||||
fn track_to_suggested(
|
||||
&self,
|
||||
@@ -1010,26 +752,22 @@ impl SuggestionsEngine {
|
||||
}
|
||||
}
|
||||
|
||||
/// Normalize a name for comparison (remove accents, lowercase)
|
||||
fn normalize_name(name: &str) -> String {
|
||||
name.to_lowercase()
|
||||
.replace('á', "a")
|
||||
.replace('é', "e")
|
||||
.replace('í', "i")
|
||||
.replace('ó', "o")
|
||||
.replace('ú', "u")
|
||||
.replace('à', "a")
|
||||
.replace('è', "e")
|
||||
.replace('ì', "i")
|
||||
.replace('ò', "o")
|
||||
.replace('ù', "u")
|
||||
.replace('ä', "a")
|
||||
.replace('ë', "e")
|
||||
.replace('ï', "i")
|
||||
.replace('ö', "o")
|
||||
.replace('ü', "u")
|
||||
.replace('ñ', "n")
|
||||
.replace('ç', "c")
|
||||
fn rank_dedup_and_truncate(
|
||||
mut tracks: Vec<SuggestedTrack>,
|
||||
max_pool_size: usize,
|
||||
) -> Vec<SuggestedTrack> {
|
||||
// Stable sorting preserves insertion order for equal scores, so playlist
|
||||
// artists (inserted first) keep priority without discarding the ranking.
|
||||
tracks.sort_by(|left, right| right.similarity_score.total_cmp(&left.similarity_score));
|
||||
|
||||
// Sorting first makes the first title/artist occurrence the highest-score
|
||||
// version, so retain now implements the deduplication promise honestly.
|
||||
let mut seen = HashSet::new();
|
||||
tracks.retain(|track| {
|
||||
seen.insert((track.title.to_lowercase(), track.artist_name.to_lowercase()))
|
||||
});
|
||||
tracks.truncate(max_pool_size);
|
||||
tracks
|
||||
}
|
||||
|
||||
/// Check if two artist names are similar enough to be considered a match
|
||||
@@ -1134,4 +872,71 @@ mod tests {
|
||||
assert_eq!(config.vector_max_age_days, 7);
|
||||
assert!(config.min_similarity > 0.0);
|
||||
}
|
||||
|
||||
fn suggested(track_id: u64, title: &str, artist: &str, score: f32) -> SuggestedTrack {
|
||||
SuggestedTrack {
|
||||
track_id,
|
||||
title: title.to_string(),
|
||||
artist_name: artist.to_string(),
|
||||
artist_id: None,
|
||||
artist_mbid: None,
|
||||
album_title: String::new(),
|
||||
album_id: String::new(),
|
||||
album_image_url: None,
|
||||
duration: 0,
|
||||
similarity_score: score,
|
||||
reason: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ranking_happens_before_pool_truncation() {
|
||||
let tracks = vec![
|
||||
suggested(1, "Lower", "Artist", 0.3),
|
||||
suggested(2, "Highest", "Artist", 0.9),
|
||||
suggested(3, "Middle", "Artist", 0.6),
|
||||
];
|
||||
|
||||
let ranked = rank_dedup_and_truncate(tracks, 2);
|
||||
|
||||
assert_eq!(
|
||||
ranked
|
||||
.iter()
|
||||
.map(|track| track.track_id)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![2, 3]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dedup_keeps_the_highest_score_version() {
|
||||
let tracks = vec![
|
||||
suggested(1, "Duplicate", "Same Artist", 0.3),
|
||||
suggested(2, "DUPLICATE", "SAME ARTIST", 0.9),
|
||||
];
|
||||
|
||||
let deduped = rank_dedup_and_truncate(tracks, 10);
|
||||
|
||||
assert_eq!(deduped.len(), 1);
|
||||
assert_eq!(deduped[0].track_id, 2);
|
||||
assert_eq!(deduped[0].similarity_score, 0.9);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn equal_scores_keep_insertion_order() {
|
||||
let tracks = vec![
|
||||
suggested(1, "Playlist Artist", "Seed", 0.8),
|
||||
suggested(2, "Related Artist", "Related", 0.8),
|
||||
];
|
||||
|
||||
let ranked = rank_dedup_and_truncate(tracks, 10);
|
||||
|
||||
assert_eq!(
|
||||
ranked
|
||||
.iter()
|
||||
.map(|track| track.track_id)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![1, 2]
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user