Matcher: bracketed subtitles no longer read as different songs (#825)
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.
This commit is contained in:
parent
ca040d2c10
commit
0939585620
4 changed files with 252 additions and 0 deletions
|
|
@ -115,6 +115,77 @@ def strip_redundant_context_qualifiers(title: str, *context_texts: str) -> str:
|
||||||
return re.sub(r"\s+", " ", out).strip()
|
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:
|
def numeric_tokens_differ(title_a: str, title_b: str) -> bool:
|
||||||
"""True when the digit-bearing tokens of two titles differ — 'Vol.4' vs
|
"""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
|
'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__ = [
|
__all__ = [
|
||||||
"titles_plausibly_same",
|
"titles_plausibly_same",
|
||||||
"strip_redundant_context_qualifiers",
|
"strip_redundant_context_qualifiers",
|
||||||
|
"strip_subtitle_qualifiers",
|
||||||
"numeric_tokens_differ",
|
"numeric_tokens_differ",
|
||||||
]
|
]
|
||||||
|
|
|
||||||
|
|
@ -7668,6 +7668,24 @@ class MusicDatabase:
|
||||||
ctx_sim *= ctx_ratio # 'Believe' vs 'Believe In Me' still penalised
|
ctx_sim *= ctx_ratio # 'Believe' vs 'Believe In Me' still penalised
|
||||||
best_title_similarity = max(best_title_similarity, ctx_sim)
|
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
|
# Word-level guard: SequenceMatcher's char ratio over-credits
|
||||||
# different songs that share a long substring or only a stopword
|
# different songs that share a long substring or only a stopword
|
||||||
# ("Dani California" vs "Californication" = 0.67; "Under The Bridge"
|
# ("Dani California" vs "Californication" = 0.67; "Under The Bridge"
|
||||||
|
|
|
||||||
|
|
@ -665,6 +665,10 @@ class PlaylistSyncService:
|
||||||
_try_title, _try_artist,
|
_try_title, _try_artist,
|
||||||
confidence_threshold=0.7, server_source=active_server,
|
confidence_threshold=0.7, server_source=active_server,
|
||||||
candidate_tracks=artist_candidates,
|
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:
|
if _cand_conf > confidence:
|
||||||
db_track, confidence = _cand_track, _cand_conf
|
db_track, confidence = _cand_track, _cand_conf
|
||||||
|
|
|
||||||
158
tests/test_subtitle_qualifier_match.py
Normal file
158
tests/test_subtitle_qualifier_match.py
Normal file
|
|
@ -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
|
||||||
Loading…
Reference in a new issue