From 142a1aaf38f845bc0026a6532caecffac67b95de Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 7 Jun 2026 10:21:23 -0700 Subject: [PATCH] =?UTF-8?q?Cover=20art:=20a=20numeric=20difference=20is=20?= =?UTF-8?q?a=20different=20release=20=E2=80=94=20Vol.4=20stops=20wearing?= =?UTF-8?q?=20Vol.4.5's=20cover?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sokhi (continued from #806): volume-numbered series ('B小町 …キャラクター ソングCD Vol.2' / 'Vol.2.5' / 'Vol.4' / 'Vol.4.5') got each other's art from both normal downloads and the retag tool. Two distinct holes, one principle: 1. The art picker's _album_matches validates by significant-token SUBSET — built to tolerate '(Deluxe)'/'- Remastered' suffixes. CJK strips out of the normalizer entirely, so Vol.4 → {b,tv,cd,vol,4}, a clean subset of Vol.4.5's {b,tv,cd,vol,4,5}: the wrong volume validated as "the same album with a suffix". Affected every fuzzy art source (iTunes, Deezer, AudioDB, Spotify) in downloads, retag, and the missing-art repair. 2. MusicBrainz match_release scores by string similarity — Vol.4 vs Vol.4.5 is 0.973, so the wrong volume could win the match outright, and its MBID then feeds Cover Art Archive with NO downstream validation (CAA is MBID-keyed, trusted by design). With Sokhi's MB metadata source this is the likely path in his logs (his release-group 404s push re-matching). The shared rule (core.text.title_match.numeric_tokens_differ): digit-bearing tokens must be IDENTICAL between the two titles. A number on one side only — volume, part, sequel, remaster year — is a different release, never a suffix. '1989' vs '1989 (Deluxe)' still matches (digits shared); 'Album' vs 'Album 2' now rejects (sequels!). Art picker rejects outright (falls through to next source / the download's own art — the designed cost of a false reject); MB matcher halves the candidate's confidence, landing it below the 70 gate while the exact-volume result is untouched. Tests: helper truth table, the exact reported pairs through _album_matches, and match_release end-to-end (wrong volume alone → no match beats a wrong MBID; exact volume beats near-identical wrong one despite lower MB score). 828 matching/metadata + 301 musicbrainz/retag/artwork tests pass. --- core/metadata/art_lookup.py | 12 ++++ core/musicbrainz_service.py | 10 +++ core/text/title_match.py | 21 ++++++- tests/matching/test_numeric_release_guard.py | 65 ++++++++++++++++++++ tests/metadata/test_art_lookup.py | 21 +++++++ 5 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 tests/matching/test_numeric_release_guard.py diff --git a/core/metadata/art_lookup.py b/core/metadata/art_lookup.py index 458dc946..983a252d 100644 --- a/core/metadata/art_lookup.py +++ b/core/metadata/art_lookup.py @@ -132,6 +132,18 @@ def _album_matches(req_artist, req_album, got_artist, got_album) -> bool: return False if not (ra <= ga or ga <= ra): return False + # Sokhi: the subset tolerance exists for '(Deluxe)'/'- Remastered' + # suffixes, but a NUMERIC difference is a different release, not a + # suffix. 'B小町 …CD Vol.4' normalizes to {b,tv,cd,vol,4} — a subset of + # Vol.4.5's {b,tv,cd,vol,4,5} — so volume 4 was hanging volume 4.5's + # cover. Any number present on only ONE side (volume, part, sequel, + # remaster year) rejects the match; the resolver then falls through to + # the next source / the download's own art, which is the designed cost + # of a false reject here. (Shared rule — the MusicBrainz release matcher + # applies the same guard so the MBID-keyed CAA path can't slip either.) + from core.text.title_match import numeric_tokens_differ + if numeric_tokens_differ(req_album, got_album): + return False ta, tg = _significant_tokens(req_artist), _significant_tokens(got_artist) if not ta: return True # requested artist unknown -> album match suffices diff --git a/core/musicbrainz_service.py b/core/musicbrainz_service.py index b7d47505..4508618d 100644 --- a/core/musicbrainz_service.py +++ b/core/musicbrainz_service.py @@ -270,6 +270,16 @@ class MusicBrainzService: # Combine scores - cap at 100 confidence = min(100, int((title_similarity * 50) + (mb_score / 100 * 30) + artist_bonus + version_bonus)) + # Numeric difference = different release. 'Vol.4' vs 'Vol.4.5' + # scores 0.97 string similarity, so a near-identical wrong + # volume could win and its MBID then feeds CAA art with NO + # downstream validation (CAA is MBID-keyed — Sokhi's wrong + # covers). Halving lands any such candidate below the 70 gate + # while leaving the exact-volume result untouched. + from core.text.title_match import numeric_tokens_differ + if numeric_tokens_differ(album_name, mb_title): + confidence = int(confidence * 0.5) + if confidence > best_confidence: best_confidence = confidence best_match = result diff --git a/core/text/title_match.py b/core/text/title_match.py index cb56334b..95f232f2 100644 --- a/core/text/title_match.py +++ b/core/text/title_match.py @@ -115,4 +115,23 @@ def strip_redundant_context_qualifiers(title: str, *context_texts: str) -> str: return re.sub(r"\s+", " ", out).strip() -__all__ = ["titles_plausibly_same", "strip_redundant_context_qualifiers"] +def numeric_tokens_differ(title_a: str, title_b: str) -> bool: + """True when the digit-bearing tokens of two titles differ — 'Vol.4' vs + 'Vol.4.5', 'Album' vs 'Album 2'. A numeric difference is a different + release (volume / part / sequel), never a '(Deluxe)'-style suffix: + string similarity ('Vol.4' vs 'Vol.4.5' = 0.97) and token-subset checks + both wave these through, which hung volume 4.5's cover art on volume 4 + (Sokhi). Shared digits on both sides ('1989' vs '1989 (Deluxe)') are + fine.""" + def _digit_tokens(text: str) -> frozenset: + tokens = re.sub(r"[^a-z0-9]+", " ", (text or "").casefold()).split() + return frozenset(t for t in tokens if any(c.isdigit() for c in t)) + + return _digit_tokens(title_a) != _digit_tokens(title_b) + + +__all__ = [ + "titles_plausibly_same", + "strip_redundant_context_qualifiers", + "numeric_tokens_differ", +] diff --git a/tests/matching/test_numeric_release_guard.py b/tests/matching/test_numeric_release_guard.py new file mode 100644 index 00000000..b2ba79c8 --- /dev/null +++ b/tests/matching/test_numeric_release_guard.py @@ -0,0 +1,65 @@ +"""Sokhi's wrong-cover-art report: 'Vol.4' must never match 'Vol.4.5'. + +Two paths served wrong art for volume-numbered series: +- the art picker's _album_matches subset check (Vol.4's tokens are a subset + of Vol.4.5's once CJK/punctuation normalizes away) — covered in + tests/metadata/test_art_lookup.py +- MusicBrainz match_release: 0.97 string similarity let the wrong volume + win, and its MBID feeds CAA art with no downstream validation — covered + here, plus the shared helper itself. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from core.text.title_match import numeric_tokens_differ +from core.musicbrainz_service import MusicBrainzService + +VOL4 = "B小町 - TVアニメ「【推しの子】」キャラクターソングCD Vol.4" +VOL45 = "B小町 - TVアニメ「【推しの子】」キャラクターソングCD Vol.4.5" + + +def test_helper_volume_and_sequel_differ(): + assert numeric_tokens_differ(VOL4, VOL45) + assert numeric_tokens_differ("Album", "Album 2") + assert numeric_tokens_differ("Now 99", "Now 100") + + +def test_helper_shared_or_no_digits_match(): + assert not numeric_tokens_differ("1989", "1989 (Deluxe)") + assert not numeric_tokens_differ(VOL4, VOL4) + assert not numeric_tokens_differ("IGOR", "IGOR (Deluxe)") + assert not numeric_tokens_differ("", "") + + +def _service_with_results(results): + svc = MusicBrainzService.__new__(MusicBrainzService) + svc.mb_client = MagicMock() + svc.mb_client.search_release.return_value = results + svc._check_cache = lambda *a, **k: None + svc._save_to_cache = lambda *a, **k: None + return svc + + +def _result(title, score=100, mbid="mb-x"): + return { + 'id': mbid, 'title': title, 'score': score, + 'artist-credit': [{'artist': {'name': 'B小町'}}], + } + + +def test_match_release_rejects_wrong_volume(): + """Only the wrong volume is returned by search → no match at all is + better than caching Vol.4.5's MBID (CAA would serve its art unvalidated).""" + svc = _service_with_results([_result(VOL45, score=100, mbid='mb-wrong')]) + assert svc.match_release(VOL4, 'B小町') is None + + +def test_match_release_prefers_exact_volume_over_near_identical(): + svc = _service_with_results([ + _result(VOL45, score=100, mbid='mb-wrong'), + _result(VOL4, score=90, mbid='mb-right'), + ]) + got = svc.match_release(VOL4, 'B小町') + assert got and got['mbid'] == 'mb-right' diff --git a/tests/metadata/test_art_lookup.py b/tests/metadata/test_art_lookup.py index 975fe1e9..92a77a82 100644 --- a/tests/metadata/test_art_lookup.py +++ b/tests/metadata/test_art_lookup.py @@ -181,6 +181,27 @@ def test_album_matches_unknown_requested_artist_allows_album_match(): assert art_lookup._album_matches("", "1989", "Taylor Swift", "1989") +def test_album_matches_rejects_numeric_difference(): + """Sokhi: same series, different volume number. CJK strips to latin + tokens, so Vol.4 was a token-subset of Vol.4.5 and inherited its art. + A number on only one side = a different release, never a suffix.""" + A = "B小町" + assert not art_lookup._album_matches( + A, "B小町 - TVアニメ「【推しの子】」キャラクターソングCD Vol.4", + A, "B小町 - TVアニメ「【推しの子】」キャラクターソングCD Vol.4.5") + assert not art_lookup._album_matches( + A, "B小町 - TVアニメ「【推しの子】」キャラクターソングCD Vol.2", + A, "B小町 - TVアニメ「【推しの子】」キャラクターソングCD Vol.2.5") + # Sequels are different albums too. + assert not art_lookup._album_matches("Artist", "Album", "Artist", "Album 2") + # Identical volume numbers still match. + assert art_lookup._album_matches( + A, "B小町 - TVアニメ「【推しの子】」キャラクターソングCD Vol.4", + A, "B小町 - TVアニメ「【推しの子】」キャラクターソングCD Vol.4") + # Numeric token shared by BOTH sides keeps non-numeric suffix tolerance. + assert art_lookup._album_matches("Taylor Swift", "1989", "Taylor Swift", "1989 (Deluxe)") + + # --------------------------------------------------------------------------- # build_art_lookup — caching + guarding # ---------------------------------------------------------------------------