Add pure artist-name comparison helper with alias awareness
Issue #442 — files tagged with one spelling of an artist's name (Japanese kanji `澤野弘之`) get quarantined when SoulSync expects the romanized spelling (`Hiroyuki Sawano`). Raw similarity comparison scored 0% across scripts. MusicBrainz exposes alternate-spelling aliases on every artist record but the verifier never consulted them. This commit adds the pure helper that does the alias-aware comparison. No I/O, no DB access, no network. Caller supplies the aliases (looked up from library DB or live MB by later commits in this PR). Default threshold matches the verifier's existing `ARTIST_MATCH_THRESHOLD` (0.6) so wiring this in preserves current pass/fail semantics on the no-alias path. # API ``` artist_names_match(expected, actual, *, aliases=None, threshold=0.6, similarity=None) -> (matched: bool, best_score: float) ``` - Direct compare first (fast path + baseline score) - If below threshold, score each alias against `actual` - First alias to clear threshold → match - Returns the best score across all candidates so callers can log the score they made the decision on ``` best_alias_match(expected, actual, aliases=None, *, similarity=None) -> (winner: Optional[str], best_score: float) ``` Companion helper for callers that want to surface WHICH alias triggered the match (debug logs, UI explanations). No threshold — purely informative. # Architectural choices - **Pure function**: no I/O. Caller (verifier, future matching-engine consumers) owns alias lookup strategy + threshold tuning. - **Custom similarity callable**: lets the verifier pass its parenthetical-stripping normaliser without this module having to know about it. Defaults to lowercase + SequenceMatcher (matches the verifier's existing behaviour). - **Defensive coercion**: aliases input handles None entries, empty strings, non-string types, sets, tuples, lists — caller may feed raw MB response data without cleaning first. - **Backward compat**: `aliases=None` or empty → behaves identically to a plain similarity check. Paths not yet wired up to alias lookup see no behaviour change. # Tests (28) - Direct compare (no aliases): exact / case / whitespace / fuzzy / different - Cross-script with aliases: Japanese ↔ romanized (reporter's case 1), Cyrillic ↔ Latin (reporter's case 2), symmetric direction, no-match fallthrough so aliases don't mask genuine mismatches - Aliases input handling: None, empty, set, tuple, None-entries, non-string entries - Threshold: default matches verifier's 0.6, custom stricter, custom looser - Custom similarity: applies to both direct + alias compare - Best-alias-match introspection - Backward compat parametrised across 5 cases # What this commit does NOT do This is the helper module + tests only. Subsequent commits in this PR populate aliases (MB worker), provide live MB lookup with cache for un-enriched artists, and wire the helper into the AcoustID verifier where the quarantine decision actually fires.
This commit is contained in:
parent
43f168a048
commit
235ada7e0f
4 changed files with 439 additions and 0 deletions
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
|
||||
0
tests/matching/__init__.py
Normal file
0
tests/matching/__init__.py
Normal file
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
|
||||
Loading…
Reference in a new issue