diff --git a/core/acoustid_verification.py b/core/acoustid_verification.py index 24930ec0..42cc5500 100644 --- a/core/acoustid_verification.py +++ b/core/acoustid_verification.py @@ -17,6 +17,7 @@ from utils.logging_config import get_logger from core.acoustid_client import AcoustIDClient from core.matching_engine import MusicMatchingEngine from core.matching.version_mismatch import is_acceptable_version_mismatch +from core.matching.script_compat import is_cross_script_mismatch from core.musicbrainz_client import MusicBrainzClient logger = get_logger("acoustid.verification") @@ -655,10 +656,32 @@ class AcoustIDVerification: and title_sim >= 0.80 and artist_sim >= ARTIST_MATCH_THRESHOLD ) - if language_script_skip or high_confidence_strong_match_skip: + # Issue #797 — the EXPECTED artist and the AcoustID-matched + # artist are written in different scripts (e.g. "Joe Hisaishi" + # vs "久石譲") yet the alias-aware comparison still confirmed + # them as the same artist (artist_sim >= threshold, bridged via + # MusicBrainz aliases). When the artist itself spans scripts the + # title almost always does too — and a romanized-vs-native title + # comparison is meaningless, so it can't be evidence the file is + # wrong. Trust the confirmed artist + the fingerprint (already + # >= MIN_ACOUSTID_SCORE to reach here) and SKIP rather than + # quarantine a correct download of a non-English artist. + # + # Deliberately narrow (the "tight" scope): keyed on the ARTIST + # spanning scripts AND being confirmed. A same-script artist + # with only a cross-script TITLE (romaji artist + kanji title) + # is NOT covered — that case keeps the stricter 0.95 floor + # above, preserving the #607 wrong-file protection. + cross_script_artist_skip = ( + best_score >= MIN_ACOUSTID_SCORE + and artist_sim >= ARTIST_MATCH_THRESHOLD + and is_cross_script_mismatch(expected_artist_name, display_artist) + ) + if (language_script_skip or high_confidence_strong_match_skip + or cross_script_artist_skip): reason = ( "likely same song in different language/script" - if language_script_skip + if (language_script_skip or cross_script_artist_skip) else "title/artist match within tolerance" ) msg = ( diff --git a/core/downloads/candidates.py b/core/downloads/candidates.py index 9510a0f5..ddce91fe 100644 --- a/core/downloads/candidates.py +++ b/core/downloads/candidates.py @@ -326,6 +326,17 @@ def attempt_download_with_candidates(task_id, candidates, track, batch_id=None, "task=%s username=%s filename=%s", task_id, username, os.path.basename(filename), ) + elif track_info and track_info.get('_skip_acoustid'): + # Issue #797 — the album-download request had the + # per-request "Skip AcoustID verification" toggle on. + # Bypass only the AcoustID gate (same as a manual + # pick); integrity + bit-depth still run. + matched_downloads_context[context_key]['_skip_quarantine_check'] = 'acoustid' + logger.info( + "[Context] Skip-AcoustID toggle — bypassing AcoustID for " + "task=%s filename=%s", + task_id, os.path.basename(filename), + ) logger.info(f"[Context] Set is_album_download: {is_album_context} (has clean data: {has_clean_spotify_data})") logger.debug(f"[Debug] Context creation - track_info: {track_info is not None}, playlist_folder_mode: {track_info.get('_playlist_folder_mode', False) if track_info else False}") diff --git a/core/downloads/master.py b/core/downloads/master.py index 3769dca3..dfcb4eab 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -347,6 +347,13 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma batch_playlist_name = 'Unknown Playlist' batch_playlist_id = playlist_id batch_source_playlist_ref = '' + # Issue #797 — per-request "Skip AcoustID verification" toggle from + # the album-download modal. When set, every track in this batch + # bypasses the AcoustID quarantine gate (the user has chosen to + # trust the metadata over fingerprint disagreement — useful for + # non-English artists whose native-script metadata AcoustID can't + # reconcile with the romanized request). + batch_skip_acoustid = False with tasks_lock: if batch_id in download_batches: force_download_all = download_batches[batch_id].get('force_download_all', False) @@ -362,6 +369,7 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma batch_source_playlist_ref = ( download_batches[batch_id].get('source_playlist_ref') or '' ).strip() + batch_skip_acoustid = bool(download_batches[batch_id].get('skip_acoustid', False)) from core.downloads.playlist_folder import ( resolve_playlist_folder_mode_for_batch, @@ -1031,6 +1039,14 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma logger.info(f"[Wishlist] Added album context for: '{track_info.get('name')}' -> '{album_ctx['name']}'") + # Issue #797 — propagate the batch-level "skip AcoustID" + # toggle onto each track so the per-track download context + # (built in core/downloads/candidates.py) can set the + # AcoustID quarantine bypass. Mirrors the _playlist_folder_mode + # threading pattern below. + if batch_skip_acoustid: + track_info['_skip_acoustid'] = True + # Add playlist folder mode flag for sync page playlists and wishlist # tracks tied to a mirrored playlist with organize_by_playlist enabled. task_pl_folder_mode = batch_playlist_folder_mode diff --git a/core/matching/script_compat.py b/core/matching/script_compat.py new file mode 100644 index 00000000..88b1cb96 --- /dev/null +++ b/core/matching/script_compat.py @@ -0,0 +1,96 @@ +"""Writing-system (script) compatibility helpers for metadata comparison. + +Issue #797 — AcoustID returns a recording's title/artist in their +*original* script (e.g. ``久石譲`` for Joe Hisaishi) while SoulSync's +expected metadata is romanized / English (``Joe Hisaishi``). A raw +string-similarity comparison between two different writing systems +scores ~0 even when they name the very same artist, so correct +downloads of non-English artists get false-quarantined. + +These pure helpers let callers DETECT that situation — "one side is +written in a non-Latin script, the other in Latin" — so the comparison +logic can stop treating an untranslatable title/artist as evidence the +file is wrong. + +Deliberately conservative: a single accented Latin character (``é``, +``ñ``, ``ü``) is still Latin, NOT a script mismatch. Only genuinely +different writing systems (CJK, Hangul, Cyrillic, Greek, Arabic, +Hebrew, Thai, …) count as "non-Latin". +""" + +from __future__ import annotations + +# Unicode ranges for non-Latin writing systems we treat as a "hard" +# script difference. Latin (incl. Latin-1 Supplement / Extended with +# diacritics) is intentionally absent — accented Latin is still Latin. +# CJK ranges mirror core.matching_engine's issue #722 detection so the +# two stay consistent. +_NONLATIN_RANGES = ( + ('Ͱ', 'Ͽ'), # Greek and Coptic + ('Ѐ', 'ӿ'), # Cyrillic + ('Ԁ', 'ԯ'), # Cyrillic Supplement + ('֐', '׿'), # Hebrew + ('؀', 'ۿ'), # Arabic + ('ݐ', 'ݿ'), # Arabic Supplement + ('฀', '๿'), # Thai + ('⺀', '⻿'), # CJK Radicals Supplement + ('぀', 'ゟ'), # Hiragana + ('゠', 'ヿ'), # Katakana + ('㐀', '䶿'), # CJK Unified Ideographs Extension A + ('一', '鿿'), # CJK Unified Ideographs + ('가', '힯'), # Hangul Syllables + ('豈', '﫿'), # CJK Compatibility Ideographs + ('ｦ', 'ￜ'), # Halfwidth Katakana / Hangul +) + + +def _is_nonlatin_char(c: str) -> bool: + """True when ``c`` belongs to a non-Latin writing system.""" + for lo, hi in _NONLATIN_RANGES: + if lo <= c <= hi: + return True + return False + + +def has_strong_nonlatin(text: str) -> bool: + """True when ``text`` contains at least one non-Latin-script letter. + + Accented Latin (``Beyoncé``, ``Sigur Rós``, ``Mötley Crüe``) returns + False — those are Latin. ``久石譲``, ``Дмитрий``, ``방탄소년단`` return True. + """ + if not text: + return False + return any(_is_nonlatin_char(c) for c in text) + + +def _has_latin_letter(text: str) -> bool: + """True when ``text`` contains an ASCII A–Z / a–z letter.""" + if not text: + return False + return any(('a' <= c <= 'z') or ('A' <= c <= 'Z') for c in text) + + +def is_cross_script_mismatch(a: str, b: str) -> bool: + """True when ``a`` and ``b`` are written in different scripts. + + Specifically: exactly one side uses a non-Latin writing system while + the other is genuine Latin text. This is the signal that a raw + similarity score between the two is meaningless (a romanized name vs + its native-script form), NOT that they name different things. + + Symmetric. Returns False when: + - both sides are Latin (ordinary English-vs-English comparison), + - both sides are non-Latin (same-script comparison still works), + - either side is empty / has no comparable letters. + """ + a_nonlatin = has_strong_nonlatin(a) + b_nonlatin = has_strong_nonlatin(b) + if a_nonlatin == b_nonlatin: + # Same script class on both sides (or neither has non-Latin) — + # similarity comparison is meaningful, no script bridge needed. + return False + # Exactly one side is non-Latin. It's only a true cross-script case + # if the OTHER side is real Latin text (not punctuation / digits). + if a_nonlatin: + return _has_latin_letter(b) + return _has_latin_letter(a) diff --git a/tests/downloads/test_downloads_candidates.py b/tests/downloads/test_downloads_candidates.py index 95ed1328..8a80a3ab 100644 --- a/tests/downloads/test_downloads_candidates.py +++ b/tests/downloads/test_downloads_candidates.py @@ -461,6 +461,45 @@ def test_auto_search_pick_does_not_inject_acoustid_bypass(): assert "_user_manual_pick" not in ctx +def test_skip_acoustid_track_flag_injects_bypass(): + """Issue #797: when the album-download request had the per-request + 'Skip AcoustID verification' toggle on, the master worker stamps + `_skip_acoustid=True` onto each track's track_info. The candidates + helper must propagate that into the post-process context as + `_skip_quarantine_check='acoustid'` so AcoustID never quarantines + this request's files (e.g. correct downloads of non-English artists + whose native-script metadata AcoustID can't reconcile).""" + deps = _build_deps() + _seed_task("t_skip_aid", track_info={"_skip_acoustid": True}) + + candidates = [_Candidate(filename="skip.flac", confidence=0.99)] + track = _Track() + + result = dc.attempt_download_with_candidates("t_skip_aid", candidates, track, batch_id="b1", deps=deps) + + assert result is True + ctx = matched_downloads_context["user1::skip.flac"] + assert ctx["_skip_quarantine_check"] == "acoustid" + # It's the toggle path, NOT a manual pick. + assert "_user_manual_pick" not in ctx + + +def test_no_skip_acoustid_flag_keeps_verification(): + """Without the toggle (no `_skip_acoustid` on track_info), AcoustID + verification must still run — the bypass is opt-in per request.""" + deps = _build_deps() + _seed_task("t_no_skip_aid", track_info={}) # no _skip_acoustid + + candidates = [_Candidate(filename="verify.flac", confidence=0.99)] + track = _Track() + + result = dc.attempt_download_with_candidates("t_no_skip_aid", candidates, track, batch_id="b1", deps=deps) + + assert result is True + ctx = matched_downloads_context["user1::verify.flac"] + assert "_skip_quarantine_check" not in ctx + + def test_equal_confidence_candidates_prefer_better_peer_quality(): """Equal-confidence Soulseek candidates use peer quality as the tiebreaker.""" deps = _build_deps() diff --git a/tests/matching/test_script_compat.py b/tests/matching/test_script_compat.py new file mode 100644 index 00000000..644d16b5 --- /dev/null +++ b/tests/matching/test_script_compat.py @@ -0,0 +1,79 @@ +"""Tests for core/matching/script_compat.py — writing-system detection. + +Issue #797 — these pin the exact boundary that the AcoustID verifier +relies on: accented Latin is still Latin (no false cross-script +trigger), but genuinely different writing systems (CJK / Hangul / +Cyrillic / Greek / Arabic / Hebrew / Thai) ARE flagged so a +romanized-vs-native artist comparison isn't treated as a real mismatch. +""" + +from __future__ import annotations + +import pytest + +from core.matching.script_compat import ( + has_strong_nonlatin, + is_cross_script_mismatch, +) + + +# --------------------------------------------------------------------------- +# has_strong_nonlatin — accented Latin must NOT count +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize('text', [ + 'Beyoncé', 'Sigur Rós', 'Mötley Crüe', 'Joe Hisaishi', + 'Kendrick Lamar', 'Dmitry Yablonsky', 'AC/DC', 'P!nk', + '', ' ', '12345', 'Café del Mar', +]) +def test_latin_and_accented_latin_is_not_nonlatin(text): + assert has_strong_nonlatin(text) is False + + +@pytest.mark.parametrize('text', [ + '久石譲', # kanji (Joe Hisaishi) + '残酷な天使のテーゼ', # kana + kanji + 'Дмитрий Яблонский', # Cyrillic + '방탄소년단', # Hangul (BTS) + 'Σωκράτης', # Greek + 'عمرو دياب', # Arabic + 'שלום', # Hebrew + 'ก้อง สหรัถ', # Thai +]) +def test_real_nonlatin_scripts_detected(text): + assert has_strong_nonlatin(text) is True + + +# --------------------------------------------------------------------------- +# is_cross_script_mismatch — the verifier's gate +# --------------------------------------------------------------------------- + +def test_romanized_vs_native_is_cross_script(): + # The reported case (#797): expected romanized, AcoustID native. + assert is_cross_script_mismatch('Joe Hisaishi', '久石譲') is True + assert is_cross_script_mismatch('Dmitry Yablonsky', 'Дмитрий Яблонский') is True + + +def test_is_symmetric(): + assert is_cross_script_mismatch('久石譲', 'Joe Hisaishi') is True + + +def test_same_latin_both_sides_is_not_mismatch(): + # English-vs-English — comparison is meaningful, no bridge. This is + # the Kendrick R.O.T.C protection surface: must stay False so the + # verifier keeps its strict FAIL path. + assert is_cross_script_mismatch('Kendrick Lamar', 'Kendrick Lamar feat. BJ') is False + assert is_cross_script_mismatch('Crown', 'Crown of Thorns') is False + + +def test_same_nonlatin_both_sides_is_not_mismatch(): + # Both native — same-script similarity still works, don't relax. + assert is_cross_script_mismatch('久石譲', '久石譲') is False + assert is_cross_script_mismatch('久石譲', '坂本龍一') is False + + +def test_empty_or_letterless_other_side_is_not_mismatch(): + # One side non-Latin but the other has no Latin LETTER to bridge to. + assert is_cross_script_mismatch('', '久石譲') is False + assert is_cross_script_mismatch('12345', '久石譲') is False + assert is_cross_script_mismatch('久石譲', ' ') is False diff --git a/tests/test_acoustid_skip_logic.py b/tests/test_acoustid_skip_logic.py index 84c92017..0d820d1b 100644 --- a/tests/test_acoustid_skip_logic.py +++ b/tests/test_acoustid_skip_logic.py @@ -185,6 +185,80 @@ def test_high_score_but_artist_mismatch_no_longer_skipped(verifier): assert result == VerificationResult.FAIL +def test_low_fingerprint_score_never_skipped_same_script_artist(verifier): + """#797 guard — the #607 protection for a SAME-SCRIPT artist (Latin + 'Yoko Takahashi' on both sides) with only a cross-script TITLE must + stay FAIL below the 0.95 floor. The #797 relaxation is keyed on the + ARTIST spanning scripts, which this case is NOT, so nothing changes + here. (Duplicates test_low_fingerprint_score_never_skipped's intent + explicitly against the new code path.)""" + _stub_lookup(verifier, recordings=[ + {'title': '残酷な天使のテーゼ', 'artist': 'Yoko Takahashi'}, + ], best_score=0.85) + + result, _msg = verifier.verify_audio_file( + '/fake/path.flac', + 'Zankoku na Tenshi no Theze', + 'Yoko Takahashi', + ) + assert result == VerificationResult.FAIL + + +# --------------------------------------------------------------------------- +# Issue #797 — non-English ARTIST whose name spans scripts +# --------------------------------------------------------------------------- + + +def test_cross_script_artist_confirmed_via_alias_skips_below_095(verifier): + """#797 headline: requested 'Joe Hisaishi' (romanized), AcoustID + returns the recording with the artist/title in their native kanji + ('久石譲'). The alias bridge confirms 久石譲 IS Joe Hisaishi, the + fingerprint is solid (0.85, above the 0.80 trust floor) but below + the old 0.95 language/script bar. Pre-#797 this FAILed and the + correct file was quarantined. Now it SKIPs.""" + with patch( + 'core.acoustid_verification._resolve_expected_artist_aliases', + return_value=['久石譲'], + ): + _stub_lookup(verifier, recordings=[ + {'title': '風のとおり道', 'artist': '久石譲'}, + ], best_score=0.85) + + result, msg = verifier.verify_audio_file( + '/fake/path.flac', + 'The Path of the Wind', + 'Joe Hisaishi', + ) + assert result == VerificationResult.SKIP + assert 'language/script' in msg.lower() + + +def test_cross_script_artist_NOT_confirmed_still_fails(verifier): + """#797 tight scope: if the alias bridge can't confirm the artist + (lookup returns nothing), we have no positive evidence the kanji + artist IS the expected one — so the #797 relaxation must NOT fire. + The relaxation only rescues a CONFIRMED cross-script artist. + + Constructed so best_rec is set (partial title overlap → non-zero + combined score) and the title stays under the strict threshold, so + the flow reaches the same skip-decision point the rescue lives at — + proving it doesn't fire without a confirmed artist.""" + with patch( + 'core.acoustid_verification._resolve_expected_artist_aliases', + return_value=[], + ): + _stub_lookup(verifier, recordings=[ + {'title': 'Summer Night', 'artist': '久石譲'}, + ], best_score=0.85) + + result, _msg = verifier.verify_audio_file( + '/fake/path.flac', + 'Summer', + 'Joe Hisaishi', + ) + assert result == VerificationResult.FAIL + + def test_old_loose_threshold_no_longer_fires_for_unrelated_titles(verifier): """Pin the negative case for the old loose threshold (title_sim >= 0.55). 'Crown' vs 'Crown of Thorns' had similarity around 0.6 diff --git a/web_server.py b/web_server.py index a02d9273..2978693f 100644 --- a/web_server.py +++ b/web_server.py @@ -18707,6 +18707,10 @@ def start_missing_tracks_process(playlist_id): is_album_download = data.get('is_album_download', False) album_context = data.get('album_context', None) artist_context = data.get('artist_context', None) + # Issue #797 — per-request "Skip AcoustID verification" toggle from the + # album-download modal. Stored on the batch and propagated per-track by + # the master worker so AcoustID never quarantines this request's files. + skip_acoustid = bool(data.get('skip_acoustid', False)) if not tracks: return jsonify({"success": False, "error": "No tracks provided"}), 400 @@ -18774,6 +18778,7 @@ def start_missing_tracks_process(playlist_id): 'album_context': album_context, 'artist_context': artist_context, 'wing_it': wing_it, + 'skip_acoustid': skip_acoustid, # #797 per-request AcoustID bypass 'batch_source': _downloads_history.detect_sync_source(playlist_id), } diff --git a/webui/static/downloads.js b/webui/static/downloads.js index d164f615..85c0f944 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -2451,6 +2451,11 @@ async function startMissingTracksProcess(playlistId) { const forceDownloadCheckbox = document.getElementById(`force-download-all-${playlistId}`); const forceDownloadAll = forceDownloadCheckbox ? forceDownloadCheckbox.checked : false; + // Issue #797 — per-request "Skip AcoustID verification" toggle. Absent + // checkbox (other call sites) → false, so behavior is unchanged there. + const skipAcoustidCheckbox = document.getElementById(`skip-acoustid-${playlistId}`); + const skipAcoustid = skipAcoustidCheckbox ? skipAcoustidCheckbox.checked : false; + // Check if playlist folder mode toggle is enabled (only for sync page playlists) const playlistFolderMode = typeof isPlaylistOrganizeEnabled === 'function' ? isPlaylistOrganizeEnabled(playlistId) @@ -2495,6 +2500,7 @@ async function startMissingTracksProcess(playlistId) { force_download_all: forceDownloadAll || isWingIt, ignore_manual_matches: forceDownloadAll, wing_it: isWingIt, + skip_acoustid: skipAcoustid, }; // If this is an artist album download, use album name and include full context diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 7eb1d78e..5e1c1e62 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -1483,6 +1483,10 @@ async function openDownloadMissingModalForArtistAlbum(virtualPlaylistId, playlis Force Download All + ${contextType === 'playlist' ? `