From 37140dff34aa8389565ef1acfc0738c7249f2ecc Mon Sep 17 00:00:00 2001 From: dev Date: Fri, 5 Jun 2026 22:55:07 +0200 Subject: [PATCH] Downloads: opt-in last-resort acceptance of repeated version mismatches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some tracks don't exist on the sources in the wanted cut — every copy is, say, the instrumental. The retry engine correctly rejects each (version mismatch) and gives up, leaving the track missing. New opt-in fallback: once a track's AcoustID retries are fully exhausted, if every quarantined candidate for it failed the SAME version mismatch (same matched version, e.g. all instrumental) and there are >= N of them, accept the best (first-tried = oldest = highest-confidence) one. Safety rules (core/imports/version_mismatch_fallback.py): - Version mismatches only. Audio/artist mismatches (different recording) and integrity/duration failures (truncated/wrong file) never participate. - All qualifying entries must share the same matched version; a mix (instrumental + live) is ambiguous → no acceptance. - Re-import bypasses ONLY the AcoustID gate; integrity/duration/bit-depth still run, so a truncated or genuinely wrong file is never let through here. - Reuses the existing quarantine approve_quarantine_entry + re-verify dispatch. Wired at the AcoustID give-up point in the verification wrapper. Two new post_processing settings surfaced in the Retry Logic tile (default off): accept_version_mismatch_fallback + version_mismatch_min_count. Pure decision core + orchestration covered by tests (11). Acceptance logged at WARNING with track + matched version. Co-Authored-By: Claude Opus 4.8 --- core/imports/pipeline.py | 59 ++++- core/imports/version_mismatch_fallback.py | 185 ++++++++++++++++ .../imports/test_version_mismatch_fallback.py | 207 ++++++++++++++++++ webui/index.html | 12 + webui/static/settings.js | 4 + 5 files changed, 466 insertions(+), 1 deletion(-) create mode 100644 core/imports/version_mismatch_fallback.py create mode 100644 tests/imports/test_version_mismatch_fallback.py diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index a2890519..d726e54a 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -35,7 +35,12 @@ from core.imports.context import ( from core.imports.file_integrity import check_audio_integrity, resolve_duration_tolerance from core.imports.filename import extract_track_number_from_filename from core.imports.guards import check_flac_bit_depth, move_to_quarantine -from core.imports.quarantine import entry_id_from_quarantined_filename +from core.imports.quarantine import ( + approve_quarantine_entry, + entry_id_from_quarantined_filename, + list_quarantine_entries, +) +from core.imports.version_mismatch_fallback import try_accept_version_mismatch_fallback from core.imports.side_effects import ( emit_track_downloaded, record_download_provenance, @@ -985,6 +990,53 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta post_process_locks.pop(context_key, None) +def _attempt_version_mismatch_fallback(context, task_id, batch_id, runtime, metadata_runtime): + """Opt-in last resort once AcoustID retries are exhausted: accept the best + quarantined version-mismatch candidate for this track instead of failing. + + Delegates the decision + safety rules to + ``core.imports.version_mismatch_fallback`` (version-mismatch only, all the + same matched version, >= min_count, AcoustID-only bypass). Returns True when + a candidate was accepted and re-dispatched — the caller then skips marking + the task failed. + """ + try: + download_path = docker_resolve_path( + config_manager.get('soulseek.download_path', './downloads') + ) + quarantine_dir = os.path.join(download_path, 'ss_quarantine') + restore_dir = os.path.join(download_path, 'Transfer') + expected_title = get_import_clean_title(context, default='') + expected_artist = get_import_clean_artist(context, default='') + if not expected_title or not expected_artist: + return False + + def _reprocess(restored_path, ctx, tid, bid): + new_key = f"vmfallback_{tid}_{int(time.time())}" + threading.Thread( + target=lambda: post_process_matched_download_with_verification( + new_key, ctx, restored_path, tid, bid, runtime, metadata_runtime + ), + daemon=True, + ).start() + + return try_accept_version_mismatch_fallback( + quarantine_dir=quarantine_dir, + restore_dir=restore_dir, + expected_title=expected_title, + expected_artist=expected_artist, + task_id=task_id, + batch_id=batch_id, + config_get=config_manager.get, + list_entries=list_quarantine_entries, + approve_entry=approve_quarantine_entry, + reprocess=_reprocess, + ) + except Exception as exc: + pp_logger.debug("[Version-Mismatch Fallback] skipped due to error: %s", exc) + return False + + def post_process_matched_download_with_verification(context_key, context, file_path, task_id, batch_id, runtime, metadata_runtime=None): on_download_completed = getattr(runtime, "on_download_completed", None) @@ -1026,6 +1078,11 @@ def post_process_matched_download_with_verification(context_key, context, file_p f"AcoustID mismatch for task {task_id} — retrying next-best candidate: {failure_msg}" ) return + # Retries exhausted. Opt-in last resort: if every quarantined + # candidate for this track failed the SAME version mismatch (e.g. all + # instrumental), accept the best one rather than leaving it missing. + if _attempt_version_mismatch_fallback(context, task_id, batch_id, runtime, metadata_runtime): + return logger.info(f"File was quarantined by AcoustID verification (task={task_id}): {failure_msg}") with tasks_lock: if task_id in download_tasks: diff --git a/core/imports/version_mismatch_fallback.py b/core/imports/version_mismatch_fallback.py new file mode 100644 index 00000000..7eaab479 --- /dev/null +++ b/core/imports/version_mismatch_fallback.py @@ -0,0 +1,185 @@ +"""Last-resort acceptance of a version-mismatched download. + +Some tracks simply don't exist on the configured sources in the wanted cut — +every copy is, say, the instrumental. The retry engine correctly rejects each +one (version mismatch) and eventually gives up, leaving the track missing. + +This module provides an OPT-IN fallback: once a track's retries are fully +exhausted, if every quarantined candidate for it failed the *same* way (same +matched version, e.g. all ``instrumental``) and there are at least ``min_count`` +of them, accept the best (first-tried) one rather than failing outright. + +Hard safety rules: +- Only ``Version mismatch`` quarantines qualify. Audio/artist mismatches + (a genuinely different recording) and integrity/duration failures + (truncated or wrong file) never participate. +- All qualifying entries must share the same matched version. A mix + (instrumental + live) is ambiguous → no acceptance. +- The chosen candidate is re-imported with only the AcoustID gate bypassed; + the integrity / duration / bit-depth gates still run, so a truncated or + corrupt file is never let through by this path. + +``select_version_mismatch_fallback`` is the pure decision core (no I/O) so it +can be tested directly. ``try_accept_version_mismatch_fallback`` wires it to the +quarantine store + re-import dispatch via injected callables. +""" + +from __future__ import annotations + +import logging +import re +from typing import Any, Callable, Dict, List, Optional + +logger = logging.getLogger(__name__) + +# Matches the reason string written by acoustid_verification's version gate: +# "Version mismatch: expected '' (<exp>) but file is '<title>' (<got>)" +# We only need the matched (<got>) version to test cross-entry consistency. +_VERSION_MISMATCH_RE = re.compile( + r"^Version mismatch:.*\bbut file is\b.*\(([^()]+)\)\s*$" +) + + +def _norm(text: Optional[str]) -> str: + return (text or "").strip().casefold() + + +def matched_version(reason: Optional[str]) -> Optional[str]: + """Return the matched version token (e.g. ``'instrumental'``) for a + Version-mismatch reason string, or None if the reason isn't a version + mismatch / can't be parsed.""" + if not reason: + return None + m = _VERSION_MISMATCH_RE.match(reason.strip()) + if not m: + return None + return m.group(1).strip().casefold() + + +def select_version_mismatch_fallback( + entries: List[Dict[str, Any]], + expected_title: str, + expected_artist: str, + min_count: int, +) -> Optional[Dict[str, Any]]: + """Pick the quarantine entry to accept as a last resort, or None. + + ``entries`` are dicts as produced by + :func:`core.imports.quarantine.list_quarantine_entries` (needs ``id``, + ``reason``, ``expected_track``, ``expected_artist``, ``has_full_context``). + + Returns the chosen entry (the first-tried = oldest = best, by ascending + ``id`` whose timestamp prefix sorts chronologically) when, for this track, + there are at least ``min_count`` version-mismatch entries that all share the + same matched version and carry full context. Otherwise None. + """ + title = _norm(expected_title) + artist = _norm(expected_artist) + + candidates = [] + for e in entries: + if not e.get("has_full_context"): + continue + if _norm(e.get("expected_track")) != title: + continue + if _norm(e.get("expected_artist")) != artist: + continue + version = matched_version(e.get("reason")) + if version is None: + continue + candidates.append((version, e)) + + if len(candidates) < max(1, int(min_count or 1)): + return None + + versions = {v for v, _ in candidates} + if len(versions) != 1: + # Inconsistent wrong versions (e.g. instrumental + live) — ambiguous, + # don't guess which the user wants. + return None + + # First tried = oldest = highest-confidence (the retry walks candidates + # best-first). The id is a "<date>_<time>_<name>" timestamp prefix, so the + # lexicographically smallest id is the earliest attempt. + return min((e for _, e in candidates), key=lambda e: e["id"]) + + +def try_accept_version_mismatch_fallback( + *, + quarantine_dir: str, + restore_dir: str, + expected_title: str, + expected_artist: str, + task_id: str, + batch_id: Optional[str], + config_get: Callable[[str, Any], Any], + list_entries: Callable[[str], List[Dict[str, Any]]], + approve_entry: Callable[..., Optional[Any]], + reprocess: Callable[..., None], +) -> bool: + """Orchestrate the last-resort acceptance. Returns True if a candidate was + accepted and re-dispatched (caller must then NOT mark the task failed). + + All I/O is injected so this is testable without a filesystem or the + web_server pipeline: + - ``config_get(key, default)`` — settings lookup. + - ``list_entries(quarantine_dir)`` — quarantine.list_quarantine_entries. + - ``approve_entry(quarantine_dir, entry_id, restore_dir)`` -> + ``(restored_path, context, trigger)`` or None — quarantine.approve_quarantine_entry. + - ``reprocess(restored_path, context, task_id, batch_id)`` — re-run the + verification pipeline on the restored file. + """ + if not config_get("post_processing.accept_version_mismatch_fallback", False): + return False + + try: + min_count = int(config_get("post_processing.version_mismatch_min_count", 2)) + except (TypeError, ValueError): + min_count = 2 + if min_count < 1: + min_count = 1 + + try: + entries = list_entries(quarantine_dir) or [] + except Exception as exc: # never let the fallback break the failure path + logger.debug("[Version-Mismatch Fallback] listing quarantine failed: %s", exc) + return False + + chosen = select_version_mismatch_fallback( + entries, expected_title, expected_artist, min_count + ) + if not chosen: + return False + + version = matched_version(chosen.get("reason")) or "?" + try: + result = approve_entry(quarantine_dir, chosen["id"], restore_dir) + except Exception as exc: + logger.error("[Version-Mismatch Fallback] approve failed for %s: %s", chosen["id"], exc) + return False + if not result: + return False + + restored_path, context, _trigger = result + if not isinstance(context, dict): + return False + # Bypass ONLY the AcoustID gate — integrity / duration / bit-depth still run, + # so a truncated or genuinely wrong file is still caught. + context["_skip_quarantine_check"] = "acoustid" + context["_version_mismatch_fallback"] = version + context["task_id"] = task_id + if batch_id: + context["batch_id"] = batch_id + + logger.warning( + "[Version-Mismatch Fallback] retries exhausted for '%s - %s'; accepting " + "best quarantined candidate (%s, entry %s) as last resort", + expected_artist, expected_title, version, chosen["id"], + ) + + try: + reprocess(restored_path, context, task_id, batch_id) + except Exception as exc: + logger.error("[Version-Mismatch Fallback] re-import dispatch failed: %s", exc) + return False + return True diff --git a/tests/imports/test_version_mismatch_fallback.py b/tests/imports/test_version_mismatch_fallback.py new file mode 100644 index 00000000..5ae4b64f --- /dev/null +++ b/tests/imports/test_version_mismatch_fallback.py @@ -0,0 +1,207 @@ +"""Last-resort acceptance of a version-mismatched quarantine candidate. + +When a track's retries are fully exhausted and EVERY quarantined candidate for +it failed the same way (same wrong version, e.g. all instrumental), the only +available version is that one — accept the best (first-tried) one instead of +leaving the track missing. Strict guards: version-mismatch only, all the same +matched version, and a minimum count. +""" + +from __future__ import annotations + +from core.imports.version_mismatch_fallback import ( + select_version_mismatch_fallback, + try_accept_version_mismatch_fallback, +) + + +def _entry(eid, reason, *, track="Barricades (Movie Ver.)", artist="Hiroyuki Sawano", + ctx=True): + return { + "id": eid, + "reason": reason, + "expected_track": track, + "expected_artist": artist, + "has_full_context": ctx, + } + + +_VM = "Version mismatch: expected 'Barricades (Movie Ver.)' (original) but file is 'Barricades <MOVIEver.> ({v})' ({v})" + + +def _vm(version): + return _VM.format(v=version) + + +def test_picks_oldest_when_all_same_version_and_count_met(): + # 3 instrumental mismatches → all same kind → pick first-tried (smallest id). + entries = [ + _entry("20260605_120300", _vm("instrumental")), + _entry("20260605_120100", _vm("instrumental")), # oldest = first tried + _entry("20260605_120200", _vm("instrumental")), + ] + chosen = select_version_mismatch_fallback(entries, "Barricades (Movie Ver.)", + "Hiroyuki Sawano", min_count=2) + assert chosen is not None + assert chosen["id"] == "20260605_120100" + + +def test_none_when_below_min_count(): + entries = [_entry("20260605_120100", _vm("instrumental"))] + assert select_version_mismatch_fallback( + entries, "Barricades (Movie Ver.)", "Hiroyuki Sawano", min_count=2) is None + + +def test_none_when_mixed_versions(): + # instrumental + live → inconsistent → never auto-accept (ambiguous). + entries = [ + _entry("20260605_120100", _vm("instrumental")), + _entry("20260605_120200", _vm("live")), + ] + assert select_version_mismatch_fallback( + entries, "Barricades (Movie Ver.)", "Hiroyuki Sawano", min_count=2) is None + + +def test_ignores_non_version_mismatch_reasons(): + # Audio mismatch (wrong artist/song) and integrity must NOT count. + entries = [ + _entry("20260605_120100", + "Audio mismatch: file identified as 'X' by 'Y' (artist=0%)"), + _entry("20260605_120200", + "Integrity check failed: Duration mismatch: file is 175s, expected 182s"), + ] + assert select_version_mismatch_fallback( + entries, "Barricades (Movie Ver.)", "Hiroyuki Sawano", min_count=1) is None + + +def test_only_counts_entries_for_this_track(): + entries = [ + _entry("20260605_120100", _vm("instrumental")), + _entry("20260605_120200", _vm("instrumental"), track="Call Your Name (Gv)"), + ] + # Only one entry matches this track → below min_count of 2. + assert select_version_mismatch_fallback( + entries, "Barricades (Movie Ver.)", "Hiroyuki Sawano", min_count=2) is None + + +def test_excludes_thin_sidecar_entries_without_context(): + # Can't approve without embedded context → exclude from the candidate pool. + entries = [ + _entry("20260605_120100", _vm("instrumental"), ctx=False), + _entry("20260605_120200", _vm("instrumental"), ctx=False), + ] + assert select_version_mismatch_fallback( + entries, "Barricades (Movie Ver.)", "Hiroyuki Sawano", min_count=2) is None + + +def test_track_match_is_case_and_space_insensitive(): + entries = [ + _entry("20260605_120100", _vm("instrumental")), + _entry("20260605_120200", _vm("instrumental")), + ] + chosen = select_version_mismatch_fallback( + entries, " barricades (movie ver.) ", "HIROYUKI SAWANO", min_count=2) + assert chosen is not None + assert chosen["id"] == "20260605_120100" + + +# ── Orchestration (try_accept_version_mismatch_fallback) ────────────────────── + +def _cfg(enabled=True, min_count=2): + values = { + "post_processing.accept_version_mismatch_fallback": enabled, + "post_processing.version_mismatch_min_count": min_count, + } + return lambda key, default=None: values.get(key, default) + + +def _two_instrumental_entries(): + return [ + _entry("20260605_120100", _vm("instrumental")), + _entry("20260605_120200", _vm("instrumental")), + ] + + +def test_orchestration_disabled_does_nothing(): + calls = {"approve": 0, "reprocess": 0} + + def approve(*a, **k): + calls["approve"] += 1 + return ("/restored.flac", {}, "acoustid") + + def reprocess(*a, **k): + calls["reprocess"] += 1 + + ok = try_accept_version_mismatch_fallback( + quarantine_dir="/q", restore_dir="/r", + expected_title="Barricades (Movie Ver.)", expected_artist="Hiroyuki Sawano", + task_id="t1", batch_id="b1", + config_get=_cfg(enabled=False), + list_entries=lambda d: _two_instrumental_entries(), + approve_entry=approve, reprocess=reprocess, + ) + assert ok is False + assert calls == {"approve": 0, "reprocess": 0} + + +def test_orchestration_accepts_and_reprocesses_with_acoustid_bypass(): + captured = {} + + def approve(qdir, entry_id, rdir): + captured["entry_id"] = entry_id + return ("/restored.flac", {"existing": 1}, "acoustid") + + def reprocess(path, context, task_id, batch_id): + captured["path"] = path + captured["context"] = context + captured["task_id"] = task_id + + ok = try_accept_version_mismatch_fallback( + quarantine_dir="/q", restore_dir="/r", + expected_title="Barricades (Movie Ver.)", expected_artist="Hiroyuki Sawano", + task_id="t1", batch_id="b1", + config_get=_cfg(), + list_entries=lambda d: _two_instrumental_entries(), + approve_entry=approve, reprocess=reprocess, + ) + assert ok is True + assert captured["entry_id"] == "20260605_120100" # oldest/best + assert captured["path"] == "/restored.flac" + assert captured["task_id"] == "t1" + # Only AcoustID bypassed — integrity/bit-depth gates still run. + assert captured["context"]["_skip_quarantine_check"] == "acoustid" + assert captured["context"]["_version_mismatch_fallback"] == "instrumental" + assert captured["context"]["task_id"] == "t1" + assert captured["context"]["batch_id"] == "b1" + + +def test_orchestration_no_candidate_does_not_reprocess(): + calls = {"reprocess": 0} + + def reprocess(*a, **k): + calls["reprocess"] += 1 + + ok = try_accept_version_mismatch_fallback( + quarantine_dir="/q", restore_dir="/r", + expected_title="Barricades (Movie Ver.)", expected_artist="Hiroyuki Sawano", + task_id="t1", batch_id="b1", + config_get=_cfg(), + list_entries=lambda d: [_entry("20260605_120100", _vm("instrumental"))], # 1 < min 2 + approve_entry=lambda *a, **k: ("/x", {}, "acoustid"), + reprocess=reprocess, + ) + assert ok is False + assert calls["reprocess"] == 0 + + +def test_orchestration_approve_failure_returns_false(): + ok = try_accept_version_mismatch_fallback( + quarantine_dir="/q", restore_dir="/r", + expected_title="Barricades (Movie Ver.)", expected_artist="Hiroyuki Sawano", + task_id="t1", batch_id="b1", + config_get=_cfg(), + list_entries=lambda d: _two_instrumental_entries(), + approve_entry=lambda *a, **k: None, # thin sidecar / move failed + reprocess=lambda *a, **k: (_ for _ in ()).throw(AssertionError("must not reprocess")), + ) + assert ok is False diff --git a/webui/index.html b/webui/index.html index 59568c18..3dbb51e6 100644 --- a/webui/index.html +++ b/webui/index.html @@ -5192,6 +5192,18 @@ <input type="number" id="retries-per-query" min="1" max="20" step="1" value="5" style="width: 100px;" autocomplete="off" data-bwignore="" data-1p-ignore="" data-lpignore="true"> <small class="settings-hint">In exhaustive mode, how many candidates to try per search query per source. The per-source budget is <em>number of search queries × this value</em>. Only used when Exhaustive retry is on.</small> </div> + <div class="form-group"> + <label class="checkbox-label"> + <input type="checkbox" id="accept-version-mismatch-fallback"> + Accept best version mismatch as last resort + </label> + <small class="settings-hint">When retries are fully exhausted and <em>every</em> candidate for a track failed the <strong>same</strong> version mismatch (e.g. only an instrumental exists), accept the best (first-tried) one instead of leaving the track missing. Only AcoustID is bypassed — integrity/duration/bit-depth checks still run, so truncated or genuinely wrong files are never let through. Off = leave such tracks failed.</small> + </div> + <div class="form-group"> + <label for="version-mismatch-min-count">Minimum matching mismatches before accepting</label> + <input type="number" id="version-mismatch-min-count" min="1" max="20" step="1" value="2" style="width: 100px;" autocomplete="off" data-bwignore="" data-1p-ignore="" data-lpignore="true"> + <small class="settings-hint">How many quarantined candidates must have failed the <em>same</em> version mismatch before the last-resort acceptance kicks in. Higher = more confirmation that no correct version exists. Only used when the option above is on.</small> + </div> </div> </div><!-- end Retry Logic body --> diff --git a/webui/static/settings.js b/webui/static/settings.js index cee5fc27..de1bc575 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -1171,6 +1171,8 @@ async function loadSettingsData() { document.getElementById('retry-next-candidate').checked = settings.post_processing?.retry_next_candidate_on_mismatch !== false; document.getElementById('retry-exhaustive').checked = settings.post_processing?.retry_exhaustive === true; document.getElementById('retries-per-query').value = settings.post_processing?.retries_per_query ?? 5; + document.getElementById('accept-version-mismatch-fallback').checked = settings.post_processing?.accept_version_mismatch_fallback === true; + document.getElementById('version-mismatch-min-count').value = settings.post_processing?.version_mismatch_min_count ?? 2; // Load service master toggles document.getElementById('embed-spotify').checked = settings.spotify?.embed_tags !== false; document.getElementById('embed-itunes').checked = settings.itunes?.embed_tags !== false; @@ -3002,6 +3004,8 @@ async function saveSettings(quiet = false) { retry_next_candidate_on_mismatch: document.getElementById('retry-next-candidate').checked, retry_exhaustive: document.getElementById('retry-exhaustive').checked, retries_per_query: Math.max(1, parseInt(document.getElementById('retries-per-query').value, 10) || 5), + accept_version_mismatch_fallback: document.getElementById('accept-version-mismatch-fallback').checked, + version_mismatch_min_count: Math.max(1, parseInt(document.getElementById('version-mismatch-min-count').value, 10) || 2), }, library: { music_paths: collectMusicPaths(),