From 0939585620d14cefe07f796b3ffb8d945988ce51 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 15:38:49 -0700 Subject: [PATCH] Matcher: bracketed subtitles no longer read as different songs (#825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit carlosjfcasero round 2 (manual-add fix didn't help — different path). His log pinned it: the mirrored sync auto-added 'Llamando a la tierra (Serenade From the Stars)' by M-Clan every run even though his library has the song (stored bare). Reproduced exactly: the subtitle restates no album context, so the #808 context strip keeps it, and the length-ratio penalty in _calculate_track_confidence crushes the pair to 0.142 (needs 0.7). Sync → "missing" → wishlist, forever; and the cleanup uses the SAME matcher, so it deterministically never removed it. Self-reinforcing. Fix at the matcher seam (benefits sync, cleanup, downloads, discography alike): core/text/title_match.strip_subtitle_qualifiers(title, other) strips a bracketed qualifier only when it (a) isn't restated in the other title, (b) contains no version-marker token (EN + ES: live/remix/acoustic/version/ dueto/directo/vivo/...), and (c) introduces no new digit token ('(Pt. 2)', '(2007)' stay different releases). Wired as a third comparison variant in _calculate_track_confidence with its own length guard. Verified against his log's other unmatched tracks: '(Live)' 0.15, '(Dueto 2007)' 0.179, '(Versión 1988)' 0.167 all still correctly blocked — version qualifiers keep their meaning; the M-Clan case goes 0.142 → 1.0 in both directions. Also: sync's check_track_exists call now passes album= (cleanup already did), enabling the album-aware fallback for multi-artist albums during sync. Tests: tests/test_subtitle_qualifier_match.py — the reported case verbatim (end-to-end through check_track_exists, both directions, batched candidate path included), EN+ES version qualifiers still blocked, numeric guard, '#769 Dani California' and '#808 OurVinyl' guards still hold. 1396 matcher/wishlist/sync tests pass. --- core/text/title_match.py | 72 +++++++++++ database/music_database.py | 18 +++ services/sync_service.py | 4 + tests/test_subtitle_qualifier_match.py | 158 +++++++++++++++++++++++++ 4 files changed, 252 insertions(+) create mode 100644 tests/test_subtitle_qualifier_match.py diff --git a/core/text/title_match.py b/core/text/title_match.py index 95f232f2..85e1c10b 100644 --- a/core/text/title_match.py +++ b/core/text/title_match.py @@ -115,6 +115,77 @@ def strip_redundant_context_qualifiers(title: str, *context_texts: str) -> str: return re.sub(r"\s+", " ", out).strip() +# Qualifier tokens that mark a genuinely DIFFERENT recording/cut — these must +# keep blocking a match. Union of the matching-engine keyword lists plus the +# Spanish markers seen in real libraries (#825: 'En Directo…', 'Versión 1988', +# 'Dueto 2007'). Titles reaching the matcher are unidecode-normalized, so the +# ASCII forms ('version') cover the accented ones ('versión'). +_VERSION_MARKER_TOKENS = frozenset({ + # English + "remix", "mix", "rmx", "live", "acoustic", "unplugged", "instrumental", + "karaoke", "demo", "demos", "edit", "version", "versions", "remaster", + "remastered", "slowed", "reverb", "sped", "spedup", "speedup", "extended", + "club", "mashup", "bootleg", "cover", "covers", "reprise", "session", + "sessions", "mono", "stereo", "duet", "rework", "dub", "vip", "single", + "radio", "alt", "alternate", "alternative", "take", "edition", "orchestral", + "symphonic", "piano", "acapella", "cappella", "nightcore", + # Distinct-track qualifiers — '(Interlude)' etc. are SEPARATE short tracks + # that share the base name with the full song; never treat as subtitles. + "interlude", "intro", "outro", "skit", "freestyle", "medley", "snippet", + # Part/volume markers whose number can be non-numeric ('Pt. II') — the + # digit guard below only catches actual digits. + "pt", "part", "vol", "ii", "iii", "iv", "vi", "vii", "viii", + # Spanish (unidecode-normalized; 'versión' → 'version' is covered above) + "directo", "vivo", "dueto", +}) + + +def strip_subtitle_qualifiers(title: str, other_title: str) -> str: + """Remove bracketed qualifiers that are SUBTITLES, not version markers. + + #825 (carlosjfcasero): the wishlist held 'Llamando a la tierra (Serenade + From the Stars)' — the song's official subtitle — while the library track + was the bare 'Llamando a la tierra'. The qualifier appears in no album or + counterpart title, so :func:`strip_redundant_context_qualifiers` keeps it, + and the length-ratio penalty then crushes an obviously-same song to ~0.14. + The sync matcher reported it missing on every run (re-adding it to the + wishlist) and the cleanup — same matcher — could never remove it. + + A qualifier is stripped only when ALL of: + * its text does not appear in ``other_title`` (if it does, the direct + comparison already handles it); + * it contains no version-marker token ('(Live)', '(Versión 1988)', + '(Dueto 2007)' keep blocking — they are different recordings); + * it introduces no digit token absent from ``other_title`` ('(Pt. 2)', + '(2007)' are different releases, never subtitles). + + Inputs should be normalized the same way the caller compares them + (lowercased / unidecode'd), like strip_redundant_context_qualifiers. + """ + if not title: + return title + + other = (other_title or "").casefold() + other_tokens = set(_TOKEN_RE.findall(other)) + + def _drop(match: re.Match) -> str: + inner = match.group(1).strip().casefold() + if not inner: + return " " + # Restated in the counterpart title — leave for the direct comparison. + if re.search(r"\b" + re.escape(inner) + r"\b", other): + return match.group(0) + tokens = _TOKEN_RE.findall(inner) + if any(t in _VERSION_MARKER_TOKENS for t in tokens): + return match.group(0) + if any(any(c.isdigit() for c in t) and t not in other_tokens for t in tokens): + return match.group(0) + return " " + + out = _QUALIFIER_RE.sub(_drop, title) + return re.sub(r"\s+", " ", out).strip() + + 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 @@ -133,5 +204,6 @@ def numeric_tokens_differ(title_a: str, title_b: str) -> bool: __all__ = [ "titles_plausibly_same", "strip_redundant_context_qualifiers", + "strip_subtitle_qualifiers", "numeric_tokens_differ", ] diff --git a/database/music_database.py b/database/music_database.py index 202b2d4c..0fc68c4c 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -7668,6 +7668,24 @@ class MusicDatabase: ctx_sim *= ctx_ratio # 'Believe' vs 'Believe In Me' still penalised best_title_similarity = max(best_title_similarity, ctx_sim) + # #825: a bracketed qualifier that is a SUBTITLE — not a version + # marker and not numeric — is the same song. 'Llamando a la tierra + # (Serenade From the Stars)' vs the library's bare 'Llamando a la + # tierra': the subtitle restates nothing (so #808 keeps it) and the + # length penalty crushed the pair to ~0.14 — sync re-added it to + # the wishlist forever and cleanup (same matcher) never removed it. + # Version qualifiers ('(Live)', '(Versión 1988)', '(Dueto 2007)') + # are kept by the helper, so their mismatch penalty still stands. + from core.text.title_match import strip_subtitle_qualifiers + sub_search = strip_subtitle_qualifiers(search_title_norm, db_title_norm) + sub_db = strip_subtitle_qualifiers(db_title_norm, search_title_norm) + if (sub_search, sub_db) != (search_title_norm, db_title_norm) and sub_search and sub_db: + sub_sim = self._string_similarity(sub_search, sub_db) + sub_ratio = min(len(sub_search), len(sub_db)) / max(len(sub_search), len(sub_db)) + if sub_ratio < 0.7: + sub_sim *= sub_ratio # stripped forms still length-guarded + best_title_similarity = max(best_title_similarity, sub_sim) + # Word-level guard: SequenceMatcher's char ratio over-credits # different songs that share a long substring or only a stopword # ("Dani California" vs "Californication" = 0.67; "Under The Bridge" diff --git a/services/sync_service.py b/services/sync_service.py index 803f2e8e..b48aa051 100644 --- a/services/sync_service.py +++ b/services/sync_service.py @@ -665,6 +665,10 @@ class PlaylistSyncService: _try_title, _try_artist, confidence_threshold=0.7, server_source=active_server, candidate_tracks=artist_candidates, + # #825: album context enables the album-aware fallback + # (multi-artist albums filed under another artist) — + # the cleanup path already passes it; sync didn't. + album=getattr(spotify_track, 'album', None) or None, ) if _cand_conf > confidence: db_track, confidence = _cand_track, _cand_conf diff --git a/tests/test_subtitle_qualifier_match.py b/tests/test_subtitle_qualifier_match.py new file mode 100644 index 00000000..a96150dc --- /dev/null +++ b/tests/test_subtitle_qualifier_match.py @@ -0,0 +1,158 @@ +"""#825: bracketed SUBTITLES must not block library-presence matching. + +carlosjfcasero's case (round 2): the mirrored-playlist sync auto-added +'Llamando a la tierra (Serenade From the Stars)' by M-Clan to the wishlist on +every run, even though his library has the song (stored bare). The subtitle is +the song's official parenthetical — it restates no album context, so the #808 +strip kept it, and the length-ratio penalty crushed the pair to ~0.14. The +sync matcher reported it missing forever AND the wishlist cleanup (the same +matcher) could never remove it. + +Fix: a bracketed qualifier with no version-marker token and no new numeric +token is a subtitle — compare with it stripped. Version qualifiers ('(Live)', +'(Versión 1988)', '(Dueto 2007)') still block, both EN and ES. +""" + +from __future__ import annotations + +import pytest + +from core.text.title_match import strip_subtitle_qualifiers +from database.music_database import MusicDatabase + + +# ── the pure helper ────────────────────────────────────────────────────────── + +def test_subtitle_is_stripped(): + out = strip_subtitle_qualifiers( + 'llamando a la tierra (serenade from the stars)', 'llamando a la tierra') + assert out == 'llamando a la tierra' + + +def test_version_markers_kept_english(): + for q in ('live', 'remix', 'acoustic', 'instrumental', 'demo', 'radio edit'): + assert strip_subtitle_qualifiers(f'song ({q})', 'song') == f'song ({q})' + + +def test_version_markers_kept_spanish(): + assert strip_subtitle_qualifiers('song (version 1988)', 'song') == 'song (version 1988)' + assert strip_subtitle_qualifiers('song (dueto 2007)', 'song') == 'song (dueto 2007)' + assert strip_subtitle_qualifiers('song (en directo en el liceu / 2008)', 'song') \ + == 'song (en directo en el liceu / 2008)' + + +def test_new_numeric_token_kept(): + # '(Pt. 2)' / '(2007)' are different releases, never subtitles. + assert strip_subtitle_qualifiers('song (pt. 2)', 'song') == 'song (pt. 2)' + assert strip_subtitle_qualifiers('song (2007)', 'song') == 'song (2007)' + + +def test_distinct_track_qualifiers_kept(): + # '(Interlude)' etc. are SEPARATE short tracks sharing the base name — + # treating them as subtitles would wrongly count the full song as owned. + for q in ('interlude', 'intro', 'outro', 'skit', 'freestyle'): + assert strip_subtitle_qualifiers(f'song ({q})', 'song') == f'song ({q})' + + +def test_roman_numeral_parts_kept(): + # No digits, so the numeric guard alone can't catch these. + assert strip_subtitle_qualifiers('song (pt. ii)', 'song') == 'song (pt. ii)' + assert strip_subtitle_qualifiers('song (part two)', 'song') == 'song (part two)' + assert strip_subtitle_qualifiers('song (vol. iii)', 'song') == 'song (vol. iii)' + + +def test_numeric_token_shared_with_other_title_is_fine(): + # The digit appears on the other side too — not a new release marker. + assert strip_subtitle_qualifiers('song 2007 (the ballad)', 'song 2007') == 'song 2007' + + +def test_qualifier_restated_in_other_title_left_for_direct_compare(): + full = 'song (the ballad)' + assert strip_subtitle_qualifiers(full, 'song (the ballad)') == full + + +def test_empty_and_plain_untouched(): + assert strip_subtitle_qualifiers('', 'x') == '' + assert strip_subtitle_qualifiers('plain title', 'other') == 'plain title' + + +# ── end to end through check_track_exists (sync + cleanup contract) ────────── + +@pytest.fixture() +def lib_db(tmp_path): + db = MusicDatabase(str(tmp_path / 'm.db')) + conn = db._get_connection() + c = conn.cursor() + c.execute("INSERT INTO artists (id, name, server_source) VALUES ('a1', 'M-Clan', 'jellyfin')") + c.execute("""INSERT INTO albums (id, title, artist_id, server_source) + VALUES ('al1', 'Usar y tirar', 'a1', 'jellyfin')""") + c.execute("""INSERT INTO tracks (id, album_id, artist_id, title, file_path, server_source) + VALUES ('t1', 'al1', 'a1', 'Llamando a la tierra', '/m/llamando.mp3', 'jellyfin')""") + c.execute("""INSERT INTO tracks (id, album_id, artist_id, title, file_path, server_source) + VALUES ('t2', 'al1', 'a1', 'Carolina', '/m/carolina.mp3', 'jellyfin')""") + conn.commit() + conn.close() + return db + + +def test_825_subtitled_search_matches_bare_library_track(lib_db): + """The reported case verbatim: playlist title carries the subtitle, the + library stores the bare title — must match (sync) and clean (cleanup).""" + match, conf = lib_db.check_track_exists( + 'Llamando a la tierra (Serenade From the Stars)', 'M-Clan', + confidence_threshold=0.7, server_source='jellyfin', + ) + assert match is not None and conf >= 0.7 + assert match.title == 'Llamando a la tierra' + + +def test_825_reverse_direction_matches(lib_db): + """Library could equally store the FULL title while the playlist has the + bare one — both directions must match.""" + conn = lib_db._get_connection() + c = conn.cursor() + c.execute("""INSERT INTO tracks (id, album_id, artist_id, title, file_path, server_source) + VALUES ('t3', 'al1', 'a1', 'Maggie (despierta)', '/m/maggie.mp3', 'jellyfin')""") + conn.commit() + conn.close() + match, conf = lib_db.check_track_exists( + 'Maggie', 'M-Clan', confidence_threshold=0.7, server_source='jellyfin') + assert match is not None and conf >= 0.7 + + +def test_live_version_still_blocked(lib_db): + match, conf = lib_db.check_track_exists( + 'Llamando a la tierra (Live)', 'M-Clan', + confidence_threshold=0.7, server_source='jellyfin', + ) + assert conf < 0.7 + + +def test_spanish_version_qualifiers_still_blocked(lib_db): + for title in ('Carolina (Versión 1988)', 'Carolina (Dueto 2007)', + 'Carolina (En Directo / 2005)'): + match, conf = lib_db.check_track_exists( + title, 'M-Clan', confidence_threshold=0.7, server_source='jellyfin') + assert conf < 0.7, title + + +def test_different_song_prefix_still_blocked(lib_db): + """Non-bracketed extensions are untouched — the length penalty stands.""" + match, conf = lib_db.check_track_exists( + 'Carolina en mi mente y otras cosas', 'M-Clan', + confidence_threshold=0.7, server_source='jellyfin', + ) + assert conf < 0.7 + + +def test_batched_candidate_path_also_fixed(lib_db): + """The sync matcher uses the candidate_tracks (batched) path — the fix must + apply there too, not just the SQL-variation path.""" + candidates = lib_db.search_tracks(artist='M-Clan', limit=50, server_source='jellyfin') + assert candidates + match, conf = lib_db.check_track_exists( + 'Llamando a la tierra (Serenade From the Stars)', 'M-Clan', + confidence_threshold=0.7, server_source='jellyfin', + candidate_tracks=candidates, + ) + assert match is not None and conf >= 0.7