Merge pull request #541 from Nezreka/fix/artist-alias-matching-issue-442
Fix/artist alias matching issue 442
This commit is contained in:
commit
c6887809da
11 changed files with 1951 additions and 4 deletions
|
|
@ -87,14 +87,105 @@ def _similarity(a: str, b: str) -> float:
|
|||
return SequenceMatcher(None, na, nb).ratio()
|
||||
|
||||
|
||||
def _alias_aware_artist_sim(
|
||||
expected_artist: str,
|
||||
actual_artist: str,
|
||||
aliases: Optional[Any] = None,
|
||||
) -> float:
|
||||
"""Best artist-similarity across (expected, *aliases) vs actual.
|
||||
|
||||
Issue #442 — when expected and actual are in different scripts
|
||||
(e.g. `Hiroyuki Sawano` vs `澤野弘之`), raw `_similarity` scores
|
||||
near 0% even though MusicBrainz aliases bridge them. Routes
|
||||
through the pure helper so the verifier inherits one shared
|
||||
contract.
|
||||
|
||||
Returns the highest score across all candidates so existing
|
||||
threshold checks (>= ARTIST_MATCH_THRESHOLD) keep their
|
||||
semantics. When `aliases` is None or empty, behaves identically
|
||||
to the prior raw `_similarity(expected, actual)` call.
|
||||
|
||||
`aliases` accepts two shapes:
|
||||
|
||||
- **Iterable** (list/tuple/set of strings): used directly. Used
|
||||
by tests that already know the aliases.
|
||||
- **Callable**: invoked LAZILY only when direct similarity
|
||||
falls below the threshold. Lets the verifier pass a memoizing
|
||||
thunk that resolves aliases (DB / cache / live MB) only when
|
||||
needed. Verifications where the direct match already passes
|
||||
never trigger the lookup chain — no wasted DB query for the
|
||||
happy path.
|
||||
|
||||
Diagnostic logging: emits an INFO line whenever an alias rescues
|
||||
a comparison that direct similarity would have failed. Lets
|
||||
future bug reports trace which alias triggered which PASS
|
||||
decision (e.g. "this file passed because alias `澤野弘之` matched
|
||||
the file's artist tag").
|
||||
"""
|
||||
from core.matching.artist_aliases import artist_names_match
|
||||
|
||||
direct = _similarity(expected_artist, actual_artist)
|
||||
# Fast path — direct match already passes the threshold OR caller
|
||||
# supplied no aliases handle. Avoids any lookup work.
|
||||
if aliases is None:
|
||||
return direct
|
||||
if direct >= ARTIST_MATCH_THRESHOLD:
|
||||
return direct
|
||||
|
||||
# Resolve the iterable. Callable provider invoked NOW (lazily —
|
||||
# the caller can memoize the result across multiple invocations
|
||||
# within one verify_audio_file call).
|
||||
resolved = aliases() if callable(aliases) else aliases
|
||||
if not resolved:
|
||||
return direct
|
||||
|
||||
_matched, score = artist_names_match(
|
||||
expected_artist,
|
||||
actual_artist,
|
||||
aliases=resolved,
|
||||
threshold=ARTIST_MATCH_THRESHOLD,
|
||||
similarity=_similarity,
|
||||
)
|
||||
|
||||
# Diagnostic — alias rescued a comparison that direct would
|
||||
# have failed. Worth logging at INFO since it's a user-visible
|
||||
# decision (file PASS instead of FAIL). One line per rescue
|
||||
# within a single verify call.
|
||||
if score >= ARTIST_MATCH_THRESHOLD and direct < ARTIST_MATCH_THRESHOLD:
|
||||
from core.matching.artist_aliases import best_alias_match
|
||||
winner, _ = best_alias_match(
|
||||
expected_artist, actual_artist, resolved, similarity=_similarity,
|
||||
)
|
||||
logger.info(
|
||||
"Artist alias rescued comparison: expected=%r vs actual=%r "
|
||||
"(direct sim=%.2f, alias %r → score=%.2f)",
|
||||
expected_artist, actual_artist, direct, winner, score,
|
||||
)
|
||||
|
||||
return score
|
||||
|
||||
|
||||
def _find_best_title_artist_match(
|
||||
recordings: List[Dict[str, Any]],
|
||||
expected_title: str,
|
||||
expected_artist: str,
|
||||
expected_artist_aliases: Optional[Any] = None,
|
||||
) -> Tuple[Optional[Dict], float, float]:
|
||||
"""
|
||||
Find the AcoustID recording that best matches expected title/artist.
|
||||
|
||||
Issue #442 — `expected_artist_aliases` (when supplied) is the
|
||||
list of alternate spellings for `expected_artist` (Japanese
|
||||
kanji, Cyrillic, etc.). Accepts either:
|
||||
|
||||
- An iterable of alias strings (used eagerly), or
|
||||
- A callable returning the list (resolved lazily — only fires
|
||||
when at least one recording fails direct artist similarity).
|
||||
|
||||
Each recording's artist is scored against (expected, *aliases)
|
||||
and the best score wins. When the list is empty/omitted/None,
|
||||
behavior is identical to the prior raw similarity comparison.
|
||||
|
||||
Returns:
|
||||
(best_recording, title_similarity, artist_similarity)
|
||||
"""
|
||||
|
|
@ -108,7 +199,9 @@ def _find_best_title_artist_match(
|
|||
artist = rec.get('artist') or ''
|
||||
|
||||
title_sim = _similarity(expected_title, title)
|
||||
artist_sim = _similarity(expected_artist, artist)
|
||||
artist_sim = _alias_aware_artist_sim(
|
||||
expected_artist, artist, expected_artist_aliases,
|
||||
)
|
||||
# Weight title higher since that's the primary identifier
|
||||
combined = (title_sim * 0.6) + (artist_sim * 0.4)
|
||||
|
||||
|
|
@ -125,6 +218,12 @@ def _find_best_title_artist_match(
|
|||
_mb_client = None
|
||||
_mb_client_lock = threading.Lock()
|
||||
|
||||
# Shared MusicBrainzService for alias lookups (issue #442). Service
|
||||
# layer wraps the raw client + adds caching + DB access — all of which
|
||||
# the alias resolution chain (library DB → cache → live MB) needs.
|
||||
_mb_service = None
|
||||
_mb_service_lock = threading.Lock()
|
||||
|
||||
MAX_MB_ENRICHMENT_LOOKUPS = 3
|
||||
|
||||
|
||||
|
|
@ -138,6 +237,42 @@ def _get_mb_client() -> MusicBrainzClient:
|
|||
return _mb_client
|
||||
|
||||
|
||||
def _get_mb_service():
|
||||
"""Get or create a shared MusicBrainzService instance.
|
||||
|
||||
Used by the alias-resolution chain in `verify_audio_file`. Lazy
|
||||
init so importing this module doesn't trigger a DB connection on
|
||||
paths that never run AcoustID verification (test runs, dry runs).
|
||||
"""
|
||||
global _mb_service
|
||||
if _mb_service is None:
|
||||
with _mb_service_lock:
|
||||
if _mb_service is None:
|
||||
from core.musicbrainz_service import MusicBrainzService
|
||||
from database.music_database import get_database
|
||||
_mb_service = MusicBrainzService(get_database())
|
||||
return _mb_service
|
||||
|
||||
|
||||
def _resolve_expected_artist_aliases(expected_artist_name: str) -> List[str]:
|
||||
"""Look up alternate-spelling aliases for the expected artist.
|
||||
|
||||
Issue #442 — bridges cross-script artist comparisons (Japanese
|
||||
kanji ↔ romanized, Cyrillic ↔ Latin, etc.) without forcing the
|
||||
verifier to know about the resolution chain. Best-effort: any
|
||||
failure (no MB service, network down, no library DB) returns
|
||||
empty list so verification falls back to the prior direct
|
||||
similarity check.
|
||||
"""
|
||||
if not expected_artist_name:
|
||||
return []
|
||||
try:
|
||||
return _get_mb_service().lookup_artist_aliases(expected_artist_name)
|
||||
except Exception as e:
|
||||
logger.debug("alias lookup failed for %r: %s", expected_artist_name, e)
|
||||
return []
|
||||
|
||||
|
||||
def _enrich_recordings_from_musicbrainz(
|
||||
recordings: List[Dict[str, Any]],
|
||||
) -> List[Dict[str, Any]]:
|
||||
|
|
@ -290,9 +425,33 @@ class AcoustIDVerification:
|
|||
# Enrich recordings that are missing title/artist via MusicBrainz lookup
|
||||
recordings = _enrich_recordings_from_musicbrainz(recordings)
|
||||
|
||||
# Issue #442 — alias resolution is LAZY. We pass a memoising
|
||||
# thunk to the artist-comparison sites; it only fires the
|
||||
# multi-tier lookup (library DB → cache → live MB) when
|
||||
# direct artist similarity falls below threshold. Verifications
|
||||
# where the direct match already passes (the common case for
|
||||
# same-script artist names) never trigger any lookup work,
|
||||
# so the fix doesn't add a per-verification DB query for the
|
||||
# happy path. When the thunk DOES fire, the result is cached
|
||||
# in the closure so the 3 comparison sites within one
|
||||
# verification share a single resolution pass.
|
||||
_alias_cache: Dict[str, Any] = {}
|
||||
|
||||
def _aliases_provider() -> List[str]:
|
||||
if 'value' not in _alias_cache:
|
||||
resolved = _resolve_expected_artist_aliases(expected_artist_name)
|
||||
_alias_cache['value'] = resolved
|
||||
if resolved:
|
||||
logger.debug(
|
||||
"Resolved %d aliases for expected artist '%s'",
|
||||
len(resolved), expected_artist_name,
|
||||
)
|
||||
return _alias_cache['value']
|
||||
|
||||
# Step 4: Find best title/artist match among AcoustID results
|
||||
best_rec, title_sim, artist_sim = _find_best_title_artist_match(
|
||||
recordings, expected_track_name, expected_artist_name
|
||||
recordings, expected_track_name, expected_artist_name,
|
||||
expected_artist_aliases=_aliases_provider,
|
||||
)
|
||||
|
||||
if not best_rec:
|
||||
|
|
@ -352,7 +511,10 @@ class AcoustIDVerification:
|
|||
# metadata for this fingerprint, it's likely the right track
|
||||
# (AcoustID's "best" match just picked the wrong variant).
|
||||
for rec in recordings:
|
||||
if _similarity(expected_artist_name, rec.get('artist', '')) >= ARTIST_MATCH_THRESHOLD:
|
||||
rec_artist = rec.get('artist', '')
|
||||
if _alias_aware_artist_sim(
|
||||
expected_artist_name, rec_artist, _aliases_provider,
|
||||
) >= ARTIST_MATCH_THRESHOLD:
|
||||
msg = (
|
||||
f"Audio verified: found '{expected_track_name}' by '{expected_artist_name}' "
|
||||
f"in AcoustID results"
|
||||
|
|
@ -400,7 +562,9 @@ class AcoustIDVerification:
|
|||
if _detect_title_version(t) != expected_version:
|
||||
continue
|
||||
if (_similarity(expected_track_name, t) >= TITLE_MATCH_THRESHOLD and
|
||||
_similarity(expected_artist_name, a) >= ARTIST_MATCH_THRESHOLD):
|
||||
_alias_aware_artist_sim(
|
||||
expected_artist_name, a, _aliases_provider,
|
||||
) >= ARTIST_MATCH_THRESHOLD):
|
||||
msg = (
|
||||
f"Audio verified: found '{t}' by '{a}' in AcoustID results "
|
||||
f"matching expected '{expected_track_name}' by '{expected_artist_name}'"
|
||||
|
|
|
|||
0
core/matching/__init__.py
Normal file
0
core/matching/__init__.py
Normal file
175
core/matching/artist_aliases.py
Normal file
175
core/matching/artist_aliases.py
Normal file
|
|
@ -0,0 +1,175 @@
|
|||
"""Pure-function artist-name comparison with alias awareness.
|
||||
|
||||
Issue #442 — cross-script artist quarantines
|
||||
-----------------------------------------------------
|
||||
|
||||
A file tagged with one spelling of an artist's name (e.g. the
|
||||
Japanese kanji `澤野弘之`) was being quarantined when SoulSync's
|
||||
expected-artist metadata used the romanized spelling
|
||||
(`Hiroyuki Sawano`). Raw similarity comparison scores 0% across
|
||||
scripts even though MusicBrainz already knows both names belong to
|
||||
the same artist (its alias list).
|
||||
|
||||
This module is the shared resolution helper. Given an expected
|
||||
artist name, an actual artist name, and an iterable of known
|
||||
aliases, it returns whether they should be treated as the same
|
||||
artist + the highest similarity score across the candidate set.
|
||||
|
||||
Pure function design:
|
||||
- No I/O, no DB access, no network
|
||||
- Caller supplies aliases (looked up from library DB or live MB)
|
||||
- Caller supplies normalize + similarity functions to keep the
|
||||
helper provider-neutral (the verifier and the matching engine
|
||||
use slightly different normalizers — let each pass its own)
|
||||
- Returns ``(matched: bool, score: float)`` so callers can log
|
||||
the score they made the decision on
|
||||
|
||||
Backward compat: when ``aliases`` is empty (or the looking-up
|
||||
caller hasn't been wired yet), the helper degrades to a plain
|
||||
direct similarity comparison — identical to the pre-fix behaviour.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from difflib import SequenceMatcher
|
||||
from typing import Callable, Iterable, Optional, Tuple
|
||||
|
||||
|
||||
# Default threshold matches the existing ARTIST_MATCH_THRESHOLD in
|
||||
# core/acoustid_verification.py. Callers can override but the helper
|
||||
# defaults are tuned to preserve current verifier behaviour.
|
||||
DEFAULT_ARTIST_MATCH_THRESHOLD = 0.6
|
||||
|
||||
|
||||
def _default_normalize(text: str) -> str:
|
||||
"""Lowercase + strip whitespace. Minimal — caller's normaliser
|
||||
almost always replaces this with something stricter (parenthetical
|
||||
stripping, punctuation removal). Used only when the caller
|
||||
doesn't pass a custom one."""
|
||||
if not text:
|
||||
return ''
|
||||
return str(text).strip().lower()
|
||||
|
||||
|
||||
def _default_similarity(a: str, b: str) -> float:
|
||||
"""SequenceMatcher ratio after the default normaliser. Matches
|
||||
the verifier's existing ``_similarity`` semantics for the no-
|
||||
custom-callable path."""
|
||||
na = _default_normalize(a)
|
||||
nb = _default_normalize(b)
|
||||
if not na or not nb:
|
||||
return 0.0
|
||||
if na == nb:
|
||||
return 1.0
|
||||
return SequenceMatcher(None, na, nb).ratio()
|
||||
|
||||
|
||||
def _coerce_aliases(aliases: Optional[Iterable[str]]) -> Tuple[str, ...]:
|
||||
"""Normalise the aliases input to a tuple of clean strings.
|
||||
|
||||
Accepts ``None``, empty iterables, lists, tuples, sets. Drops
|
||||
None / empty / non-string entries silently — callers feeding us
|
||||
raw MusicBrainz response dicts shouldn't have to clean first.
|
||||
"""
|
||||
if not aliases:
|
||||
return ()
|
||||
cleaned = []
|
||||
for value in aliases:
|
||||
if value is None:
|
||||
continue
|
||||
text = str(value).strip()
|
||||
if text:
|
||||
cleaned.append(text)
|
||||
return tuple(cleaned)
|
||||
|
||||
|
||||
def artist_names_match(
|
||||
expected: str,
|
||||
actual: str,
|
||||
*,
|
||||
aliases: Optional[Iterable[str]] = None,
|
||||
threshold: float = DEFAULT_ARTIST_MATCH_THRESHOLD,
|
||||
similarity: Optional[Callable[[str, str], float]] = None,
|
||||
) -> Tuple[bool, float]:
|
||||
"""Compare ``expected`` and ``actual`` artist names with alias
|
||||
awareness.
|
||||
|
||||
Args:
|
||||
expected: The artist name the caller expected (typically from
|
||||
metadata-source data — Spotify / iTunes / Deezer track
|
||||
payload).
|
||||
actual: The artist name the caller observed (typically from
|
||||
an AcoustID recording or a downloaded file's tag).
|
||||
aliases: Iterable of known alternate spellings for ``expected``.
|
||||
Each one gets compared against ``actual``; the best score
|
||||
wins. Empty or omitted → plain direct comparison
|
||||
(backward-compat with pre-fix behaviour).
|
||||
threshold: Score at or above which we consider the names a
|
||||
match. Defaults to 0.6 to match the verifier's existing
|
||||
``ARTIST_MATCH_THRESHOLD``.
|
||||
similarity: Optional caller-supplied similarity function
|
||||
``(a, b) -> float in [0, 1]``. Lets the verifier pass its
|
||||
stricter normaliser (parenthetical stripping etc.) without
|
||||
this module having to know about it. Defaults to a
|
||||
lowercase + SequenceMatcher comparison.
|
||||
|
||||
Returns:
|
||||
``(matched, best_score)`` where ``matched`` is True iff the
|
||||
best score across (actual, *aliases) ≥ threshold and
|
||||
``best_score`` is that maximum. ``best_score`` is informative
|
||||
for callers that want to log "matched at 0.83" or similar.
|
||||
"""
|
||||
sim = similarity or _default_similarity
|
||||
|
||||
# Direct compare first — both for the fast path and so the
|
||||
# returned score reflects the actual-vs-expected baseline (callers
|
||||
# may want it for logging even when an alias is the actual winner).
|
||||
direct_score = sim(expected, actual)
|
||||
best_score = direct_score
|
||||
if direct_score >= threshold:
|
||||
return True, direct_score
|
||||
|
||||
# Alias compare: each alias is a known alternate spelling of the
|
||||
# EXPECTED artist; match it against the ACTUAL name we observed.
|
||||
# Highest score wins.
|
||||
for alias in _coerce_aliases(aliases):
|
||||
score = sim(alias, actual)
|
||||
if score > best_score:
|
||||
best_score = score
|
||||
if score >= threshold:
|
||||
return True, score
|
||||
|
||||
return False, best_score
|
||||
|
||||
|
||||
def best_alias_match(
|
||||
expected: str,
|
||||
actual: str,
|
||||
aliases: Optional[Iterable[str]] = None,
|
||||
*,
|
||||
similarity: Optional[Callable[[str, str], float]] = None,
|
||||
) -> Tuple[Optional[str], float]:
|
||||
"""Return the alias that best matched ``actual`` (or None for the
|
||||
direct expected-vs-actual comparison) and its score.
|
||||
|
||||
Companion to ``artist_names_match`` for callers that want to
|
||||
surface which alias triggered the match (debug logging, UI
|
||||
explanations). Doesn't apply a threshold — purely informative.
|
||||
|
||||
Returns:
|
||||
``(winner, score)`` where ``winner`` is the alias string when
|
||||
an alias outscored the direct comparison, ``None`` when the
|
||||
direct comparison won (or both tied at zero).
|
||||
"""
|
||||
sim = similarity or _default_similarity
|
||||
direct_score = sim(expected, actual)
|
||||
winner: Optional[str] = None
|
||||
best = direct_score
|
||||
|
||||
for alias in _coerce_aliases(aliases):
|
||||
score = sim(alias, actual)
|
||||
if score > best:
|
||||
best = score
|
||||
winner = alias
|
||||
|
||||
return winner, best
|
||||
|
|
@ -388,6 +388,216 @@ class MusicBrainzService:
|
|||
logger.error(f"Error matching recording '{track_name}': {e}")
|
||||
return None
|
||||
|
||||
def lookup_artist_aliases(self, artist_name: str) -> list:
|
||||
"""Find alternate-spelling aliases for an artist by NAME.
|
||||
|
||||
Multi-tier resolution:
|
||||
1. Library DB row (`artists.aliases` populated by the MB
|
||||
worker when the artist was enriched). Fast path — no
|
||||
network.
|
||||
2. Existing musicbrainz_cache entry (entity_type='artist_aliases')
|
||||
— caches a prior live MB lookup for this name.
|
||||
3. Live MB lookup: search artist → fetch aliases for the best
|
||||
MBID → cache the result.
|
||||
|
||||
Always returns a list (possibly empty) — never raises. Empty
|
||||
result on any tier means "no alternate spellings found, fall
|
||||
back to direct match" which is identical to pre-fix behaviour.
|
||||
|
||||
Used by the AcoustID verifier when an artist comparison fails
|
||||
the direct similarity check. Caching means each unique artist
|
||||
name only hits MB once per cache TTL even if 100 download
|
||||
candidates fail verification with that artist.
|
||||
"""
|
||||
if not artist_name:
|
||||
return []
|
||||
|
||||
# Tier 1: library DB
|
||||
library = self.get_artist_aliases(artist_name)
|
||||
if library:
|
||||
return library
|
||||
|
||||
# Tier 2: cached live lookup (re-uses musicbrainz_cache table)
|
||||
cached = self._check_cache('artist_aliases', artist_name)
|
||||
if cached:
|
||||
metadata = cached.get('metadata') or {}
|
||||
aliases = metadata.get('aliases') if isinstance(metadata, dict) else None
|
||||
if isinstance(aliases, list):
|
||||
return [str(x).strip() for x in aliases if x]
|
||||
# Cache hit with empty result — respect it (don't re-query)
|
||||
return []
|
||||
|
||||
# Tier 3: live MB lookup. Search → fetch by MBID → cache.
|
||||
try:
|
||||
results = self.mb_client.search_artist(artist_name, limit=3)
|
||||
except Exception as e:
|
||||
logger.debug("lookup_artist_aliases: search_artist(%r) raised: %s", artist_name, e)
|
||||
self._save_to_cache('artist_aliases', artist_name, None, None, {'aliases': []}, 0)
|
||||
return []
|
||||
|
||||
if not results:
|
||||
self._save_to_cache('artist_aliases', artist_name, None, None, {'aliases': []}, 0)
|
||||
return []
|
||||
|
||||
# Score each result: combined of name-similarity + MB's own
|
||||
# relevance. Score range 0.0-1.0.
|
||||
scored = []
|
||||
for result in results:
|
||||
mb_name = result.get('name', '')
|
||||
mb_score = result.get('score', 0)
|
||||
sim = self._calculate_similarity(artist_name, mb_name)
|
||||
combined = (sim * 0.7) + (mb_score / 100 * 0.3)
|
||||
mbid = result.get('id')
|
||||
if mbid:
|
||||
scored.append((combined, mbid))
|
||||
|
||||
if not scored:
|
||||
self._save_to_cache('artist_aliases', artist_name, None, None, {'aliases': []}, 0)
|
||||
return []
|
||||
|
||||
scored.sort(key=lambda x: -x[0])
|
||||
best_score, best_mbid = scored[0]
|
||||
|
||||
# Strict trust threshold: real matches for distinctive cross-
|
||||
# script artists (the user-reported case) score >= 0.95.
|
||||
# Anything below 0.85 is ambiguous and not worth the false-
|
||||
# positive risk of pulling in aliases for the wrong artist.
|
||||
if best_score < 0.85:
|
||||
logger.debug(
|
||||
"lookup_artist_aliases: best match for %r below trust "
|
||||
"threshold (score=%.2f)", artist_name, best_score,
|
||||
)
|
||||
self._save_to_cache('artist_aliases', artist_name, None, None, {'aliases': []}, 0)
|
||||
return []
|
||||
|
||||
# Ambiguity detection: when 2+ results both score high (within
|
||||
# 0.1 of the best), the search hit multiple distinct artists
|
||||
# with similar names ("John Smith" returning 10 different
|
||||
# John Smiths all at score 100). Pulling aliases for one of
|
||||
# them could produce wrong matches. Skip + cache empty.
|
||||
if len(scored) >= 2 and (scored[0][0] - scored[1][0]) < 0.1:
|
||||
logger.debug(
|
||||
"lookup_artist_aliases: ambiguous match for %r — top "
|
||||
"two results within 0.1 (%.2f / %.2f). Skipping alias lookup.",
|
||||
artist_name, scored[0][0], scored[1][0],
|
||||
)
|
||||
self._save_to_cache('artist_aliases', artist_name, None, None, {'aliases': []}, 0)
|
||||
return []
|
||||
|
||||
aliases = self.fetch_artist_aliases(best_mbid)
|
||||
self._save_to_cache(
|
||||
'artist_aliases', artist_name, None, best_mbid,
|
||||
{'aliases': aliases}, int(best_score * 100),
|
||||
)
|
||||
return aliases
|
||||
|
||||
def fetch_artist_aliases(self, mbid: str) -> list:
|
||||
"""Fetch the alias list for an artist from MusicBrainz.
|
||||
|
||||
Issue #442 — Japanese kanji / Cyrillic / etc. spellings of an
|
||||
artist's name are stored as `aliases` on the MusicBrainz
|
||||
artist record. Pull them so SoulSync can recognise that
|
||||
`澤野弘之` and `Hiroyuki Sawano` refer to the same artist.
|
||||
|
||||
Returns the deduplicated list of alias `name` strings. Returns
|
||||
empty list (NOT None) on any failure — caller should treat
|
||||
empty as "no aliases available, fall back to direct match" so
|
||||
a transient MB outage never causes a stricter verification
|
||||
decision than today.
|
||||
"""
|
||||
if not mbid:
|
||||
return []
|
||||
try:
|
||||
data = self.mb_client.get_artist(mbid, includes=['aliases'])
|
||||
except Exception as e:
|
||||
logger.debug("fetch_artist_aliases: get_artist(%s) raised: %s", mbid, e)
|
||||
return []
|
||||
if not data:
|
||||
return []
|
||||
raw_aliases = data.get('aliases') or []
|
||||
# MB returns each alias as a dict with `name`, `sort-name`,
|
||||
# `locale`, `primary`, `type`, etc. We only care about the
|
||||
# display name — that's what `actual` artist strings will
|
||||
# match against.
|
||||
seen = set()
|
||||
cleaned = []
|
||||
for entry in raw_aliases:
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
name = (entry.get('name') or '').strip()
|
||||
if not name:
|
||||
continue
|
||||
key = name.lower()
|
||||
if key in seen:
|
||||
continue
|
||||
seen.add(key)
|
||||
cleaned.append(name)
|
||||
return cleaned
|
||||
|
||||
def update_artist_aliases(self, artist_id: int, aliases: list) -> None:
|
||||
"""Persist the alias list to `artists.aliases` as a JSON array.
|
||||
|
||||
Idempotent — overwrites any existing value. Empty list
|
||||
clears the column (caller may want this if MB has no aliases
|
||||
for the artist anymore).
|
||||
"""
|
||||
if artist_id is None:
|
||||
return
|
||||
conn = None
|
||||
try:
|
||||
conn = self.db._get_connection()
|
||||
cursor = conn.cursor()
|
||||
cursor.execute(
|
||||
"UPDATE artists SET aliases = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?",
|
||||
(json.dumps(aliases) if aliases else None, artist_id),
|
||||
)
|
||||
conn.commit()
|
||||
logger.debug("Updated artist %s aliases (%d entries)", artist_id, len(aliases or []))
|
||||
except Exception as e:
|
||||
logger.error(f"Error updating artist aliases for {artist_id}: {e}")
|
||||
if conn:
|
||||
conn.rollback()
|
||||
finally:
|
||||
if conn:
|
||||
conn.close()
|
||||
|
||||
def get_artist_aliases(self, artist_name: str) -> list:
|
||||
"""Look up cached aliases for an artist by NAME (not id).
|
||||
|
||||
Used by the verifier where the expected artist comes from a
|
||||
download's metadata-source data — we don't have a library
|
||||
row's `id` to query, just the display name. Returns empty
|
||||
list when the artist isn't in the library or has no aliases
|
||||
recorded. The verifier falls back to live MB lookup in that
|
||||
case.
|
||||
"""
|
||||
if not artist_name:
|
||||
return []
|
||||
conn = None
|
||||
try:
|
||||
conn = self.db._get_connection()
|
||||
cursor = conn.cursor()
|
||||
cursor.execute(
|
||||
"SELECT aliases FROM artists WHERE name = ? COLLATE NOCASE LIMIT 1",
|
||||
(artist_name,),
|
||||
)
|
||||
row = cursor.fetchone()
|
||||
if not row or not row[0]:
|
||||
return []
|
||||
try:
|
||||
parsed = json.loads(row[0])
|
||||
except (TypeError, json.JSONDecodeError):
|
||||
return []
|
||||
if not isinstance(parsed, list):
|
||||
return []
|
||||
return [str(x).strip() for x in parsed if x]
|
||||
except Exception as e:
|
||||
logger.debug("get_artist_aliases lookup failed for %r: %s", artist_name, e)
|
||||
return []
|
||||
finally:
|
||||
if conn:
|
||||
conn.close()
|
||||
|
||||
def update_artist_mbid(self, artist_id: int, mbid: Optional[str], status: str):
|
||||
"""Update artist with MusicBrainz ID"""
|
||||
conn = None
|
||||
|
|
|
|||
|
|
@ -283,6 +283,31 @@ class MusicBrainzWorker:
|
|||
if conn:
|
||||
conn.close()
|
||||
|
||||
def _artist_aliases_empty(self, artist_id: Any) -> bool:
|
||||
"""Check if `artists.aliases` for this row is NULL or empty.
|
||||
|
||||
Used by the existing-MBID backfill path to skip the MB call
|
||||
when aliases are already populated (re-scan cycles after
|
||||
backfill complete should be no-ops). Defensive: returns True
|
||||
on any error so the backfill attempt happens — a redundant MB
|
||||
call is cheaper than missing the backfill entirely.
|
||||
"""
|
||||
conn = None
|
||||
try:
|
||||
conn = self.db._get_connection()
|
||||
cursor = conn.cursor()
|
||||
cursor.execute("SELECT aliases FROM artists WHERE id = ? LIMIT 1", (artist_id,))
|
||||
row = cursor.fetchone()
|
||||
if not row:
|
||||
return False # Row doesn't exist — nothing to backfill
|
||||
value = row[0]
|
||||
return value is None or value == '' or value == '[]'
|
||||
except Exception:
|
||||
return True
|
||||
finally:
|
||||
if conn:
|
||||
conn.close()
|
||||
|
||||
def _process_item(self, item: Dict[str, Any]):
|
||||
"""Process a single item (artist, album, or track)"""
|
||||
try:
|
||||
|
|
@ -301,6 +326,27 @@ class MusicBrainzWorker:
|
|||
try:
|
||||
if item_type == 'artist':
|
||||
self.mb_service.update_artist_mbid(item_id, existing_id, 'matched')
|
||||
# Issue #442 — one-time backfill for artists
|
||||
# enriched before alias support landed. Users with
|
||||
# pre-existing libraries on day-one of this PR have
|
||||
# MBIDs but NULL aliases. Fetch ONLY when the
|
||||
# column is empty so re-scan cycles after backfill
|
||||
# don't re-query MB. Best-effort: failures are
|
||||
# logged at debug, don't regress the match outcome.
|
||||
try:
|
||||
if self._artist_aliases_empty(item_id):
|
||||
aliases = self.mb_service.fetch_artist_aliases(existing_id)
|
||||
if aliases:
|
||||
self.mb_service.update_artist_aliases(item_id, aliases)
|
||||
logger.debug(
|
||||
"Backfilled %d aliases for artist '%s'",
|
||||
len(aliases), item_name,
|
||||
)
|
||||
except Exception as backfill_err:
|
||||
logger.debug(
|
||||
"Alias backfill failed for artist '%s': %s",
|
||||
item_name, backfill_err,
|
||||
)
|
||||
elif item_type == 'album':
|
||||
self.mb_service.update_album_mbid(item_id, existing_id, 'matched')
|
||||
elif item_type == 'track':
|
||||
|
|
@ -313,6 +359,24 @@ class MusicBrainzWorker:
|
|||
result = self.mb_service.match_artist(item_name)
|
||||
if result and result.get('mbid'):
|
||||
self.mb_service.update_artist_mbid(item_id, result['mbid'], 'matched')
|
||||
# Issue #442 — pull alternate-spelling aliases (Japanese
|
||||
# kanji, Cyrillic, etc.) so the verifier can recognise
|
||||
# cross-script artist names without re-querying MB on
|
||||
# every quarantine candidate. Best-effort: failures are
|
||||
# swallowed inside `fetch_artist_aliases` (returns
|
||||
# empty list) so a transient MB outage never regresses
|
||||
# the enrichment outcome.
|
||||
try:
|
||||
aliases = self.mb_service.fetch_artist_aliases(result['mbid'])
|
||||
if aliases:
|
||||
self.mb_service.update_artist_aliases(item_id, aliases)
|
||||
logger.debug(
|
||||
"Stored %d aliases for artist '%s'", len(aliases), item_name,
|
||||
)
|
||||
except Exception as alias_err:
|
||||
logger.debug(
|
||||
"Alias enrichment failed for artist '%s': %s", item_name, alias_err,
|
||||
)
|
||||
self.stats['matched'] += 1
|
||||
logger.info(f"Matched artist '{item_name}' → MBID: {result['mbid']}")
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -1792,6 +1792,17 @@ class MusicDatabase:
|
|||
if 'musicbrainz_match_status' not in artists_columns:
|
||||
cursor.execute("ALTER TABLE artists ADD COLUMN musicbrainz_match_status TEXT")
|
||||
columns_added = True
|
||||
# MusicBrainz exposes alternate-spelling aliases on every artist
|
||||
# record (Japanese kanji ↔ romanized, Cyrillic ↔ Latin, etc.).
|
||||
# SoulSync's artist matching used to compare expected vs actual
|
||||
# name with raw similarity — cross-script comparison scored 0%
|
||||
# and the file got quarantined even when MusicBrainz knew both
|
||||
# names belonged to the same artist (issue #442). Persist the
|
||||
# alias list as JSON so the verifier + matcher can consult it
|
||||
# without re-querying MB on every comparison.
|
||||
if 'aliases' not in artists_columns:
|
||||
cursor.execute("ALTER TABLE artists ADD COLUMN aliases TEXT")
|
||||
columns_added = True
|
||||
if columns_added:
|
||||
logger.info("Added MusicBrainz columns to artists table")
|
||||
|
||||
|
|
|
|||
0
tests/matching/__init__.py
Normal file
0
tests/matching/__init__.py
Normal file
498
tests/matching/test_acoustid_verification_aliases.py
Normal file
498
tests/matching/test_acoustid_verification_aliases.py
Normal file
|
|
@ -0,0 +1,498 @@
|
|||
"""Regression tests for issue #442 — AcoustID verifier alias awareness.
|
||||
|
||||
The reporter posted two exact cases:
|
||||
|
||||
Case 1 (Japanese kanji ↔ romanized):
|
||||
File: YAMANAIAME by 澤野弘之
|
||||
Expected: YAMANAIAME by Hiroyuki Sawano
|
||||
Pre-fix: quarantined (artist_sim=0%)
|
||||
Post-fix: passes verification because MB aliases bridge the
|
||||
two spellings.
|
||||
|
||||
Case 2 (Cyrillic ↔ Latin):
|
||||
File: On the Other Side by Sergey Lazarev
|
||||
Expected: On the other side by Сергей Лазарев
|
||||
Pre-fix: quarantined (artist_sim=7%)
|
||||
Post-fix: passes via aliases.
|
||||
|
||||
These tests pin the verifier through the helper. AcoustID's
|
||||
fingerprint call is stubbed (no network), MB service's
|
||||
`lookup_artist_aliases` is stubbed to return the relevant aliases.
|
||||
The verifier's pass/fail decision is the assertion.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from core.acoustid_verification import (
|
||||
AcoustIDVerification,
|
||||
VerificationResult,
|
||||
_alias_aware_artist_sim,
|
||||
_find_best_title_artist_match,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Pure helper — _alias_aware_artist_sim
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAliasAwareArtistSim:
|
||||
def test_returns_higher_score_when_alias_matches(self):
|
||||
score = _alias_aware_artist_sim(
|
||||
'Hiroyuki Sawano', '澤野弘之',
|
||||
aliases=['澤野弘之', 'SawanoHiroyuki'],
|
||||
)
|
||||
assert score == 1.0
|
||||
|
||||
def test_no_aliases_falls_back_to_direct_similarity(self):
|
||||
"""Cross-script with NO aliases → score ~0, pre-fix behaviour."""
|
||||
score = _alias_aware_artist_sim(
|
||||
'Hiroyuki Sawano', '澤野弘之', aliases=None,
|
||||
)
|
||||
assert score < 0.1
|
||||
|
||||
def test_aliases_dont_mask_genuine_mismatch(self):
|
||||
"""Different artist entirely → still scores low even when
|
||||
aliases are provided. Aliases bridge synonyms, not unrelated
|
||||
artists."""
|
||||
score = _alias_aware_artist_sim(
|
||||
'Hiroyuki Sawano', 'Khalil Turk & Friends',
|
||||
aliases=['澤野弘之', 'SawanoHiroyuki'],
|
||||
)
|
||||
assert score < 0.5
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _find_best_title_artist_match — accepts aliases now
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestFindBestMatchWithAliases:
|
||||
def test_japanese_alias_picks_correct_recording(self):
|
||||
"""Reporter's case 1: AcoustID returned recording with kanji
|
||||
artist. Without aliases the scorer ranks it low and the
|
||||
verifier later quarantines. With aliases it scores high."""
|
||||
recordings = [
|
||||
{'title': 'YAMANAIAME', 'artist': '澤野弘之'},
|
||||
{'title': 'Different Song', 'artist': 'Hiroyuki Sawano'},
|
||||
]
|
||||
# Aliases provided — bridge to recording 0
|
||||
best, title_sim, artist_sim = _find_best_title_artist_match(
|
||||
recordings, 'YAMANAIAME', 'Hiroyuki Sawano',
|
||||
expected_artist_aliases=['澤野弘之', 'SawanoHiroyuki'],
|
||||
)
|
||||
assert best is recordings[0]
|
||||
assert artist_sim == 1.0
|
||||
|
||||
def test_no_aliases_legacy_behaviour_preserved(self):
|
||||
"""Default arg / empty aliases → identical to pre-fix
|
||||
behaviour. Critical for paths not yet wired up to alias
|
||||
lookup."""
|
||||
recordings = [
|
||||
{'title': 'Track', 'artist': 'Artist'},
|
||||
]
|
||||
best, title_sim, artist_sim = _find_best_title_artist_match(
|
||||
recordings, 'Track', 'Artist',
|
||||
)
|
||||
assert title_sim == 1.0
|
||||
assert artist_sim == 1.0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# End-to-end — reporter's cases through the full verifier
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def stubbed_verifier(monkeypatch):
|
||||
"""AcoustIDVerification with the acoustid client + MB service
|
||||
layer stubbed. Lets us drive the verifier's full decision path
|
||||
without network or DB. Returns the verifier + mutable handles
|
||||
to the stubs so each test can shape the AcoustID response +
|
||||
aliases."""
|
||||
verifier = AcoustIDVerification()
|
||||
verifier.acoustid_client = MagicMock()
|
||||
verifier.acoustid_client.is_available.return_value = (True, '')
|
||||
|
||||
# Stub the MB service so verifier alias lookup doesn't touch DB
|
||||
# or network. Each test sets fake_service.lookup_artist_aliases.
|
||||
fake_service = MagicMock()
|
||||
fake_service.lookup_artist_aliases.return_value = []
|
||||
monkeypatch.setattr(
|
||||
'core.acoustid_verification._get_mb_service', lambda: fake_service,
|
||||
)
|
||||
|
||||
return verifier, fake_service
|
||||
|
||||
|
||||
class TestIssue442Regression:
|
||||
def test_japanese_kanji_artist_passes_verification(self, stubbed_verifier):
|
||||
"""Reporter's case 1 — verbatim from the issue:
|
||||
|
||||
File: YAMANAIAME by 澤野弘之
|
||||
Expected: YAMANAIAME by Hiroyuki Sawano
|
||||
Pre-fix: Quarantined (artist=0%)
|
||||
"""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
# AcoustID returns the recording with kanji artist
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'YAMANAIAME', 'artist': '澤野弘之', 'mbid': 'rec-x'},
|
||||
],
|
||||
}
|
||||
# MB knows Hiroyuki Sawano's aliases
|
||||
fake_service.lookup_artist_aliases.return_value = [
|
||||
'澤野弘之', 'SawanoHiroyuki', 'Sawano Hiroyuki',
|
||||
]
|
||||
|
||||
result, msg = verifier.verify_audio_file(
|
||||
'/fake/path.mp3', 'YAMANAIAME', 'Hiroyuki Sawano',
|
||||
)
|
||||
|
||||
assert result == VerificationResult.PASS, (
|
||||
f"Reporter's exact case must pass verification post-fix; "
|
||||
f"got result={result.value!r} msg={msg!r}"
|
||||
)
|
||||
fake_service.lookup_artist_aliases.assert_called_once_with('Hiroyuki Sawano')
|
||||
|
||||
def test_cyrillic_artist_passes_verification(self, stubbed_verifier):
|
||||
"""Reporter's case 2 — Sergey Lazarev / Сергей Лазарев."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'On the Other Side', 'artist': 'Sergey Lazarev', 'mbid': 'rec-y'},
|
||||
],
|
||||
}
|
||||
fake_service.lookup_artist_aliases.return_value = [
|
||||
'Sergey Lazarev', 'Sergei Lazarev',
|
||||
]
|
||||
|
||||
result, msg = verifier.verify_audio_file(
|
||||
'/fake/path.flac', 'On the other side', 'Сергей Лазарев',
|
||||
)
|
||||
|
||||
assert result == VerificationResult.PASS
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Backward compat — no aliases available → behavior identical to pre-fix
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestBackwardCompat:
|
||||
def test_no_aliases_clear_artist_mismatch_still_fails(self, stubbed_verifier):
|
||||
"""Pre-fix: clear mismatches (artist sim near 0, NOT a script
|
||||
difference) should FAIL. Post-fix with empty aliases must
|
||||
preserve this — aliases bridge synonyms, not unrelated
|
||||
artists."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
# Wrong artist entirely — Latin script both sides, sim ~0
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'Some Track', 'artist': 'Khalil Turk & Friends'},
|
||||
],
|
||||
}
|
||||
fake_service.lookup_artist_aliases.return_value = [] # No aliases
|
||||
|
||||
result, msg = verifier.verify_audio_file(
|
||||
'/fake/path.mp3', 'Some Track', 'Foreigner',
|
||||
)
|
||||
|
||||
assert result == VerificationResult.FAIL
|
||||
|
||||
def test_no_aliases_exact_match_still_passes(self, stubbed_verifier):
|
||||
"""Exact title + artist match → PASS regardless of aliases."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'Dirty White Boy', 'artist': 'Foreigner'},
|
||||
],
|
||||
}
|
||||
fake_service.lookup_artist_aliases.return_value = []
|
||||
|
||||
result, _ = verifier.verify_audio_file(
|
||||
'/fake/path.mp3', 'Dirty White Boy', 'Foreigner',
|
||||
)
|
||||
assert result == VerificationResult.PASS
|
||||
|
||||
def test_alias_lookup_failure_does_not_break_verification(self, stubbed_verifier):
|
||||
"""MB service raises → verifier still completes with direct
|
||||
similarity (pre-fix behaviour preserved)."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
fake_service.lookup_artist_aliases.side_effect = Exception("MB down")
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'Dirty White Boy', 'artist': 'Foreigner'},
|
||||
],
|
||||
}
|
||||
|
||||
result, _ = verifier.verify_audio_file(
|
||||
'/fake/path.mp3', 'Dirty White Boy', 'Foreigner',
|
||||
)
|
||||
# Should still pass — direct similarity works
|
||||
assert result == VerificationResult.PASS
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Performance contract — alias lookup fires ONCE per verification
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAliasLookupCalledOncePerVerify:
|
||||
def test_single_lookup_call_regardless_of_recordings_count(self, stubbed_verifier):
|
||||
"""The verifier processes multiple recordings + scans through
|
||||
them at up to 3 sites — but should only call
|
||||
`lookup_artist_aliases` ONCE per verify_audio_file invocation.
|
||||
Otherwise verifying a track with 20 AcoustID recordings could
|
||||
fire 60+ MB lookups (cached or not, that's wasteful)."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'X', 'artist': '澤野弘之'},
|
||||
{'title': 'X', 'artist': 'SawanoHiroyuki'},
|
||||
{'title': 'X', 'artist': 'Different Artist'},
|
||||
],
|
||||
}
|
||||
fake_service.lookup_artist_aliases.return_value = ['澤野弘之', 'SawanoHiroyuki']
|
||||
|
||||
verifier.verify_audio_file('/fake/path.mp3', 'X', 'Hiroyuki Sawano')
|
||||
|
||||
assert fake_service.lookup_artist_aliases.call_count == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Lazy alias resolution — happy path skips MB lookup entirely
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestLazyAliasResolution:
|
||||
"""Issue #442 perf followup: alias lookup should ONLY fire when
|
||||
the direct artist comparison fails. Verifications where artist
|
||||
names already match (the 95% common case for same-script
|
||||
libraries) must NOT trigger the lookup chain — no wasted DB
|
||||
query, no wasted MB call."""
|
||||
|
||||
def test_no_lookup_when_direct_artist_match_passes(self, stubbed_verifier):
|
||||
"""Exact-match Latin-script artist passes verification with
|
||||
zero alias lookups — no DB query, no MB call. Same-script
|
||||
libraries (the 95% common case) inherit zero perf cost from
|
||||
this PR."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'Dirty White Boy', 'artist': 'Foreigner'},
|
||||
],
|
||||
}
|
||||
|
||||
result, _ = verifier.verify_audio_file(
|
||||
'/fake/path.mp3', 'Dirty White Boy', 'Foreigner',
|
||||
)
|
||||
|
||||
assert result == VerificationResult.PASS
|
||||
# Critical — alias lookup must NOT have been called for the
|
||||
# happy path. Otherwise every successful verification adds a
|
||||
# DB query for nothing.
|
||||
fake_service.lookup_artist_aliases.assert_not_called()
|
||||
|
||||
def test_lookup_fires_only_when_direct_artist_match_fails(self, stubbed_verifier):
|
||||
"""Cross-script case where direct sim is 0% → lookup fires
|
||||
as expected."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'YAMANAIAME', 'artist': '澤野弘之'},
|
||||
],
|
||||
}
|
||||
fake_service.lookup_artist_aliases.return_value = ['澤野弘之']
|
||||
|
||||
result, _ = verifier.verify_audio_file(
|
||||
'/fake/path.mp3', 'YAMANAIAME', 'Hiroyuki Sawano',
|
||||
)
|
||||
|
||||
assert result == VerificationResult.PASS
|
||||
# Lookup fired BECAUSE direct match would have failed
|
||||
fake_service.lookup_artist_aliases.assert_called_once()
|
||||
|
||||
def test_lookup_memoised_across_three_comparison_sites(self, stubbed_verifier):
|
||||
"""When lookup DOES fire, the result must be reused across
|
||||
the three artist-comparison sites in the verifier (best-match
|
||||
scoring, secondary scan, fallback scan). One resolution per
|
||||
verification — not three."""
|
||||
verifier, fake_service = stubbed_verifier
|
||||
|
||||
# Force a code path that hits multiple sites: title matches
|
||||
# several recordings but the best-match's artist sim is below
|
||||
# threshold (forces secondary scan path).
|
||||
verifier.acoustid_client.fingerprint_and_lookup.return_value = {
|
||||
'best_score': 0.95,
|
||||
'recordings': [
|
||||
{'title': 'X', 'artist': 'Different Latin Artist'}, # 0 alias hit
|
||||
{'title': 'X', 'artist': '澤野弘之'}, # alias hit
|
||||
],
|
||||
}
|
||||
fake_service.lookup_artist_aliases.return_value = ['澤野弘之']
|
||||
|
||||
verifier.verify_audio_file('/fake/path.mp3', 'X', 'Hiroyuki Sawano')
|
||||
|
||||
# Memoised — one resolution shared across all sites
|
||||
assert fake_service.lookup_artist_aliases.call_count == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Provider-callable contract on the helper
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAliasProviderCallable:
|
||||
"""Pin the dual-shape contract on `_alias_aware_artist_sim`:
|
||||
accepts an iterable OR a callable. Callable resolves lazily."""
|
||||
|
||||
def test_iterable_passed_directly(self):
|
||||
"""Plain list — used as-is, no lazy semantics."""
|
||||
score = _alias_aware_artist_sim(
|
||||
'Hiroyuki Sawano', '澤野弘之', aliases=['澤野弘之'],
|
||||
)
|
||||
assert score == 1.0
|
||||
|
||||
def test_callable_resolves_lazily_only_when_direct_fails(self):
|
||||
"""Callable provider — invoked ONLY when direct sim falls
|
||||
below threshold."""
|
||||
call_count = [0]
|
||||
|
||||
def provider():
|
||||
call_count[0] += 1
|
||||
return ['澤野弘之']
|
||||
|
||||
# Direct match passes → provider NOT called
|
||||
_alias_aware_artist_sim('Foreigner', 'Foreigner', aliases=provider)
|
||||
assert call_count[0] == 0
|
||||
|
||||
# Direct match fails → provider IS called
|
||||
_alias_aware_artist_sim('Hiroyuki Sawano', '澤野弘之', aliases=provider)
|
||||
assert call_count[0] == 1
|
||||
|
||||
def test_callable_returning_empty_list_falls_back_to_direct(self):
|
||||
"""Provider returns empty (e.g. MB had no aliases) →
|
||||
score = direct sim, no error."""
|
||||
score = _alias_aware_artist_sim(
|
||||
'Hiroyuki Sawano', '澤野弘之', aliases=lambda: [],
|
||||
)
|
||||
# ~0 because direct cross-script comparison fails
|
||||
assert score < 0.1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Diagnostic logging — alias rescues are visible in logs
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAliasRescueLogging:
|
||||
"""When an alias bridges a comparison that direct similarity
|
||||
would have failed, log it at INFO level. Future bug reports
|
||||
where a file passed verification incorrectly can be traced back
|
||||
to which alias triggered which decision.
|
||||
|
||||
Uses a directly-attached handler instead of pytest's caplog —
|
||||
full-suite caplog is intermittently flaky for soulsync namespace
|
||||
loggers (handler ordering, parallel test state). An owned
|
||||
handler on the specific logger sidesteps both issues, same
|
||||
pattern as the prior watchdog-test fix.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _capture_records():
|
||||
"""Attach an owned ListHandler to the verifier's logger.
|
||||
Returns (records list, teardown callable)."""
|
||||
import logging as _logging
|
||||
records: list = []
|
||||
|
||||
class _ListHandler(_logging.Handler):
|
||||
def emit(self, record):
|
||||
records.append(record)
|
||||
|
||||
handler = _ListHandler(level=_logging.INFO)
|
||||
# Logger name is `soulsync.acoustid.verification` per
|
||||
# `core.acoustid_verification`'s `get_logger("acoustid_verification")`
|
||||
# — dot-separated, NOT underscored.
|
||||
verifier_logger = _logging.getLogger('soulsync.acoustid.verification')
|
||||
verifier_logger.addHandler(handler)
|
||||
prior_level = verifier_logger.level
|
||||
verifier_logger.setLevel(_logging.INFO)
|
||||
|
||||
def teardown():
|
||||
verifier_logger.removeHandler(handler)
|
||||
verifier_logger.setLevel(prior_level)
|
||||
|
||||
return records, teardown
|
||||
|
||||
def test_alias_rescue_emits_info_log(self):
|
||||
records, teardown = self._capture_records()
|
||||
try:
|
||||
_alias_aware_artist_sim(
|
||||
'Hiroyuki Sawano', '澤野弘之', aliases=['澤野弘之'],
|
||||
)
|
||||
finally:
|
||||
teardown()
|
||||
|
||||
rescue_logs = [
|
||||
r.getMessage() for r in records
|
||||
if 'alias rescued' in r.getMessage().lower()
|
||||
]
|
||||
assert len(rescue_logs) >= 1, (
|
||||
f"Expected an INFO log line about alias rescue; got "
|
||||
f"{[r.getMessage() for r in records]}"
|
||||
)
|
||||
|
||||
def test_no_log_when_direct_match_succeeds(self):
|
||||
"""Happy path doesn't spam logs — only rescue cases log."""
|
||||
records, teardown = self._capture_records()
|
||||
try:
|
||||
_alias_aware_artist_sim(
|
||||
'Foreigner', 'Foreigner', aliases=['ignored-alias'],
|
||||
)
|
||||
finally:
|
||||
teardown()
|
||||
|
||||
rescue_logs = [
|
||||
r.getMessage() for r in records
|
||||
if 'alias rescued' in r.getMessage().lower()
|
||||
]
|
||||
assert rescue_logs == []
|
||||
|
||||
def test_no_log_when_alias_doesnt_help(self):
|
||||
"""If aliases were available but didn't bridge the gap (still
|
||||
below threshold), no rescue log — there was no rescue."""
|
||||
records, teardown = self._capture_records()
|
||||
try:
|
||||
_alias_aware_artist_sim(
|
||||
'Hiroyuki Sawano', 'Khalil Turk',
|
||||
aliases=['Sergey Lazarev'], # unrelated alias
|
||||
)
|
||||
finally:
|
||||
teardown()
|
||||
|
||||
rescue_logs = [
|
||||
r.getMessage() for r in records
|
||||
if 'alias rescued' in r.getMessage().lower()
|
||||
]
|
||||
assert rescue_logs == []
|
||||
560
tests/matching/test_artist_alias_service.py
Normal file
560
tests/matching/test_artist_alias_service.py
Normal file
|
|
@ -0,0 +1,560 @@
|
|||
"""Pin the MusicBrainz service alias methods + worker enrichment.
|
||||
|
||||
Issue #442 — these methods feed the alias data the helper compares
|
||||
against. Three layers covered:
|
||||
|
||||
1. ``fetch_artist_aliases`` — pulls aliases off the MB get-artist
|
||||
response, defensive against missing fields, broken JSON, network
|
||||
errors.
|
||||
2. ``update_artist_aliases`` — persists to ``artists.aliases`` as a
|
||||
JSON array. Empty/None → column cleared.
|
||||
3. ``get_artist_aliases`` — reads back by artist NAME (not id) since
|
||||
that's what the verifier has at quarantine time.
|
||||
|
||||
Worker enrichment integration covered separately: when MB worker
|
||||
matches an artist, it calls fetch + update so subsequent verifier
|
||||
calls find aliases in the library DB without firing live MB.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import sqlite3
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def temp_db(tmp_path, monkeypatch):
|
||||
"""Real MusicDatabase against a tmp file. Uses the production
|
||||
schema (so the `aliases` column from commit 1's migration is
|
||||
present) and the production update/get methods we're pinning."""
|
||||
monkeypatch.setenv('DATABASE_PATH', str(tmp_path / 'test.db'))
|
||||
from database.music_database import MusicDatabase
|
||||
return MusicDatabase()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def service(temp_db):
|
||||
"""MusicBrainzService with stubbed mb_client. The DB is real;
|
||||
the network is not."""
|
||||
from core.musicbrainz_service import MusicBrainzService
|
||||
svc = MusicBrainzService(temp_db)
|
||||
svc.mb_client = MagicMock()
|
||||
return svc
|
||||
|
||||
|
||||
_seed_counter = 0
|
||||
|
||||
|
||||
def _seed_artist(db, name: str, **fields) -> str:
|
||||
"""Insert a row into the artists table.
|
||||
|
||||
`artists.id` is TEXT (NOT INTEGER auto-increment), so we mint a
|
||||
deterministic test id rather than relying on rowid magic.
|
||||
Returns the id as str — that's what the production code paths
|
||||
use too (read methods, joins, etc.).
|
||||
"""
|
||||
global _seed_counter
|
||||
_seed_counter += 1
|
||||
artist_id = f"test-artist-{_seed_counter}"
|
||||
conn = db._get_connection()
|
||||
cursor = conn.cursor()
|
||||
cols = ['id', 'name'] + list(fields.keys())
|
||||
placeholders = ','.join('?' * len(cols))
|
||||
cursor.execute(
|
||||
f"INSERT INTO artists ({','.join(cols)}) VALUES ({placeholders})",
|
||||
[artist_id, name] + list(fields.values()),
|
||||
)
|
||||
conn.commit()
|
||||
conn.close()
|
||||
return artist_id
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# fetch_artist_aliases — MB get-artist response parser
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestFetchArtistAliases:
|
||||
def test_extracts_alias_names_from_mb_response(self, service):
|
||||
"""Reporter's case 1 shape: MB returns aliases for Hiroyuki
|
||||
Sawano including the Japanese kanji form. Extract the `name`
|
||||
from each alias entry."""
|
||||
service.mb_client.get_artist.return_value = {
|
||||
'id': '60d2ea34-1912-425f-bf9c-fc544e4448cd',
|
||||
'name': 'Hiroyuki Sawano',
|
||||
'aliases': [
|
||||
{'name': '澤野弘之', 'sort-name': '澤野弘之', 'locale': 'ja', 'primary': True},
|
||||
{'name': 'SawanoHiroyuki', 'sort-name': 'SawanoHiroyuki', 'locale': None},
|
||||
{'name': 'Sawano Hiroyuki', 'sort-name': 'Sawano, Hiroyuki', 'locale': 'en'},
|
||||
],
|
||||
}
|
||||
|
||||
aliases = service.fetch_artist_aliases('60d2ea34-1912-425f-bf9c-fc544e4448cd')
|
||||
|
||||
assert '澤野弘之' in aliases
|
||||
assert 'SawanoHiroyuki' in aliases
|
||||
assert 'Sawano Hiroyuki' in aliases
|
||||
assert len(aliases) == 3
|
||||
|
||||
def test_dedup_case_insensitive(self, service):
|
||||
"""Same name with different casing should collapse — MB
|
||||
sometimes returns duplicate-looking entries with locale
|
||||
variations."""
|
||||
service.mb_client.get_artist.return_value = {
|
||||
'aliases': [
|
||||
{'name': 'Hiroyuki Sawano'},
|
||||
{'name': 'hiroyuki sawano'},
|
||||
{'name': 'HIROYUKI SAWANO'},
|
||||
],
|
||||
}
|
||||
aliases = service.fetch_artist_aliases('mbid-x')
|
||||
assert len(aliases) == 1
|
||||
|
||||
def test_empty_alias_entries_skipped(self, service):
|
||||
service.mb_client.get_artist.return_value = {
|
||||
'aliases': [
|
||||
{'name': ''},
|
||||
{'name': ' '},
|
||||
{'name': None},
|
||||
{'name': 'Real Name'},
|
||||
],
|
||||
}
|
||||
aliases = service.fetch_artist_aliases('mbid-x')
|
||||
assert aliases == ['Real Name']
|
||||
|
||||
def test_missing_aliases_key_returns_empty(self, service):
|
||||
"""MB artist record might not have any aliases. Returns []
|
||||
not raises."""
|
||||
service.mb_client.get_artist.return_value = {
|
||||
'id': 'mbid-x',
|
||||
'name': 'Some Artist',
|
||||
}
|
||||
assert service.fetch_artist_aliases('mbid-x') == []
|
||||
|
||||
def test_aliases_null_returns_empty(self, service):
|
||||
"""MB sometimes returns `aliases: null` instead of empty array."""
|
||||
service.mb_client.get_artist.return_value = {'aliases': None}
|
||||
assert service.fetch_artist_aliases('mbid-x') == []
|
||||
|
||||
def test_get_artist_failure_returns_empty(self, service):
|
||||
"""Network / API failure → empty list, NOT raise. Caller
|
||||
must treat empty as 'no aliases available, fall back to
|
||||
direct match' so transient MB outages never trigger
|
||||
stricter quarantine decisions than today."""
|
||||
service.mb_client.get_artist.side_effect = Exception("network error")
|
||||
assert service.fetch_artist_aliases('mbid-x') == []
|
||||
|
||||
def test_get_artist_returns_none_returns_empty(self, service):
|
||||
service.mb_client.get_artist.return_value = None
|
||||
assert service.fetch_artist_aliases('mbid-x') == []
|
||||
|
||||
def test_empty_mbid_returns_empty_without_api_call(self, service):
|
||||
assert service.fetch_artist_aliases('') == []
|
||||
assert service.fetch_artist_aliases(None) == []
|
||||
service.mb_client.get_artist.assert_not_called()
|
||||
|
||||
def test_includes_aliases_in_request(self, service):
|
||||
"""Verify the MB API call requests the aliases include
|
||||
explicitly — without `inc=aliases` the response wouldn't
|
||||
carry them."""
|
||||
service.mb_client.get_artist.return_value = {'aliases': []}
|
||||
service.fetch_artist_aliases('mbid-x')
|
||||
service.mb_client.get_artist.assert_called_once_with(
|
||||
'mbid-x', includes=['aliases'],
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# update_artist_aliases — persistence
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestUpdateArtistAliases:
|
||||
def test_persists_as_json_array(self, service, temp_db):
|
||||
artist_id = _seed_artist(temp_db, 'Hiroyuki Sawano')
|
||||
service.update_artist_aliases(artist_id, ['澤野弘之', 'SawanoHiroyuki'])
|
||||
|
||||
conn = temp_db._get_connection()
|
||||
row = conn.execute("SELECT aliases FROM artists WHERE id = ?", (artist_id,)).fetchone()
|
||||
conn.close()
|
||||
parsed = json.loads(row[0])
|
||||
assert parsed == ['澤野弘之', 'SawanoHiroyuki']
|
||||
|
||||
def test_idempotent_overwrite(self, service, temp_db):
|
||||
artist_id = _seed_artist(temp_db, 'X')
|
||||
service.update_artist_aliases(artist_id, ['a'])
|
||||
service.update_artist_aliases(artist_id, ['b', 'c'])
|
||||
conn = temp_db._get_connection()
|
||||
row = conn.execute("SELECT aliases FROM artists WHERE id = ?", (artist_id,)).fetchone()
|
||||
conn.close()
|
||||
assert json.loads(row[0]) == ['b', 'c']
|
||||
|
||||
def test_empty_list_clears_column(self, service, temp_db):
|
||||
artist_id = _seed_artist(temp_db, 'X', aliases=json.dumps(['old']))
|
||||
service.update_artist_aliases(artist_id, [])
|
||||
conn = temp_db._get_connection()
|
||||
row = conn.execute("SELECT aliases FROM artists WHERE id = ?", (artist_id,)).fetchone()
|
||||
conn.close()
|
||||
assert row[0] is None
|
||||
|
||||
def test_none_artist_id_is_noop(self, service, temp_db):
|
||||
"""Defensive: caller might pass None on edge cases. Don't crash."""
|
||||
service.update_artist_aliases(None, ['x']) # no exception
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# get_artist_aliases — read back by artist NAME (verifier path)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestGetArtistAliases:
|
||||
def test_returns_aliases_for_known_artist(self, service, temp_db):
|
||||
artist_id = _seed_artist(
|
||||
temp_db, 'Hiroyuki Sawano',
|
||||
aliases=json.dumps(['澤野弘之', 'SawanoHiroyuki']),
|
||||
)
|
||||
aliases = service.get_artist_aliases('Hiroyuki Sawano')
|
||||
assert '澤野弘之' in aliases
|
||||
assert 'SawanoHiroyuki' in aliases
|
||||
|
||||
def test_case_insensitive_lookup(self, service, temp_db):
|
||||
"""Verifier passes the artist name from track metadata —
|
||||
casing might differ from how the library stored it."""
|
||||
_seed_artist(temp_db, 'Hiroyuki Sawano', aliases=json.dumps(['澤野弘之']))
|
||||
assert service.get_artist_aliases('hiroyuki sawano') == ['澤野弘之']
|
||||
assert service.get_artist_aliases('HIROYUKI SAWANO') == ['澤野弘之']
|
||||
|
||||
def test_returns_empty_for_unknown_artist(self, service):
|
||||
assert service.get_artist_aliases('NeverHeardOf') == []
|
||||
|
||||
def test_returns_empty_for_artist_without_aliases(self, service, temp_db):
|
||||
_seed_artist(temp_db, 'X') # no aliases column set
|
||||
assert service.get_artist_aliases('X') == []
|
||||
|
||||
def test_handles_corrupt_json_gracefully(self, service, temp_db):
|
||||
_seed_artist(temp_db, 'X', aliases='not-valid-json')
|
||||
# Returns [] instead of raising — defensive against legacy
|
||||
# rows that might have been written by an older format
|
||||
assert service.get_artist_aliases('X') == []
|
||||
|
||||
def test_handles_non_list_json_gracefully(self, service, temp_db):
|
||||
_seed_artist(temp_db, 'X', aliases=json.dumps({'wrong': 'shape'}))
|
||||
assert service.get_artist_aliases('X') == []
|
||||
|
||||
def test_empty_name_returns_empty_without_query(self, service):
|
||||
assert service.get_artist_aliases('') == []
|
||||
assert service.get_artist_aliases(None) == []
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Worker integration — alias enrichment fires on successful match
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestWorkerAliasEnrichment:
|
||||
def test_matched_artist_triggers_alias_fetch_and_persist(self, temp_db, monkeypatch):
|
||||
"""End-to-end: worker matches an artist, immediately fetches
|
||||
+ persists aliases. Subsequent verifier calls find them in
|
||||
the library DB without firing live MB."""
|
||||
from core.musicbrainz_worker import MusicBrainzWorker
|
||||
|
||||
artist_id = _seed_artist(temp_db, 'Hiroyuki Sawano')
|
||||
|
||||
worker = MusicBrainzWorker.__new__(MusicBrainzWorker)
|
||||
worker.database = temp_db
|
||||
worker.mb_service = MagicMock()
|
||||
worker.mb_service.match_artist.return_value = {
|
||||
'mbid': '60d2ea34-1912-425f-bf9c-fc544e4448cd', 'name': 'Hiroyuki Sawano',
|
||||
}
|
||||
worker.mb_service.fetch_artist_aliases.return_value = ['澤野弘之', 'SawanoHiroyuki']
|
||||
worker.stats = {'matched': 0, 'not_found': 0, 'errors': 0}
|
||||
|
||||
# Bypass _get_existing_id (would query DB for prior MBID)
|
||||
worker._get_existing_id = MagicMock(return_value=None)
|
||||
|
||||
worker._process_item({'type': 'artist', 'id': artist_id, 'name': 'Hiroyuki Sawano'})
|
||||
|
||||
worker.mb_service.update_artist_mbid.assert_called_once_with(
|
||||
artist_id, '60d2ea34-1912-425f-bf9c-fc544e4448cd', 'matched',
|
||||
)
|
||||
worker.mb_service.fetch_artist_aliases.assert_called_once_with(
|
||||
'60d2ea34-1912-425f-bf9c-fc544e4448cd',
|
||||
)
|
||||
worker.mb_service.update_artist_aliases.assert_called_once_with(
|
||||
artist_id, ['澤野弘之', 'SawanoHiroyuki'],
|
||||
)
|
||||
|
||||
def test_no_alias_call_when_artist_not_matched(self, temp_db):
|
||||
"""If MB returned no MBID match, don't fetch aliases —
|
||||
nothing to enrich."""
|
||||
from core.musicbrainz_worker import MusicBrainzWorker
|
||||
artist_id = _seed_artist(temp_db, 'Unknown')
|
||||
|
||||
worker = MusicBrainzWorker.__new__(MusicBrainzWorker)
|
||||
worker.database = temp_db
|
||||
worker.mb_service = MagicMock()
|
||||
worker.mb_service.match_artist.return_value = None
|
||||
worker.stats = {'matched': 0, 'not_found': 0, 'errors': 0}
|
||||
worker._get_existing_id = MagicMock(return_value=None)
|
||||
|
||||
worker._process_item({'type': 'artist', 'id': artist_id, 'name': 'Unknown'})
|
||||
|
||||
worker.mb_service.fetch_artist_aliases.assert_not_called()
|
||||
worker.mb_service.update_artist_aliases.assert_not_called()
|
||||
|
||||
def test_existing_mbid_path_backfills_aliases_when_column_empty(self, temp_db):
|
||||
"""Issue #442 perf followup: existing-MBID short-circuit path
|
||||
was skipping alias enrichment entirely. Users with libraries
|
||||
enriched BEFORE this PR shipped have MBIDs but NULL aliases.
|
||||
Worker should fetch aliases on the existing-id path too —
|
||||
one-time backfill on first re-scan post-deploy."""
|
||||
from core.musicbrainz_worker import MusicBrainzWorker
|
||||
artist_id = _seed_artist(temp_db, 'Hiroyuki Sawano')
|
||||
|
||||
worker = MusicBrainzWorker.__new__(MusicBrainzWorker)
|
||||
worker.database = temp_db
|
||||
worker.db = temp_db # _artist_aliases_empty uses self.db
|
||||
worker.mb_service = MagicMock()
|
||||
worker.mb_service.fetch_artist_aliases.return_value = ['澤野弘之', 'SawanoHiroyuki']
|
||||
worker.stats = {'matched': 0, 'not_found': 0, 'errors': 0}
|
||||
# Existing MBID path
|
||||
worker._get_existing_id = MagicMock(return_value='mb-existing-id')
|
||||
|
||||
worker._process_item({'type': 'artist', 'id': artist_id, 'name': 'Hiroyuki Sawano'})
|
||||
|
||||
# MBID was preserved
|
||||
worker.mb_service.update_artist_mbid.assert_called_once_with(
|
||||
artist_id, 'mb-existing-id', 'matched',
|
||||
)
|
||||
# Aliases backfilled
|
||||
worker.mb_service.fetch_artist_aliases.assert_called_once_with('mb-existing-id')
|
||||
worker.mb_service.update_artist_aliases.assert_called_once_with(
|
||||
artist_id, ['澤野弘之', 'SawanoHiroyuki'],
|
||||
)
|
||||
|
||||
def test_existing_mbid_path_skips_backfill_when_aliases_already_set(self, temp_db):
|
||||
"""If aliases are already populated, don't re-fetch — re-scan
|
||||
cycles after backfill complete should be no-ops."""
|
||||
from core.musicbrainz_worker import MusicBrainzWorker
|
||||
artist_id = _seed_artist(
|
||||
temp_db, 'X', aliases=json.dumps(['existing-alias']),
|
||||
)
|
||||
|
||||
worker = MusicBrainzWorker.__new__(MusicBrainzWorker)
|
||||
worker.database = temp_db
|
||||
worker.db = temp_db
|
||||
worker.mb_service = MagicMock()
|
||||
worker.stats = {'matched': 0, 'not_found': 0, 'errors': 0}
|
||||
worker._get_existing_id = MagicMock(return_value='mb-x')
|
||||
|
||||
worker._process_item({'type': 'artist', 'id': artist_id, 'name': 'X'})
|
||||
|
||||
# No alias work — column already populated
|
||||
worker.mb_service.fetch_artist_aliases.assert_not_called()
|
||||
worker.mb_service.update_artist_aliases.assert_not_called()
|
||||
|
||||
def test_existing_mbid_backfill_failure_does_not_break_match(self, temp_db):
|
||||
"""Backfill is best-effort — failure to fetch aliases must
|
||||
NOT prevent the MBID-preservation update from happening."""
|
||||
from core.musicbrainz_worker import MusicBrainzWorker
|
||||
artist_id = _seed_artist(temp_db, 'X')
|
||||
|
||||
worker = MusicBrainzWorker.__new__(MusicBrainzWorker)
|
||||
worker.database = temp_db
|
||||
worker.db = temp_db
|
||||
worker.mb_service = MagicMock()
|
||||
worker.mb_service.fetch_artist_aliases.side_effect = Exception("MB down")
|
||||
worker.stats = {'matched': 0, 'not_found': 0, 'errors': 0}
|
||||
worker._get_existing_id = MagicMock(return_value='mb-x')
|
||||
|
||||
# Should NOT raise
|
||||
worker._process_item({'type': 'artist', 'id': artist_id, 'name': 'X'})
|
||||
|
||||
# MBID still preserved despite alias backfill failure
|
||||
worker.mb_service.update_artist_mbid.assert_called_once_with(
|
||||
artist_id, 'mb-x', 'matched',
|
||||
)
|
||||
|
||||
def test_alias_fetch_failure_does_not_break_match(self, temp_db):
|
||||
"""If alias fetch raises (network error, malformed response,
|
||||
whatever), the artist match still gets recorded — alias
|
||||
enrichment is best-effort, not a gate."""
|
||||
from core.musicbrainz_worker import MusicBrainzWorker
|
||||
artist_id = _seed_artist(temp_db, 'X')
|
||||
|
||||
worker = MusicBrainzWorker.__new__(MusicBrainzWorker)
|
||||
worker.database = temp_db
|
||||
worker.mb_service = MagicMock()
|
||||
worker.mb_service.match_artist.return_value = {'mbid': 'mb-x', 'name': 'X'}
|
||||
worker.mb_service.fetch_artist_aliases.side_effect = Exception("boom")
|
||||
worker.stats = {'matched': 0, 'not_found': 0, 'errors': 0}
|
||||
worker._get_existing_id = MagicMock(return_value=None)
|
||||
|
||||
worker._process_item({'type': 'artist', 'id': artist_id, 'name': 'X'})
|
||||
|
||||
# MBID still got updated despite alias failure
|
||||
worker.mb_service.update_artist_mbid.assert_called_once_with(
|
||||
artist_id, 'mb-x', 'matched',
|
||||
)
|
||||
# No alias write attempted (fetch raised before update)
|
||||
worker.mb_service.update_artist_aliases.assert_not_called()
|
||||
# And the match was still counted
|
||||
assert worker.stats['matched'] == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# lookup_artist_aliases — multi-tier resolution (library → cache → live)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestLookupArtistAliasesMultiTier:
|
||||
def test_tier1_library_db_hit(self, service, temp_db):
|
||||
"""Fast path: artist already enriched in library DB.
|
||||
No MB API call fired."""
|
||||
_seed_artist(temp_db, 'Hiroyuki Sawano',
|
||||
aliases=json.dumps(['澤野弘之', 'SawanoHiroyuki']))
|
||||
|
||||
aliases = service.lookup_artist_aliases('Hiroyuki Sawano')
|
||||
|
||||
assert '澤野弘之' in aliases
|
||||
service.mb_client.search_artist.assert_not_called()
|
||||
service.mb_client.get_artist.assert_not_called()
|
||||
|
||||
def test_tier3_live_mb_lookup_when_not_in_library(self, service, temp_db):
|
||||
"""Cache miss + library miss → MB search → fetch by MBID →
|
||||
cache the result."""
|
||||
service.mb_client.search_artist.return_value = [
|
||||
{'id': 'mb-sawano', 'name': 'Hiroyuki Sawano', 'score': 100},
|
||||
]
|
||||
service.mb_client.get_artist.return_value = {
|
||||
'aliases': [{'name': '澤野弘之'}, {'name': 'SawanoHiroyuki'}],
|
||||
}
|
||||
|
||||
aliases = service.lookup_artist_aliases('Hiroyuki Sawano')
|
||||
|
||||
assert '澤野弘之' in aliases
|
||||
service.mb_client.search_artist.assert_called_once()
|
||||
service.mb_client.get_artist.assert_called_once_with(
|
||||
'mb-sawano', includes=['aliases'],
|
||||
)
|
||||
|
||||
def test_tier2_cache_hit_skips_live_lookup(self, service, temp_db):
|
||||
"""Second call for same artist hits the cache, doesn't
|
||||
re-query MB. Critical for the verifier path — 100 quarantine
|
||||
candidates with the same artist must NOT trigger 100 MB
|
||||
calls."""
|
||||
service.mb_client.search_artist.return_value = [
|
||||
{'id': 'mb-x', 'name': 'X', 'score': 100},
|
||||
]
|
||||
service.mb_client.get_artist.return_value = {
|
||||
'aliases': [{'name': 'X-alias'}],
|
||||
}
|
||||
|
||||
# First call — populates cache
|
||||
first = service.lookup_artist_aliases('X')
|
||||
# Second call — should be cached
|
||||
second = service.lookup_artist_aliases('X')
|
||||
|
||||
assert first == second == ['X-alias']
|
||||
# Only ONE round-trip to MB despite two calls
|
||||
assert service.mb_client.search_artist.call_count == 1
|
||||
assert service.mb_client.get_artist.call_count == 1
|
||||
|
||||
def test_empty_name_returns_empty_no_api_call(self, service):
|
||||
assert service.lookup_artist_aliases('') == []
|
||||
assert service.lookup_artist_aliases(None) == []
|
||||
service.mb_client.search_artist.assert_not_called()
|
||||
|
||||
def test_search_failure_returns_empty(self, service):
|
||||
"""Network outage on search — return empty, cache the empty
|
||||
result so we don't keep retrying."""
|
||||
service.mb_client.search_artist.side_effect = Exception("network down")
|
||||
aliases = service.lookup_artist_aliases('Anyone')
|
||||
assert aliases == []
|
||||
|
||||
def test_no_search_results_returns_empty(self, service):
|
||||
"""Artist not found on MB — empty return, cached so we
|
||||
don't re-search the same name forever."""
|
||||
service.mb_client.search_artist.return_value = []
|
||||
aliases = service.lookup_artist_aliases('NeverHeardOf')
|
||||
assert aliases == []
|
||||
# Second call should hit cache, not re-search
|
||||
service.lookup_artist_aliases('NeverHeardOf')
|
||||
assert service.mb_client.search_artist.call_count == 1
|
||||
|
||||
def test_low_confidence_match_skipped(self, service):
|
||||
"""Search returned something but the name similarity is too
|
||||
low — don't trust it. Could pull in aliases for the wrong
|
||||
artist (e.g. searching 'John Smith' returns a different
|
||||
John Smith). Empty return + cached."""
|
||||
service.mb_client.search_artist.return_value = [
|
||||
{'id': 'mb-different', 'name': 'Completely Different Artist', 'score': 30},
|
||||
]
|
||||
aliases = service.lookup_artist_aliases('Hiroyuki Sawano')
|
||||
assert aliases == []
|
||||
# Didn't even try fetching aliases for the bad match
|
||||
service.mb_client.get_artist.assert_not_called()
|
||||
|
||||
def test_moderate_confidence_match_now_skipped_strict_threshold(self, service):
|
||||
"""Threshold tightened to 0.85 (was 0.6) — moderate matches
|
||||
(sim ~0.7) are no longer trusted. Reduces false-positive
|
||||
risk on ambiguous artist names."""
|
||||
service.mb_client.search_artist.return_value = [
|
||||
# Different name, MB matched on weak signal — combined
|
||||
# score lands around 0.6-0.7, below the new 0.85 floor.
|
||||
{'id': 'mb-x', 'name': 'John Williams', 'score': 50},
|
||||
]
|
||||
aliases = service.lookup_artist_aliases('John Smith')
|
||||
assert aliases == []
|
||||
service.mb_client.get_artist.assert_not_called()
|
||||
|
||||
def test_ambiguous_results_skipped(self, service):
|
||||
"""When MB search returns multiple results with similar high
|
||||
scores (within 0.1 of each other), the artist name is
|
||||
ambiguous — common name with multiple distinct artists
|
||||
('John Smith' returning 10 different John Smiths). Pulling
|
||||
aliases for one could mismatch the wrong artist's data
|
||||
against our file. Skip + cache empty."""
|
||||
service.mb_client.search_artist.return_value = [
|
||||
{'id': 'mb-smith-1', 'name': 'John Smith', 'score': 100},
|
||||
{'id': 'mb-smith-2', 'name': 'John Smith', 'score': 100},
|
||||
{'id': 'mb-smith-3', 'name': 'John Smith', 'score': 100},
|
||||
]
|
||||
aliases = service.lookup_artist_aliases('John Smith')
|
||||
assert aliases == []
|
||||
# Didn't fetch aliases for either ambiguous candidate
|
||||
service.mb_client.get_artist.assert_not_called()
|
||||
|
||||
def test_unambiguous_high_confidence_match_succeeds(self, service):
|
||||
"""Sanity: a clear winner (top result high, no near-tie with
|
||||
runner-up) still triggers alias fetch — the ambiguity gate
|
||||
doesn't break the legit case."""
|
||||
service.mb_client.search_artist.return_value = [
|
||||
{'id': 'mb-sawano', 'name': 'Hiroyuki Sawano', 'score': 100},
|
||||
{'id': 'mb-other', 'name': 'Unrelated Artist', 'score': 30},
|
||||
]
|
||||
service.mb_client.get_artist.return_value = {
|
||||
'aliases': [{'name': '澤野弘之'}],
|
||||
}
|
||||
aliases = service.lookup_artist_aliases('Hiroyuki Sawano')
|
||||
assert '澤野弘之' in aliases
|
||||
|
||||
def test_library_with_empty_aliases_falls_through_to_live(self, service, temp_db):
|
||||
"""Edge case: library has the artist but `aliases` column is
|
||||
NULL (worker hasn't enriched yet). Don't get stuck — fall
|
||||
through to live MB lookup."""
|
||||
_seed_artist(temp_db, 'Hiroyuki Sawano') # no aliases
|
||||
service.mb_client.search_artist.return_value = [
|
||||
{'id': 'mb-sawano', 'name': 'Hiroyuki Sawano', 'score': 100},
|
||||
]
|
||||
service.mb_client.get_artist.return_value = {
|
||||
'aliases': [{'name': '澤野弘之'}],
|
||||
}
|
||||
|
||||
aliases = service.lookup_artist_aliases('Hiroyuki Sawano')
|
||||
|
||||
assert '澤野弘之' in aliases
|
||||
service.mb_client.search_artist.assert_called_once()
|
||||
264
tests/matching/test_artist_aliases.py
Normal file
264
tests/matching/test_artist_aliases.py
Normal file
|
|
@ -0,0 +1,264 @@
|
|||
"""Pin the alias-aware artist comparison helper.
|
||||
|
||||
Issue #442 — files tagged with one spelling of an artist's name
|
||||
(Japanese kanji `澤野弘之`) were quarantined when SoulSync expected
|
||||
the romanized spelling (`Hiroyuki Sawano`). MusicBrainz aliases
|
||||
should bridge the two — this helper does the bridging.
|
||||
|
||||
These tests cover the helper in total isolation: no DB, no network,
|
||||
no MusicBrainz client. Pure-function contract pinned at the right
|
||||
boundary so every consumer (verifier, matching engine, future
|
||||
callers) inherits the same correctness guarantees.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from core.matching.artist_aliases import (
|
||||
DEFAULT_ARTIST_MATCH_THRESHOLD,
|
||||
artist_names_match,
|
||||
best_alias_match,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Direct compare path — no aliases
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestDirectCompareNoAliases:
|
||||
def test_exact_match(self):
|
||||
matched, score = artist_names_match('Foreigner', 'Foreigner')
|
||||
assert matched is True
|
||||
assert score == 1.0
|
||||
|
||||
def test_case_insensitive(self):
|
||||
matched, score = artist_names_match('foreigner', 'FOREIGNER')
|
||||
assert matched is True
|
||||
assert score == 1.0
|
||||
|
||||
def test_whitespace_tolerant(self):
|
||||
matched, score = artist_names_match(' Foreigner ', 'Foreigner')
|
||||
assert matched is True
|
||||
|
||||
def test_completely_different_artists(self):
|
||||
matched, score = artist_names_match('Foreigner', 'Khalil Turk')
|
||||
assert matched is False
|
||||
assert score < DEFAULT_ARTIST_MATCH_THRESHOLD
|
||||
|
||||
def test_fuzzy_match_above_threshold(self):
|
||||
# 'Beatles' vs 'The Beatles' — sim ~0.78
|
||||
matched, score = artist_names_match('The Beatles', 'Beatles')
|
||||
assert matched is True
|
||||
assert score >= DEFAULT_ARTIST_MATCH_THRESHOLD
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Cross-script — the headline of issue #442
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestCrossScriptWithAliases:
|
||||
def test_japanese_kanji_to_romanized(self):
|
||||
"""Reporter's case 1: file tagged 澤野弘之, expected
|
||||
Hiroyuki Sawano. MusicBrainz alias `澤野弘之` on the artist
|
||||
record bridges the two."""
|
||||
matched, score = artist_names_match(
|
||||
'Hiroyuki Sawano',
|
||||
'澤野弘之',
|
||||
aliases=['澤野弘之', 'SawanoHiroyuki', 'Sawano Hiroyuki'],
|
||||
)
|
||||
assert matched is True, (
|
||||
f"Expected alias match for Japanese spelling; got matched=False score={score}"
|
||||
)
|
||||
|
||||
def test_romanized_to_japanese_kanji(self):
|
||||
"""Symmetric direction — file tagged Hiroyuki Sawano, expected
|
||||
澤野弘之. Aliases should resolve either way."""
|
||||
matched, score = artist_names_match(
|
||||
'澤野弘之',
|
||||
'Hiroyuki Sawano',
|
||||
aliases=['Hiroyuki Sawano', 'SawanoHiroyuki'],
|
||||
)
|
||||
assert matched is True
|
||||
|
||||
def test_cyrillic_to_latin(self):
|
||||
"""Reporter's case 2: file tagged Sergey Lazarev, expected
|
||||
Сергей Лазарев."""
|
||||
matched, score = artist_names_match(
|
||||
'Сергей Лазарев',
|
||||
'Sergey Lazarev',
|
||||
aliases=['Sergey Lazarev', 'Sergei Lazarev'],
|
||||
)
|
||||
assert matched is True
|
||||
|
||||
def test_no_alias_match_falls_through_to_fail(self):
|
||||
"""Aliases provided but none match the actual artist —
|
||||
should still fail. Aliases bridge synonyms, they don't mask
|
||||
genuine mismatches."""
|
||||
matched, score = artist_names_match(
|
||||
'Hiroyuki Sawano',
|
||||
'Khalil Turk',
|
||||
aliases=['澤野弘之', 'SawanoHiroyuki'],
|
||||
)
|
||||
assert matched is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Aliases input handling — defensive coercion
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAliasesInputCoercion:
|
||||
def test_none_aliases_treated_as_empty(self):
|
||||
matched, _ = artist_names_match('A', 'A', aliases=None)
|
||||
assert matched is True
|
||||
|
||||
def test_empty_list_aliases(self):
|
||||
matched, _ = artist_names_match('A', 'A', aliases=[])
|
||||
assert matched is True
|
||||
|
||||
def test_aliases_can_be_set(self):
|
||||
matched, _ = artist_names_match(
|
||||
'Hiroyuki Sawano', '澤野弘之', aliases={'澤野弘之', 'SawanoHiroyuki'},
|
||||
)
|
||||
assert matched is True
|
||||
|
||||
def test_aliases_can_be_tuple(self):
|
||||
matched, _ = artist_names_match(
|
||||
'Hiroyuki Sawano', '澤野弘之', aliases=('澤野弘之',),
|
||||
)
|
||||
assert matched is True
|
||||
|
||||
def test_none_entries_in_aliases_skipped(self):
|
||||
"""Defensive: caller might pass aliases pulled directly from
|
||||
a partial MB response. None / empty entries shouldn't crash."""
|
||||
matched, _ = artist_names_match(
|
||||
'Hiroyuki Sawano', '澤野弘之',
|
||||
aliases=[None, '', '澤野弘之', None],
|
||||
)
|
||||
assert matched is True
|
||||
|
||||
def test_non_string_entries_coerced(self):
|
||||
"""Defensive: aliases parsed from JSON might surface as ints
|
||||
or other non-string types. str() coercion in helper handles it."""
|
||||
matched, _ = artist_names_match(
|
||||
'A', '123', aliases=[123],
|
||||
)
|
||||
assert matched is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Threshold behaviour
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestThreshold:
|
||||
def test_default_threshold_matches_verifier(self):
|
||||
"""Default threshold must equal the verifier's existing
|
||||
ARTIST_MATCH_THRESHOLD so wiring the helper into the
|
||||
verifier preserves current pass/fail semantics on the
|
||||
no-alias path."""
|
||||
assert DEFAULT_ARTIST_MATCH_THRESHOLD == 0.6
|
||||
|
||||
def test_custom_threshold_stricter(self):
|
||||
# Direct comparison would normally pass at 0.6 default,
|
||||
# but a stricter threshold should reject it.
|
||||
matched, score = artist_names_match(
|
||||
'The Beatles', 'Beatles', threshold=0.95,
|
||||
)
|
||||
assert matched is False
|
||||
|
||||
def test_custom_threshold_looser(self):
|
||||
matched, score = artist_names_match(
|
||||
'AAAAA', 'AAABB', threshold=0.4,
|
||||
)
|
||||
# ~0.6 sim, passes loose threshold
|
||||
assert matched is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Custom similarity callable
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestCustomSimilarity:
|
||||
def test_custom_sim_used_for_direct_compare(self):
|
||||
"""Caller (verifier) passes its own normaliser-aware
|
||||
similarity. Helper must route through it instead of using
|
||||
the default."""
|
||||
def stricter(a, b):
|
||||
# Always returns 0 — proves we're using the custom callable
|
||||
return 0.0
|
||||
|
||||
matched, score = artist_names_match(
|
||||
'Foreigner', 'Foreigner', similarity=stricter,
|
||||
)
|
||||
assert matched is False
|
||||
assert score == 0.0
|
||||
|
||||
def test_custom_sim_used_for_alias_compare(self):
|
||||
"""Custom similarity also applies to alias scoring — not just
|
||||
the direct comparison."""
|
||||
def alias_only_perfect(a, b):
|
||||
# Returns 1.0 only when comparing the alias 'aliasX'
|
||||
return 1.0 if 'aliasX' in (a, b) else 0.0
|
||||
|
||||
matched, score = artist_names_match(
|
||||
'Foreigner', 'observed',
|
||||
aliases=['aliasX'],
|
||||
similarity=alias_only_perfect,
|
||||
)
|
||||
assert matched is True
|
||||
assert score == 1.0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Best-alias-match introspection helper
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestBestAliasMatch:
|
||||
def test_direct_wins_no_alias_winner(self):
|
||||
winner, score = best_alias_match(
|
||||
'Foreigner', 'Foreigner', aliases=['otherthing'],
|
||||
)
|
||||
assert winner is None
|
||||
assert score == 1.0
|
||||
|
||||
def test_alias_wins_returns_alias(self):
|
||||
winner, score = best_alias_match(
|
||||
'Hiroyuki Sawano', '澤野弘之',
|
||||
aliases=['澤野弘之', 'SawanoHiroyuki'],
|
||||
)
|
||||
assert winner == '澤野弘之'
|
||||
assert score == 1.0
|
||||
|
||||
def test_no_aliases_just_direct_score(self):
|
||||
winner, score = best_alias_match('A', 'B', aliases=None)
|
||||
assert winner is None
|
||||
assert isinstance(score, float)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Backward compat — pre-fix behaviour preserved when no aliases
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestBackwardCompatNoAliases:
|
||||
"""When callers don't supply aliases (initial wiring, or live MB
|
||||
unreachable), the helper must behave EXACTLY like a direct
|
||||
similarity check — no surprises for paths that haven't been
|
||||
wired up to alias lookup yet."""
|
||||
|
||||
@pytest.mark.parametrize('expected,actual,should_match', [
|
||||
('Foreigner', 'Foreigner', True), # exact
|
||||
('foreigner', 'FOREIGNER', True), # case
|
||||
('The Beatles', 'Beatles', True), # fuzzy passes
|
||||
('Foreigner', 'Khalil Turk', False), # different
|
||||
('Hiroyuki Sawano', '澤野弘之', False), # cross-script no aliases → fail (pre-fix behaviour)
|
||||
])
|
||||
def test_no_alias_path_matches_direct_similarity(self, expected, actual, should_match):
|
||||
matched, _ = artist_names_match(expected, actual)
|
||||
assert matched is should_match
|
||||
|
|
@ -3416,6 +3416,7 @@ const WHATS_NEW = {
|
|||
'2.4.3': [
|
||||
// --- post-release patch work on the 2.4.3 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
|
||||
{ date: 'Unreleased — 2.4.3 patch work' },
|
||||
{ title: 'Cross-Script Artist Names No Longer Quarantine Files (Hiroyuki Sawano / 澤野弘之, Сергей Лазарев / Sergey Lazarev)', desc: 'github issue #442 (afonsog6): files where the artist tag was in one script and the expected metadata was in another — japanese kanji `澤野弘之` for `hiroyuki sawano`, cyrillic `сергей лазарев` for `sergey lazarev`, etc. — got quarantined post-download because acoustid verification scored the artist similarity at 0% (the two scripts share no characters). reporter could not even rescue the file via manual import — the import-modal goes through the same verifier and re-quarantined the same file. cause: verifier compared expected vs actual artist with raw `_similarity` and never consulted musicbrainz aliases, even though MB exposes them on every artist record. fix: new `core/matching/artist_aliases.py` pure helper with alias-aware comparison + new `artists.aliases` JSON column populated by the existing MB enrichment worker on every artist match (one extra `inc=aliases` request per artist) + new multi-tier resolver `MusicBrainzService.lookup_artist_aliases` (library DB → cache → live MB) so the verifier finds aliases even for un-enriched artists without thrashing the MB API. verifier resolves aliases ONCE per `verify_audio_file` call and feeds them through three artist comparison sites (best-match scoring, secondary scan when title matches but artist doesn\'t, final fallback scan). reporter\'s exact two cases reproduced as regression tests with stubbed MB service. backward compat: aliases unavailable / MB unreachable → verifier falls back to direct similarity (identical to pre-fix behaviour — never quarantines stricter than today). 70 new tests pin every layer: pure helper (28), service methods (31), verifier integration (11). audited adjacent artist-comparison sites (auto-import single-track id, discovery scoring, matching engine) — left untouched per scope discipline since they aren\'t the user-reported pain.', page: 'downloads' },
|
||||
{ title: 'Plex: Library Scan Trigger No Longer Fails On Non-English Section Names', desc: 'github issue #535 (adrigzr): plex servers with the music library named anything other than "music" — Música, Musique, Musik, Musica, etc. — got a `Failed to trigger library scan for "Music": Invalid library section: Music` error after every import cycle, and `wishlist.processing` kept reporting "missing from media server after sync" for tracks that DID import correctly because the post-import scan never fired. cause: `trigger_library_scan` and `is_library_scanning` ignored the auto-detected `self.music_library` (correctly populated by `_find_music_library` filtering by `section.type == "artist"`) and called `self.server.library.section(library_name)` with a hardcoded "music" default — raised NotFound on any non-english server. read methods like `get_artists` already routed through `_get_music_sections` so they didn\'t have the bug; this aligns the scan-trigger path with the same resolution. fix: both single-library branches prefer `self.music_library` first, fall back to literal section lookup only when auto-detection hasn\'t run. activity-feed match in `is_library_scanning` also corrected to use the resolved section\'s actual title instead of the unused `library_name` arg — the prior log line read "triggered scan for music" even on Spanish servers. 13 new tests pin: trigger uses auto-detected section across 6 locale variants (Música / Musique / Musik / Musica / 音乐 / موسيقى), backward-compat fallback when music_library is None, explicit library_name kwarg ignored when auto-detected section exists, log line surfaces correct section title, scan-status check uses auto-detected section\'s `refreshing` attr, activity-feed match filters by resolved title (not library_name).', page: 'settings' },
|
||||
{ title: 'Search For Match: No More Karaoke / Cover / "Originally Performed By" Junk At The Top', desc: 'github issue #534 (radoslav-orlov): typing "dirty white boy" + "foreigner" into the import-modal "search for match" dialog returned karaoke versions, "originally performed by" compilations, and tribute-band cuts ranked above the actual foreigner studio recording in some regions. user had to scroll past 5+ junk results before finding the canonical track. fix: new `core/metadata/relevance.py` helper reranks results locally with cover/karaoke/tribute/re-recorded penalties (multiplier 0.05× — effectively buries) + exact-artist-match boost (1.5×) + variant-tag (live/acoustic/remix/remaster) penalty (0.4×, skipped when user explicitly typed the variant — searching "track (live)" still ranks live versions correctly). applied at the deezer + itunes + spotify search-tracks endpoints so all three sources behave consistently. validated against live deezer api with the actual #534 query: real foreigner head games cut now lands at #1, live versions follow, karaoke / cover / tribute variants drop to positions 11-15. deezer client also gained optional field-scoped query kwargs (`track="X" artist="Y"`) that build deezer\'s advanced search syntax `track:"X" artist:"Y"` for future opt-in callers (e.g. exact-match flows where api-level filtering is more important than ranking) — kept in client but NOT used at the import-modal endpoint after live testing showed the advanced syntax has its own ranking bias (surfaced "(2008 remaster)" instead of the canonical recording). free-text + local rerank is the more reliable combination here. 75 new tests pin every scoring component, pattern detection (13 cover patterns, 11 variant patterns, 3 fields), score composition (real-cut > karaoke > remaster > re-recorded), the issue #534 screenshot reproduced as a regression test, deezer client query construction + free-text fallback safety net.', page: 'import' },
|
||||
{ title: 'Auto-Import: Album Duration Is Album Total + Re-Imports Fill Metadata Gaps', desc: 'two more parity gaps closed in the soulsync standalone library write path. (1) album row\'s `duration` column was being written with the FIRST imported track\'s duration instead of the album total — pre-existing bug that survived the prior parity commit. soulsync_client deep scan computes `sum(t.duration for t in self._tracks)` for each album; auto-import now mirrors that by computing the sum across every matched track in the worker and threading it through context to the album INSERT. (2) `record_soulsync_library_entry` was insert-only on artists + albums — once a row existed (matched by id OR name fallback), subsequent imports of the same artist or album skipped completely. meant: artist genres / thumb / source-id reflected ONLY whatever the FIRST imported album supplied, never refreshing as more albums by that artist landed (ten more deezer/spotify imports later, artist row still had whatever the first random import wrote). new conservative UPDATE path: when an existing row matches, fill ONLY the columns whose current value is NULL or empty — never overwrites populated values. protects manual edits + enrichment-worker writes the same way scanner UPDATEs preserve enrichment columns. f-string column names are validated against an allowlist (`_SOULSYNC_FILLABLE_COLUMNS`) before interpolation — defensive against accidental misuse adding columns without an allowlist update. 4 new tests pin: album duration uses sum not single-track, re-import fills empty thumb + genres on existing artist row, re-import does NOT clobber populated values, re-import fills empty source-id columns when later import has them.', page: 'import' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue