diff --git a/core/matching/__init__.py b/core/matching/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/core/matching/artist_aliases.py b/core/matching/artist_aliases.py new file mode 100644 index 00000000..1b1e5565 --- /dev/null +++ b/core/matching/artist_aliases.py @@ -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 diff --git a/tests/matching/__init__.py b/tests/matching/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/matching/test_artist_aliases.py b/tests/matching/test_artist_aliases.py new file mode 100644 index 00000000..382b5f81 --- /dev/null +++ b/tests/matching/test_artist_aliases.py @@ -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