From cf0a17c14a25ec11139f7b2174eb42cf47f943d8 Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 10 Jun 2026 14:43:57 +0200 Subject: [PATCH 01/14] feat(verification): shared normalize() core for import + scan Co-Authored-By: Claude Opus 4.8 --- core/matching/audio_verification.py | 59 +++++++++++++++++++ .../matching/test_audio_verification_core.py | 22 +++++++ 2 files changed, 81 insertions(+) create mode 100644 core/matching/audio_verification.py create mode 100644 tests/matching/test_audio_verification_core.py diff --git a/core/matching/audio_verification.py b/core/matching/audio_verification.py new file mode 100644 index 00000000..45ff48e6 --- /dev/null +++ b/core/matching/audio_verification.py @@ -0,0 +1,59 @@ +"""Shared audio-verification decision core (pure; no file/DB I/O). + +Single source of truth for normalization + the PASS/SKIP/FAIL decision used by +BOTH import-time verification (``core/acoustid_verification.py``) and the library +scan (``core/repair_jobs/acoustid_scanner.py``). Historically each path had its +own ``_normalize`` and decision branches that drifted apart and produced +inconsistent results (a correct cross-script anime-OST track passed at import but +was false-flagged by the scan). Centralising the decision here means the +thresholds, normalization, alias-aware comparison, cross-script handling, version +gate and duration guard are defined exactly once. +""" + +import re +from difflib import SequenceMatcher + +# Thresholds — the single definition both paths share. +MIN_ACOUSTID_SCORE = 0.80 # Minimum fingerprint score to trust a match. +TITLE_MATCH_THRESHOLD = 0.70 # Title similarity to consider a match. +ARTIST_MATCH_THRESHOLD = 0.60 # Artist similarity to consider a match. + + +def normalize(text: str) -> str: + """Normalize a title/artist for comparison. + + lowercase; strip ``()`` / ``[]`` / ``<>`` annotations (version tags, + performer credits like ````); strip trailing + version / featuring tags; KEEP CJK characters (``\\w`` is unicode-aware) so + Japanese/Chinese/Korean titles produce a comparable form instead of an empty + string; collapse whitespace. + """ + if not text: + return "" + s = text.lower().strip() + # Annotations that are metadata, not core identity. + s = re.sub(r'\s*\([^)]*\)', '', s) + s = re.sub(r'\s*\[[^\]]*\]', '', s) + s = re.sub(r'\s*<[^>]*>', '', s) + # Trailing featuring / version tags. + s = re.sub(r'\s+(?:feat\.?|ft\.?|featuring)\s+.*$', '', s, flags=re.IGNORECASE) + s = re.sub( + r'\s*-\s*(?:vocal|instrumental|acoustic|live|remix|cover|clean|explicit|' + r'radio\s*edit|original\s*mix|extended\s*mix|club\s*mix)\s*$', + '', s, flags=re.IGNORECASE, + ) + s = re.sub(r'\s*-\s*from\s+.+$', '', s, flags=re.IGNORECASE) + # Drop remaining punctuation but keep word chars (incl. CJK) + spaces. + s = re.sub(r'[^\w\s]', '', s) + s = re.sub(r'\s+', ' ', s).strip() + return s + + +def similarity(a: str, b: str) -> float: + """Similarity (0.0–1.0) between two strings after normalization.""" + na, nb = normalize(a), normalize(b) + if not na or not nb: + return 0.0 + if na == nb: + return 1.0 + return SequenceMatcher(None, na, nb).ratio() diff --git a/tests/matching/test_audio_verification_core.py b/tests/matching/test_audio_verification_core.py new file mode 100644 index 00000000..a9dd2e85 --- /dev/null +++ b/tests/matching/test_audio_verification_core.py @@ -0,0 +1,22 @@ +"""Shared audio-verification decision core: normalize() + evaluate(). + +One place for normalization + the PASS/SKIP/FAIL decision used by BOTH import-time +verification and the library AcoustID scan, so the two paths can't drift apart. +""" + +from core.matching.audio_verification import normalize + + +def test_normalize_strips_paren_bracket_angle_and_keeps_cjk(): + assert normalize("澤野弘之 ") == "澤野弘之" + assert normalize("Clarity (Live at X) [Remastered]") == "clarity" + assert normalize("Attack on Titan ") == "attack on titan" + + +def test_normalize_strips_version_and_featuring(): + assert normalize("In My Feelings - Instrumental") == "in my feelings" + assert normalize("Song feat. Someone") == "song" + + +def test_normalize_keeps_plain_text(): + assert normalize("Sawano Hiroyuki") == "sawano hiroyuki" From d989f252204126fc2ee29388586b78173e6f03f9 Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 10 Jun 2026 14:46:32 +0200 Subject: [PATCH 02/14] feat(verification): shared evaluate() PASS/SKIP/FAIL decision core Co-Authored-By: Claude Opus 4.8 --- core/matching/audio_verification.py | 189 ++++++++++++++++++ .../matching/test_audio_verification_core.py | 54 ++++- 2 files changed, 242 insertions(+), 1 deletion(-) diff --git a/core/matching/audio_verification.py b/core/matching/audio_verification.py index 45ff48e6..72da6e8b 100644 --- a/core/matching/audio_verification.py +++ b/core/matching/audio_verification.py @@ -11,12 +11,32 @@ gate and duration guard are defined exactly once. """ import re +from dataclasses import dataclass from difflib import SequenceMatcher +from enum import Enum +from typing import Any, List, Optional # Thresholds — the single definition both paths share. MIN_ACOUSTID_SCORE = 0.80 # Minimum fingerprint score to trust a match. TITLE_MATCH_THRESHOLD = 0.70 # Title similarity to consider a match. ARTIST_MATCH_THRESHOLD = 0.60 # Artist similarity to consider a match. +CLEAR_MISMATCH_THRESHOLD = 0.30 # Below this artist sim = clear wrong song. + + +class Decision(Enum): + PASS = "pass" + SKIP = "skip" + FAIL = "fail" + + +@dataclass +class Outcome: + decision: Decision + title_sim: float = 0.0 + artist_sim: float = 0.0 + matched_title: str = "" + matched_artist: str = "" + reason: str = "" def normalize(text: str) -> str: @@ -57,3 +77,172 @@ def similarity(a: str, b: str) -> float: if na == nb: return 1.0 return SequenceMatcher(None, na, nb).ratio() + + +_match_engine = None + + +def _detect_title_version(title: str) -> str: + """Version label ('original'/'instrumental'/'live'/'remix'/...) for a title.""" + global _match_engine + if not title: + return 'original' + if _match_engine is None: + from core.matching_engine import MusicMatchingEngine + _match_engine = MusicMatchingEngine() + version_type, _ = _match_engine.detect_version_type(title) + return version_type + + +def _alias_aware_artist_sim(expected_artist: str, actual_artist: str, + aliases: Optional[Any] = None) -> float: + """Best artist similarity across (expected, *aliases) vs actual. + + Bridges cross-script artist comparisons (kanji↔romaji etc) when MusicBrainz + aliases are available. ``aliases`` is an iterable of alias strings, or a + callable resolving them lazily (only invoked when direct similarity falls + below threshold — keeps the happy path lookup-free). + """ + from core.matching.artist_aliases import artist_names_match + + direct = similarity(expected_artist, actual_artist) + if aliases is None: + return direct + if direct >= ARTIST_MATCH_THRESHOLD: + return direct + resolved = aliases() if callable(aliases) else aliases + if not resolved: + return direct + _matched, score = artist_names_match( + expected_artist, actual_artist, aliases=resolved, + threshold=ARTIST_MATCH_THRESHOLD, similarity=similarity, + ) + return score + + +def _find_best_title_artist_match(recordings, expected_title, expected_artist, + aliases=None): + """Return (best_recording, title_sim, artist_sim) — title weighted higher.""" + best_rec = None + best_title_sim = 0.0 + best_artist_sim = 0.0 + best_combined = 0.0 + for rec in recordings: + title = rec.get('title') or '' + artist = rec.get('artist') or '' + title_sim = similarity(expected_title, title) + artist_sim = _alias_aware_artist_sim(expected_artist, artist, aliases) + combined = (title_sim * 0.6) + (artist_sim * 0.4) + if combined > best_combined: + best_combined = combined + best_rec = rec + best_title_sim = title_sim + best_artist_sim = artist_sim + return best_rec, best_title_sim, best_artist_sim + + +def evaluate(expected_title: str, expected_artist: str, + recordings: List[dict], *, fingerprint_score: float, + file_duration_s: Optional[float] = None, + aliases_provider: Optional[Any] = None) -> Outcome: + """Decide PASS / SKIP / FAIL for a fingerprinted file against expected + title/artist. Pure: no I/O. Shared by import verification and library scan. + + ``aliases_provider``: iterable or callable of expected-artist aliases + (kanji/cyrillic/etc) used to bridge cross-script comparisons. + ``file_duration_s``: when provided, a strong duration mismatch downgrades a + would-be FAIL to SKIP (fingerprint hash collision guard, used by the scan). + """ + from core.matching.script_compat import is_cross_script_mismatch + from core.matching.acoustid_candidates import ( + duration_mismatches_strongly, find_matching_recording, + ) + from core.matching.version_mismatch import is_acceptable_version_mismatch + + best_rec, title_sim, artist_sim = _find_best_title_artist_match( + recordings, expected_title, expected_artist, aliases_provider, + ) + if not best_rec: + return Outcome(Decision.SKIP, reason="No recordings with title/artist info") + + matched_title = best_rec.get('title', '?') or '?' + matched_artist = best_rec.get('artist', '?') or '?' + + def out(dec, reason): + return Outcome(dec, title_sim, artist_sim, matched_title, matched_artist, reason) + + # Version gate: original vs instrumental/live/remix is a real difference. + expected_version = _detect_title_version(expected_title) + matched_version = _detect_title_version(matched_title) + if expected_version != matched_version: + if not is_acceptable_version_mismatch( + expected_version, matched_version, + fingerprint_score=fingerprint_score, + title_similarity=title_sim, artist_similarity=artist_sim, + ): + return out(Decision.FAIL, + f"Version mismatch: expected ({expected_version}) " + f"but file is ({matched_version})") + + # Clean match. + if title_sim >= TITLE_MATCH_THRESHOLD and artist_sim >= ARTIST_MATCH_THRESHOLD: + return out(Decision.PASS, "Audio verified") + + # Title matches, artist doesn't — cover/collab vs genuinely wrong. + if title_sim >= TITLE_MATCH_THRESHOLD and artist_sim < ARTIST_MATCH_THRESHOLD: + for rec in recordings: + if _alias_aware_artist_sim( + expected_artist, rec.get('artist', ''), aliases_provider, + ) >= ARTIST_MATCH_THRESHOLD: + return out(Decision.PASS, "Expected artist found in AcoustID results") + if artist_sim < CLEAR_MISMATCH_THRESHOLD: + if file_duration_s and duration_mismatches_strongly( + file_duration_s, best_rec.get('duration') or best_rec.get('length')): + return out(Decision.SKIP, "Duration mismatch (fingerprint collision)") + return out(Decision.FAIL, + f"Audio mismatch: '{matched_title}' by '{matched_artist}' " + f"— expected artist not found") + return out(Decision.SKIP, "Title matches but artist ambiguous (cover/collab?)") + + # Title doesn't match — scan all recordings for a version-matched hit. + def _title_sim(a, b): + return similarity(a, b) + + def _artist_sim(ea, aa): + return _alias_aware_artist_sim(ea, aa, aliases_provider) + + candidate = None + for rec in recordings: + if _detect_title_version(rec.get('title') or '') != expected_version: + continue + if (similarity(expected_title, rec.get('title') or '') >= TITLE_MATCH_THRESHOLD + and _alias_aware_artist_sim( + expected_artist, rec.get('artist', ''), aliases_provider, + ) >= ARTIST_MATCH_THRESHOLD): + candidate = rec + break + if candidate is not None: + return out(Decision.PASS, "Scan match found in AcoustID results") + + # High-confidence / cross-script skips (don't quarantine a correct file). + has_non_ascii = (any(ord(c) > 127 for c in (expected_title or '')) + or any(ord(c) > 127 for c in matched_title)) + language_script_skip = (fingerprint_score >= 0.95 and has_non_ascii + and artist_sim >= ARTIST_MATCH_THRESHOLD) + high_confidence_strong_match_skip = (fingerprint_score >= 0.95 + and title_sim >= 0.80 + and artist_sim >= ARTIST_MATCH_THRESHOLD) + cross_script_artist_skip = (fingerprint_score >= MIN_ACOUSTID_SCORE + and artist_sim >= ARTIST_MATCH_THRESHOLD + and is_cross_script_mismatch(expected_artist, matched_artist)) + if (language_script_skip or high_confidence_strong_match_skip + or cross_script_artist_skip): + return out(Decision.SKIP, "Likely same song in different language/script") + + if file_duration_s and duration_mismatches_strongly( + file_duration_s, best_rec.get('duration') or best_rec.get('length')): + return out(Decision.SKIP, "Duration mismatch (fingerprint collision)") + + return out(Decision.FAIL, + f"Audio mismatch: file identified as '{matched_title}' by " + f"'{matched_artist}', expected '{expected_title}' by '{expected_artist}'") diff --git a/tests/matching/test_audio_verification_core.py b/tests/matching/test_audio_verification_core.py index a9dd2e85..efc8162c 100644 --- a/tests/matching/test_audio_verification_core.py +++ b/tests/matching/test_audio_verification_core.py @@ -4,7 +4,59 @@ One place for normalization + the PASS/SKIP/FAIL decision used by BOTH import-ti verification and the library AcoustID scan, so the two paths can't drift apart. """ -from core.matching.audio_verification import normalize +from core.matching.audio_verification import normalize, evaluate, Decision + + +def _rec(title, artist, duration=None): + return {"title": title, "artist": artist, "duration": duration} + + +def test_cross_script_vocal_credit_clean_match_passes(): + # Sawano / 澤野弘之 — <> stripped, alias bridges the artist, + # title matches too -> clean PASS (must never FAIL/quarantine). + out = evaluate( + "Call Your Name", "Sawano Hiroyuki", + [_rec("call your name", "澤野弘之 ")], + fingerprint_score=0.95, + aliases_provider=lambda: ["澤野弘之"], + ) + assert out.decision == Decision.PASS + + +def test_cross_script_ipa_title_skips_not_fails(): + # AcoustID returns an IPA-transcribed title that can't string-match, but the + # artist bridges cross-script -> SKIP (import anyway), never FAIL. + out = evaluate( + "Attack on Titan", "Sawano Hiroyuki", + [_rec("ətˈæk 0N tάɪtn", "澤野弘之")], + fingerprint_score=0.95, + aliases_provider=lambda: ["澤野弘之"], + ) + assert out.decision == Decision.SKIP + + +def test_clean_cross_script_match_passes(): + out = evaluate( + "Xl-Tt", "Sawano Hiroyuki", + [_rec("xl-tt", "澤野弘之")], + fingerprint_score=0.95, + aliases_provider=lambda: ["澤野弘之"], + ) + assert out.decision == Decision.PASS + + +def test_genuine_wrong_song_fails(): + out = evaluate( + "Yellow", "Coldplay", + [_rec("Rich Interlude", "Kendrick Lamar")], + fingerprint_score=0.85, + ) + assert out.decision == Decision.FAIL + + +def test_no_recordings_skips(): + out = evaluate("Whatever", "Someone", [], fingerprint_score=0.9) + assert out.decision == Decision.SKIP def test_normalize_strips_paren_bracket_angle_and_keeps_cjk(): From 967ad2a02619fef7723bd0675d560c5ca657399d Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 10 Jun 2026 14:56:45 +0200 Subject: [PATCH 03/14] refactor(verification): import path delegates to shared core verify_audio_file now calls audio_verification.evaluate() and re-exports normalize/similarity/_alias_aware_artist_sim from the core, so import and the library scan can no longer drift apart. Alias-rescue diagnostic moved to the core. Co-Authored-By: Claude Opus 4.8 --- core/acoustid_verification.py | 430 ++---------------- core/matching/audio_verification.py | 16 + .../test_acoustid_verification_aliases.py | 8 +- 3 files changed, 62 insertions(+), 392 deletions(-) diff --git a/core/acoustid_verification.py b/core/acoustid_verification.py index 42cc5500..a399dda6 100644 --- a/core/acoustid_verification.py +++ b/core/acoustid_verification.py @@ -56,166 +56,29 @@ class VerificationResult(Enum): ERROR = "error" # Lookup errored (invalid key / rate limit / no backend) - continue, but flag it -def _normalize(text: str) -> str: - """Normalize a string for comparison: lowercase, strip parentheticals, punctuation.""" - if not text: - return "" - s = text.lower().strip() - # Remove ALL parenthetical suffixes — these are metadata annotations, not core title - # Covers: (Live), (Remastered), (Parody of ...), (from "..." Soundtrack), (feat. ...), etc. - s = re.sub(r'\s*\([^)]*\)', '', s) - # Remove ALL square bracket suffixes: [Live], [Remastered], [Deluxe], etc. - s = re.sub(r'\s*\[[^\]]*\]', '', s) - # Remove trailing featuring info not in parentheses: "feat. ...", "ft. ...", "featuring ..." - s = re.sub(r'\s+(?:feat\.?|ft\.?|featuring)\s+.*$', '', s, flags=re.IGNORECASE) - # Remove dash-separated version tags: "- Vocal", "- Instrumental", "- Acoustic", etc. - s = re.sub(r'\s*-\s*(?:vocal|instrumental|acoustic|live|remix|cover|clean|explicit|radio\s*edit|original\s*mix|extended\s*mix|club\s*mix)\s*$', '', s, flags=re.IGNORECASE) - # Remove soundtrack/source subtitles: ' - From "..." Soundtrack', ' - from the film ...' - s = re.sub(r'\s*-\s*from\s+.+$', '', s, flags=re.IGNORECASE) - # Remove non-alphanumeric except spaces - s = re.sub(r'[^\w\s]', '', s) - # Collapse whitespace - s = re.sub(r'\s+', ' ', s).strip() - return s +# normalize() + similarity() + the alias-aware comparison now live in the shared +# decision core (core/matching/audio_verification.py) so import-time verification +# and the library scan share ONE definition — the <>-strip fix, CJK handling and +# thresholds can't drift apart again. Names kept (`_normalize` etc.) for existing +# importers/tests. +from core.matching.audio_verification import ( # noqa: E402 + normalize as _normalize, + similarity as _similarity, + _alias_aware_artist_sim, + _find_best_title_artist_match as _core_find_best_title_artist_match, + evaluate as _core_evaluate, + Decision as _CoreDecision, +) -def _similarity(a: str, b: str) -> float: - """Calculate similarity between two strings (0.0-1.0) after normalization.""" - na = _normalize(a) - nb = _normalize(b) - if not na or not nb: - return 0.0 - if na == nb: - return 1.0 - return SequenceMatcher(None, na, nb).ratio() - - -def _alias_aware_artist_sim( - expected_artist: str, - actual_artist: str, - aliases: Optional[Any] = None, -) -> float: - """Best artist-similarity across (expected, *aliases) vs actual. - - Issue #442 — when expected and actual are in different scripts - (e.g. `Hiroyuki Sawano` vs `澤野弘之`), raw `_similarity` scores - near 0% even though MusicBrainz aliases bridge them. Routes - through the pure helper so the verifier inherits one shared - contract. - - Returns the highest score across all candidates so existing - threshold checks (>= ARTIST_MATCH_THRESHOLD) keep their - semantics. When `aliases` is None or empty, behaves identically - to the prior raw `_similarity(expected, actual)` call. - - `aliases` accepts two shapes: - - - **Iterable** (list/tuple/set of strings): used directly. Used - by tests that already know the aliases. - - **Callable**: invoked LAZILY only when direct similarity - falls below the threshold. Lets the verifier pass a memoizing - thunk that resolves aliases (DB / cache / live MB) only when - needed. Verifications where the direct match already passes - never trigger the lookup chain — no wasted DB query for the - happy path. - - Diagnostic logging: emits an INFO line whenever an alias rescues - a comparison that direct similarity would have failed. Lets - future bug reports trace which alias triggered which PASS - decision (e.g. "this file passed because alias `澤野弘之` matched - the file's artist tag"). - """ - from core.matching.artist_aliases import artist_names_match - - direct = _similarity(expected_artist, actual_artist) - # Fast path — direct match already passes the threshold OR caller - # supplied no aliases handle. Avoids any lookup work. - if aliases is None: - return direct - if direct >= ARTIST_MATCH_THRESHOLD: - return direct - - # Resolve the iterable. Callable provider invoked NOW (lazily — - # the caller can memoize the result across multiple invocations - # within one verify_audio_file call). - resolved = aliases() if callable(aliases) else aliases - if not resolved: - return direct - - _matched, score = artist_names_match( - expected_artist, - actual_artist, - aliases=resolved, - threshold=ARTIST_MATCH_THRESHOLD, - similarity=_similarity, +def _find_best_title_artist_match(recordings, expected_title, expected_artist, + expected_artist_aliases=None): + """Back-compat wrapper around the shared core matcher (keeps the + ``expected_artist_aliases`` kwarg name for existing callers/tests).""" + return _core_find_best_title_artist_match( + recordings, expected_title, expected_artist, expected_artist_aliases, ) - # Diagnostic — alias rescued a comparison that direct would - # have failed. Worth logging at INFO since it's a user-visible - # decision (file PASS instead of FAIL). One line per rescue - # within a single verify call. - if score >= ARTIST_MATCH_THRESHOLD and direct < ARTIST_MATCH_THRESHOLD: - from core.matching.artist_aliases import best_alias_match - winner, _ = best_alias_match( - expected_artist, actual_artist, resolved, similarity=_similarity, - ) - logger.info( - "Artist alias rescued comparison: expected=%r vs actual=%r " - "(direct sim=%.2f, alias %r → score=%.2f)", - expected_artist, actual_artist, direct, winner, score, - ) - - return score - - -def _find_best_title_artist_match( - recordings: List[Dict[str, Any]], - expected_title: str, - expected_artist: str, - expected_artist_aliases: Optional[Any] = None, -) -> Tuple[Optional[Dict], float, float]: - """ - Find the AcoustID recording that best matches expected title/artist. - - Issue #442 — `expected_artist_aliases` (when supplied) is the - list of alternate spellings for `expected_artist` (Japanese - kanji, Cyrillic, etc.). Accepts either: - - - An iterable of alias strings (used eagerly), or - - A callable returning the list (resolved lazily — only fires - when at least one recording fails direct artist similarity). - - Each recording's artist is scored against (expected, *aliases) - and the best score wins. When the list is empty/omitted/None, - behavior is identical to the prior raw similarity comparison. - - Returns: - (best_recording, title_similarity, artist_similarity) - """ - best_rec = None - best_title_sim = 0.0 - best_artist_sim = 0.0 - best_combined = 0.0 - - for rec in recordings: - title = rec.get('title') or '' - artist = rec.get('artist') or '' - - title_sim = _similarity(expected_title, title) - artist_sim = _alias_aware_artist_sim( - expected_artist, artist, expected_artist_aliases, - ) - # Weight title higher since that's the primary identifier - combined = (title_sim * 0.6) + (artist_sim * 0.4) - - if combined > best_combined: - best_combined = combined - best_rec = rec - best_title_sim = title_sim - best_artist_sim = artist_sim - - return best_rec, best_title_sim, best_artist_sim - # Shared MusicBrainz client for enrichment lookups _mb_client = None @@ -466,241 +329,32 @@ class AcoustIDVerification: ) return _alias_cache['value'] - # Step 4: Find best title/artist match among AcoustID results - best_rec, title_sim, artist_sim = _find_best_title_artist_match( - recordings, expected_track_name, expected_artist_name, - expected_artist_aliases=_aliases_provider, + # Steps 4-5: delegate the PASS/SKIP/FAIL decision to the shared core + # (core/matching/audio_verification.evaluate) so import verification + # and the library scan apply identical logic. + outcome = _core_evaluate( + expected_track_name, expected_artist_name, recordings, + fingerprint_score=best_score, + aliases_provider=_aliases_provider, ) - - if not best_rec: - return VerificationResult.SKIP, "No recordings with title/artist info" - - matched_title = best_rec.get('title', '?') - matched_artist = best_rec.get('artist', '?') - logger.info( - f"Best match: '{matched_title}' by '{matched_artist}' " - f"(title_sim={title_sim:.2f}, artist_sim={artist_sim:.2f})" + "Best match: '%s' by '%s' (title_sim=%.2f, artist_sim=%.2f) -> %s", + outcome.matched_title, outcome.matched_artist, + outcome.title_sim, outcome.artist_sim, outcome.decision.value, ) - - # Step 4b: Version-mismatch gate. - # - # The ``_normalize`` step deliberately strips parentheticals and - # version tags ("(Instrumental)", "- Live", etc) so that legit - # name variations don't fail the title-similarity comparison. - # That same stripping made it impossible to tell a vocal track - # apart from its instrumental: "In My Feelings" and "In My - # Feelings (Instrumental)" both normalize to "in my feelings", - # the title sim ends up 1.0, and the file passes verification - # even though it's the wrong cut. - # - # Detect the version on each side BEFORE normalization runs. - # If the expected track and the AcoustID-matched recording - # disagree on version (one is original, the other is - # instrumental / live / remix / acoustic / etc), reject — the - # fingerprint identified a real song but it's not the one the - # caller asked for. - expected_version = _detect_title_version(expected_track_name) - matched_version = _detect_title_version(matched_title) - if expected_version != matched_version: - # Issue #607 (AfonsoG6): MusicBrainz often stores live - # recordings with bare titles ("Clarity") while the - # release entry carries the venue annotation ("Clarity - # (Live at Blossom Music Center, ...)"). The fingerprint - # correctly identifies the LIVE recording; only the - # title text is bare. Helper accepts the one-sided bare - # case when fingerprint + bare-title + artist all agree. - # Two-sided version mismatches (live vs remix etc) stay - # strict — those are genuinely different recordings. - if is_acceptable_version_mismatch( - expected_version, matched_version, - fingerprint_score=best_score, - title_similarity=title_sim, - artist_similarity=artist_sim, - ): - logger.info( - f"AcoustID version annotation differs (expected={expected_version}, " - f"matched={matched_version}) but fingerprint+title+artist all match — " - f"accepting (likely MB metadata gap on a live/version-annotated recording)" - ) - else: - msg = ( - f"Version mismatch: expected '{expected_track_name}' ({expected_version}) " - f"but file is '{matched_title}' ({matched_version})" - ) - logger.warning(f"AcoustID verification FAILED (version mismatch) - {msg}") - return VerificationResult.FAIL, msg - - # Step 5: Decide pass/fail based on similarity - if title_sim >= TITLE_MATCH_THRESHOLD and artist_sim >= ARTIST_MATCH_THRESHOLD: - msg = ( - f"Audio verified: '{matched_title}' by '{matched_artist}' " - f"matches expected '{expected_track_name}' by '{expected_artist_name}' " - f"(title={title_sim:.0%}, artist={artist_sim:.0%})" - ) - logger.info(f"AcoustID verification PASSED - {msg}") - return VerificationResult.PASS, msg - - # Title matches but artist doesn't — could be a cover/collab OR a - # genuinely different track with the same name. Distinguish the - # two by checking whether the expected artist appears anywhere in - # AcoustID's returned recordings. - if title_sim >= TITLE_MATCH_THRESHOLD and artist_sim < ARTIST_MATCH_THRESHOLD: - # First: if the expected artist is present in ANY recording's - # metadata for this fingerprint, it's likely the right track - # (AcoustID's "best" match just picked the wrong variant). - for rec in recordings: - rec_artist = rec.get('artist', '') - if _alias_aware_artist_sim( - expected_artist_name, rec_artist, _aliases_provider, - ) >= ARTIST_MATCH_THRESHOLD: - msg = ( - f"Audio verified: found '{expected_track_name}' by '{expected_artist_name}' " - f"in AcoustID results" - ) - logger.info(f"AcoustID verification PASSED (secondary match) - {msg}") - return VerificationResult.PASS, msg - - # Expected artist wasn't found anywhere. Decide between: - # - FAIL: clear mismatch, e.g. "Tom Walker" (sim ~0.2) when - # expecting "Maduk" — different song with same name - # - SKIP: ambiguous, e.g. collab / alt credit / formatting - # difference (sim 0.3-0.6) - # - # The 0.3 cutoff catches hard mismatches while preserving the - # benefit of the doubt for borderline artist formatting. - CLEAR_MISMATCH_THRESHOLD = 0.3 - if artist_sim < CLEAR_MISMATCH_THRESHOLD: - msg = ( - f"Audio mismatch: file identified as '{matched_title}' by '{matched_artist}', " - f"expected '{expected_track_name}' by '{expected_artist_name}' " - f"(title={title_sim:.0%}, artist={artist_sim:.0%}) — " - f"expected artist not found in any AcoustID recording" - ) - logger.warning(f"AcoustID verification FAILED (clear artist mismatch) - {msg}") - return VerificationResult.FAIL, msg - - msg = ( - f"Title matches but artist unclear: " - f"AcoustID='{matched_title}' by '{matched_artist}', " - f"expected '{expected_track_name}' by '{expected_artist_name}' " - f"(artist_sim={artist_sim:.0%} — ambiguous, could be cover/collab)" - ) - logger.info(f"AcoustID verification SKIPPED - {msg}") - return VerificationResult.SKIP, msg - - # Title doesn't match — check ALL recordings for any title/artist match - # (the best combined match might not be the right one if there are many results) - # Skip recordings whose version (instrumental/live/etc) disagrees with - # what the caller asked for — the version mismatch above checked - # only the best recording, but a wrong-version variant could still - # win this fallback scan if its bare title matched. - for rec in recordings: - t = rec.get('title') or '' - a = rec.get('artist') or '' - if _detect_title_version(t) != expected_version: - continue - if (_similarity(expected_track_name, t) >= TITLE_MATCH_THRESHOLD and - _alias_aware_artist_sim( - expected_artist_name, a, _aliases_provider, - ) >= ARTIST_MATCH_THRESHOLD): - msg = ( - f"Audio verified: found '{t}' by '{a}' in AcoustID results " - f"matching expected '{expected_track_name}' by '{expected_artist_name}'" - ) - logger.info(f"AcoustID verification PASSED (scan match) - {msg}") - return VerificationResult.PASS, msg - - # No match found — but if fingerprint score is very high (≥0.95) - # AND we have evidence the mismatch is a language/script case - # (rather than two genuinely different songs by the same artist), - # skip rather than quarantine a correct file. Two routes: - # - # (a) Either side of the comparison contains non-ASCII characters - # — strong signal of transliteration / kanji↔roman cases. - # Artist must still be a strong match to use this path. - # (b) Both title AND artist similarity are very high (the song - # is recognizably the same with minor punctuation / casing - # differences that fell below the strict match thresholds). - # - # The OLD logic was ``title_sim >= 0.55 OR artist_sim >= match``. - # That fired for English-vs-English songs by the same artist that - # share NO actual content — e.g. "R.O.T.C (Interlude)" by - # Kendrick Lamar getting accepted as "Rich (Interlude)" by - # Kendrick Lamar because the artist matched perfectly and - # "interlude" was shared in both titles. Reported by user when - # downloading Mr. Morale: three tracks (Rich Interlude, Savior - # Interlude, Savior) all received the wrong R.O.T.C audio file - # because of this leak. - # Use the BEST matching recording's strings here (not - # `recordings[0]`) so the failure message reports the same - # candidate the title/artist similarity scores came from. - # Issue #607 (AfonsoG6) example 1: the prior code mixed - # `recordings[0]`'s strings (which can be empty) with - # `best_rec`'s scores, producing nonsense reasons like - # "file identified as '' by '' (artist=100%)" when a later - # recording in the list scored well on artist. - display_title = matched_title or '?' - display_artist = matched_artist or '?' - has_non_ascii = ( - any(ord(c) > 127 for c in (expected_track_name or '')) - or any(ord(c) > 127 for c in display_title) - ) - language_script_skip = ( - best_score >= 0.95 - and has_non_ascii - and artist_sim >= ARTIST_MATCH_THRESHOLD - ) - high_confidence_strong_match_skip = ( - best_score >= 0.95 - and title_sim >= 0.80 - and artist_sim >= ARTIST_MATCH_THRESHOLD - ) - # 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 or cross_script_artist_skip) - else "title/artist match within tolerance" - ) - msg = ( - f"Title/artist mismatch but fingerprint confidence very high ({best_score:.2f}): " - f"AcoustID='{display_title}' by '{display_artist}', " - f"expected '{expected_track_name}' by '{expected_artist_name}' — " - f"{reason}" - ) - logger.info(f"AcoustID verification SKIPPED (high confidence) - {msg}") - return VerificationResult.SKIP, msg - - # Low fingerprint score + no metadata match — file is likely wrong. - msg = ( - f"Audio mismatch: file identified as '{display_title}' by '{display_artist}', " - f"expected '{expected_track_name}' by '{expected_artist_name}' " - f"(title={title_sim:.0%}, artist={artist_sim:.0%})" - ) - logger.warning(f"AcoustID verification FAILED - {msg}") - return VerificationResult.FAIL, msg + _decision_map = { + _CoreDecision.PASS: VerificationResult.PASS, + _CoreDecision.SKIP: VerificationResult.SKIP, + _CoreDecision.FAIL: VerificationResult.FAIL, + } + result = _decision_map[outcome.decision] + if result == VerificationResult.PASS: + logger.info("AcoustID verification PASSED - %s", outcome.reason) + elif result == VerificationResult.FAIL: + logger.warning("AcoustID verification FAILED - %s", outcome.reason) + else: + logger.info("AcoustID verification SKIPPED - %s", outcome.reason) + return result, outcome.reason except Exception as e: # Any unexpected error -> SKIP (fail open) diff --git a/core/matching/audio_verification.py b/core/matching/audio_verification.py index 72da6e8b..f72bdbb6 100644 --- a/core/matching/audio_verification.py +++ b/core/matching/audio_verification.py @@ -16,6 +16,10 @@ from difflib import SequenceMatcher from enum import Enum from typing import Any, List, Optional +from utils.logging_config import get_logger + +logger = get_logger("audio_verification") + # Thresholds — the single definition both paths share. MIN_ACOUSTID_SCORE = 0.80 # Minimum fingerprint score to trust a match. TITLE_MATCH_THRESHOLD = 0.70 # Title similarity to consider a match. @@ -117,6 +121,18 @@ def _alias_aware_artist_sim(expected_artist: str, actual_artist: str, expected_artist, actual_artist, aliases=resolved, threshold=ARTIST_MATCH_THRESHOLD, similarity=similarity, ) + # Diagnostic: an alias rescued a comparison direct similarity would have + # failed. INFO since it's a user-visible decision (PASS instead of FAIL). + if score >= ARTIST_MATCH_THRESHOLD and direct < ARTIST_MATCH_THRESHOLD: + from core.matching.artist_aliases import best_alias_match + winner, _ = best_alias_match( + expected_artist, actual_artist, resolved, similarity=similarity, + ) + logger.info( + "Artist alias rescued comparison: expected=%r vs actual=%r " + "(direct sim=%.2f, alias %r → score=%.2f)", + expected_artist, actual_artist, direct, winner, score, + ) return score diff --git a/tests/matching/test_acoustid_verification_aliases.py b/tests/matching/test_acoustid_verification_aliases.py index 987130c0..87874eeb 100644 --- a/tests/matching/test_acoustid_verification_aliases.py +++ b/tests/matching/test_acoustid_verification_aliases.py @@ -431,10 +431,10 @@ class TestAliasRescueLogging: records.append(record) handler = _ListHandler(level=_logging.INFO) - # Logger name is `soulsync.acoustid.verification` per - # `core.acoustid_verification`'s `get_logger("acoustid_verification")` - # — dot-separated, NOT underscored. - verifier_logger = _logging.getLogger('soulsync.acoustid.verification') + # The alias-aware comparison now lives in the shared core + # (`core.matching.audio_verification`, logger `audio_verification`), + # which is where the rescue diagnostic is emitted. + verifier_logger = _logging.getLogger('soulsync.audio_verification') verifier_logger.addHandler(handler) prior_level = verifier_logger.level verifier_logger.setLevel(_logging.INFO) From b981230d0748d904df6d07c2e2a97f4904c6e2a0 Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 10 Jun 2026 15:13:43 +0200 Subject: [PATCH 04/14] refactor(scanner): use shared verification core; stop false-flagging cross-script The library AcoustID scan now calls audio_verification.evaluate() (alias-aware artist match + cross-script SKIP) instead of its own non-ASCII-stripping _normalize and threshold logic, so it no longer false-flags correct anime-OST / kanji tracks. Duration-collision guard kept as a scanner pre-check on the top recording. evaluate() is now purely a title/artist/version/cross-script decision. Co-Authored-By: Claude Opus 4.8 --- core/matching/audio_verification.py | 17 +-- core/repair_jobs/acoustid_scanner.py | 161 +++++++++------------------ tests/test_acoustid_scanner.py | 41 ++++++- 3 files changed, 95 insertions(+), 124 deletions(-) diff --git a/core/matching/audio_verification.py b/core/matching/audio_verification.py index f72bdbb6..cd1b2c17 100644 --- a/core/matching/audio_verification.py +++ b/core/matching/audio_verification.py @@ -159,20 +159,18 @@ def _find_best_title_artist_match(recordings, expected_title, expected_artist, def evaluate(expected_title: str, expected_artist: str, recordings: List[dict], *, fingerprint_score: float, - file_duration_s: Optional[float] = None, aliases_provider: Optional[Any] = None) -> Outcome: """Decide PASS / SKIP / FAIL for a fingerprinted file against expected title/artist. Pure: no I/O. Shared by import verification and library scan. ``aliases_provider``: iterable or callable of expected-artist aliases (kanji/cyrillic/etc) used to bridge cross-script comparisons. - ``file_duration_s``: when provided, a strong duration mismatch downgrades a - would-be FAIL to SKIP (fingerprint hash collision guard, used by the scan). + + Note: fingerprint-collision duration checks are the caller's responsibility + (the library scan pre-checks the top recording's length before calling this) + so the decision here stays purely about title/artist/version identity. """ from core.matching.script_compat import is_cross_script_mismatch - from core.matching.acoustid_candidates import ( - duration_mismatches_strongly, find_matching_recording, - ) from core.matching.version_mismatch import is_acceptable_version_mismatch best_rec, title_sim, artist_sim = _find_best_title_artist_match( @@ -212,9 +210,6 @@ def evaluate(expected_title: str, expected_artist: str, ) >= ARTIST_MATCH_THRESHOLD: return out(Decision.PASS, "Expected artist found in AcoustID results") if artist_sim < CLEAR_MISMATCH_THRESHOLD: - if file_duration_s and duration_mismatches_strongly( - file_duration_s, best_rec.get('duration') or best_rec.get('length')): - return out(Decision.SKIP, "Duration mismatch (fingerprint collision)") return out(Decision.FAIL, f"Audio mismatch: '{matched_title}' by '{matched_artist}' " f"— expected artist not found") @@ -255,10 +250,6 @@ def evaluate(expected_title: str, expected_artist: str, or cross_script_artist_skip): return out(Decision.SKIP, "Likely same song in different language/script") - if file_duration_s and duration_mismatches_strongly( - file_duration_s, best_rec.get('duration') or best_rec.get('length')): - return out(Decision.SKIP, "Duration mismatch (fingerprint collision)") - return out(Decision.FAIL, f"Audio mismatch: file identified as '{matched_title}' by " f"'{matched_artist}', expected '{expected_title}' by '{expected_artist}'") diff --git a/core/repair_jobs/acoustid_scanner.py b/core/repair_jobs/acoustid_scanner.py index 00f900a1..f7a4acba 100644 --- a/core/repair_jobs/acoustid_scanner.py +++ b/core/repair_jobs/acoustid_scanner.py @@ -15,6 +15,9 @@ from typing import Optional from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger +from core.matching.audio_verification import evaluate, Decision +from core.matching.acoustid_candidates import duration_mismatches_strongly +from core.acoustid_verification import _resolve_expected_artist_aliases logger = get_logger("repair_job.acoustid") @@ -220,112 +223,60 @@ class AcoustIDScannerJob(RepairJob): or expected['artist'] ) - # Normalize and compare - norm_expected_title = _normalize(expected['title']) - norm_aid_title = _normalize(aid_title) - norm_expected_artist = _normalize(expected_artist) - norm_aid_artist = _normalize(aid_artist) - - title_sim = SequenceMatcher(None, norm_expected_title, norm_aid_title).ratio() - # Issue (Foxxify Discord report): AcoustID returns the FULL artist - # credit (e.g. `Okayracer, aldrch & poptropicaslutz!`) while the - # library DB carries only the primary artist (`Okayracer`). Raw - # similarity scores ~43% — well below threshold — so multi-artist - # tracks get flagged as Wrong Song even though the primary IS in - # the credit. Route through the shared `artist_names_match` helper - # which splits the credit on common separators (comma, ampersand, - # feat./ft./with/vs., etc.) and checks each token. Primary-in- - # credit cases now resolve at 100% match instead of 43%. - # - # Pass RAW artist strings (not pre-normalised) so the splitter - # can recognise the separators. The helper applies its own - # case + whitespace normalisation internally per token. - if norm_expected_artist: - from core.matching.artist_aliases import artist_names_match - - _, artist_sim = artist_names_match( - expected_artist, - aid_artist, - threshold=artist_threshold, - ) - else: - artist_sim = 1.0 - - if title_sim >= title_threshold and artist_sim >= artist_threshold: - return - - # Issue #587 (Foxxify) — top recording's metadata mismatched, but - # AcoustID often returns multiple recordings per fingerprint - # (sample collisions, multi-MB-record cases). Check ALL of them - # before flagging — if any candidate's metadata matches expected - # title + artist, the file IS the right song and AcoustID's top - # match was just a wrong-credited recording. - from core.matching.acoustid_candidates import ( - duration_mismatches_strongly, - find_matching_recording, - ) - from core.matching.artist_aliases import artist_names_match - - def _scanner_title_sim(a, b): - return SequenceMatcher(None, _normalize(a), _normalize(b)).ratio() - - def _scanner_artist_sim(expected_a, actual_a): - _, score = artist_names_match(expected_a, actual_a, threshold=artist_threshold) - return score - - candidate_match, _, _ = find_matching_recording( - fp_result.get('recordings') or [], - expected['title'], - expected_artist, - title_threshold=title_threshold, - artist_threshold=artist_threshold, - similarity=_scanner_title_sim, - artist_similarity=_scanner_artist_sim, - ) - if candidate_match is not None: - # A lower-ranked candidate matched — file IS the right song. - # No finding. + # Fingerprint-collision guard: when the TOP recording's length is wildly + # different from the file, the fingerprint hit is a hash collision (the + # 17-min mashup → 5-min track case), not a real match — skip BEFORE any + # title/artist/version analysis so it can't surface as a false finding. + try: + file_duration_s = (expected.get('duration_ms') or 0) / 1000.0 + except Exception: + file_duration_s = 0.0 + cand_duration_s = best_recording.get('duration') or best_recording.get('length') + if file_duration_s and duration_mismatches_strongly(file_duration_s, cand_duration_s): if context.report_progress: context.report_progress( - log_line=( - f'Resolved (lower-ranked candidate match): {fname} — ' - f'expected "{expected["title"]}" matched candidate ' - f'"{candidate_match.get("title")}" by ' - f'"{candidate_match.get("artist")}"' - ), + log_line=(f'Skipped (duration mismatch suggests fingerprint ' + f'collision): {fname}'), + log_type='skip') + return + + # Decision via the shared verification core — identical logic to import- + # time verification (alias-aware artist match + cross-script SKIP), so the + # scan no longer false-flags correct cross-script tracks. Only a FAIL + # produces a "Wrong download" finding. + _alias_cache = {} + + def _aliases(): + if 'v' not in _alias_cache: + try: + _alias_cache['v'] = _resolve_expected_artist_aliases(expected_artist) + except Exception: + _alias_cache['v'] = [] + return _alias_cache['v'] + + outcome = evaluate( + expected['title'], expected_artist, fp_result['recordings'], + fingerprint_score=best_score, + aliases_provider=_aliases, + ) + + if outcome.decision != Decision.FAIL: + if context.report_progress: + context.report_progress( + log_line=f'OK ({outcome.decision.value}): {fname} — {outcome.reason}', log_type='ok', ) return - # Issue #587 (Foxxify "17min mashup → 5min track") — duration - # guard against fingerprint hash collisions. When the file's - # actual duration differs from AcoustID's matched recording by - # more than max(60s, 35%), the fingerprint is almost certainly - # a sample/intro collision, not a real recording match. Don't - # produce a confident "Wrong Song" finding. - try: - file_duration_s = (expected.get('duration_ms') or 0) / 1000.0 - except Exception: - file_duration_s = 0 - candidate_duration_s = best_recording.get('duration') - if candidate_duration_s is None and best_recording.get('length'): - candidate_duration_s = best_recording.get('length') - if duration_mismatches_strongly(file_duration_s, candidate_duration_s): - if context.report_progress: - context.report_progress( - log_line=( - f'Skipped (duration mismatch suggests fingerprint collision): ' - f'{fname} — expected {file_duration_s:.0f}s, AcoustID ' - f'candidate {candidate_duration_s:.0f}s' - ), - log_type='skip', - ) - return + title_sim = outcome.title_sim + artist_sim = outcome.artist_sim + matched_title = outcome.matched_title or aid_title + matched_artist = outcome.matched_artist or aid_artist - # Mismatch detected + # Mismatch (FAIL) — create finding. if context.report_progress: context.report_progress( - log_line=f'Mismatch: {fname} — expected "{expected["title"]}", got "{aid_title}"', + log_line=f'Mismatch: {fname} — expected "{expected["title"]}", got "{matched_title}"', log_type='error' ) if context.create_finding: @@ -337,18 +288,18 @@ class AcoustIDScannerJob(RepairJob): entity_type='track', entity_id=str(track_id), file_path=fpath, - title=f'Wrong download: "{expected["title"]}" is actually "{aid_title}"', + title=f'Wrong download: "{expected["title"]}" is actually "{matched_title}"', description=( f'Expected "{expected["title"]}" by {expected_artist}, ' - f'but audio fingerprint matches "{aid_title}" by {aid_artist} ' + f'but audio fingerprint matches "{matched_title}" by {matched_artist} ' f'(fingerprint: {best_score:.0%}, title match: {title_sim:.0%}, ' f'artist match: {artist_sim:.0%})' ), details={ 'expected_title': expected['title'], 'expected_artist': expected_artist, - 'acoustid_title': aid_title, - 'acoustid_artist': aid_artist, + 'acoustid_title': matched_title, + 'acoustid_artist': matched_artist, 'fingerprint_score': round(best_score, 3), 'title_similarity': round(title_sim, 3), 'artist_similarity': round(artist_sim, 3), @@ -480,11 +431,3 @@ class AcoustIDScannerJob(RepairJob): finally: if conn: conn.close() - - -def _normalize(text: str) -> str: - t = text.lower() - t = re.sub(r'\(.*?\)', '', t) - t = re.sub(r'\[.*?\]', '', t) - t = re.sub(r'[^a-z0-9 ]', '', t) - return t.strip() diff --git a/tests/test_acoustid_scanner.py b/tests/test_acoustid_scanner.py index 1993b099..9c2f7413 100644 --- a/tests/test_acoustid_scanner.py +++ b/tests/test_acoustid_scanner.py @@ -199,7 +199,11 @@ def test_scanner_still_flags_genuine_artist_mismatch(): 'best_score': 0.99, 'recordings': [{ 'title': 'Some Track', - 'artist': 'Different Band, Other Person & Random Featuring', + # Clearly-different multi-value credit (artist sim < 0.30). The + # unified core gives 0.30-0.60 ("ambiguous") the benefit of the + # doubt, so a genuine-mismatch assertion needs an artist that's + # unambiguously different. + 'artist': 'Metallica, Slayer & Anthrax', }], }, ) @@ -468,7 +472,9 @@ def test_scanner_falls_back_to_db_when_file_tag_missing(monkeypatch): 'best_score': 0.99, 'recordings': [{ 'title': 'Some Track', - 'artist': 'Different Band', + # Unambiguously different artist (sim < 0.30) so the unified + # core flags it (0.30-0.60 would be treated as ambiguous). + 'artist': 'Metallica', }], }, ) @@ -779,3 +785,34 @@ def test_scanner_still_flags_when_duration_matches(): ) assert len(captured_findings) == 1 + + +def test_scanner_does_not_flag_cross_script_when_alias_bridges(monkeypatch): + """Anime-OST track: AcoustID returns the kanji artist with a + credit. With the MusicBrainz alias bridging 澤野弘之 ↔ Sawano Hiroyuki, the + unified verification core recognises the match, so the library scan must NOT + create a false 'Wrong download' finding (it did before, stripping all + non-ASCII and never consulting aliases).""" + import core.repair_jobs.acoustid_scanner as scanner_mod + monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", + lambda name: ["澤野弘之"], raising=False) + job = AcoustIDScannerJob() + captured = [] + context = _make_finding_capturing_context( + track_row=("7", "Call Your Name", "Sawano Hiroyuki", + "/music/cyn.flac", 15, "Attack on Titan OST", None, None), + captured=captured, + ) + fake_acoustid = SimpleNamespace( + fingerprint_and_lookup=lambda fpath: { + 'best_score': 0.97, + 'recordings': [{'title': 'call your name', + 'artist': '澤野弘之 '}], + }, + ) + result = JobResultStub() + job._scan_file('/music/cyn.flac', '7', + {'title': 'Call Your Name', 'artist': 'Sawano Hiroyuki'}, + fake_acoustid, context, result, + fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6) + assert captured == [], f"cross-script track false-flagged: {captured}" From 8e6820dbdf6cf6d5368c3372a1aefb9df4756fe0 Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 10 Jun 2026 18:30:49 +0200 Subject: [PATCH 05/14] feat(verification): status vocabulary, DB column, SOULSYNC_VERIFICATION tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also: evaluate() treats an empty expected artist as title-only comparison (old scanner behaviour — a missing DB artist is no evidence of a wrong file), and the thresholds are now defined once in the core and re-exported. Co-Authored-By: Claude Opus 4.8 --- core/acoustid_verification.py | 9 ++-- core/matching/audio_verification.py | 7 +++ core/matching/verification_status.py | 48 ++++++++++++++++++ core/tag_writer.py | 46 +++++++++++++++++ database/music_database.py | 5 ++ .../matching/test_audio_verification_core.py | 9 ++++ tests/matching/test_verification_status.py | 26 ++++++++++ tests/test_verification_tag_roundtrip.py | 50 +++++++++++++++++++ 8 files changed, 196 insertions(+), 4 deletions(-) create mode 100644 core/matching/verification_status.py create mode 100644 tests/matching/test_verification_status.py create mode 100644 tests/test_verification_tag_roundtrip.py diff --git a/core/acoustid_verification.py b/core/acoustid_verification.py index a399dda6..0d10b613 100644 --- a/core/acoustid_verification.py +++ b/core/acoustid_verification.py @@ -22,10 +22,11 @@ from core.musicbrainz_client import MusicBrainzClient logger = get_logger("acoustid.verification") -# Thresholds -MIN_ACOUSTID_SCORE = 0.80 # Minimum AcoustID fingerprint score to trust -TITLE_MATCH_THRESHOLD = 0.70 # Title similarity needed to consider a match -ARTIST_MATCH_THRESHOLD = 0.60 # Artist similarity needed to consider a match +# Thresholds — single definition lives in the shared core; re-exported here so +# existing importers keep working and the values can't drift between paths. +from core.matching.audio_verification import ( # noqa: E402 + MIN_ACOUSTID_SCORE, TITLE_MATCH_THRESHOLD, ARTIST_MATCH_THRESHOLD, +) # Single matching-engine instance so version detection reuses the same patterns # used by the pre-download Soulseek matcher (remix / live / acoustic / diff --git a/core/matching/audio_verification.py b/core/matching/audio_verification.py index cd1b2c17..06b90949 100644 --- a/core/matching/audio_verification.py +++ b/core/matching/audio_verification.py @@ -173,9 +173,16 @@ def evaluate(expected_title: str, expected_artist: str, from core.matching.script_compat import is_cross_script_mismatch from core.matching.version_mismatch import is_acceptable_version_mismatch + # No expected artist on record (legacy/compilation rows): compare on title + # only — the old scanner treated this as artist-match=1.0 and a missing DB + # value is no evidence the file is wrong. + no_expected_artist = not normalize(expected_artist or '') + best_rec, title_sim, artist_sim = _find_best_title_artist_match( recordings, expected_title, expected_artist, aliases_provider, ) + if no_expected_artist: + artist_sim = 1.0 if not best_rec: return Outcome(Decision.SKIP, reason="No recordings with title/artist info") diff --git a/core/matching/verification_status.py b/core/matching/verification_status.py new file mode 100644 index 00000000..4b7569c9 --- /dev/null +++ b/core/matching/verification_status.py @@ -0,0 +1,48 @@ +"""Verification-status vocabulary for imported tracks. + +Three states, persisted in the DB (``tracks.verification_status``) AND as an +embedded file tag (``SOULSYNC_VERIFICATION``) so the information survives DB +resets and travels with the file: + +- ``verified`` — clean AcoustID PASS at import time. +- ``unverified`` — AcoustID SKIP (cross-script / ambiguous / no match in + the AcoustID DB). Imported, but not hard-confirmed. +- ``force_imported`` — accepted via the version-mismatch fallback after the + retry budget was exhausted (user opted in via + ``post_processing.accept_version_mismatch_fallback``). + A later library scan will still re-check these but + reports them as informational, clearly marked. + +Quarantined files are never imported, so they carry no status. +""" + +VERIFIED = 'verified' +UNVERIFIED = 'unverified' +FORCE_IMPORTED = 'force_imported' + +ALL_STATUSES = (VERIFIED, UNVERIFIED, FORCE_IMPORTED) + +# The file tag name (Vorbis comment key / ID3 TXXX desc / MP4 freeform). +TAG_NAME = 'SOULSYNC_VERIFICATION' + + +def status_from_acoustid_result(result_value): + """Map an AcoustID verification result string ('pass'/'skip'/...) to a + status. 'disabled'/'error'/unknown return None — no claim either way.""" + if result_value == 'pass': + return VERIFIED + if result_value == 'skip': + return UNVERIFIED + return None + + +def status_for_import(context: dict): + """Status for a just-imported file from its pipeline context. + + The version-mismatch fallback flag wins: a force-accepted file is + ``force_imported`` regardless of what the (earlier, failed) verification + said about the candidate. + """ + if context.get('_version_mismatch_fallback'): + return FORCE_IMPORTED + return status_from_acoustid_result(context.get('_acoustid_result')) diff --git a/core/tag_writer.py b/core/tag_writer.py index b0d4c6fa..816e69cd 100644 --- a/core/tag_writer.py +++ b/core/tag_writer.py @@ -44,6 +44,8 @@ def read_file_tags(file_path: str) -> Dict[str, Any]: 'replaygain_track_peak': None, 'replaygain_album_gain': None, 'replaygain_album_peak': None, + # SoulSync verification status ('verified'/'unverified'/'force_imported') + 'verification_status': None, } if not file_path or not os.path.exists(file_path): @@ -80,6 +82,10 @@ def read_file_tags(file_path: str) -> Dict[str, Any]: except (ValueError, TypeError): pass result['has_cover_art'] = bool(audio.tags.getall('APIC')) + for fr in audio.tags.getall('TXXX'): + if getattr(fr, 'desc', '') == 'SOULSYNC_VERIFICATION' and fr.text: + result['verification_status'] = str(fr.text[0]) + break elif isinstance(audio, (FLAC, OggVorbis)) or type(audio).__name__ == 'OggOpus': # FLAC / OGG @@ -102,6 +108,7 @@ def read_file_tags(file_path: str) -> Dict[str, Any]: else: # OGG doesn't have a standard picture field we can easily check result['has_cover_art'] = False + result['verification_status'] = _vorbis_first(audio, 'soulsync_verification') elif isinstance(audio, MP4): # MP4 / M4A @@ -118,6 +125,12 @@ def read_file_tags(file_path: str) -> Dict[str, Any]: if disk: result['disc_number'] = disk[0][0] if isinstance(disk[0], tuple) else None result['has_cover_art'] = bool(audio.tags.get('covr', [])) if audio.tags else False + vs = (audio.tags or {}).get('----:com.soulsync:VERIFICATION') + if vs: + raw = vs[0] + result['verification_status'] = ( + raw.decode('utf-8', 'ignore') if isinstance(raw, bytes) else str(raw) + ) except Exception as e: result['error'] = str(e) @@ -156,6 +169,39 @@ def is_placeholder_meta(value: Any) -> bool: return s == '' or s in _PLACEHOLDER_META_VALUES +def write_verification_status(file_path: str, status: str) -> bool: + """Embed the SoulSync verification status into the file's tags. + + Vorbis comment ``SOULSYNC_VERIFICATION`` (FLAC/OGG/Opus), ID3 + ``TXXX:SOULSYNC_VERIFICATION`` (MP3), MP4 freeform + ``----:com.soulsync:VERIFICATION``. The tag travels with the file so the + status survives DB resets; the AcoustID scan reads it back via + ``read_file_tags`` to refresh the DB column and to mark force-imported + fallbacks in its findings. Never raises; returns success. + """ + if not status or not file_path or not os.path.exists(file_path): + return False + try: + audio = MutagenFile(file_path) + if audio is None: + return False + if getattr(audio, 'tags', None) is None and hasattr(audio, 'add_tags'): + audio.add_tags() + if isinstance(audio.tags, ID3): + audio.tags.delall('TXXX:SOULSYNC_VERIFICATION') + audio.tags.add(TXXX(encoding=3, desc='SOULSYNC_VERIFICATION', text=[status])) + elif isinstance(audio, MP4): + audio.tags['----:com.soulsync:VERIFICATION'] = [status.encode('utf-8')] + else: + # Vorbis-comment family (FLAC / OggVorbis / OggOpus) + audio['SOULSYNC_VERIFICATION'] = [status] + audio.save() + return True + except Exception as e: + logger.debug("write_verification_status failed for %s: %s", file_path, e) + return False + + def guard_placeholder_overwrite(db_val: Any, file_val: Any) -> Any: """#800 guard: never replace a real file value with a placeholder. diff --git a/database/music_database.py b/database/music_database.py index 7b352a34..2b2b3881 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -2409,6 +2409,11 @@ class MusicDatabase: if 'musicbrainz_match_status' not in tracks_columns: cursor.execute("ALTER TABLE tracks ADD COLUMN musicbrainz_match_status TEXT") added_tracks = True + if 'verification_status' not in tracks_columns: + # 'verified' / 'unverified' / 'force_imported' — set at import, + # refreshed by the AcoustID scan (which reads the file tag). + cursor.execute("ALTER TABLE tracks ADD COLUMN verification_status TEXT") + added_tracks = True if added_tracks: columns_added = True logger.info("Added MusicBrainz columns to tracks table") diff --git a/tests/matching/test_audio_verification_core.py b/tests/matching/test_audio_verification_core.py index efc8162c..09f6a2aa 100644 --- a/tests/matching/test_audio_verification_core.py +++ b/tests/matching/test_audio_verification_core.py @@ -72,3 +72,12 @@ def test_normalize_strips_version_and_featuring(): def test_normalize_keeps_plain_text(): assert normalize("Sawano Hiroyuki") == "sawano hiroyuki" + + +def test_empty_expected_artist_does_not_fail(): + # Old scanner treated a missing expected artist as artist-match=1.0 + # (compare title only). The unified core must not FAIL a track just + # because the DB has no artist value. + out = evaluate("Some Track", "", [_rec("Some Track", "Whoever")], + fingerprint_score=0.95) + assert out.decision == Decision.PASS diff --git a/tests/matching/test_verification_status.py b/tests/matching/test_verification_status.py new file mode 100644 index 00000000..d488710e --- /dev/null +++ b/tests/matching/test_verification_status.py @@ -0,0 +1,26 @@ +"""Verification-status vocabulary + mapping (DB column / file tag / UI badge).""" + +from core.matching.verification_status import ( + VERIFIED, UNVERIFIED, FORCE_IMPORTED, + status_from_acoustid_result, status_for_import, +) + + +def test_acoustid_result_maps_to_status(): + assert status_from_acoustid_result('pass') == VERIFIED + assert status_from_acoustid_result('skip') == UNVERIFIED + # disabled / error / unknown -> no claim either way + assert status_from_acoustid_result('disabled') is None + assert status_from_acoustid_result('error') is None + assert status_from_acoustid_result(None) is None + + +def test_force_import_context_wins_over_acoustid(): + ctx = {'_version_mismatch_fallback': 'instrumental', '_acoustid_result': 'pass'} + assert status_for_import(ctx) == FORCE_IMPORTED + + +def test_status_for_import_falls_back_to_acoustid_result(): + assert status_for_import({'_acoustid_result': 'pass'}) == VERIFIED + assert status_for_import({'_acoustid_result': 'skip'}) == UNVERIFIED + assert status_for_import({}) is None diff --git a/tests/test_verification_tag_roundtrip.py b/tests/test_verification_tag_roundtrip.py new file mode 100644 index 00000000..974dd7b0 --- /dev/null +++ b/tests/test_verification_tag_roundtrip.py @@ -0,0 +1,50 @@ +"""SOULSYNC_VERIFICATION file tag: write + read back (travels with the file, +survives DB resets; the AcoustID scan reads it to refresh the DB column).""" + +import shutil +import subprocess + +import pytest + +from core.tag_writer import read_file_tags, write_verification_status + + +pytestmark = pytest.mark.skipif( + shutil.which('ffmpeg') is None, reason='ffmpeg required to build test audio' +) + + +def _make_flac(path): + subprocess.run( + ['ffmpeg', '-loglevel', 'error', '-y', '-f', 'lavfi', + '-i', 'anullsrc=r=44100:cl=mono', '-t', '0.1', str(path)], + check=True, + ) + + +def test_flac_verification_tag_roundtrip(tmp_path): + f = tmp_path / 'x.flac' + _make_flac(f) + assert write_verification_status(str(f), 'force_imported') is True + tags = read_file_tags(str(f)) + assert tags.get('verification_status') == 'force_imported' + + +def test_overwrite_existing_status(tmp_path): + f = tmp_path / 'y.flac' + _make_flac(f) + write_verification_status(str(f), 'unverified') + write_verification_status(str(f), 'verified') + assert read_file_tags(str(f)).get('verification_status') == 'verified' + + +def test_missing_file_returns_false_not_raises(tmp_path): + assert write_verification_status(str(tmp_path / 'nope.flac'), 'verified') is False + + +def test_db_migration_adds_verification_status_column(tmp_path): + from database.music_database import MusicDatabase + db = MusicDatabase(str(tmp_path / 't.db')) + with db._get_connection() as conn: + cols = [r[1] for r in conn.execute('PRAGMA table_info(tracks)').fetchall()] + assert 'verification_status' in cols From 9d1d09a57141a9566bc3a1dfd9ec66c64a6a4d9f Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 10 Jun 2026 18:36:23 +0200 Subject: [PATCH 06/14] feat(verification): persist status (db+tag), surface on Downloads, scan-aware force-imports - import pipeline writes SOULSYNC_VERIFICATION tag + context status (verified / unverified / force_imported via version-mismatch fallback) - downloads payload + UI badge (tooltip explains each state) - AcoustID scan reads the tag: refreshes tracks.verification_status, reports force-imported mismatches as informational (clearly marked), optional skip via job setting skip_force_imported - evaluate(): empty expected artist = title-only comparison (old scanner behaviour); thresholds single-sourced in the core Co-Authored-By: Claude Opus 4.8 --- core/auto_import_worker.py | 39 +- core/downloads/status.py | 4 + core/imports/folder_artist.py | 52 +++ core/imports/pipeline.py | 18 + core/repair_jobs/acoustid_scanner.py | 45 ++- ...06-10-acoustid-verification-unification.md | 378 ++++++++++++++++++ ...coustid-verification-unification-design.md | 153 +++++++ tests/imports/test_folder_artist_override.py | 85 ++++ ...st_acoustid_normalize_angle_annotations.py | 36 ++ tests/test_acoustid_scanner.py | 52 +++ webui/index.html | 16 + webui/static/downloads.js | 27 +- webui/static/settings.js | 3 + webui/static/style.css | 6 + 14 files changed, 886 insertions(+), 28 deletions(-) create mode 100644 core/imports/folder_artist.py create mode 100644 docs/superpowers/plans/2026-06-10-acoustid-verification-unification.md create mode 100644 docs/superpowers/specs/2026-06-10-acoustid-verification-unification-design.md create mode 100644 tests/imports/test_folder_artist_override.py create mode 100644 tests/test_acoustid_normalize_angle_annotations.py diff --git a/core/auto_import_worker.py b/core/auto_import_worker.py index 2b49bf48..39cc0c22 100644 --- a/core/auto_import_worker.py +++ b/core/auto_import_worker.py @@ -21,6 +21,7 @@ from datetime import datetime from difflib import SequenceMatcher from typing import Any, Callable, Dict, List, Optional +from core.imports.folder_artist import resolve_folder_artist from utils.logging_config import get_logger logger = get_logger("auto_import") @@ -1576,31 +1577,21 @@ class AutoImportWorker: album_name = identification.get('album_name', 'Unknown') image_url = identification.get('image_url', '') - # Parent folder artist override: if the staging folder structure is - # Artist/Albums/AlbumName or Artist/AlbumName, use the parent folder - # as the artist name when the tag-extracted artist looks wrong. - # This handles mixtapes/compilations where embedded tags have DJ names. + # Parent folder artist override — OPT-IN via import.folder_artist_override + # (default off). When enabled it uses the top Staging folder as the artist + # for Artist/Album or Artist//Album layouts, which helps + # mixtapes/compilations whose embedded tags carry DJ names. Off by default + # because it otherwise clobbers a confidently metadata-identified artist + # when a user stages a mixed pile under a single container folder (the + # "soulsync" mass-mislabel incident). try: - staging_root = self._resolve_staging_path() or self.staging_path - rel_path = os.path.relpath(candidate.path, staging_root) - parts = [p for p in rel_path.replace('\\', '/').split('/') if p] - - # parts[0] = artist folder, parts[1] = album or category subfolder, etc. - # Only attempt override if there's at least 2 levels (artist/album) - folder_artist = None - if len(parts) >= 2: - _category_names = {'albums', 'singles', 'eps', 'compilations', 'mixtapes', - 'discography', 'music', 'downloads'} - if len(parts) >= 3 and parts[1].lower() in _category_names: - # Artist/Albums/AlbumFolder → parts[0] is artist - folder_artist = parts[0] - elif parts[0].lower() not in _category_names: - # Artist/AlbumFolder → parts[0] is artist - folder_artist = parts[0] - - if folder_artist and folder_artist.lower() != artist_name.lower(): - logger.info(f"[Auto-Import] Parent folder artist '{folder_artist}' differs from tag artist '{artist_name}' — using folder artist") - artist_name = folder_artist + if self._config_manager.get('import.folder_artist_override', False): + staging_root = self._resolve_staging_path() or self.staging_path + rel_path = os.path.relpath(candidate.path, staging_root) + folder_artist = resolve_folder_artist(rel_path, artist_name, enabled=True) + if folder_artist: + logger.info(f"[Auto-Import] Parent folder artist '{folder_artist}' differs from tag artist '{artist_name}' — using folder artist") + artist_name = folder_artist except Exception as e: logger.debug("folder artist override failed: %s", e) release_date = identification.get('release_date', '') or album_data.get('release_date', '') diff --git a/core/downloads/status.py b/core/downloads/status.py index 010581b9..46016634 100644 --- a/core/downloads/status.py +++ b/core/downloads/status.py @@ -340,6 +340,9 @@ def build_batch_status_data(batch_id: str, batch: dict, live_transfers_lookup: d 'error_message': task.get('error_message'), # Surface failure reasons to UI 'quarantine_entry_id': task.get('quarantine_entry_id'), 'has_candidates': bool(task.get('cached_candidates')), # Whether search found results (for clickable review) + # 'verified' / 'unverified' / 'force_imported' — set by the + # import pipeline once post-processing finishes. + 'verification_status': task.get('verification_status'), } _ti = task.get('track_info') if isinstance(task.get('track_info'), dict) else {} task_filename = task.get('filename') or _ti.get('filename') @@ -737,6 +740,7 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: 'status': status, 'progress': progress, 'error': task.get('error_message'), + 'verification_status': task.get('verification_status'), 'batch_id': batch_id, 'batch_name': batch.get('playlist_name') or batch.get('album_name') or '', 'batch_source': batch.get('source_page') or batch.get('initiated_from') or '', diff --git a/core/imports/folder_artist.py b/core/imports/folder_artist.py new file mode 100644 index 00000000..61faecb2 --- /dev/null +++ b/core/imports/folder_artist.py @@ -0,0 +1,52 @@ +"""Opt-in "parent folder artist" resolution for imports. + +Historically the auto-import worker derived the artist from the top Staging +folder whenever the path had >=2 levels and that folder wasn't a category word +(albums/singles/eps/...). It did so *unconditionally*, overriding even a +confidently metadata-identified artist — which mass-mislabelled files when a +user staged everything under one container folder (see the "soulsync" incident). + +This module isolates that decision as a pure function so it can be: +- gated behind an opt-in setting (``import.folder_artist_override``, + default off), and +- unit-tested without standing up the whole import worker. +""" + +import os + +# Top-level folder names that denote a *category*, not an artist. +DEFAULT_CATEGORY_NAMES = frozenset({ + 'albums', 'singles', 'eps', 'compilations', 'mixtapes', + 'discography', 'music', 'downloads', +}) + + +def resolve_folder_artist(rel_path, identified_artist, enabled, + category_names=DEFAULT_CATEGORY_NAMES): + """Return the folder-derived artist to use, or ``None`` to keep the + already-identified artist. + + When ``enabled`` is False this always returns ``None`` — the import keeps + whatever artist the metadata match produced. Only when explicitly enabled + does it fall back to the staging folder name, and even then never when the + folder already equals the identified artist. + + ``rel_path`` is the candidate's path relative to the staging root. + """ + if not enabled: + return None + + parts = [p for p in rel_path.replace('\\', '/').split('/') if p] + + folder_artist = None + if len(parts) >= 2: + if len(parts) >= 3 and parts[1].lower() in category_names: + # Artist/Albums/AlbumFolder -> parts[0] is the artist + folder_artist = parts[0] + elif parts[0].lower() not in category_names: + # Artist/AlbumFolder -> parts[0] is the artist + folder_artist = parts[0] + + if folder_artist and folder_artist.lower() != (identified_artist or '').lower(): + return folder_artist + return None diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index 792af6d6..3995173a 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -944,6 +944,20 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta final_path = downsampled_path context['_final_processed_path'] = final_path + # Persist the verification status (verified / unverified / force_imported) + # as an embedded tag so it travels with the file and survives DB resets. + # Written BEFORE the lossy copy so the copy inherits it. Also stashed on + # the context so the wrapper can surface it on the Downloads page. + try: + from core.matching.verification_status import status_for_import + from core.tag_writer import write_verification_status + _verif_status = status_for_import(context) + if _verif_status: + context['_verification_status'] = _verif_status + write_verification_status(final_path, _verif_status) + except Exception as _vs_err: + logger.debug(f"verification-status persist skipped: {_vs_err}") + blasphemy_path = create_lossy_copy(final_path) if blasphemy_path: context['_final_processed_path'] = blasphemy_path @@ -1219,6 +1233,8 @@ def post_process_matched_download_with_verification(context_key, context, file_p with tasks_lock: if task_id in download_tasks: _mark_task_completed(task_id, context.get('track_info')) + if context.get('_verification_status'): + download_tasks[task_id]['verification_status'] = context['_verification_status'] with matched_context_lock: if context_key in matched_downloads_context: del matched_downloads_context[context_key] @@ -1231,6 +1247,8 @@ def post_process_matched_download_with_verification(context_key, context, file_p if task_id in download_tasks: _mark_task_completed(task_id, context.get('track_info')) download_tasks[task_id]['metadata_enhanced'] = True + if context.get('_verification_status'): + download_tasks[task_id]['verification_status'] = context['_verification_status'] redownload_ctx = download_tasks[task_id].get('_redownload_context') with matched_context_lock: diff --git a/core/repair_jobs/acoustid_scanner.py b/core/repair_jobs/acoustid_scanner.py index f7a4acba..ecf1bd3d 100644 --- a/core/repair_jobs/acoustid_scanner.py +++ b/core/repair_jobs/acoustid_scanner.py @@ -53,6 +53,10 @@ class AcoustIDScannerJob(RepairJob): 'title_similarity': 0.70, 'artist_similarity': 0.60, 'batch_size': 200, + # Skip tracks the user force-imported via the version-mismatch + # fallback (they are expected to mismatch; default: still scan them + # but report as informational). + 'skip_force_imported': False, } auto_fix = False # User chooses fix action per finding @@ -223,6 +227,24 @@ class AcoustIDScannerJob(RepairJob): or expected['artist'] ) + # Verification status from the embedded SOULSYNC_VERIFICATION tag. + # force_imported = user accepted this file as best candidate after the + # retry budget was exhausted — a mismatch here is EXPECTED. Either skip + # (job setting) or downgrade the finding to informational below. + file_verif_status = None + try: + from core.tag_writer import read_file_tags as _rft + file_verif_status = (_rft(fpath) or {}).get('verification_status') + except Exception: + pass + if file_verif_status == 'force_imported' and \ + self._get_settings(context).get('skip_force_imported', False): + if context.report_progress: + context.report_progress( + log_line=f'Skipped (force-imported fallback): {fname}', + log_type='skip') + return + # Fingerprint-collision guard: when the TOP recording's length is wildly # different from the file, the fingerprint hit is a hash collision (the # 17-min mashup → 5-min track case), not a real match — skip BEFORE any @@ -260,6 +282,18 @@ class AcoustIDScannerJob(RepairJob): aliases_provider=_aliases, ) + # Refresh the DB column from the file tag (the tag travels with the + # file and survives DB resets; the tracks row is the UI-facing cache). + if file_verif_status: + try: + conn = context.db._get_connection() + conn.cursor().execute( + "UPDATE tracks SET verification_status = ? WHERE id = ?", + (file_verif_status, track_id)) + getattr(conn, 'commit', lambda: None)() + except Exception as e: + logger.debug("verification_status refresh failed for %s: %s", track_id, e) + if outcome.decision != Decision.FAIL: if context.report_progress: context.report_progress( @@ -280,7 +314,13 @@ class AcoustIDScannerJob(RepairJob): log_type='error' ) if context.create_finding: - severity = 'warning' if best_score >= 0.90 else 'info' + _is_force = file_verif_status == 'force_imported' + severity = 'info' if _is_force else ('warning' if best_score >= 0.90 else 'info') + _title = ( + f'Force-imported (fallback): "{expected["title"]}" is actually "{matched_title}"' + if _is_force else + f'Wrong download: "{expected["title"]}" is actually "{matched_title}"' + ) inserted = context.create_finding( job_id=self.job_id, finding_type='acoustid_mismatch', @@ -288,7 +328,7 @@ class AcoustIDScannerJob(RepairJob): entity_type='track', entity_id=str(track_id), file_path=fpath, - title=f'Wrong download: "{expected["title"]}" is actually "{matched_title}"', + title=_title, description=( f'Expected "{expected["title"]}" by {expected_artist}, ' f'but audio fingerprint matches "{matched_title}" by {matched_artist} ' @@ -307,6 +347,7 @@ class AcoustIDScannerJob(RepairJob): 'artist_thumb_url': expected.get('artist_thumb_url'), 'album_title': expected.get('album_title', ''), 'track_number': expected.get('track_number'), + 'force_imported': file_verif_status == 'force_imported', } ) if inserted: diff --git a/docs/superpowers/plans/2026-06-10-acoustid-verification-unification.md b/docs/superpowers/plans/2026-06-10-acoustid-verification-unification.md new file mode 100644 index 00000000..5516bbc3 --- /dev/null +++ b/docs/superpowers/plans/2026-06-10-acoustid-verification-unification.md @@ -0,0 +1,378 @@ +# AcoustID Verification Unification + Status Tracking — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make import-time verification and the library AcoustID scan share ONE decision core, and persist a per-track verification status (DB + file tag) surfaced on the Downloads page. + +**Architecture:** Extract a pure `core/matching/audio_verification.py` (`normalize` + `evaluate`) built on the existing shared helpers (`artist_aliases`, `script_compat`, `acoustid_candidates`, `version_mismatch`). `acoustid_verification.verify_audio_file` (import) and `acoustid_scanner._scan_file` (scan) delegate the decision to it. A new `tracks.verification_status` column + `SOULSYNC_VERIFICATION` file tag record `verified` / `unverified` / `force_imported`; the Downloads page shows a badge. + +**Tech Stack:** Python 3.11, pytest (`.venv/bin/python -m pytest`), mutagen, SQLite, vanilla JS webui. + +**Spec:** `docs/superpowers/specs/2026-06-10-acoustid-verification-unification-design.md` + +--- + +## File Structure + +- **Create** `core/matching/audio_verification.py` — the shared `normalize()` + `evaluate()` decision core (pure, no I/O). Single source of truth for normalization, thresholds, alias-aware comparison, cross-script SKIP, version gate, duration guard. +- **Create** `core/matching/verification_status.py` — the status vocabulary (`VERIFIED`, `UNVERIFIED`, `FORCE_IMPORTED`) + `status_from_outcome(decision)` + `status_from_context(context)` mapping helpers. Tiny, pure, testable. +- **Modify** `core/acoustid_verification.py` — `_normalize`/`_similarity`/`_alias_aware_artist_sim`/`_find_best_title_artist_match` and the decision branches in `verify_audio_file` delegate to the core. Keep the import-facing wrapper (fingerprint lookup, MB alias provider, `VerificationResult`). +- **Modify** `core/repair_jobs/acoustid_scanner.py` — drop the private `_normalize` (line 485) and the inline decision; call `normalize` + `evaluate`. +- **Modify** `database/music_database.py` — additive migration: `tracks.verification_status TEXT`. +- **Modify** `core/tag_writer.py` — write `SOULSYNC_VERIFICATION` in `_write_vorbis`/`_write_id3`/`_write_mp4`; read it in `read_file_tags`. +- **Modify** `core/imports/pipeline.py` — compute + persist the status (DB + tag) after post-processing; stash on the download context. +- **Modify** `web_server.py` + `webui/static/downloads.js` + `webui/index.html` — surface the status badge. + +--- + +## Task 1: Core `normalize()` + +**Files:** +- Create: `core/matching/audio_verification.py` +- Test: `tests/matching/test_audio_verification_core.py` + +- [ ] **Step 1: Write the failing test** + +```python +from core.matching.audio_verification import normalize + +def test_normalize_strips_paren_bracket_angle_and_keeps_cjk(): + assert normalize("澤野弘之 ") == "澤野弘之" + assert normalize("Clarity (Live at X) [Remastered]") == "clarity" + assert normalize("Attack on Titan ") == "attack on titan" + +def test_normalize_strips_version_and_featuring(): + assert normalize("In My Feelings - Instrumental") == "in my feelings" + assert normalize("Song feat. Someone") == "song" +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `.venv/bin/python -m pytest tests/matching/test_audio_verification_core.py -q` +Expected: FAIL — `ModuleNotFoundError: core.matching.audio_verification`. + +- [ ] **Step 3: Write minimal implementation** + +Port the current (already-correct) body of `core/acoustid_verification.py::_normalize` +verbatim into the new module (it already strips `()`/`[]`/`<>`/version/featuring and +keeps CJK via `\w`). Add the thresholds as module constants for later tasks. + +```python +"""Shared audio-verification decision core (pure; no file/DB I/O). + +Single source of truth for normalization + the PASS/SKIP/FAIL decision used by +BOTH import-time verification (core/acoustid_verification.py) and the library +scan (core/repair_jobs/acoustid_scanner.py). +""" +import re +from difflib import SequenceMatcher + +MIN_ACOUSTID_SCORE = 0.80 +TITLE_MATCH_THRESHOLD = 0.70 +ARTIST_MATCH_THRESHOLD = 0.60 + +def normalize(text: str) -> str: + if not text: + return "" + s = text.lower().strip() + s = re.sub(r'\s*\([^)]*\)', '', s) + s = re.sub(r'\s*\[[^\]]*\]', '', s) + s = re.sub(r'\s*<[^>]*>', '', s) + s = re.sub(r'\s+(?:feat\.?|ft\.?|featuring)\s+.*$', '', s, flags=re.IGNORECASE) + s = re.sub(r'\s*-\s*(?:vocal|instrumental|acoustic|live|remix|cover|clean|explicit|radio\s*edit|original\s*mix|extended\s*mix|club\s*mix)\s*$', '', s, flags=re.IGNORECASE) + s = re.sub(r'\s*-\s*from\s+.+$', '', s, flags=re.IGNORECASE) + s = re.sub(r'[^\w\s]', '', s) + s = re.sub(r'\s+', ' ', s).strip() + return s + +def similarity(a: str, b: str) -> float: + na, nb = normalize(a), normalize(b) + if not na or not nb: + return 0.0 + if na == nb: + return 1.0 + return SequenceMatcher(None, na, nb).ratio() +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `.venv/bin/python -m pytest tests/matching/test_audio_verification_core.py -q` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add core/matching/audio_verification.py tests/matching/test_audio_verification_core.py +git commit -m "feat(verification): shared normalize() core for import + scan" +``` + +--- + +## Task 2: Core `evaluate()` decision + +**Files:** +- Modify: `core/matching/audio_verification.py` +- Test: `tests/matching/test_audio_verification_core.py` + +`evaluate` reproduces the import-path decision (it is the richer of the two), so it +is behaviour-preserving for import and an upgrade for scan. Port the logic from +`acoustid_verification.verify_audio_file` Steps 4b–end (version gate via +`is_acceptable_version_mismatch`, alias-aware artist sim, secondary/scan match via +`find_matching_recording`, cross-script SKIP via `is_cross_script_mismatch`, +duration guard via `duration_mismatches_strongly`). + +- [ ] **Step 1: Write the failing tests** (the three real-world cases) + +```python +from core.matching.audio_verification import evaluate, Decision + +REC = lambda t, a, d=None: {"title": t, "artist": a, "duration": d} + +def test_cross_script_artist_with_vocal_credit_skips_not_fails(): + # Sawano / 澤野弘之 + IPA title -> SKIP, never FAIL + out = evaluate( + "Call Your Name", "Sawano Hiroyuki", + [REC("call your name", "澤野弘之 ")], + fingerprint_score=0.95, + aliases_provider=lambda: ["澤野弘之"], + ) + assert out.decision == Decision.SKIP + +def test_clean_match_passes(): + out = evaluate("Xl-Tt", "Sawano Hiroyuki", + [REC("xl-tt", "澤野弘之")], fingerprint_score=0.95, + aliases_provider=lambda: ["澤野弘之"]) + assert out.decision == Decision.PASS + +def test_genuine_wrong_song_fails(): + out = evaluate("Yellow", "Coldplay", + [REC("Rich Interlude", "Kendrick Lamar")], fingerprint_score=0.85) + assert out.decision == Decision.FAIL +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `.venv/bin/python -m pytest tests/matching/test_audio_verification_core.py -k evaluate -q` +Expected: FAIL — `cannot import name 'evaluate'`. + +- [ ] **Step 3: Implement `evaluate` + `Decision` + `Outcome`** + +Add an `enum Decision {PASS, SKIP, FAIL}`, a dataclass `Outcome(decision, title_sim, +artist_sim, matched_title, matched_artist, reason)`, and `_alias_aware_artist_sim` +(ported from acoustid_verification). Port the decision sequence from +`verify_audio_file` lines 470–703 into `evaluate`, taking `recordings` + +`fingerprint_score` + `file_duration_s` + `aliases_provider` as params and +returning an `Outcome` instead of `(VerificationResult, msg)`. Reuse the existing +imports: `from core.matching.artist_aliases import artist_names_match, best_alias_match`, +`from core.matching.script_compat import is_cross_script_mismatch`, +`from core.matching.acoustid_candidates import find_matching_recording, duration_mismatches_strongly`, +`from core.matching.version_mismatch import is_acceptable_version_mismatch`, and +`MusicMatchingEngine().detect_version_type` for `_detect_title_version`. + +- [ ] **Step 4: Run to verify pass** + +Run: `.venv/bin/python -m pytest tests/matching/test_audio_verification_core.py -q` +Expected: PASS (all). + +- [ ] **Step 5: Commit** + +```bash +git add core/matching/audio_verification.py tests/matching/test_audio_verification_core.py +git commit -m "feat(verification): shared evaluate() PASS/SKIP/FAIL decision core" +``` + +--- + +## Task 3: Import path delegates to the core + +**Files:** +- Modify: `core/acoustid_verification.py` +- Test (regression): `tests/test_acoustid_skip_logic.py`, `tests/test_acoustid_version_mismatch.py`, `tests/test_acoustid_normalize_angle_annotations.py`, `tests/test_acoustid_error_reporting.py` + +- [ ] **Step 1: Run the existing suite to capture green baseline** + +Run: `.venv/bin/python -m pytest tests/test_acoustid_skip_logic.py tests/test_acoustid_version_mismatch.py tests/test_acoustid_normalize_angle_annotations.py tests/test_acoustid_error_reporting.py -q` +Expected: PASS (baseline before refactor). + +- [ ] **Step 2: Refactor** + +In `core/acoustid_verification.py`: replace the bodies of `_normalize`/`_similarity` +with re-exports from the core (`from core.matching.audio_verification import normalize as _normalize, similarity as _similarity`). Replace the decision block in +`verify_audio_file` (Steps 4b–end) with a single call to `evaluate(...)`, then map +`Outcome.decision` → `VerificationResult` (PASS→PASS, SKIP→SKIP, FAIL→FAIL) and pass +`Outcome.reason` through. Keep the existing fingerprint lookup, MB enrichment, and +`_resolve_expected_artist_aliases` thunk (pass it as `aliases_provider`). + +- [ ] **Step 3: Run regression suite** + +Run: same command as Step 1. +Expected: PASS (unchanged behaviour for import). + +- [ ] **Step 4: Commit** + +```bash +git add core/acoustid_verification.py +git commit -m "refactor(verification): import path delegates to shared core" +``` + +--- + +## Task 4: Scanner uses the core (gains alias bridge + cross-script SKIP) + +**Files:** +- Modify: `core/repair_jobs/acoustid_scanner.py` +- Test: `tests/test_acoustid_scanner.py` (regression) + new case + +- [ ] **Step 1: Write the failing test** (cross-script track must NOT create a finding) + +```python +# tests/test_acoustid_scanner_cross_script.py +def test_scanner_does_not_flag_cross_script_anime_ost(monkeypatch): + # Build a scan over one track expected "Call Your Name" / "Sawano Hiroyuki" + # whose fingerprint returns "澤野弘之 "; assert findings_created == 0. + ... +``` + +(Concrete construction mirrors `tests/test_acoustid_scanner.py` fixtures; assert the +mismatch branch is NOT reached.) + +- [ ] **Step 2: Run to verify failure** + +Run: `.venv/bin/python -m pytest tests/test_acoustid_scanner_cross_script.py -q` +Expected: FAIL — a finding IS created (current scanner strips non-ASCII, no alias). + +- [ ] **Step 3: Refactor scanner** + +Delete `_normalize` (line 485) and import `normalize` from the core. Replace the +`_scan_file` similarity + decision block (the `title_sim`/`artist_sim`/finding logic) +with a call to `evaluate(...)` passing `fp_result['recordings']`, `best_score`, +`file_duration_s`, and an `aliases_provider` (reuse the MB alias lookup, or pass the +DB-resolved aliases). Create a finding only when `Outcome.decision == Decision.FAIL`. + +- [ ] **Step 4: Run new + existing scanner tests** + +Run: `.venv/bin/python -m pytest tests/test_acoustid_scanner.py tests/test_acoustid_scanner_cross_script.py -q` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add core/repair_jobs/acoustid_scanner.py tests/test_acoustid_scanner_cross_script.py +git commit -m "refactor(scanner): use shared verification core; stop false-flagging cross-script" +``` + +--- + +## Task 5: Status vocabulary + mapping + +**Files:** +- Create: `core/matching/verification_status.py` +- Test: `tests/matching/test_verification_status.py` + +- [ ] **Step 1: Failing test** + +```python +from core.matching.verification_status import ( + VERIFIED, UNVERIFIED, FORCE_IMPORTED, status_from_decision, status_from_context) +from core.matching.audio_verification import Decision + +def test_decision_maps_to_status(): + assert status_from_decision(Decision.PASS) == VERIFIED + assert status_from_decision(Decision.SKIP) == UNVERIFIED + +def test_force_import_context_wins(): + assert status_from_context({"_version_mismatch_fallback": "instrumental"}) == FORCE_IMPORTED + assert status_from_context({}) is None +``` + +- [ ] **Step 2: Run → fail.** `.venv/bin/python -m pytest tests/matching/test_verification_status.py -q` + +- [ ] **Step 3: Implement** the three string constants + the two pure mappers. + +- [ ] **Step 4: Run → pass.** + +- [ ] **Step 5: Commit** `feat(verification): status vocabulary + mappers`. + +--- + +## Task 6: DB migration `tracks.verification_status` + +**Files:** +- Modify: `database/music_database.py` +- Test: `tests/test_verification_status_migration.py` + +- [ ] **Step 1: Failing test** — open a fresh DB, assert `verification_status` is in `PRAGMA table_info(tracks)`. +- [ ] **Step 2: Run → fail.** +- [ ] **Step 3: Implement** following the existing additive pattern: + +```python +cursor.execute("PRAGMA table_info(tracks)") +cols = [r[1] for r in cursor.fetchall()] +if 'verification_status' not in cols: + cursor.execute("ALTER TABLE tracks ADD COLUMN verification_status TEXT") +``` +(Place beside the other `tracks` ADD COLUMN migrations, e.g. near line 2372.) + +- [ ] **Step 4: Run → pass.** +- [ ] **Step 5: Commit** `feat(db): add tracks.verification_status`. + +--- + +## Task 7: Write + read the `SOULSYNC_VERIFICATION` tag + +**Files:** +- Modify: `core/tag_writer.py` +- Test: `tests/test_verification_tag_roundtrip.py` + +- [ ] **Step 1: Failing test** — write a FLAC with `db_data={'verification_status': 'unverified', ...}`, read it back via `read_file_tags`, assert `tags['verification_status'] == 'unverified'`. (Build the FLAC with ffmpeg as in `/tmp` test harness; or reuse an existing tag-writer test fixture.) +- [ ] **Step 2: Run → fail.** +- [ ] **Step 3: Implement** — in `_write_vorbis` set `audio['SOULSYNC_VERIFICATION']=[status]`; in `_write_id3` add `TXXX(desc='SOULSYNC_VERIFICATION', text=[status])`; in `_write_mp4` set `----:com.soulsync:VERIFICATION`. Read all three back in `read_file_tags` into key `verification_status`. Only write when `db_data.get('verification_status')` is set (non-fatal on error). +- [ ] **Step 4: Run → pass.** +- [ ] **Step 5: Commit** `feat(tags): persist SOULSYNC_VERIFICATION tag`. + +--- + +## Task 8: Persist status at import (DB + tag + context) + +**Files:** +- Modify: `core/imports/pipeline.py` +- Test: `tests/imports/test_import_verification_status.py` + +- [ ] **Step 1: Failing test** — drive `post_process_matched_download` (or the helper that records the library row) with a PASS verification result and assert the track row's `verification_status == 'verified'`; with `context['_version_mismatch_fallback']` set assert `'force_imported'`. +- [ ] **Step 2: Run → fail.** +- [ ] **Step 3: Implement** — after verification, compute `status = status_from_context(context) or status_from_decision(verification_decision)`; thread it into `db_data` (so the tag write in Task 7 picks it up) and into the library-row write (`tracks.verification_status`); also stash `context['_verification_status'] = status` for the Downloads payload. +- [ ] **Step 4: Run → pass.** +- [ ] **Step 5: Commit** `feat(import): record verification status (db+tag+context)`. + +--- + +## Task 9: Downloads page badge + +**Files:** +- Modify: `web_server.py` (download-status payload), `webui/static/downloads.js`, `webui/index.html` (badge styles if needed) +- Test: manual + a small JS-free assertion if a payload test exists + +- [ ] **Step 1:** Add `verification_status` to the per-task download payload in `web_server.py` (read from `context['_verification_status']` / the task record). +- [ ] **Step 2:** In `downloads.js`, render a badge per completed row: ✓ verified / ⚠ unverified / ⚑ force-imported, keyed on `task.verification_status`. +- [ ] **Step 3:** Add minimal badge CSS in `index.html`/`style.css` mirroring existing status pills. +- [ ] **Step 4:** Manual verify: import one clean + one cross-script track; confirm ✓ and ⚠ badges. +- [ ] **Step 5: Commit** `feat(ui): show verification status badge on Downloads`. + +--- + +## Final verification + +- [ ] Run the full AcoustID + import suites: + `.venv/bin/python -m pytest tests/test_acoustid_*.py tests/matching/ tests/imports/ -q` + Expected: all PASS. +- [ ] Build + push image (per build-deploy loop) for live testing. + +## Self-review notes + +- Spec coverage: unify (Tasks 1–4), status values (Task 5), DB (6), tag (7), + import wiring (8), UI (9) — all spec sections mapped. +- Behaviour-preserving for import (Task 3 regression-gated); intentional change for + scan (Task 4 — the fix). +- Tasks 4, 8, 9 reference existing fixtures/APIs the executor must read live + (`test_acoustid_scanner.py` fixtures, the download payload shape, `downloads.js` + row render) — flagged as such rather than guessed. diff --git a/docs/superpowers/specs/2026-06-10-acoustid-verification-unification-design.md b/docs/superpowers/specs/2026-06-10-acoustid-verification-unification-design.md new file mode 100644 index 00000000..954564db --- /dev/null +++ b/docs/superpowers/specs/2026-06-10-acoustid-verification-unification-design.md @@ -0,0 +1,153 @@ +# AcoustID Verification: Unify the Pipeline + Persist Verification Status + +Date: 2026-06-10 +Branch: `fix/import-folder-artist-override-optin` + +## Problem + +Audio verification logic is **duplicated** across two code paths that have +drifted apart, producing inconsistent results ("komische Fehler"): + +- **Import time** — `core/acoustid_verification.py` (`verify_audio_file`, + `_normalize` @ line 59, alias-aware artist sim, cross-script SKIP, version + gate, duration guard). Keeps CJK characters; strips `()`/`[]`/`<>` (after the + recent fix). +- **Library scan** — `core/repair_jobs/acoustid_scanner.py` (its OWN `_normalize` + @ line 485, which strips **all non-ASCII** — kanji/IPA vanish — and has NO + cross-script alias bridge / SKIP). A full library scan therefore false-flags + correct cross-script tracks (e.g. anime OSTs credited `澤野弘之 `). + +Two `_normalize`s + two decision paths = the same bug must be fixed twice and +they diverge. There is also no record of *how* a track passed verification, so a +force-imported or skipped-but-imported track looks identical to a cleanly +verified one. + +## Goals + +1. **One verification core** used by BOTH import and scan. Normalization, + alias-aware comparison, cross-script handling, version gate, duration guard, + and thresholds live in exactly one place. +2. **Persist a verification status** per track in the DB **and** as a file tag, + so it survives DB resets / file moves and can drive the UI. +3. **Surface the status on the Downloads page** as an info badge. + +## Consumer map (discovered 2026-06-10) + +In scope (call the new core): +- `core/imports/pipeline.py:340-373` — the only caller of + `AcoustIDVerification.verify_audio_file` (import path). +- `core/repair_jobs/acoustid_scanner.py` — the library-scan job. + +Shared helpers (stay shared, unchanged; the core builds on them): +- `core/matching/artist_aliases.py::artist_names_match` / `best_alias_match` +- `core/matching/script_compat.py::is_cross_script_mismatch` +- `core/matching/acoustid_candidates.py::find_matching_recording`, + `duration_mismatches_strongly` +- `core/matching/version_mismatch.py::is_acceptable_version_mismatch` + +Status hook: +- `core/imports/version_mismatch_fallback.py` sets + `context["_version_mismatch_fallback"]` when it force-accepts the best + quarantined candidate after retries are exhausted → maps to `force_imported`. + Dispatched from `core/downloads/task_worker.py` via `try_version_mismatch_fallback`. + +Deliberately OUT of scope (consume a shared helper but serve other purposes; +NOT merged): +- `core/repair_worker.py::_album_fill_artist_names_match` — Album-Completeness + auto-fill gate. +- `core/matching/album_context_title.py::_normalize` — trivial album-grouping + normalize. +- `core/downloads/task_worker.py`, `core/matching_engine.py` — pre-download + (Soulseek candidate) matching, not post-download verification. + +## Design + +### 1. Shared core — `core/matching/audio_verification.py` (new) + +The single home for the verification *decision*. Pure where possible (no file +I/O, no DB) so it is unit-testable in isolation. Callers keep their own I/O +(fingerprinting, quarantine, finding creation, tag writing). + +``` +def normalize(text: str) -> str + # lowercase; strip ()/[]/<> annotations; strip version (- Live, etc) + + # featuring tags; KEEP CJK (\w under unicode); collapse whitespace. + # Single source of truth — replaces acoustid_verification._normalize AND + # acoustid_scanner._normalize. + +def evaluate( + expected_title, expected_artist, recordings, *, + fingerprint_score, file_duration_s=None, aliases_provider=None, +) -> Outcome + # Outcome(decision, title_sim, artist_sim, matched_title, matched_artist, reason) + # decision in {PASS, SKIP, FAIL}. + # Encapsulates: alias-aware artist sim, version-mismatch gate, duration + # collision guard, cross-script SKIP, thresholds. Built on the shared helpers. +``` + +Thresholds (`TITLE_MATCH_THRESHOLD`, `ARTIST_MATCH_THRESHOLD`, `MIN_ACOUSTID_SCORE`) +move into the core as the single definition. + +Caller mapping: +- **Import** (`acoustid_verification.verify_audio_file`): PASS → import + + status `verified`; SKIP → import + status `unverified`; FAIL → quarantine. +- **Scan** (`acoustid_scanner._scan_file`): PASS/SKIP → no finding (optionally + refresh the stored status); FAIL → create `acoustid_mismatch` finding. + +`acoustid_verification.py` keeps `verify_audio_file` as the import-facing wrapper +(fingerprint lookup, MB enrichment, alias provider, returns `VerificationResult`) +but delegates the decision to `evaluate`. `acoustid_scanner.py` drops its private +`_normalize` and decision branches and calls `normalize` + `evaluate`. + +### 2. Verification status + +Values (the three the user selected; quarantined files are not imported, so they +carry no status): +- `verified` — clean AcoustID PASS. +- `unverified` — SKIP: cross-script / ambiguous / no AcoustID match. Imported, + not hard-confirmed. +- `force_imported` — accepted via `version_mismatch_fallback` after retries + exhausted. + +### 3. Storage — DB + tag + +- **DB:** new nullable column `tracks.verification_status TEXT` (idempotent + migration in `database/music_database.py`). Set at import; refreshed by the + scan. Mirrored onto the download record/context so the Downloads page can show + it before a library re-scan. +- **Tag:** `SOULSYNC_VERIFICATION=` written via `core/tag_writer.py` + (Vorbis comment / ID3 `TXXX`). Travels with the file; the scanner reads it and + may skip re-verifying an already-`verified` file (perf bonus) and to display. + +### 4. Downloads page UI + +A small, info-only badge per row sourced from the status: +- ✓ `verified` · ⚠ `unverified` · ⚑ `force_imported`. +Wired in `webui/static/downloads.js` (+ the download-status API payload). + +### 5. Testing (TDD) + +- Unit-test the core `normalize` (incl. `<>`/`()`/`[]`, CJK retention) and + `evaluate` (Sawano/IPA cross-script → SKIP not FAIL; vocal-credit artist → + alias 100%; version mismatch → FAIL; duration collision → no FAIL). +- Wire import + scanner to the core; their existing tests + (`test_acoustid_skip_logic`, `test_acoustid_scanner`, + `test_acoustid_version_mismatch`, `test_acoustid_error_reporting`, + `test_acoustid_normalize_angle_annotations`) act as regression. +- New tests for the status: force-import → `force_imported`; PASS → `verified`; + SKIP → `unverified`; tag written + read back; DB column populated. + +## Rollout / risk + +- Pure-core extraction is behaviour-preserving for the import path (existing + tests pin it); the scan path *changes* (gains alias bridge + cross-script SKIP) + — that is the intended fix. +- DB migration is additive (nullable column), safe on existing DBs. +- Tag writing reuses the existing `tag_writer` path; failures are non-fatal. +- Staged on the current branch with TDD; ships in the next image build. + +## Out of scope (now) + +Unifying the pre-download Soulseek matcher or the album-completeness gate. They +consume shared helpers but are different decisions; folding them in would widen +blast radius without serving this goal. diff --git a/tests/imports/test_folder_artist_override.py b/tests/imports/test_folder_artist_override.py new file mode 100644 index 00000000..8bc102d8 --- /dev/null +++ b/tests/imports/test_folder_artist_override.py @@ -0,0 +1,85 @@ +"""Folder-artist override is opt-in and never clobbers an identified artist. + +Background +---------- + +The auto-import "parent folder artist override" used to fire unconditionally: +whenever the Staging path had >=2 levels and the top folder wasn't a category +word, it replaced the (already metadata-identified) artist with the top folder +name. A user who staged a mixed pile of singles under one container folder +named ``soulsync`` therefore got the album-artist of *every* file forced to +"soulsync" — even when the track was confidently resolved to a real artist +with a Spotify/MusicBrainz id. Navidrome groups by album-artist, so 65 singles +collapsed under a bogus "soulsync" artist. + +``resolve_folder_artist`` is the extracted, pure decision. It must: +- return ``None`` (i.e. keep the identified artist) when the feature is OFF, + regardless of folder structure — this is the regression guard; +- only when explicitly enabled, reproduce the original folder-derived artist. +""" + +from core.imports.folder_artist import resolve_folder_artist + + +# --- OFF by default: never override (the bug fix) --------------------------- + +def test_disabled_keeps_identified_artist_even_with_artist_album_structure(): + # The 'soulsync' mass mis-file: identified artist is real, folder is a + # generic container. With the feature off it must NOT be overridden. + assert resolve_folder_artist( + "soulsync/Bunny Girl/01 - Bunny Girl.flac", + identified_artist="1nonly, Ciscaux", + enabled=False, + ) is None + + +def test_disabled_returns_none_for_clean_artist_album_path(): + assert resolve_folder_artist( + "AC+DC/Back In Black/01 - Hells Bells.flac", + identified_artist="AC/DC", + enabled=False, + ) is None + + +# --- Enabled: original behaviour is available on request -------------------- + +def test_enabled_uses_top_folder_as_artist_when_it_differs(): + assert resolve_folder_artist( + "soulsync/Bunny Girl/01 - Bunny Girl.flac", + identified_artist="1nonly, Ciscaux", + enabled=True, + ) == "soulsync" + + +def test_enabled_skips_category_folder_and_uses_artist_above_it(): + assert resolve_folder_artist( + "Pink Floyd/Albums/The Wall/01 - In the Flesh.flac", + identified_artist="Some DJ", + enabled=True, + ) == "Pink Floyd" + + +def test_enabled_flat_file_has_no_folder_artist(): + assert resolve_folder_artist( + "01 - Bitch Lasagna.flac", + identified_artist="PewDiePie", + enabled=True, + ) is None + + +def test_enabled_no_override_when_folder_matches_identified(): + # Folder already equals the identified artist (case-insensitive) -> no-op. + assert resolve_folder_artist( + "AC_DC/Back In Black/01 - Hells Bells.flac", + identified_artist="ac_dc", + enabled=True, + ) is None + + +def test_enabled_top_level_category_word_is_not_an_artist(): + # 'singles/Track/file' — top folder is a category, not an artist. + assert resolve_folder_artist( + "singles/Headache/01 - Headache.flac", + identified_artist="Asal", + enabled=True, + ) is None diff --git a/tests/test_acoustid_normalize_angle_annotations.py b/tests/test_acoustid_normalize_angle_annotations.py new file mode 100644 index 00000000..42e34e19 --- /dev/null +++ b/tests/test_acoustid_normalize_angle_annotations.py @@ -0,0 +1,36 @@ +"""`_normalize` must strip ``<...>`` annotations like the AcoustID/MusicBrainz +vocalist credit ``澤野弘之 ``. + +User report: a correct anime-OST track ("Attack on Titan" by "Sawano Hiroyuki") +was false-quarantined. AcoustID returned the artist as +``澤野弘之 ``. The kanji ``澤野弘之`` IS the artist and the +MusicBrainz alias bridge matches it — but `_normalize` stripped ``()`` and +``[]`` annotations, NOT ``<...>``, so the trailing "vocal mika kobayashi" words +diluted the alias comparison down to ~0.28 (below ARTIST_MATCH_THRESHOLD). That +in turn blocked the existing cross-script SKIP safety net (issue #797), which is +gated on ``artist_sim >= threshold``, so the file FAILED and was quarantined. + +Stripping ``<...>`` restores the artist to ``澤野弘之`` so the alias match (and +thus the cross-script SKIP) works. +""" + +from core.acoustid_verification import _normalize, _similarity + + +def test_normalize_strips_angle_bracket_vocalist_annotation(): + assert _normalize("澤野弘之 ") == "澤野弘之" + + +def test_normalize_strips_angle_brackets_latin(): + assert _normalize("Attack on Titan ") == "attack on titan" + + +def test_vocalist_annotation_no_longer_dilutes_artist_similarity(): + # The kanji artist with a vocalist credit must compare as identical to the + # bare kanji artist — this is what lets the alias bridge clear the threshold. + assert _similarity("澤野弘之", "澤野弘之 ") == 1.0 + + +def test_normalize_keeps_plain_text_untouched(): + # Guard: no angle brackets -> unchanged behaviour. + assert _normalize("Sawano Hiroyuki") == "sawano hiroyuki" diff --git a/tests/test_acoustid_scanner.py b/tests/test_acoustid_scanner.py index 9c2f7413..ce04b82f 100644 --- a/tests/test_acoustid_scanner.py +++ b/tests/test_acoustid_scanner.py @@ -816,3 +816,55 @@ def test_scanner_does_not_flag_cross_script_when_alias_bridges(monkeypatch): fake_acoustid, context, result, fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6) assert captured == [], f"cross-script track false-flagged: {captured}" + + +def _force_imported_scan(monkeypatch, *, skip_setting): + """Drive a scan over a force-imported file whose fingerprint clearly + mismatches. Returns the captured findings.""" + import core.repair_jobs.acoustid_scanner as scanner_mod + monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", + lambda name: [], raising=False) + monkeypatch.setattr( + 'core.tag_writer.read_file_tags', + lambda fpath: {'artist': None, 'verification_status': 'force_imported'}, + ) + job = AcoustIDScannerJob() + if skip_setting: + monkeypatch.setattr(job, '_get_settings', + lambda ctx: {**job.default_settings, + 'skip_force_imported': True}) + captured = [] + context = _make_finding_capturing_context( + track_row=("42", "Wanted Song", "Real Artist", + "/music/ws.flac", 1, "Album", None, None), + captured=captured, + ) + fake_acoustid = SimpleNamespace( + fingerprint_and_lookup=lambda fpath: { + 'best_score': 0.99, + 'recordings': [{'title': 'Wanted Song - Instrumental', + 'artist': 'Real Artist'}], + }, + ) + result = JobResultStub() + job._scan_file('/music/ws.flac', '42', + {'title': 'Wanted Song', 'artist': 'Real Artist'}, + fake_acoustid, context, result, + fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6) + return captured + + +def test_force_imported_mismatch_is_reported_as_informational(monkeypatch): + # The user opted into the fallback, so the scan must still TELL them the + # file is e.g. an instrumental — but as 'info', clearly marked, not as a + # red Wrong-download warning. + captured = _force_imported_scan(monkeypatch, skip_setting=False) + assert len(captured) == 1 + assert captured[0]['severity'] == 'info' + assert captured[0]['details'].get('force_imported') is True + assert 'Force-imported' in captured[0]['title'] + + +def test_force_imported_can_be_skipped_via_setting(monkeypatch): + captured = _force_imported_scan(monkeypatch, skip_setting=True) + assert captured == [] diff --git a/webui/index.html b/webui/index.html index 0fdd3e47..ce1e38e1 100644 --- a/webui/index.html +++ b/webui/index.html @@ -6201,6 +6201,22 @@ (e.g. MP3), it will be replaced with the higher quality version (e.g. FLAC). If disabled, existing tracks are always kept and the import file is skipped. + +
+ +
+
+ On by default (legacy behaviour). When enabled, files staged as + Artist/Album/… (or Artist/Albums/Album/…) take the top + folder as the album artist — handy for mixtapes/compilations whose embedded tags + carry DJ names. Turn this off if you drop a mixed pile of songs + under one container folder: otherwise every file's artist gets overwritten with + that folder's name (the cause of the "soulsync" mass-mislabel). With it off, the + metadata-identified artist is always kept. +
diff --git a/webui/static/downloads.js b/webui/static/downloads.js index aeca1bd1..dad32572 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -3668,7 +3668,23 @@ function processModalStatusUpdate(playlistId, data) { case 'searching': statusText = '🔍 Searching...'; break; case 'downloading': statusText = `⏬ Downloading... ${Math.round(task.progress || 0)}%`; break; case 'post_processing': statusText = '⌛ Processing...'; break; - case 'completed': statusText = '✅ Completed'; completedCount++; break; + case 'completed': { + statusText = '✅ Completed'; + // Verification badge — how this file passed verification: + // verified = clean AcoustID pass; unverified = couldn't be + // hard-confirmed (cross-script/ambiguous/no fingerprint match); + // force_imported = accepted as best candidate after the retry + // budget was exhausted (version-mismatch fallback). + if (task.verification_status === 'force_imported') { + statusText += ' ⚑'; + } else if (task.verification_status === 'unverified') { + statusText += ' ⚠'; + } else if (task.verification_status === 'verified') { + statusText += ' ✔'; + } + completedCount++; + break; + } case 'not_found': statusText = '🔇 Not Found'; notFoundCount++; break; case 'failed': { // Distinguish quarantine outcomes from generic @@ -3701,7 +3717,14 @@ function processModalStatusUpdate(playlistId, data) { delete statusEl.dataset.quarantineReason; delete statusEl.dataset.quarantineTrack; delete statusEl.dataset.detailOpen; - statusEl.textContent = statusText; + // statusText is static markup only; the verif-badge span is the + // one case that needs HTML. Everything else stays textContent + // (XSS-safe default). + if (statusText.includes('class="verif-badge')) { + statusEl.innerHTML = statusText; + } else { + statusEl.textContent = statusText; + } // Visual-only hooks: the cell carries its state for the badge // styling, the row glows while a track is actively working. statusEl.dataset.state = isQuarantinedTask ? 'quarantined' diff --git a/webui/static/settings.js b/webui/static/settings.js index 30288c68..429dfc63 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -1297,6 +1297,8 @@ async function loadSettingsData() { // Populate Import settings document.getElementById('import-replace-lower-quality').checked = settings.import?.replace_lower_quality === true; + const _folderArtistEl = document.getElementById('import-folder-artist-override'); + if (_folderArtistEl) _folderArtistEl.checked = settings.import?.folder_artist_override === true; // Populate M3U Export settings document.getElementById('m3u-export-enabled').checked = settings.m3u_export?.enabled === true; @@ -3114,6 +3116,7 @@ async function saveSettings(quiet = false) { }, import: { replace_lower_quality: document.getElementById('import-replace-lower-quality').checked, + folder_artist_override: document.getElementById('import-folder-artist-override')?.checked === true, staging_path: document.getElementById('staging-path').value || './Staging' }, lossy_copy: { diff --git a/webui/static/style.css b/webui/static/style.css index ca06db9d..1a46aada 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -67892,3 +67892,9 @@ body.em-scroll-lock { overflow: hidden; } padding: 7px 11px; color: #fff; font-size: 0.82rem; width: 150px; } .ma-token-input:focus { outline: none; border-color: var(--ma-brand); } + +/* Verification badge on completed downloads (verified / unverified / force-imported) */ +.verif-badge { display: inline-block; margin-left: 4px; font-size: 11px; cursor: help; border-radius: 999px; padding: 0 5px; line-height: 16px; } +.verif-badge.verif-ok { color: #2ecc71; background: rgba(46,204,113,0.12); } +.verif-badge.verif-unverified { color: #f1c40f; background: rgba(241,196,15,0.14); } +.verif-badge.verif-force { color: #e67e22; background: rgba(230,126,34,0.16); } From 2a11dc961a57ab2469a55097e7cb5c4715ef5acb Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 10 Jun 2026 19:03:55 +0200 Subject: [PATCH 07/14] feat(verification): persist status into library_history, badge on Downloads completed list The persistent Completed list is built from library_history (not live tasks), so the badge never showed after a session ended. Column added (additive), written at import, passed through _build_history_download_item, rendered by _adlVerifBadge next to the status label. Co-Authored-By: Claude Opus 4.8 --- core/downloads/status.py | 1 + core/imports/side_effects.py | 1 + database/music_database.py | 11 ++++++----- webui/static/pages-extra.js | 23 ++++++++++++++++++++++- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/core/downloads/status.py b/core/downloads/status.py index 46016634..c1c0eefd 100644 --- a/core/downloads/status.py +++ b/core/downloads/status.py @@ -657,6 +657,7 @@ def _build_history_download_item(entry: dict) -> dict: 'priority': _STATUS_PRIORITY['completed'], 'quality': entry.get('quality') or '', 'file_path': entry.get('file_path') or '', + 'verification_status': entry.get('verification_status'), 'is_persistent_history': True, } diff --git a/core/imports/side_effects.py b/core/imports/side_effects.py index 1c84f287..20fe0017 100644 --- a/core/imports/side_effects.py +++ b/core/imports/side_effects.py @@ -268,6 +268,7 @@ def record_library_history_download(context: Dict[str, Any]) -> None: source_artist=source_artist, origin=origin, origin_context=origin_context, + verification_status=context.get("_verification_status"), ) except Exception as e: logger.debug("library history record failed: %s", e) diff --git a/database/music_database.py b/database/music_database.py index 2b2b3881..6aed3f6b 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -636,7 +636,7 @@ class MusicDatabase: if 'download_source' not in lh_cols: cursor.execute("ALTER TABLE library_history ADD COLUMN download_source TEXT") logger.info("Added download_source column to library_history") - for _col in ['source_track_id', 'source_track_title', 'source_filename', 'acoustid_result', 'source_artist']: + for _col in ['source_track_id', 'source_track_title', 'source_filename', 'acoustid_result', 'source_artist', 'verification_status']: if _col not in lh_cols: cursor.execute(f"ALTER TABLE library_history ADD COLUMN {_col} TEXT") logger.info(f"Added {_col} column to library_history") @@ -12811,7 +12811,7 @@ class MusicDatabase: quality=None, server_source=None, file_path=None, thumb_url=None, download_source=None, source_track_id=None, source_track_title=None, source_filename=None, acoustid_result=None, source_artist=None, - origin=None, origin_context=None): + origin=None, origin_context=None, verification_status=None): """Record a download or import event to the library history table. ``origin``/``origin_context`` record what TRIGGERED the download @@ -12824,11 +12824,12 @@ class MusicDatabase: INSERT INTO library_history (event_type, title, artist_name, album_name, quality, server_source, file_path, thumb_url, download_source, source_track_id, source_track_title, source_filename, - acoustid_result, source_artist, origin, origin_context) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + acoustid_result, source_artist, origin, origin_context, + verification_status) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) """, (event_type, title, artist_name, album_name, quality, server_source, file_path, thumb_url, download_source, source_track_id, source_track_title, source_filename, - acoustid_result, source_artist, origin, origin_context)) + acoustid_result, source_artist, origin, origin_context, verification_status)) conn.commit() return True except Exception as e: diff --git a/webui/static/pages-extra.js b/webui/static/pages-extra.js index d930bb37..83483aa6 100644 --- a/webui/static/pages-extra.js +++ b/webui/static/pages-extra.js @@ -2342,6 +2342,26 @@ function _updateDlNavBadge(count) { } } +function _adlVerifBadge(dl) { + // Verification badge for completed downloads — how this file passed + // verification (status comes from library_history / the live task): + // verified = clean AcoustID pass; unverified = imported but not + // hard-confirmed (cross-script/ambiguous/no fingerprint match); + // force_imported = accepted as best candidate after the retry budget was + // exhausted (version-mismatch fallback). + if (dl.status !== 'completed') return ''; + if (dl.verification_status === 'force_imported') { + return ' ⚑'; + } + if (dl.verification_status === 'unverified') { + return ' ⚠'; + } + if (dl.verification_status === 'verified') { + return ' ✔'; + } + return ''; +} + function _adlRender() { const list = document.getElementById('adl-list'); const empty = document.getElementById('adl-empty'); @@ -2449,6 +2469,7 @@ function _adlRender() { for (const dl of section.items) { const statusClass = _adlStatusClass(dl.status); const statusLabel = _adlStatusLabel(dl.status); + // (verification badge appended next to the label via _adlVerifBadge) const title = _adlEsc(dl.title || 'Unknown Track'); const artist = _adlEsc(dl.artist || ''); const album = _adlEsc(dl.album || ''); @@ -2488,7 +2509,7 @@ function _adlRender() {
- ${statusLabel} + ${statusLabel}${_adlVerifBadge(dl)}
${cancelBtnHtml} `; From 8dcad2be4e4e6f3c3b20e8f93c9b37724c0ddffb Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 10 Jun 2026 19:12:40 +0200 Subject: [PATCH 08/14] feat(downloads): Unverified review filter + visible retry progress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - '⚠ Unverified' filter pill on the Downloads page lists completed downloads whose verification status is unverified/force_imported (review queue) - the quarantine-retry engine's attempt counter (already tracked internally) is now surfaced: task.retry_info ('2/5') shows next to Searching/Downloading in the modal and as 🔁 on the Downloads page rows, with the trigger in the tooltip Co-Authored-By: Claude Opus 4.8 --- core/downloads/monitor.py | 5 +++++ core/downloads/status.py | 5 +++++ webui/index.html | 1 + webui/static/downloads.js | 12 ++++++++++-- webui/static/pages-extra.js | 5 ++++- webui/static/style.css | 1 + 6 files changed, 26 insertions(+), 3 deletions(-) diff --git a/core/downloads/monitor.py b/core/downloads/monitor.py index 472e521c..60ab4585 100644 --- a/core/downloads/monitor.py +++ b/core/downloads/monitor.py @@ -250,6 +250,11 @@ def requeue_quarantined_task_for_retry(task_id, batch_id, trigger): task.pop('quarantine_entry_id', None) task['status'] = 'searching' task['status_change_time'] = time.time() + # Surface the retry progress to the UI ("attempt 2/5" next to the + # status while the task goes around again). Cleared implicitly on + # completion (UI only renders it for active/queued states). + task['retry_info'] = attempt_desc + task['retry_trigger'] = trigger logger.info( f"[Retry:{trigger}] Re-queuing task {task_id} for next-best candidate " diff --git a/core/downloads/status.py b/core/downloads/status.py index c1c0eefd..4acd8248 100644 --- a/core/downloads/status.py +++ b/core/downloads/status.py @@ -343,6 +343,9 @@ def build_batch_status_data(batch_id: str, batch: dict, live_transfers_lookup: d # 'verified' / 'unverified' / 'force_imported' — set by the # import pipeline once post-processing finishes. 'verification_status': task.get('verification_status'), + # "2/5" while the quarantine-retry engine walks candidates. + 'retry_info': task.get('retry_info'), + 'retry_trigger': task.get('retry_trigger'), } _ti = task.get('track_info') if isinstance(task.get('track_info'), dict) else {} task_filename = task.get('filename') or _ti.get('filename') @@ -742,6 +745,8 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: 'progress': progress, 'error': task.get('error_message'), 'verification_status': task.get('verification_status'), + 'retry_info': task.get('retry_info'), + 'retry_trigger': task.get('retry_trigger'), 'batch_id': batch_id, 'batch_name': batch.get('playlist_name') or batch.get('album_name') or '', 'batch_source': batch.get('source_page') or batch.get('initiated_from') or '', diff --git a/webui/index.html b/webui/index.html index ce1e38e1..d46ee112 100644 --- a/webui/index.html +++ b/webui/index.html @@ -2390,6 +2390,7 @@ +
diff --git a/webui/static/downloads.js b/webui/static/downloads.js index dad32572..ae58ee4e 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -3665,8 +3665,16 @@ function processModalStatusUpdate(playlistId, data) { } else { switch (task.status) { case 'pending': statusText = '⏸️ Pending'; break; - case 'searching': statusText = '🔍 Searching...'; break; - case 'downloading': statusText = `⏬ Downloading... ${Math.round(task.progress || 0)}%`; break; + case 'searching': + statusText = '🔍 Searching...'; + // Quarantine-retry engine: show which attempt we're on + // ("retry 2/5") while it walks the next-best candidates. + if (task.retry_info) statusText += ` 🔁 retry ${task.retry_info}`; + break; + case 'downloading': + statusText = `⏬ Downloading... ${Math.round(task.progress || 0)}%`; + if (task.retry_info) statusText += ` 🔁 retry ${task.retry_info}`; + break; case 'post_processing': statusText = '⌛ Processing...'; break; case 'completed': { statusText = '✅ Completed'; diff --git a/webui/static/pages-extra.js b/webui/static/pages-extra.js index 83483aa6..5581afe5 100644 --- a/webui/static/pages-extra.js +++ b/webui/static/pages-extra.js @@ -2384,6 +2384,9 @@ function _adlRender() { if (_adlFilter === 'active') filtered = filtered.filter(d => activeStatuses.includes(d.status)); else if (_adlFilter === 'queued') filtered = filtered.filter(d => queuedStatuses.includes(d.status)); else if (_adlFilter === 'completed') filtered = filtered.filter(d => completedStatuses.includes(d.status)); + else if (_adlFilter === 'unverified') filtered = filtered.filter(d => + completedStatuses.includes(d.status) && + (d.verification_status === 'unverified' || d.verification_status === 'force_imported')); else if (_adlFilter === 'failed') filtered = filtered.filter(d => failedStatuses.includes(d.status)); const completedN = _adlData.filter(d => @@ -2509,7 +2512,7 @@ function _adlRender() {
- ${statusLabel}${_adlVerifBadge(dl)} + ${statusLabel}${_adlVerifBadge(dl)}${dl.retry_info && (statusClass === 'active' || statusClass === 'queued') ? ` 🔁 ${_adlEsc(String(dl.retry_info))}` : ''}
${cancelBtnHtml} `; diff --git a/webui/static/style.css b/webui/static/style.css index 1a46aada..7e74308a 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -67898,3 +67898,4 @@ body.em-scroll-lock { overflow: hidden; } .verif-badge.verif-ok { color: #2ecc71; background: rgba(46,204,113,0.12); } .verif-badge.verif-unverified { color: #f1c40f; background: rgba(241,196,15,0.14); } .verif-badge.verif-force { color: #e67e22; background: rgba(230,126,34,0.16); } +.adl-retry-info { margin-left: 6px; font-size: 11px; color: #e67e22; cursor: help; } From 41536384c34c4f6e555e6297a8997b975181f705 Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 10 Jun 2026 19:25:07 +0200 Subject: [PATCH 09/14] fix(verification): persist status on ALL pipeline success exits + history backfill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pipeline has three success exits (simple download, playlist folder mode, main) but only the main one persisted the verification status — force-imported playlist tracks got no tag, no history status, and never appeared in the Unverified filter. Extracted _persist_verification_status() and call it at every exit. One-time idempotent backfill derives status for existing history rows from their recorded acoustid_result (pass->verified, skip->unverified). Co-Authored-By: Claude Opus 4.8 --- core/imports/pipeline.py | 37 ++++++++++++++++++++++++------------- database/music_database.py | 17 +++++++++++++++++ 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index 3995173a..ec027f06 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -182,6 +182,26 @@ def build_import_pipeline_runtime( ) + +def _persist_verification_status(context, final_path): + """Compute + persist the verification status (verified / unverified / + force_imported) for a finished import: embedded tag on the file, plus + context['_verification_status'] for the history row and the Downloads UI. + MUST be called on EVERY success exit of post-processing (main, playlist + folder mode, simple download) — a missed exit means no badge and no tag. + Never raises.""" + try: + from core.matching.verification_status import status_for_import + from core.tag_writer import write_verification_status + status = status_for_import(context) + if status: + context['_verification_status'] = status + if final_path: + write_verification_status(str(final_path), status) + except Exception as _vs_err: + logger.debug(f"verification-status persist skipped: {_vs_err}") + + def post_process_matched_download(context_key, context, file_path, runtime, metadata_runtime=None): on_download_completed = getattr(runtime, "on_download_completed", None) automation_engine = getattr(runtime, "automation_engine", None) @@ -450,6 +470,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta logger.info(f"Simple download post-processing complete: {activity_target}") context['_simple_download_completed'] = True context['_final_path'] = str(destination) + _persist_verification_status(context, destination) emit_track_downloaded(context, automation_engine) record_library_history_download(context) record_download_provenance(context) @@ -627,6 +648,8 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta final_path = downsampled_path context['_final_processed_path'] = final_path + _persist_verification_status(context, final_path) + blasphemy_path = create_lossy_copy(final_path) if blasphemy_path: context['_final_processed_path'] = blasphemy_path @@ -944,19 +967,7 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta final_path = downsampled_path context['_final_processed_path'] = final_path - # Persist the verification status (verified / unverified / force_imported) - # as an embedded tag so it travels with the file and survives DB resets. - # Written BEFORE the lossy copy so the copy inherits it. Also stashed on - # the context so the wrapper can surface it on the Downloads page. - try: - from core.matching.verification_status import status_for_import - from core.tag_writer import write_verification_status - _verif_status = status_for_import(context) - if _verif_status: - context['_verification_status'] = _verif_status - write_verification_status(final_path, _verif_status) - except Exception as _vs_err: - logger.debug(f"verification-status persist skipped: {_vs_err}") + _persist_verification_status(context, final_path) blasphemy_path = create_lossy_copy(final_path) if blasphemy_path: diff --git a/database/music_database.py b/database/music_database.py index 6aed3f6b..2547d740 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -641,6 +641,23 @@ class MusicDatabase: cursor.execute(f"ALTER TABLE library_history ADD COLUMN {_col} TEXT") logger.info(f"Added {_col} column to library_history") + # One-time backfill: derive verification_status for history rows + # written before the column existed (or by pipeline exits that + # missed it) from the acoustid_result those imports already + # recorded (pass->verified, skip->unverified). force_imported + # can't be derived retroactively. Idempotent: only fills NULLs. + cursor.execute(""" + UPDATE library_history SET verification_status = + CASE acoustid_result + WHEN 'pass' THEN 'verified' + WHEN 'skip' THEN 'unverified' + END + WHERE verification_status IS NULL + AND acoustid_result IN ('pass', 'skip') + """) + if cursor.rowcount: + logger.info("Backfilled verification_status from acoustid_result (%d rows)", cursor.rowcount) + # Migration: download-origin provenance — what TRIGGERED a download # ('watchlist' + artist / 'playlist' + playlist name). Read by the # origin-history modal on the watchlist + sync pages. From 97b40cbd432a10c2b7dbf1ec98406703c1559407 Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 10 Jun 2026 19:40:07 +0200 Subject: [PATCH 10/14] =?UTF-8?q?feat(verification):=20review=20queue=20?= =?UTF-8?q?=E2=80=94=20listen/compare/approve/delete=20unverified=20downlo?= =?UTF-8?q?ads?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ⚠ Unverified filter rows gain actions: inline play (range-streamed from the history file path, server-side only), YouTube compare, Approve -> new human_verified status (tag + history + tracks; AcoustID scanner skips these entirely), Delete (file + entry) - API: /api/verification//stream|approve|delete (path only from DB row) - backfill: history rows with acoustid_result='fail' that exist at all were imported despite the failure = force_imported (covers pre-fix fallback imports like the user's 'My Ordinary Life') Co-Authored-By: Claude Opus 4.8 --- core/matching/verification_status.py | 5 +- core/repair_jobs/acoustid_scanner.py | 7 +++ database/music_database.py | 3 +- tests/test_acoustid_scanner.py | 20 ++++++++ web_server.py | 70 +++++++++++++++++++++++++++ webui/static/downloads.js | 2 + webui/static/pages-extra.js | 71 ++++++++++++++++++++++++++++ webui/static/style.css | 7 +++ 8 files changed, 183 insertions(+), 2 deletions(-) diff --git a/core/matching/verification_status.py b/core/matching/verification_status.py index 4b7569c9..05be1050 100644 --- a/core/matching/verification_status.py +++ b/core/matching/verification_status.py @@ -19,8 +19,11 @@ Quarantined files are never imported, so they carry no status. VERIFIED = 'verified' UNVERIFIED = 'unverified' FORCE_IMPORTED = 'force_imported' +# Set by the user via the review queue ("yes, this file IS the right track"). +# Outranks machine states: the scanner skips these entirely. +HUMAN_VERIFIED = 'human_verified' -ALL_STATUSES = (VERIFIED, UNVERIFIED, FORCE_IMPORTED) +ALL_STATUSES = (VERIFIED, UNVERIFIED, FORCE_IMPORTED, HUMAN_VERIFIED) # The file tag name (Vorbis comment key / ID3 TXXX desc / MP4 freeform). TAG_NAME = 'SOULSYNC_VERIFICATION' diff --git a/core/repair_jobs/acoustid_scanner.py b/core/repair_jobs/acoustid_scanner.py index ecf1bd3d..dd7db7af 100644 --- a/core/repair_jobs/acoustid_scanner.py +++ b/core/repair_jobs/acoustid_scanner.py @@ -237,6 +237,13 @@ class AcoustIDScannerJob(RepairJob): file_verif_status = (_rft(fpath) or {}).get('verification_status') except Exception: pass + if file_verif_status == 'human_verified': + # The user explicitly confirmed this file via the review queue — + # never second-guess a human decision. + if context.report_progress: + context.report_progress( + log_line=f'Skipped (human-verified): {fname}', log_type='skip') + return if file_verif_status == 'force_imported' and \ self._get_settings(context).get('skip_force_imported', False): if context.report_progress: diff --git a/database/music_database.py b/database/music_database.py index 2547d740..f131a03b 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -651,9 +651,10 @@ class MusicDatabase: CASE acoustid_result WHEN 'pass' THEN 'verified' WHEN 'skip' THEN 'unverified' + WHEN 'fail' THEN 'force_imported' END WHERE verification_status IS NULL - AND acoustid_result IN ('pass', 'skip') + AND acoustid_result IN ('pass', 'skip', 'fail') """) if cursor.rowcount: logger.info("Backfilled verification_status from acoustid_result (%d rows)", cursor.rowcount) diff --git a/tests/test_acoustid_scanner.py b/tests/test_acoustid_scanner.py index ce04b82f..089d594e 100644 --- a/tests/test_acoustid_scanner.py +++ b/tests/test_acoustid_scanner.py @@ -868,3 +868,23 @@ def test_force_imported_mismatch_is_reported_as_informational(monkeypatch): def test_force_imported_can_be_skipped_via_setting(monkeypatch): captured = _force_imported_scan(monkeypatch, skip_setting=True) assert captured == [] + + +def test_human_verified_files_are_never_scanned(monkeypatch): + import core.repair_jobs.acoustid_scanner as scanner_mod + monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", + lambda name: [], raising=False) + monkeypatch.setattr('core.tag_writer.read_file_tags', + lambda fpath: {'artist': None, 'verification_status': 'human_verified'}) + job = AcoustIDScannerJob() + captured = [] + context = _make_finding_capturing_context( + track_row=("7", "T", "A", "/music/t.flac", 1, "Al", None, None), + captured=captured) + fake = SimpleNamespace(fingerprint_and_lookup=lambda f: { + 'best_score': 0.99, + 'recordings': [{'title': 'Totally Different', 'artist': 'Metallica'}]}) + job._scan_file('/music/t.flac', '7', {'title': 'T', 'artist': 'A'}, + fake, context, JobResultStub(), + fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6) + assert captured == [] diff --git a/web_server.py b/web_server.py index 54c47f33..29feac4d 100644 --- a/web_server.py +++ b/web_server.py @@ -7512,6 +7512,76 @@ def stream_quarantine_item(entry_id): return jsonify({"error": str(e)}), 500 +@app.route('/api/verification//stream', methods=['GET']) +def stream_verification_item(history_id): + """Stream a completed download for the verification review queue (listen + before approving). Path comes ONLY from the history row — no client paths.""" + try: + rows = get_database().get_library_history_rows_by_ids([history_id]) + if not rows or not rows[0].get('file_path'): + return jsonify({"error": "History entry or file path not found"}), 404 + file_path = rows[0]['file_path'] + if not os.path.exists(file_path): + return jsonify({"error": "File not found on disk"}), 404 + ext = os.path.splitext(file_path)[1].lower().lstrip('.') + mimetype = _AUDIO_MIME_TYPES.get(ext, 'audio/mpeg') + return _serve_audio_file_with_range(file_path, mimetype_override=mimetype) + except Exception as e: + logger.error(f"[Verification] Error streaming history {history_id}: {e}") + return jsonify({"error": str(e)}), 500 + + +@app.route('/api/verification//approve', methods=['POST']) +def approve_verification_item(history_id): + """User confirmed the file IS the right track: set human_verified on the + history row, the file tag, and (best-effort) the tracks row. The AcoustID + scanner skips human-verified files entirely.""" + try: + from core.matching.verification_status import HUMAN_VERIFIED + from core.tag_writer import write_verification_status + db = get_database() + rows = db.get_library_history_rows_by_ids([history_id]) + if not rows: + return jsonify({"success": False, "error": "History entry not found"}), 404 + file_path = rows[0].get('file_path') or '' + conn = db._get_connection() + conn.cursor().execute( + "UPDATE library_history SET verification_status = ? WHERE id = ?", + (HUMAN_VERIFIED, history_id)) + if file_path: + conn.cursor().execute( + "UPDATE tracks SET verification_status = ? WHERE file_path = ?", + (HUMAN_VERIFIED, file_path)) + conn.commit() + tag_written = bool(file_path) and write_verification_status(file_path, HUMAN_VERIFIED) + return jsonify({"success": True, "tag_written": tag_written}) + except Exception as e: + logger.error(f"[Verification] Approve failed for {history_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/verification//delete', methods=['POST']) +def delete_verification_item(history_id): + """User decided the file is wrong: delete it from disk and drop the + history row (the media-server mirror cleans the tracks row on next scan).""" + try: + db = get_database() + rows = db.get_library_history_rows_by_ids([history_id]) + if not rows: + return jsonify({"success": False, "error": "History entry not found"}), 404 + file_path = rows[0].get('file_path') or '' + file_deleted = False + if file_path and os.path.exists(file_path): + os.remove(file_path) + file_deleted = True + logger.info(f"[Verification] Deleted rejected file: {file_path}") + db.delete_library_history_rows([history_id]) + return jsonify({"success": True, "file_deleted": file_deleted}) + except Exception as e: + logger.error(f"[Verification] Delete failed for {history_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + @app.route('/api/quarantine//recover', methods=['POST']) def recover_quarantine_item(entry_id): """Fallback for legacy thin sidecars: move file into Staging so the user diff --git a/webui/static/downloads.js b/webui/static/downloads.js index ae58ee4e..8a50d453 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -3689,6 +3689,8 @@ function processModalStatusUpdate(playlistId, data) { statusText += ' ⚠'; } else if (task.verification_status === 'verified') { statusText += ' ✔'; + } else if (task.verification_status === 'human_verified') { + statusText += ' 🛡✔'; } completedCount++; break; diff --git a/webui/static/pages-extra.js b/webui/static/pages-extra.js index 5581afe5..141eabfb 100644 --- a/webui/static/pages-extra.js +++ b/webui/static/pages-extra.js @@ -2359,9 +2359,78 @@ function _adlVerifBadge(dl) { if (dl.verification_status === 'verified') { return ' ✔'; } + if (dl.verification_status === 'human_verified') { + return ' 🛡✔'; + } return ''; } +// ---- Verification review queue (the ⚠ Unverified filter) ---- +let _verifAudio = null; +let _verifAudioId = null; + +function verifHistoryId(dl) { + // Persistent history rows carry task_id 'history-'. + if (!dl.is_persistent_history || !dl.task_id) return null; + const m = String(dl.task_id).match(/^history-(\d+)$/); + return m ? m[1] : null; +} + +function _adlReviewActions(dl) { + if (_adlFilter !== 'unverified') return ''; + const hid = verifHistoryId(dl); + if (!hid) return ''; + const q = encodeURIComponent(`${dl.artist || ''} ${dl.title || ''}`.trim()); + const playing = _verifAudioId === hid ? ' playing' : ''; + return `
+ + + + +
`; +} + +function verifTogglePlay(hid, btn) { + if (_verifAudio && _verifAudioId === hid) { + _verifAudio.pause(); _verifAudio = null; _verifAudioId = null; + if (btn) { btn.textContent = '▶'; btn.classList.remove('playing'); } + return; + } + if (_verifAudio) { try { _verifAudio.pause(); } catch (e) {} } + document.querySelectorAll('.verif-act-play.playing').forEach(b => { b.textContent = '▶'; b.classList.remove('playing'); }); + _verifAudio = new Audio(`/api/verification/${hid}/stream`); + _verifAudioId = hid; + _verifAudio.play().catch(() => { showToast && showToast('Could not play file', 'error'); }); + _verifAudio.onended = () => { _verifAudioId = null; if (btn) { btn.textContent = '▶'; btn.classList.remove('playing'); } }; + if (btn) { btn.textContent = '⏸'; btn.classList.add('playing'); } +} + +async function verifApprove(hid, btn) { + if (btn) btn.disabled = true; + try { + const r = await fetch(`/api/verification/${hid}/approve`, { method: 'POST' }); + const d = await r.json(); + if (d.success) { showToast && showToast('Marked as human-verified 🛡✔', 'success'); _adlFetch(); } + else showToast && showToast(d.error || 'Approve failed', 'error'); + } catch (e) { showToast && showToast('Approve failed', 'error'); } + if (btn) btn.disabled = false; +} + +async function verifDelete(hid, btn) { + if (!confirm('Delete this file from disk and remove the entry?')) return; + if (btn) btn.disabled = true; + try { + const r = await fetch(`/api/verification/${hid}/delete`, { method: 'POST' }); + const d = await r.json(); + if (d.success) { showToast && showToast('File deleted', 'success'); _adlFetch(); } + else showToast && showToast(d.error || 'Delete failed', 'error'); + } catch (e) { showToast && showToast('Delete failed', 'error'); } + if (btn) btn.disabled = false; +} +window.verifTogglePlay = verifTogglePlay; +window.verifApprove = verifApprove; +window.verifDelete = verifDelete; + function _adlRender() { const list = document.getElementById('adl-list'); const empty = document.getElementById('adl-empty'); @@ -2387,6 +2456,7 @@ function _adlRender() { else if (_adlFilter === 'unverified') filtered = filtered.filter(d => completedStatuses.includes(d.status) && (d.verification_status === 'unverified' || d.verification_status === 'force_imported')); + // (review banner injected below when this filter is active) else if (_adlFilter === 'failed') filtered = filtered.filter(d => failedStatuses.includes(d.status)); const completedN = _adlData.filter(d => @@ -2514,6 +2584,7 @@ function _adlRender() { ${statusLabel}${_adlVerifBadge(dl)}${dl.retry_info && (statusClass === 'active' || statusClass === 'queued') ? ` 🔁 ${_adlEsc(String(dl.retry_info))}` : ''} + ${_adlReviewActions(dl)} ${cancelBtnHtml} `; } diff --git a/webui/static/style.css b/webui/static/style.css index 7e74308a..35567a5d 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -67899,3 +67899,10 @@ body.em-scroll-lock { overflow: hidden; } .verif-badge.verif-unverified { color: #f1c40f; background: rgba(241,196,15,0.14); } .verif-badge.verif-force { color: #e67e22; background: rgba(230,126,34,0.16); } .adl-retry-info { margin-left: 6px; font-size: 11px; color: #e67e22; cursor: help; } +.verif-badge.verif-human { color: #3498db; background: rgba(52,152,219,0.14); } +.verif-actions { display: inline-flex; gap: 6px; margin-left: 10px; align-items: center; } +.verif-act { border: 1px solid rgba(255,255,255,0.18); background: rgba(255,255,255,0.06); color: rgba(255,255,255,0.85); border-radius: 6px; padding: 2px 8px; font-size: 12px; cursor: pointer; line-height: 18px; } +.verif-act:hover { background: rgba(255,255,255,0.14); } +.verif-act-ok:hover { background: rgba(46,204,113,0.25); border-color: rgba(46,204,113,0.5); } +.verif-act-del:hover { background: rgba(231,76,60,0.25); border-color: rgba(231,76,60,0.5); } +.verif-act-play.playing { background: rgba(var(--accent-rgb),0.3); } From 5896f2dcc6cb0be258fb51415dca0a3b57867738 Mon Sep 17 00:00:00 2001 From: dev Date: Thu, 11 Jun 2026 01:09:05 +0200 Subject: [PATCH 11/14] fix: eager config load + check acoustid.enabled for verification pill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On fresh page load the Downloads pill now immediately reflects whether Download Verification is enabled (calls _verifLoadConfig in loadActiveDownloadsPage instead of only on first filter click). Also changed /api/verification/config to check the `acoustid.enabled` toggle rather than the raw api_key string — matches the UI setting "Enable Download Verification". Co-Authored-By: Claude Sonnet 4.6 --- web_server.py | 361 ++++++++++++++++++++++++++-- webui/static/pages-extra.js | 460 ++++++++++++++++++++++++++++++++++-- 2 files changed, 782 insertions(+), 39 deletions(-) diff --git a/web_server.py b/web_server.py index 29feac4d..121455d6 100644 --- a/web_server.py +++ b/web_server.py @@ -7512,18 +7512,76 @@ def stream_quarantine_item(entry_id): return jsonify({"error": str(e)}), 500 +def _get_library_history_row(history_id): + """Fetch one full library_history row as a dict (or None).""" + conn = get_database()._get_connection() + cursor = conn.cursor() + cursor.execute("SELECT * FROM library_history WHERE id = ?", (history_id,)) + row = cursor.fetchone() + return dict(row) if row else None + + +def _resolve_history_audio_path(row): + """Resolve a library_history row to a playable on-disk file. + + The recorded path can go stale: Docker↔host prefix differences, or the + media server / organizer renaming files with exotic titles (e.g. + 凸】♀】♂】←Titan) after import. Fallback chain: + 1. the recorded path as-is, + 2. `_resolve_library_file_path` (transfer/download/library prefix swap), + 3. the tracks table — the media-server mirror knows the CURRENT path for + this title+artist even after a rename — resolved the same way. + """ + raw_path = (row.get('file_path') or '').strip() + if raw_path and os.path.exists(raw_path): + return raw_path + resolved = _resolve_library_file_path(raw_path) if raw_path else None + if resolved and os.path.exists(resolved): + return resolved + title = (row.get('title') or '').strip() + if not title: + return None + artist = (row.get('artist_name') or '').strip() + try: + conn = get_database()._get_connection() + cursor = conn.cursor() + cursor.execute( + "SELECT file_path FROM tracks WHERE file_path IS NOT NULL AND LOWER(title) = LOWER(?)", + (title,)) + candidates = [r[0] for r in cursor.fetchall() if r[0]] + except Exception as e: + logger.debug(f"[Verification] tracks-table path fallback failed: {e}") + return None + # Same-title collisions across artists exist (and delete() trusts this + # path), so be strict: when the row names an artist, only accept + # candidates whose path mentions it; otherwise only an unambiguous + # single candidate. + artist_l = artist.lower() + if artist_l: + candidates = [p for p in candidates if artist_l in p.lower()] + elif len(candidates) != 1: + return None + for cand in candidates: + cand_resolved = _resolve_library_file_path(cand) + if cand_resolved and os.path.exists(cand_resolved): + return cand_resolved + return None + + @app.route('/api/verification//stream', methods=['GET']) def stream_verification_item(history_id): """Stream a completed download for the verification review queue (listen before approving). Path comes ONLY from the history row — no client paths.""" try: - rows = get_database().get_library_history_rows_by_ids([history_id]) - if not rows or not rows[0].get('file_path'): - return jsonify({"error": "History entry or file path not found"}), 404 - file_path = rows[0]['file_path'] - if not os.path.exists(file_path): + row = _get_library_history_row(history_id) + if not row: + return jsonify({"error": "History entry not found"}), 404 + file_path = _resolve_history_audio_path(row) + if not file_path: return jsonify({"error": "File not found on disk"}), 404 - ext = os.path.splitext(file_path)[1].lower().lstrip('.') + # _AUDIO_MIME_TYPES keys keep the dot ('.flac') — don't strip it, or + # everything falls back to audio/mpeg and FLAC playback breaks. + ext = os.path.splitext(file_path)[1].lower() mimetype = _AUDIO_MIME_TYPES.get(ext, 'audio/mpeg') return _serve_audio_file_with_range(file_path, mimetype_override=mimetype) except Exception as e: @@ -7531,6 +7589,264 @@ def stream_verification_item(history_id): return jsonify({"error": str(e)}), 500 +@app.route('/api/verification//entry', methods=['GET']) +def get_verification_entry(history_id): + """Full library_history row for one review-queue item — feeds the Audit + Trail modal when opened from the Downloads page (where the history-page + entry cache is not populated).""" + try: + row = _get_library_history_row(history_id) + if not row: + return jsonify({"success": False, "error": "History entry not found"}), 404 + return jsonify({"success": True, "entry": row}) + except Exception as e: + logger.error(f"[Verification] Entry fetch failed for {history_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/verification/config', methods=['GET']) +def get_verification_config(): + """Whether AcoustID/download-verification is enabled — if not, the review + queue collapses to quarantine-only in the UI.""" + try: + enabled = bool(config_manager.get('acoustid.enabled', False)) + return jsonify({"success": True, "acoustid_enabled": enabled}) + except Exception as e: + return jsonify({"success": True, "acoustid_enabled": True, "error": str(e)}) + + +def _audio_file_duration_ms(path): + """Best-effort duration of an on-disk audio file (0 when unreadable). + mutagen detects the format from content, so this also works for + quarantined files whose extension was swapped to `.quarantined`.""" + try: + import mutagen + mf = mutagen.File(path) + if mf and mf.info and getattr(mf.info, 'length', 0): + return int(mf.info.length * 1000) + except Exception: + pass + return 0 + + +def _set_review_play_session(file_path, title, artist, album, mimetype=None): + """Point THIS listener's media-player session at a local file — same + mechanism as /api/library/play, so the bottom player UI drives playback + (seek/stop/volume) instead of an invisible Audio element.""" + sess = _current_stream_state() + with sess.lock: + sess.update({ + "status": "ready", + "progress": 100, + "track_info": { + "title": title or os.path.basename(file_path), + "artist": artist or 'Unknown Artist', + "album": album or '', + }, + "file_path": file_path, + "stream_url": None, + "error_message": None, + "is_library": True, + # Content-Type hint for /stream/audio — needed for quarantined + # files whose on-disk extension is `.quarantined`. Keyed to the + # exact path so a stale hint can never leak onto another file. + "mimetype_override": mimetype, + "mimetype_override_path": file_path if mimetype else None, + }) + + +@app.route('/api/verification//play', methods=['POST']) +def play_verification_item(history_id): + """Load the downloaded file into the media player (review queue ▶).""" + try: + row = _get_library_history_row(history_id) + if not row: + return jsonify({"success": False, "error": "History entry not found"}), 404 + file_path = _resolve_history_audio_path(row) + if not file_path: + return jsonify({"success": False, "error": "File not found on disk"}), 404 + _set_review_play_session( + file_path, row.get('title'), row.get('artist_name'), row.get('album_name')) + return jsonify({"success": True, "track_info": { + "title": row.get('title') or os.path.basename(file_path), + "artist": row.get('artist_name') or '', + "album": row.get('album_name') or '', + "image_url": row.get('thumb_url') or None, + }}) + except Exception as e: + logger.error(f"[Verification] Play failed for {history_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/verification//compare-stream', methods=['POST']) +def compare_stream_verification_item(history_id): + """Find the expected track on Soulseek/streaming sources for an A/B + comparison — the SAME pipeline as the /search page play button, but fed + server-side so the local file's duration guides candidate ranking (a + missing duration lets e.g. 10-hour YouTube loops win and time out).""" + try: + row = _get_library_history_row(history_id) + if not row: + return jsonify({"success": False, "error": "History entry not found"}), 404 + local = _resolve_history_audio_path(row) + duration_ms = _audio_file_duration_ms(local) if local else 0 + result = _search_stream.stream_search_track( + track_name=row.get('title') or '', + artist_name=row.get('artist_name') or '', + album_name=row.get('album_name') or '', + duration_ms=duration_ms, + config_manager=config_manager, + download_orchestrator=download_orchestrator, + matching_engine=matching_engine, + run_async=run_async, + ) + if result is None: + return jsonify({"success": False, + "error": "No suitable stream candidate found"}), 404 + result['title'] = row.get('title') or '' + result['artist'] = row.get('artist_name') or '' + result['album'] = row.get('album_name') or '' + result['image_url'] = row.get('thumb_url') or None + return jsonify({"success": True, "result": result}) + except Exception as e: + logger.error(f"[Verification] Compare-stream failed for {history_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +def _get_quarantine_entry(entry_id): + from core.imports.quarantine import list_quarantine_entries + for entry in list_quarantine_entries(_get_quarantine_dir()): + if entry.get('id') == entry_id: + return entry + return None + + +@app.route('/api/quarantine//play', methods=['POST']) +def play_quarantine_item(entry_id): + """Load a quarantined file into the media player (review queue ▶).""" + try: + from core.imports.quarantine import get_quarantine_entry_stream_info + info = get_quarantine_entry_stream_info(_get_quarantine_dir(), entry_id) + if info is None: + return jsonify({"success": False, "error": "Quarantined file not found"}), 404 + file_path, extension = info + entry = _get_quarantine_entry(entry_id) or {} + title = entry.get('expected_track') or entry.get('original_filename') or os.path.basename(file_path) + _set_review_play_session( + file_path, f"{title} (quarantined)", entry.get('expected_artist'), '', + mimetype=_AUDIO_MIME_TYPES.get(extension, 'audio/mpeg')) + return jsonify({"success": True, "track_info": { + "title": f"{title} (quarantined)", + "artist": entry.get('expected_artist') or '', + "album": '', + }}) + except Exception as e: + logger.error(f"[Quarantine] Play failed for {entry_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/quarantine//compare-stream', methods=['POST']) +def compare_stream_quarantine_item(entry_id): + """Stream-search the EXPECTED track for a quarantined file (A/B compare), + using the quarantined file's duration to guide candidate ranking.""" + try: + from core.imports.quarantine import get_quarantine_entry_stream_info + entry = _get_quarantine_entry(entry_id) + if not entry: + return jsonify({"success": False, "error": "Quarantine entry not found"}), 404 + track_name = entry.get('expected_track') or '' + artist_name = entry.get('expected_artist') or '' + if not track_name: + return jsonify({"success": False, + "error": "Entry has no expected-track metadata"}), 400 + info = get_quarantine_entry_stream_info(_get_quarantine_dir(), entry_id) + duration_ms = _audio_file_duration_ms(info[0]) if info else 0 + result = _search_stream.stream_search_track( + track_name=track_name, + artist_name=artist_name, + album_name='', + duration_ms=duration_ms, + config_manager=config_manager, + download_orchestrator=download_orchestrator, + matching_engine=matching_engine, + run_async=run_async, + ) + if result is None: + return jsonify({"success": False, + "error": "No suitable stream candidate found"}), 404 + result['title'] = track_name + result['artist'] = artist_name + return jsonify({"success": True, "result": result}) + except Exception as e: + logger.error(f"[Quarantine] Compare-stream failed for {entry_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/quarantine//entry', methods=['GET']) +def get_quarantine_audit_entry(entry_id): + """Synthesize a library_history-shaped entry from a quarantine sidecar so + the review queue opens the SAME Audit Trail modal for quarantined files + (they were never imported, so no history row exists). ``id`` is None on + purpose — the modal fetches tags/lyrics through ``_file_tags_url``.""" + try: + from core.imports.quarantine import ( + get_quarantine_entry_context, get_quarantine_entry_stream_info) + entry = _get_quarantine_entry(entry_id) + if not entry: + return jsonify({"success": False, "error": "Quarantine entry not found"}), 404 + ctx = get_quarantine_entry_context(_get_quarantine_dir(), entry_id) + info = get_quarantine_entry_stream_info(_get_quarantine_dir(), entry_id) + osr = ctx.get('original_search_result') if isinstance(ctx.get('original_search_result'), dict) else {} + username = (osr.get('username') or '') if isinstance(osr, dict) else '' + streaming = ('tidal', 'youtube', 'qobuz', 'hifi', 'deezer_dl', + 'lidarr', 'soundcloud', 'amazon') + ti = ctx.get('track_info') if isinstance(ctx.get('track_info'), dict) else {} + album_raw = ti.get('album', '') + album_name = album_raw.get('name', '') if isinstance(album_raw, dict) else str(album_raw or '') + synthetic = { + 'id': None, + 'event_type': 'download', + 'title': entry.get('expected_track') or entry.get('original_filename') or '', + 'artist_name': entry.get('expected_artist') or '', + 'album_name': album_name, + 'created_at': entry.get('timestamp') or '', + 'thumb_url': entry.get('thumb_url') or '', + 'file_path': info[0] if info else '', + 'quality': ctx.get('_audio_quality') or '', + 'download_source': username if username in streaming else ('soulseek' if username else ''), + 'source_filename': entry.get('source_filename') or '', + 'source_artist': (osr.get('artist') or '') if isinstance(osr, dict) else '', + 'source_track_title': (osr.get('title') or osr.get('name') or '') if isinstance(osr, dict) else '', + 'acoustid_result': 'fail' if entry.get('trigger') == 'acoustid' else None, + 'verification_status': None, + '_quarantined': True, + '_quarantine_reason': entry.get('reason') or '', + '_file_tags_url': f"/api/quarantine/{entry_id}/file-tags", + } + return jsonify({"success": True, "entry": synthetic}) + except Exception as e: + logger.error(f"[Quarantine] Audit entry failed for {entry_id}: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/quarantine//file-tags', methods=['GET']) +def get_quarantine_file_tags(entry_id): + """Embedded tags of a quarantined file — feeds the Audit modal's Tags / + Lyrics tabs. mutagen detects the format from content, so the swapped + `.quarantined` extension is no obstacle.""" + try: + from core.imports.quarantine import get_quarantine_entry_stream_info + from core.library.file_tags import read_embedded_tags + info = get_quarantine_entry_stream_info(_get_quarantine_dir(), entry_id) + if info is None: + return jsonify({'success': False, 'error': 'Quarantined file not found'}), 404 + result = read_embedded_tags(info[0]) + return jsonify({'success': True, **result}) + except Exception as e: + logger.error(f"[Quarantine] File-tags failed for {entry_id}: {e}") + return jsonify({'success': False, 'error': str(e)}), 500 + + @app.route('/api/verification//approve', methods=['POST']) def approve_verification_item(history_id): """User confirmed the file IS the right track: set human_verified on the @@ -7540,20 +7856,22 @@ def approve_verification_item(history_id): from core.matching.verification_status import HUMAN_VERIFIED from core.tag_writer import write_verification_status db = get_database() - rows = db.get_library_history_rows_by_ids([history_id]) - if not rows: + row = _get_library_history_row(history_id) + if not row: return jsonify({"success": False, "error": "History entry not found"}), 404 - file_path = rows[0].get('file_path') or '' + file_path = row.get('file_path') or '' + on_disk = _resolve_history_audio_path(row) conn = db._get_connection() conn.cursor().execute( "UPDATE library_history SET verification_status = ? WHERE id = ?", (HUMAN_VERIFIED, history_id)) - if file_path: + # The tracks row may carry either the recorded or the resolved path. + for p in {p for p in (file_path, on_disk) if p}: conn.cursor().execute( "UPDATE tracks SET verification_status = ? WHERE file_path = ?", - (HUMAN_VERIFIED, file_path)) + (HUMAN_VERIFIED, p)) conn.commit() - tag_written = bool(file_path) and write_verification_status(file_path, HUMAN_VERIFIED) + tag_written = bool(on_disk) and write_verification_status(on_disk, HUMAN_VERIFIED) return jsonify({"success": True, "tag_written": tag_written}) except Exception as e: logger.error(f"[Verification] Approve failed for {history_id}: {e}") @@ -7566,15 +7884,15 @@ def delete_verification_item(history_id): history row (the media-server mirror cleans the tracks row on next scan).""" try: db = get_database() - rows = db.get_library_history_rows_by_ids([history_id]) - if not rows: + row = _get_library_history_row(history_id) + if not row: return jsonify({"success": False, "error": "History entry not found"}), 404 - file_path = rows[0].get('file_path') or '' + on_disk = _resolve_history_audio_path(row) file_deleted = False - if file_path and os.path.exists(file_path): - os.remove(file_path) + if on_disk and os.path.exists(on_disk): + os.remove(on_disk) file_deleted = True - logger.info(f"[Verification] Deleted rejected file: {file_path}") + logger.info(f"[Verification] Deleted rejected file: {on_disk}") db.delete_library_history_rows([history_id]) return jsonify({"success": True, "file_deleted": file_deleted}) except Exception as e: @@ -12776,6 +13094,11 @@ def stream_audio(): stream_url = sess.get("stream_url") if not file_path and not stream_url: return jsonify({"error": "No audio file ready for streaming"}), 404 + # Content-Type hint set by the quarantine player (on-disk extension + # is `.quarantined`). Path-keyed so it can't leak onto other files. + mimetype_override = (sess.get("mimetype_override") + if sess.get("mimetype_override_path") == file_path + else None) # Library track played via the media server's stream API (#809). if stream_url: @@ -12783,7 +13106,7 @@ def stream_audio(): return _proxy_stream_url_with_range(stream_url) logger.info(f"Serving audio file: {os.path.basename(file_path)}") - return _serve_audio_file_with_range(file_path) + return _serve_audio_file_with_range(file_path, mimetype_override=mimetype_override) except Exception as e: logger.error(f"Error serving audio file: {e}") return jsonify({"error": str(e)}), 500 diff --git a/webui/static/pages-extra.js b/webui/static/pages-extra.js index 141eabfb..a866322d 100644 --- a/webui/static/pages-extra.js +++ b/webui/static/pages-extra.js @@ -2282,6 +2282,7 @@ function _adlRenderBatchSummary(activeBatches) { } function loadActiveDownloadsPage() { + _verifLoadConfig(); _adlFetch(); _adlFetchBatchHistory(); // Poll downloads every 2 seconds, history every 60 seconds @@ -2365,9 +2366,7 @@ function _adlVerifBadge(dl) { return ''; } -// ---- Verification review queue (the ⚠ Unverified filter) ---- -let _verifAudio = null; -let _verifAudioId = null; +// ---- Verification review queue (the ⚠ Unverified/Quarantine filter) ---- function verifHistoryId(dl) { // Persistent history rows carry task_id 'history-'. @@ -2376,33 +2375,95 @@ function verifHistoryId(dl) { return m ? m[1] : null; } +function _verifTimeAgo(iso) { + return (typeof formatHistoryTime === 'function' && iso) ? formatHistoryTime(iso) : ''; +} + +function _verifReasonBadge(dl) { + // Glanceable badge in the style of the library-history quarantine tab. + if (dl.verification_status === 'force_imported') { + return 'FORCE-IMPORTED'; + } + if (dl.verification_status === 'unverified') { + return 'ACOUSTID UNCONFIRMED'; + } + return ''; +} + function _adlReviewActions(dl) { if (_adlFilter !== 'unverified') return ''; const hid = verifHistoryId(dl); if (!hid) return ''; - const q = encodeURIComponent(`${dl.artist || ''} ${dl.title || ''}`.trim()); - const playing = _verifAudioId === hid ? ' playing' : ''; + const timeAgo = _verifTimeAgo(dl.created_at); return `
- - + ${_verifReasonBadge(dl)} + ${timeAgo ? `${timeAgo}` : ''} + + +
`; } -function verifTogglePlay(hid, btn) { - if (_verifAudio && _verifAudioId === hid) { - _verifAudio.pause(); _verifAudio = null; _verifAudioId = null; - if (btn) { btn.textContent = '▶'; btn.classList.remove('playing'); } - return; +async function verifPlay(hid) { + // Plays the LOCAL file through the global media player (same machinery as + // library playback) — full player UI with seek/stop instead of an + // invisible Audio element that re-renders wiped. + const dl = _adlData.find(d => verifHistoryId(d) === String(hid)); + try { + if (typeof setTrackInfo === 'function') { + setTrackInfo({ + title: (dl && dl.title) || 'Review track', + artist: (dl && dl.artist) || '', + album: (dl && dl.album) || '', + is_library: true, + image_url: (dl && dl.artwork) || null, + }); + } + if (typeof showLoadingAnimation === 'function') showLoadingAnimation(); + const r = await fetch(`/api/verification/${hid}/play`, { method: 'POST' }); + const d = await r.json(); + if (!d.success) throw new Error(d.error || 'Playback failed'); + await startAudioPlayback(); + } catch (e) { + if (typeof hideLoadingAnimation === 'function') hideLoadingAnimation(); + showToast && showToast('Playback failed: ' + e.message, 'error'); + } +} + +async function verifCompare(hid, btn) { + // Same pipeline as the /search page play button — run server-side so the + // local file's duration guides candidate ranking (avoids e.g. 10-hour + // YouTube loops winning the match and timing out). + if (btn) { btn.disabled = true; btn.textContent = '…'; } + showToast && showToast('Searching stream for comparison…', 'info'); + try { + const res = await fetch(`/api/verification/${hid}/compare-stream`, { method: 'POST' }); + const data = await res.json(); + if (data.success && data.result && typeof startStream === 'function') { + await startStream(data.result); + } else { + showToast && showToast(data.error || 'No stream candidate found for comparison', 'error'); + } + } catch (e) { + showToast && showToast('Stream failed: ' + e.message, 'error'); + } + if (btn) { btn.disabled = false; btn.textContent = '⇆'; } +} + +async function verifAudit(hid) { + try { + const r = await fetch(`/api/verification/${hid}/entry`); + const d = await r.json(); + if (d.success && d.entry && typeof openDownloadAuditModal === 'function') { + openDownloadAuditModal(d.entry); + } else { + showToast && showToast(d.error || 'Audit data not available', 'error'); + } + } catch (e) { + showToast && showToast('Audit load failed', 'error'); } - if (_verifAudio) { try { _verifAudio.pause(); } catch (e) {} } - document.querySelectorAll('.verif-act-play.playing').forEach(b => { b.textContent = '▶'; b.classList.remove('playing'); }); - _verifAudio = new Audio(`/api/verification/${hid}/stream`); - _verifAudioId = hid; - _verifAudio.play().catch(() => { showToast && showToast('Could not play file', 'error'); }); - _verifAudio.onended = () => { _verifAudioId = null; if (btn) { btn.textContent = '▶'; btn.classList.remove('playing'); } }; - if (btn) { btn.textContent = '⏸'; btn.classList.add('playing'); } } async function verifApprove(hid, btn) { @@ -2427,9 +2488,326 @@ async function verifDelete(hid, btn) { } catch (e) { showToast && showToast('Delete failed', 'error'); } if (btn) btn.disabled = false; } -window.verifTogglePlay = verifTogglePlay; +// ---- Quarantine sub-view inside the ⚠ filter ---- +// The review queue covers BOTH kinds of suspect files: imported-but- +// unconfirmed (unverified/force_imported) and not-imported-at-all +// (quarantined). One place to listen, compare, approve or delete. +let _verifSubView = 'unverified'; +let _verifQuarEntries = []; +let _verifQuarLoaded = false; +let _verifQuarLoading = false; +// Expanded 🔍 detail panels, keyed by quarantine entry id — survives the +// polling re-render (which rebuilds the rows every few seconds). +const _verifQuarOpenDetails = new Set(); +// null = not fetched yet (assume enabled). Without an AcoustID API key +// nothing ever gets a verification status, so the review queue collapses +// to quarantine-only. +let _verifAcoustidEnabled = null; +let _verifConfigLoading = false; + +async function _verifLoadConfig() { + if (_verifAcoustidEnabled !== null || _verifConfigLoading) return; + _verifConfigLoading = true; + try { + const r = await fetch('/api/verification/config'); + const d = await r.json(); + _verifAcoustidEnabled = !!(d && d.acoustid_enabled); + } catch (e) { _verifAcoustidEnabled = true; } + _verifConfigLoading = false; + if (_verifAcoustidEnabled === false) { + _verifSubView = 'quarantine'; + const pill = document.querySelector('.adl-pill[data-filter="unverified"]'); + if (pill) { + pill.textContent = '🛡 Quarantine'; + pill.title = 'Files that failed import checks and were NOT imported. (AcoustID is not configured, so there is no unverified review queue.)'; + } + if (_adlFilter === 'unverified') { _verifLoadQuarantine(true); _adlRender(); } + } +} + +function verifSetSubView(v) { + if (_verifAcoustidEnabled === false) v = 'quarantine'; + _verifSubView = v === 'quarantine' ? 'quarantine' : 'unverified'; + if (_verifSubView === 'quarantine') _verifLoadQuarantine(true); + _adlRender(); +} + +async function _verifLoadQuarantine(force) { + if (_verifQuarLoading || (_verifQuarLoaded && !force)) return; + _verifQuarLoading = true; + try { + const r = await fetch('/api/quarantine/list'); + const d = await r.json(); + _verifQuarEntries = (d.success && Array.isArray(d.entries)) ? d.entries : []; + } catch (e) { _verifQuarEntries = []; } + _verifQuarLoaded = true; + _verifQuarLoading = false; + if (_adlFilter === 'unverified') _adlRender(); +} + +const _VERIF_QUAR_TRIGGERS = { + integrity: ['DURATION / INTEGRITY', 'verif-rb-int'], + acoustid: ['ACOUSTID MISMATCH', 'verif-rb-force'], + bit_depth: ['BIT DEPTH FILTER', 'verif-rb-int'], +}; + +function _verifQuarRows() { + if (!_verifQuarLoaded) return '
Loading quarantine…
'; + if (!_verifQuarEntries.length) return ''; + let html = ''; + _verifQuarEntries.forEach((q, idx) => { + const title = _adlEsc(q.expected_track || q.original_filename || q.filename || 'Unknown file'); + const meta = [_adlEsc(q.expected_artist || ''), _adlEsc(q.original_filename || '')].filter(Boolean).join(' — '); + const [trigLabel, trigClass] = _VERIF_QUAR_TRIGGERS[q.trigger] || ['QUARANTINED', 'verif-rb-unv']; + const timeAgo = _verifTimeAgo(q.timestamp); + const approveBtn = q.has_full_context + ? `` + : ``; + const details = [ + q.reason ? `
Reason: ${_adlEsc(q.reason)}
` : '', + q.source_username ? `
Source uploader: ${_adlEsc(q.source_username)}
` : '', + q.source_filename ? `
Original Soulseek file: ${_adlEsc(q.source_filename)}
` : '', + q.timestamp ? `
Quarantined: ${_adlEsc(q.timestamp)}
` : '', + ].filter(Boolean).join(''); + const detailsOpen = _verifQuarOpenDetails.has(q.id); + const artHtml = q.thumb_url + ? `` + : '
'; + html += `
+ ${artHtml} +
+
${title}
+ ${meta ? `
${meta}
` : ''} +
${details || 'No further details in the sidecar.'}
+
+
+ ${trigLabel} + ${timeAgo ? `${timeAgo}` : ''} + + + + ${approveBtn} + +
+
`; + }); + return html; +} + +function verifQuarInspect(idx) { + // Open-state lives in a Set keyed by entry id (not the DOM) — the polling + // re-render rebuilds the rows every few seconds and would collapse a + // DOM-only toggle right after the click. + const q = _verifQuarEntries[idx]; + if (!q) return; + if (_verifQuarOpenDetails.has(q.id)) _verifQuarOpenDetails.delete(q.id); + else _verifQuarOpenDetails.add(q.id); + const el = document.getElementById(`verif-quar-details-${idx}`); + if (el) el.style.display = _verifQuarOpenDetails.has(q.id) ? '' : 'none'; +} + +async function verifQuarAudit(idx) { + // Same Audit Trail modal as the unverified rows — the backend synthesizes + // a history-shaped entry from the quarantine sidecar (these files were + // never imported, so no real history row exists). + const q = _verifQuarEntries[idx]; if (!q) return; + try { + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/entry`); + const d = await r.json(); + if (d.success && d.entry && typeof openDownloadAuditModal === 'function') { + openDownloadAuditModal(d.entry); + } else { + showToast && showToast(d.error || 'Audit data not available', 'error'); + } + } catch (e) { + showToast && showToast('Audit load failed', 'error'); + } +} + +async function verifQuarPlay(idx) { + const q = _verifQuarEntries[idx]; if (!q) return; + try { + if (typeof setTrackInfo === 'function') { + setTrackInfo({ + title: `${q.expected_track || q.original_filename || 'Quarantined file'} (quarantined)`, + artist: q.expected_artist || '', + album: '', + is_library: true, + }); + } + if (typeof showLoadingAnimation === 'function') showLoadingAnimation(); + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/play`, { method: 'POST' }); + const d = await r.json(); + if (!d.success) throw new Error(d.error || 'Playback failed'); + await startAudioPlayback(); + } catch (e) { + if (typeof hideLoadingAnimation === 'function') hideLoadingAnimation(); + showToast && showToast('Playback failed: ' + e.message, 'error'); + } +} + +async function verifQuarCompare(idx, btn) { + const q = _verifQuarEntries[idx]; if (!q) return; + if (btn) { btn.disabled = true; btn.textContent = '…'; } + showToast && showToast(`Searching stream for "${q.expected_track || ''}"…`, 'info'); + try { + const res = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/compare-stream`, { method: 'POST' }); + const data = await res.json(); + if (data.success && data.result && typeof startStream === 'function') { + await startStream(data.result); + } else { + showToast && showToast(data.error || 'No stream candidate found for comparison', 'error'); + } + } catch (e) { + showToast && showToast('Stream failed: ' + e.message, 'error'); + } + if (btn) { btn.disabled = false; btn.textContent = '⇆'; } +} + +async function verifQuarApprove(idx, btn) { + const q = _verifQuarEntries[idx]; if (!q) return; + if (btn) btn.disabled = true; + try { + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/approve`, { method: 'POST' }); + const d = await r.json(); + if (d.success) { + showToast && showToast('Approved — re-importing, will be marked human-verified 🛡✔', 'success'); + _verifLoadQuarantine(true); + } else { + showToast && showToast(d.error || 'Approve failed', 'error'); + } + } catch (e) { showToast && showToast('Approve failed', 'error'); } + if (btn) btn.disabled = false; +} + +async function verifQuarRecover(idx, btn) { + const q = _verifQuarEntries[idx]; if (!q) return; + if (btn) btn.disabled = true; + try { + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/recover`, { method: 'POST' }); + const d = await r.json(); + if (d.success) { + showToast && showToast('Moved to Staging — finish via the Import page', 'success'); + _verifLoadQuarantine(true); + } else { + showToast && showToast(d.error || 'Recover failed', 'error'); + } + } catch (e) { showToast && showToast('Recover failed', 'error'); } + if (btn) btn.disabled = false; +} + +async function verifQuarDelete(idx, btn) { + const q = _verifQuarEntries[idx]; if (!q) return; + if (!confirm('Permanently delete this quarantined file?')) return; + if (btn) btn.disabled = true; + try { + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}`, { method: 'DELETE' }); + const d = await r.json(); + if (d.success) { showToast && showToast('Quarantined file deleted', 'success'); _verifLoadQuarantine(true); } + else showToast && showToast(d.error || 'Delete failed', 'error'); + } catch (e) { showToast && showToast('Delete failed', 'error'); } + if (btn) btn.disabled = false; +} + +function _verifUnverifiedIds() { + const done = ['completed', 'skipped', 'already_owned']; + return _adlData + .filter(d => done.includes(d.status) && + (d.verification_status === 'unverified' || d.verification_status === 'force_imported')) + .map(d => verifHistoryId(d)) + .filter(Boolean); +} + +async function verifApproveAll(btn) { + const ids = _verifUnverifiedIds(); + if (!ids.length) return; + if (!confirm(`Mark all ${ids.length} unverified entries as human-verified? The AcoustID scanner will skip them from now on.`)) return; + if (btn) btn.disabled = true; + let ok = 0; + for (const id of ids) { + try { + const r = await fetch(`/api/verification/${id}/approve`, { method: 'POST' }); + const d = await r.json(); + if (d.success) ok++; + } catch (e) {} + } + showToast && showToast(`Approved ${ok}/${ids.length} entries 🛡✔`, ok === ids.length ? 'success' : 'error'); + if (btn) btn.disabled = false; + _adlFetch(); +} + +async function verifDeleteAll(btn) { + const ids = _verifUnverifiedIds(); + if (!ids.length) return; + if (!confirm(`Delete ALL ${ids.length} unverified files from disk and remove their entries? This cannot be undone.`)) return; + if (btn) btn.disabled = true; + let ok = 0; + for (const id of ids) { + try { + const r = await fetch(`/api/verification/${id}/delete`, { method: 'POST' }); + const d = await r.json(); + if (d.success) ok++; + } catch (e) {} + } + showToast && showToast(`Deleted ${ok}/${ids.length} files`, ok === ids.length ? 'success' : 'error'); + if (btn) btn.disabled = false; + _adlFetch(); +} + +async function verifQuarApproveAll(btn) { + const entries = _verifQuarEntries.filter(q => q.has_full_context); + if (!entries.length) { + showToast && showToast('No one-click-approvable entries (legacy sidecars need Recover)', 'info'); + return; + } + if (!confirm(`Approve and re-import all ${entries.length} quarantined files? They will be imported into the library marked human-verified.`)) return; + if (btn) btn.disabled = true; + let ok = 0; + for (const q of entries) { + try { + const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}/approve`, { method: 'POST' }); + const d = await r.json(); + if (d.success) ok++; + } catch (e) {} + // Each approve spawns a server-side re-import thread — stagger them. + await new Promise(res => setTimeout(res, 500)); + } + showToast && showToast(`Approved ${ok}/${entries.length} — re-importing as human-verified`, ok === entries.length ? 'success' : 'error'); + if (btn) btn.disabled = false; + _verifLoadQuarantine(true); +} + +async function verifQuarClearAll(btn) { + if (!_verifQuarEntries.length) return; + if (!confirm(`Permanently delete ALL ${_verifQuarEntries.length} quarantined files? This cannot be undone.`)) return; + if (btn) btn.disabled = true; + try { + const r = await fetch('/api/quarantine/clear', { method: 'POST' }); + const d = await r.json(); + if (d.success) showToast && showToast('Quarantine cleared', 'success'); + else showToast && showToast(d.error || 'Clear failed', 'error'); + } catch (e) { showToast && showToast('Clear failed', 'error'); } + if (btn) btn.disabled = false; + _verifLoadQuarantine(true); +} + +window.verifPlay = verifPlay; window.verifApprove = verifApprove; window.verifDelete = verifDelete; +window.verifCompare = verifCompare; +window.verifAudit = verifAudit; +window.verifSetSubView = verifSetSubView; +window.verifApproveAll = verifApproveAll; +window.verifDeleteAll = verifDeleteAll; +window.verifQuarPlay = verifQuarPlay; +window.verifQuarCompare = verifQuarCompare; +window.verifQuarInspect = verifQuarInspect; +window.verifQuarAudit = verifQuarAudit; +window.verifQuarApprove = verifQuarApprove; +window.verifQuarRecover = verifQuarRecover; +window.verifQuarDelete = verifQuarDelete; +window.verifQuarApproveAll = verifQuarApproveAll; +window.verifQuarClearAll = verifQuarClearAll; function _adlRender() { const list = document.getElementById('adl-list'); @@ -2506,6 +2884,48 @@ function _adlRender() { existingBanner.style.display = 'none'; } + // Review queue sub-view toggle: unverified imports ⇄ quarantined files. + let verifBanner = document.getElementById('verif-subview-banner'); + if (_adlFilter === 'unverified') { + _verifLoadConfig(); // no-op once fetched + if (!verifBanner) { + verifBanner = document.createElement('div'); + verifBanner.id = 'verif-subview-banner'; + verifBanner.className = 'adl-batch-filter-banner'; + list.parentNode.insertBefore(verifBanner, list); + } + const quarCount = _verifQuarLoaded ? ` (${_verifQuarEntries.length})` : ''; + const bulkBtns = _verifSubView === 'quarantine' + ? ` + ` + : ` + `; + // Without an AcoustID key nothing ever gets a verification status — + // hide the pointless Unverified pill and show quarantine only. + const unvPill = _verifAcoustidEnabled === false + ? '' + : ``; + verifBanner.innerHTML = ` + ${unvPill} + + + ${bulkBtns}`; + verifBanner.style.display = ''; + if (!_verifQuarLoaded) _verifLoadQuarantine(false); // count for the pill + } else if (verifBanner) { + verifBanner.style.display = 'none'; + } + + if (_adlFilter === 'unverified' && _verifSubView === 'quarantine') { + const qhtml = _verifQuarRows(); + const qEmptyEl = document.getElementById('adl-empty'); + const qEmptyHtml = qEmptyEl ? qEmptyEl.outerHTML : ''; + list.innerHTML = qEmptyHtml + qhtml; + const qNewEmpty = document.getElementById('adl-empty'); + if (qNewEmpty) qNewEmpty.style.display = qhtml ? 'none' : ''; + return; + } + if (filtered.length === 0) { if (empty) empty.style.display = ''; // Clear any existing rows but keep the empty message From 37ea6604c7024d6e1b0201d9ea682fbd97c1a25d Mon Sep 17 00:00:00 2001 From: dev Date: Thu, 11 Jun 2026 01:24:22 +0200 Subject: [PATCH 12/14] Fix import artist override and verification review --- config/config.example.json | 7 +- config/settings.py | 8 +- core/auto_import_worker.py | 13 +-- core/imports/quarantine.py | 46 +++++++++ core/matching/verification_status.py | 13 ++- core/repair_jobs/acoustid_scanner.py | 100 ++++++++++++++----- tests/imports/test_folder_artist_override.py | 13 ++- tests/matching/test_verification_status.py | 12 ++- tests/test_acoustid_scanner.py | 91 ++++++++++++++--- webui/index.html | 2 +- webui/static/style.css | 30 ++++++ webui/static/wishlist-tools.js | 62 +++++++++--- 12 files changed, 326 insertions(+), 71 deletions(-) diff --git a/config/config.example.json b/config/config.example.json index efbb1da9..0543822c 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -56,6 +56,11 @@ "playlist_path": "$playlist/$artist - $title" } }, + "import": { + "staging_path": "./Staging", + "replace_lower_quality": false, + "folder_artist_override": true + }, "lossy_copy": { "enabled": false, "bitrate": "320", @@ -67,4 +72,4 @@ "listenbrainz": { "token": "LISTENBRAINZ_TOKEN" } -} \ No newline at end of file +} diff --git a/config/settings.py b/config/settings.py index c32c5b75..a522d4a7 100644 --- a/config/settings.py +++ b/config/settings.py @@ -684,7 +684,13 @@ class ConfigManager: }, "import": { "staging_path": "./Staging", - "replace_lower_quality": False + "replace_lower_quality": False, + # Use the top Staging folder as the artist (Artist/Album layouts, + # mixtapes). On by default to preserve the long-standing import + # behaviour for existing users. Turn OFF if you stage a mixed pile + # of songs under one container folder, otherwise that folder's name + # overrides every metadata-identified artist (the "soulsync" case). + "folder_artist_override": True }, "m3u_export": { "enabled": False, diff --git a/core/auto_import_worker.py b/core/auto_import_worker.py index 39cc0c22..8bfbea34 100644 --- a/core/auto_import_worker.py +++ b/core/auto_import_worker.py @@ -1577,15 +1577,12 @@ class AutoImportWorker: album_name = identification.get('album_name', 'Unknown') image_url = identification.get('image_url', '') - # Parent folder artist override — OPT-IN via import.folder_artist_override - # (default off). When enabled it uses the top Staging folder as the artist - # for Artist/Album or Artist//Album layouts, which helps - # mixtapes/compilations whose embedded tags carry DJ names. Off by default - # because it otherwise clobbers a confidently metadata-identified artist - # when a user stages a mixed pile under a single container folder (the - # "soulsync" mass-mislabel incident). + # Parent folder artist override via import.folder_artist_override. + # Default on to preserve the legacy Artist/Album staging behavior. + # Users who stage mixed piles under one container folder can turn it off + # to keep the metadata-identified artist. try: - if self._config_manager.get('import.folder_artist_override', False): + if self._config_manager.get('import.folder_artist_override', True): staging_root = self._resolve_staging_path() or self.staging_path rel_path = os.path.relpath(candidate.path, staging_root) folder_artist = resolve_folder_artist(rel_path, artist_name, enabled=True) diff --git a/core/imports/quarantine.py b/core/imports/quarantine.py index 3df79f85..0e9af594 100644 --- a/core/imports/quarantine.py +++ b/core/imports/quarantine.py @@ -219,6 +219,7 @@ def list_quarantine_entries(quarantine_dir: str) -> List[Dict[str, Any]]: "trigger": sidecar.get("trigger", "unknown"), "source_username": source_username, "source_filename": source_filename, + "thumb_url": _extract_context_thumb(ctx), } ) @@ -226,6 +227,51 @@ def list_quarantine_entries(quarantine_dir: str) -> List[Dict[str, Any]]: return entries +def get_quarantine_entry_context(quarantine_dir: str, entry_id: str) -> Dict[str, Any]: + """The sidecar's embedded pipeline ``context`` dict for one entry. + Returns {} for thin/legacy sidecars, missing entries or read errors.""" + _, sidecar_path = _resolve_entry_paths(quarantine_dir, entry_id) + if not sidecar_path or not os.path.isfile(sidecar_path): + return {} + try: + with open(sidecar_path, encoding="utf-8") as f: + loaded = json.load(f) + ctx = loaded.get("context") if isinstance(loaded, dict) else None + return ctx if isinstance(ctx, dict) else {} + except Exception as exc: + logger.debug("quarantine context read failed for %s: %s", entry_id, exc) + return {} + + +def _extract_context_thumb(ctx: Dict[str, Any]) -> str: + """Album-art URL from a sidecar's pipeline context — same lookup chain the + library-history recorder uses (album/spotify_album image, then album_info, + then the track_info's embedded album images). Empty string when absent.""" + def _first_image(album: Any) -> str: + if not isinstance(album, dict): + return "" + url = album.get("image_url") or "" + if url: + return url + images = album.get("images") or [] + if images and isinstance(images[0], dict): + return images[0].get("url", "") or "" + return "" + + thumb = _first_image(ctx.get("album")) or _first_image(ctx.get("spotify_album")) + if not thumb: + album_info = ctx.get("album_info") + if isinstance(album_info, dict): + thumb = album_info.get("album_image_url", "") or "" + if not thumb: + ti = ctx.get("track_info") + if isinstance(ti, dict): + thumb = _first_image(ti.get("album")) + if not thumb: + thumb = ti.get("image_url", "") or "" + return thumb + + def _resolve_entry_paths(quarantine_dir: str, entry_id: str) -> Tuple[Optional[str], Optional[str]]: """Locate the `.quarantined` file + JSON sidecar for an entry id. diff --git a/core/matching/verification_status.py b/core/matching/verification_status.py index 05be1050..f064ea1f 100644 --- a/core/matching/verification_status.py +++ b/core/matching/verification_status.py @@ -42,10 +42,17 @@ def status_from_acoustid_result(result_value): def status_for_import(context: dict): """Status for a just-imported file from its pipeline context. - The version-mismatch fallback flag wins: a force-accepted file is - ``force_imported`` regardless of what the (earlier, failed) verification - said about the candidate. + Priority order: + 1. A quarantine entry the user explicitly approved ("yes, import this + exact file") is a human decision — ``human_verified``, outranking + whatever the machine said about the candidate earlier. + 2. The version-mismatch fallback flag: a force-accepted file is + ``force_imported`` regardless of what the (earlier, failed) + verification said. + 3. The AcoustID result of this pipeline run. """ + if context.get('_approved_quarantine_trigger'): + return HUMAN_VERIFIED if context.get('_version_mismatch_fallback'): return FORCE_IMPORTED return status_from_acoustid_result(context.get('_acoustid_result')) diff --git a/core/repair_jobs/acoustid_scanner.py b/core/repair_jobs/acoustid_scanner.py index dd7db7af..916829b6 100644 --- a/core/repair_jobs/acoustid_scanner.py +++ b/core/repair_jobs/acoustid_scanner.py @@ -53,10 +53,6 @@ class AcoustIDScannerJob(RepairJob): 'title_similarity': 0.70, 'artist_similarity': 0.60, 'batch_size': 200, - # Skip tracks the user force-imported via the version-mismatch - # fallback (they are expected to mismatch; default: still scan them - # but report as informational). - 'skip_force_imported': False, } auto_fix = False # User chooses fix action per finding @@ -228,9 +224,11 @@ class AcoustIDScannerJob(RepairJob): ) # Verification status from the embedded SOULSYNC_VERIFICATION tag. - # force_imported = user accepted this file as best candidate after the - # retry budget was exhausted — a mismatch here is EXPECTED. Either skip - # (job setting) or downgrade the finding to informational below. + # Only a human decision short-circuits the scan: the user explicitly + # confirmed the file via the review queue. Everything else (verified / + # unverified / force_imported / untagged) is re-checked; force_imported + # mismatches are reported as informational below since a mismatch + # there is EXPECTED (the user accepted the best candidate). file_verif_status = None try: from core.tag_writer import read_file_tags as _rft @@ -244,13 +242,6 @@ class AcoustIDScannerJob(RepairJob): context.report_progress( log_line=f'Skipped (human-verified): {fname}', log_type='skip') return - if file_verif_status == 'force_imported' and \ - self._get_settings(context).get('skip_force_imported', False): - if context.report_progress: - context.report_progress( - log_line=f'Skipped (force-imported fallback): {fname}', - log_type='skip') - return # Fingerprint-collision guard: when the TOP recording's length is wildly # different from the file, the fingerprint hit is a hash collision (the @@ -289,17 +280,26 @@ class AcoustIDScannerJob(RepairJob): aliases_provider=_aliases, ) - # Refresh the DB column from the file tag (the tag travels with the - # file and survives DB resets; the tracks row is the UI-facing cache). - if file_verif_status: - try: - conn = context.db._get_connection() - conn.cursor().execute( - "UPDATE tracks SET verification_status = ? WHERE id = ?", - (file_verif_status, track_id)) - getattr(conn, 'commit', lambda: None)() - except Exception as e: - logger.debug("verification_status refresh failed for %s: %s", track_id, e) + # Persist the scan outcome so it feeds the same review pipeline as + # import-time verification: PASS backfills 'verified' on untagged or + # previously-unverified files; SKIP (ambiguous / cross-script / no + # hard confirmation) marks untagged files 'unverified' so they surface + # in the Downloads-page review queue. force_imported is never blessed + # here (normalize() strips version words, so an instrumental can PASS + # the title check) and 'verified' is never downgraded by a SKIP (the + # import-time check ran with richer candidate metadata). FAIL keeps + # the finding flow below. + new_status = file_verif_status + if outcome.decision == Decision.PASS and file_verif_status in (None, '', 'unverified'): + new_status = 'verified' + elif outcome.decision == Decision.SKIP and not file_verif_status: + new_status = 'unverified' + if new_status: + self._persist_status( + context, track_id, fpath, + (expected.get('file_path') or '').strip() or None, + new_status, write_tag=(new_status != file_verif_status), + expected=expected) if outcome.decision != Decision.FAIL: if context.report_progress: @@ -362,6 +362,56 @@ class AcoustIDScannerJob(RepairJob): else: result.findings_skipped_dedup += 1 + def _persist_status(self, context, track_id, fpath, db_path, status, write_tag, + expected=None): + """Persist a verification status to the file tag (durable, travels with + the file), the tracks row (UI cache) and any library_history rows for + this file (feeds the Unverified review queue on the Downloads page). + ``db_path`` is the unresolved DB-side path — history rows may store + either form, so both are matched. + + Files SoulSync never downloaded have no history row at all — for an + 'unverified' outcome one is inserted (download_source 'acoustid_scan') + so EVERY scan-flagged file lands in the review queue, not just past + downloads. Re-scans then match this row via file_path (no duplicates). + """ + if not status: + return + if write_tag: + try: + from core.tag_writer import write_verification_status + write_verification_status(fpath, status) + except Exception as e: + logger.debug("verification tag write failed for %s: %s", fpath, e) + try: + conn = context.db._get_connection() + cur = conn.cursor() + cur.execute( + "UPDATE tracks SET verification_status = ? WHERE id = ?", + (status, track_id)) + matched = 0 + for p in {p for p in (fpath, db_path) if p}: + cur.execute( + "UPDATE library_history SET verification_status = ? WHERE file_path = ?", + (status, p)) + matched += max(getattr(cur, 'rowcount', 0) or 0, 0) + if status == 'unverified' and matched == 0: + exp = expected or {} + cur.execute( + """INSERT INTO library_history + (event_type, title, artist_name, album_name, file_path, + thumb_url, download_source, verification_status) + VALUES ('download', ?, ?, ?, ?, ?, 'acoustid_scan', ?)""", + (exp.get('title') or os.path.basename(fpath), + exp.get('artist') or None, + exp.get('album_title') or None, + db_path or fpath, + exp.get('album_thumb_url') or None, + status)) + getattr(conn, 'commit', lambda: None)() + except Exception as e: + logger.debug("verification_status persist failed for %s: %s", track_id, e) + def _load_db_tracks(self, context: JobContext) -> dict: """Load all tracks from DB keyed by track ID.""" tracks = {} diff --git a/tests/imports/test_folder_artist_override.py b/tests/imports/test_folder_artist_override.py index 8bc102d8..ad069d2b 100644 --- a/tests/imports/test_folder_artist_override.py +++ b/tests/imports/test_folder_artist_override.py @@ -1,4 +1,4 @@ -"""Folder-artist override is opt-in and never clobbers an identified artist. +"""Folder-artist override can be disabled to keep an identified artist. Background ---------- @@ -12,16 +12,15 @@ named ``soulsync`` therefore got the album-artist of *every* file forced to with a Spotify/MusicBrainz id. Navidrome groups by album-artist, so 65 singles collapsed under a bogus "soulsync" artist. -``resolve_folder_artist`` is the extracted, pure decision. It must: -- return ``None`` (i.e. keep the identified artist) when the feature is OFF, - regardless of folder structure — this is the regression guard; -- only when explicitly enabled, reproduce the original folder-derived artist. +``resolve_folder_artist`` is the extracted, pure decision. It must keep the +identified artist when the feature is OFF, and reproduce the original +folder-derived artist when enabled. """ from core.imports.folder_artist import resolve_folder_artist -# --- OFF by default: never override (the bug fix) --------------------------- +# --- Disabled: never override ----------------------------------------------- def test_disabled_keeps_identified_artist_even_with_artist_album_structure(): # The 'soulsync' mass mis-file: identified artist is real, folder is a @@ -41,7 +40,7 @@ def test_disabled_returns_none_for_clean_artist_album_path(): ) is None -# --- Enabled: original behaviour is available on request -------------------- +# --- Enabled: original behaviour -------------------------------------------- def test_enabled_uses_top_folder_as_artist_when_it_differs(): assert resolve_folder_artist( diff --git a/tests/matching/test_verification_status.py b/tests/matching/test_verification_status.py index d488710e..5e37813e 100644 --- a/tests/matching/test_verification_status.py +++ b/tests/matching/test_verification_status.py @@ -1,7 +1,7 @@ """Verification-status vocabulary + mapping (DB column / file tag / UI badge).""" from core.matching.verification_status import ( - VERIFIED, UNVERIFIED, FORCE_IMPORTED, + VERIFIED, UNVERIFIED, FORCE_IMPORTED, HUMAN_VERIFIED, status_from_acoustid_result, status_for_import, ) @@ -24,3 +24,13 @@ def test_status_for_import_falls_back_to_acoustid_result(): assert status_for_import({'_acoustid_result': 'pass'}) == VERIFIED assert status_for_import({'_acoustid_result': 'skip'}) == UNVERIFIED assert status_for_import({}) is None + + +def test_approved_quarantine_import_is_human_verified(): + # The user clicked Approve on a quarantine entry — an explicit human + # decision about THIS file. Outranks both the fallback flag and whatever + # the (earlier, failed) verification said. + ctx = {'_approved_quarantine_trigger': 'acoustid', + '_version_mismatch_fallback': 'instrumental', + '_acoustid_result': 'fail'} + assert status_for_import(ctx) == HUMAN_VERIFIED diff --git a/tests/test_acoustid_scanner.py b/tests/test_acoustid_scanner.py index 089d594e..1c83cac6 100644 --- a/tests/test_acoustid_scanner.py +++ b/tests/test_acoustid_scanner.py @@ -818,7 +818,7 @@ def test_scanner_does_not_flag_cross_script_when_alias_bridges(monkeypatch): assert captured == [], f"cross-script track false-flagged: {captured}" -def _force_imported_scan(monkeypatch, *, skip_setting): +def _force_imported_scan(monkeypatch): """Drive a scan over a force-imported file whose fingerprint clearly mismatches. Returns the captured findings.""" import core.repair_jobs.acoustid_scanner as scanner_mod @@ -829,10 +829,6 @@ def _force_imported_scan(monkeypatch, *, skip_setting): lambda fpath: {'artist': None, 'verification_status': 'force_imported'}, ) job = AcoustIDScannerJob() - if skip_setting: - monkeypatch.setattr(job, '_get_settings', - lambda ctx: {**job.default_settings, - 'skip_force_imported': True}) captured = [] context = _make_finding_capturing_context( track_row=("42", "Wanted Song", "Real Artist", @@ -857,19 +853,14 @@ def _force_imported_scan(monkeypatch, *, skip_setting): def test_force_imported_mismatch_is_reported_as_informational(monkeypatch): # The user opted into the fallback, so the scan must still TELL them the # file is e.g. an instrumental — but as 'info', clearly marked, not as a - # red Wrong-download warning. - captured = _force_imported_scan(monkeypatch, skip_setting=False) + # red Wrong-download warning. Only human_verified short-circuits the scan. + captured = _force_imported_scan(monkeypatch) assert len(captured) == 1 assert captured[0]['severity'] == 'info' assert captured[0]['details'].get('force_imported') is True assert 'Force-imported' in captured[0]['title'] -def test_force_imported_can_be_skipped_via_setting(monkeypatch): - captured = _force_imported_scan(monkeypatch, skip_setting=True) - assert captured == [] - - def test_human_verified_files_are_never_scanned(monkeypatch): import core.repair_jobs.acoustid_scanner as scanner_mod monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", @@ -888,3 +879,79 @@ def test_human_verified_files_are_never_scanned(monkeypatch): fake, context, JobResultStub(), fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6) assert captured == [] + + +# --------------------------------------------------------------------------- +# Scan-outcome persistence — the scan feeds the same review pipeline as +# import-time verification (tag + tracks row + library_history rows). +# --------------------------------------------------------------------------- + + +def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_artist): + """Drive one _scan_file call and return (status_updates, tag_writes) where + status_updates is the list of (query, params) UPDATEs the scanner ran.""" + import core.repair_jobs.acoustid_scanner as scanner_mod + monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases", + lambda name: [], raising=False) + monkeypatch.setattr( + 'core.tag_writer.read_file_tags', + lambda fpath: {'artist': None, 'verification_status': file_status}) + tag_writes = [] + monkeypatch.setattr( + 'core.tag_writer.write_verification_status', + lambda fpath, status: tag_writes.append((fpath, status)) or True) + job = AcoustIDScannerJob() + captured = [] + context = _make_finding_capturing_context( + track_row=("9", "Call Your Name", expected_artist, + "/music/cyn.flac", 1, "Album", None, None), + captured=captured) + fake = SimpleNamespace(fingerprint_and_lookup=lambda f: { + 'best_score': 0.97, + 'recordings': [{'title': 'Call Your Name', 'artist': aid_artist}]}) + job._scan_file('/music/cyn.flac', '9', + {'title': 'Call Your Name', 'artist': expected_artist}, + fake, context, JobResultStub(), + fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6) + conn = context.db._get_connection() + updates = [(q, p) for q, p in conn.cursor().executed + if 'verification_status' in q] + return updates, tag_writes, captured + + +def test_scan_pass_backfills_verified_status(monkeypatch): + # Untagged file + clean fingerprint PASS → the scan backfills 'verified' + # into the tag, the tracks row AND library_history (review-queue feed). + updates, tag_writes, captured = _run_persistence_scan( + monkeypatch, file_status=None, + aid_artist='Sawano Hiroyuki', expected_artist='Sawano Hiroyuki') + assert captured == [] + assert tag_writes == [('/music/cyn.flac', 'verified')] + assert any('tracks' in q and p == ('verified', '9') for q, p in updates) + assert any('library_history' in q and p == ('verified', '/music/cyn.flac') + for q, p in updates) + + +def test_scan_skip_marks_untagged_file_unverified(monkeypatch): + # Title matches but the artist is ambiguous (cover/collab band?) → SKIP. + # An untagged file gets 'unverified' so it surfaces in the Downloads-page + # review queue instead of silently passing or being deleted. + updates, tag_writes, captured = _run_persistence_scan( + monkeypatch, file_status=None, + aid_artist='Mantilla', expected_artist='Metallica') + assert captured == [] + assert tag_writes == [('/music/cyn.flac', 'unverified')] + assert any('tracks' in q and p == ('unverified', '9') for q, p in updates) + assert any('library_history' in q and p == ('unverified', '/music/cyn.flac') + for q, p in updates) + + +def test_scan_skip_does_not_downgrade_verified(monkeypatch): + # A SKIP must not downgrade an import-time 'verified' (that check ran with + # richer candidate metadata). Status is refreshed, tag untouched. + updates, tag_writes, captured = _run_persistence_scan( + monkeypatch, file_status='verified', + aid_artist='Mantilla', expected_artist='Metallica') + assert captured == [] + assert tag_writes == [] + assert any('tracks' in q and p == ('verified', '9') for q, p in updates) diff --git a/webui/index.html b/webui/index.html index d46ee112..e9c836ce 100644 --- a/webui/index.html +++ b/webui/index.html @@ -2390,7 +2390,7 @@ - +
diff --git a/webui/static/style.css b/webui/static/style.css index 35567a5d..241bf33c 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -10864,6 +10864,26 @@ body.helper-mode-active #dashboard-activity-feed:hover { border-color: rgba(239, 83, 80, 0.3); color: #ef5350; } +.download-audit-hero-pill-verified { + background: rgba(74, 222, 128, 0.12); + border-color: rgba(74, 222, 128, 0.3); + color: #4ade80; +} +.download-audit-hero-pill-human_verified { + background: rgba(52, 152, 219, 0.12); + border-color: rgba(52, 152, 219, 0.35); + color: #3498db; +} +.download-audit-hero-pill-force_imported { + background: rgba(230, 126, 34, 0.12); + border-color: rgba(230, 126, 34, 0.35); + color: #e67e22; +} +.download-audit-hero-pill-unverified { + background: rgba(241, 196, 15, 0.12); + border-color: rgba(241, 196, 15, 0.35); + color: #f1c40f; +} /* Tab bar */ .download-audit-tabs { @@ -67906,3 +67926,13 @@ body.em-scroll-lock { overflow: hidden; } .verif-act-ok:hover { background: rgba(46,204,113,0.25); border-color: rgba(46,204,113,0.5); } .verif-act-del:hover { background: rgba(231,76,60,0.25); border-color: rgba(231,76,60,0.5); } .verif-act-play.playing { background: rgba(var(--accent-rgb),0.3); } +/* Review-queue row annotations (library-history-quarantine style) */ +.verif-reason-badge { display: inline-block; font-size: 9.5px; font-weight: 600; letter-spacing: 0.04em; border: 1px solid; border-radius: 999px; padding: 1px 7px; line-height: 14px; white-space: nowrap; cursor: help; } +.verif-rb-unv { color: #f1c40f; border-color: rgba(241,196,15,0.5); } +.verif-rb-force { color: #ef5350; border-color: rgba(239,83,80,0.5); } +.verif-rb-int { color: #facc15; border-color: rgba(250,204,21,0.5); } +.verif-time { font-size: 11px; color: rgba(255,255,255,0.45); white-space: nowrap; } +.verif-quar-details { margin-top: 6px; font-size: 11.5px; color: rgba(255,255,255,0.6); line-height: 1.5; word-break: break-all; } +.verif-detail-label { color: rgba(255,255,255,0.4); margin-right: 4px; } +.verif-banner-spacer { flex: 1; } +.verif-bulk-danger { border-color: rgba(248,113,113,0.4) !important; color: #f87171 !important; } diff --git a/webui/static/wishlist-tools.js b/webui/static/wishlist-tools.js index 1842e72a..481938b4 100644 --- a/webui/static/wishlist-tools.js +++ b/webui/static/wishlist-tools.js @@ -3794,8 +3794,10 @@ function openDownloadAuditModal(entry) { // Live tag read for the Tags tab. Fire-and-forget on open so the // data is ready by the time the user switches to the Tags tab. - if (entry.id != null) { - fetchAndRenderEmbeddedTags(entry.id); + // Synthetic entries (quarantine review queue) have no history id but + // carry their own tags URL in _file_tags_url. + if (entry.id != null || entry._file_tags_url) { + fetchAndRenderEmbeddedTags(entry.id, entry._file_tags_url); } } @@ -3809,11 +3811,9 @@ function switchAuditTab(tabName) { if (body) body.innerHTML = renderDownloadAuditTabPanel(tabName, _downloadAuditEntry); // Re-fire the live fetch when switching to Tags (in case it // raced past first mount before the user got there). - if (tabName === 'tags' && _downloadAuditEntry.id != null) { - fetchAndRenderEmbeddedTags(_downloadAuditEntry.id); - } - if (tabName === 'lyrics' && _downloadAuditEntry.id != null) { - fetchAndRenderEmbeddedTags(_downloadAuditEntry.id); + if ((tabName === 'tags' || tabName === 'lyrics') && + (_downloadAuditEntry.id != null || _downloadAuditEntry._file_tags_url)) { + fetchAndRenderEmbeddedTags(_downloadAuditEntry.id, _downloadAuditEntry._file_tags_url); } } window.switchAuditTab = switchAuditTab; @@ -3851,7 +3851,14 @@ function renderDownloadAuditHero(entry) { const pills = []; if (entry.download_source) pills.push(_auditHeroPill('source', entry.download_source)); if (entry.quality) pills.push(_auditHeroPill('quality', entry.quality)); - if (entry.acoustid_result) pills.push(_auditHeroPill('verify', formatAcoustidLabel(entry.acoustid_result), entry.acoustid_result)); + const _vsLabels = { verified: 'Verified', unverified: 'Unverified', force_imported: 'Force-imported', human_verified: 'Human verified' }; + if (entry.verification_status && _vsLabels[entry.verification_status]) { + pills.push(_auditHeroPill('verify', _vsLabels[entry.verification_status], entry.verification_status)); + } else if (entry._quarantined) { + pills.push(_auditHeroPill('verify', 'Quarantined', 'fail')); + } else if (entry.acoustid_result) { + pills.push(_auditHeroPill('verify', formatAcoustidLabel(entry.acoustid_result), entry.acoustid_result)); + } return ` ${art} @@ -3952,7 +3959,7 @@ function renderEmbeddedTagsSection(entry) { return renderDownloadAuditTagsTab(entry); } -async function fetchAndRenderEmbeddedTags(historyId) { +async function fetchAndRenderEmbeddedTags(historyId, urlOverride) { const tagsSlot = document.getElementById('download-audit-tags-body'); const lyricsSlot = document.getElementById('download-audit-lyrics-body'); if (!tagsSlot && !lyricsSlot) return; @@ -3962,7 +3969,7 @@ async function fetchAndRenderEmbeddedTags(historyId) { if (lyricsSlot) lyricsSlot.innerHTML = html; }; try { - const resp = await fetch(`/api/library/history/${historyId}/file-tags`); + const resp = await fetch(urlOverride || `/api/library/history/${historyId}/file-tags`); if (!resp.ok) { renderError(`Could not read file tags (HTTP ${resp.status}).`); return; } const data = await resp.json(); if (!data.success) { renderError(data.error || 'Could not read file tags.'); return; } @@ -4258,8 +4265,8 @@ function buildDownloadAuditSteps(entry) { { key: 'verify', title: 'Verification', - status: auditStatusFromAcoustid(entry.acoustid_result), - detail: buildAcoustidDetail(entry.acoustid_result), + status: auditVerifyStatus(entry), + detail: buildVerificationDetail(entry), meta: [], }, (() => { @@ -4376,6 +4383,37 @@ function buildSourceMatchMeta(entry) { return out; } +function auditVerifyStatus(entry) { + // The persisted verification status is the final word — it captures + // outcomes the raw acoustid_result can't express (force-imported after + // N retries, human approval via the review queue). + if (entry._quarantined) return 'error'; + if (entry.verification_status === 'human_verified') return 'complete'; + if (entry.verification_status === 'verified') return 'complete'; + if (entry.verification_status === 'force_imported') return 'partial'; + if (entry.verification_status === 'unverified') return 'partial'; + return auditStatusFromAcoustid(entry.acoustid_result); +} + +function buildVerificationDetail(entry) { + if (entry._quarantined) { + return `Quarantined — the file was NOT imported. ${entry._quarantine_reason || ''}`.trim(); + } + if (entry.verification_status === 'force_imported') { + return 'Force-imported: accepted as the best available candidate after the retry budget was exhausted (version-mismatch fallback). The AcoustID check was bypassed for this final re-import — earlier attempts had failed it.'; + } + if (entry.verification_status === 'human_verified') { + return 'Human verified: you confirmed this file via the review queue. The AcoustID scanner skips it.'; + } + if (entry.verification_status === 'unverified') { + return 'Imported but not hard-confirmed: AcoustID could not verify this file (ambiguous / cross-script metadata / no fingerprint match). Review it under Unverified/Quarantine on the Downloads page.'; + } + if (entry.verification_status === 'verified') { + return 'AcoustID fingerprint matched the expected track.'; + } + return buildAcoustidDetail(entry.acoustid_result); +} + function auditStatusFromAcoustid(result) { if (!result) return 'unknown'; if (result === 'pass') return 'complete'; From e7bc77cb0412f0418038cb7946886da910999fdd Mon Sep 17 00:00:00 2001 From: dev Date: Thu, 11 Jun 2026 01:53:05 +0200 Subject: [PATCH 13/14] Use app confirm modal for verification review actions --- core/imports/folder_artist.py | 2 +- webui/static/pages-extra.js | 47 +++++++++++++++++++++++++++++------ 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/core/imports/folder_artist.py b/core/imports/folder_artist.py index 61faecb2..d767b02c 100644 --- a/core/imports/folder_artist.py +++ b/core/imports/folder_artist.py @@ -8,7 +8,7 @@ user staged everything under one container folder (see the "soulsync" incident). This module isolates that decision as a pure function so it can be: - gated behind an opt-in setting (``import.folder_artist_override``, - default off), and + default on for legacy compatibility), and - unit-tested without standing up the whole import worker. """ diff --git a/webui/static/pages-extra.js b/webui/static/pages-extra.js index a866322d..9fd21696 100644 --- a/webui/static/pages-extra.js +++ b/webui/static/pages-extra.js @@ -2478,7 +2478,13 @@ async function verifApprove(hid, btn) { } async function verifDelete(hid, btn) { - if (!confirm('Delete this file from disk and remove the entry?')) return; + if (!await showConfirmDialog({ + title: 'Delete Unverified File', + message: 'This permanently deletes the downloaded file from disk and removes the review entry. Cannot be undone.', + confirmText: 'Delete', + cancelText: 'Cancel', + destructive: true, + })) return; if (btn) btn.disabled = true; try { const r = await fetch(`/api/verification/${hid}/delete`, { method: 'POST' }); @@ -2698,7 +2704,13 @@ async function verifQuarRecover(idx, btn) { async function verifQuarDelete(idx, btn) { const q = _verifQuarEntries[idx]; if (!q) return; - if (!confirm('Permanently delete this quarantined file?')) return; + if (!await showConfirmDialog({ + title: 'Delete Quarantined File', + message: 'This permanently removes the file and its metadata sidecar. Cannot be undone.', + confirmText: 'Delete', + cancelText: 'Cancel', + destructive: true, + })) return; if (btn) btn.disabled = true; try { const r = await fetch(`/api/quarantine/${encodeURIComponent(q.id)}`, { method: 'DELETE' }); @@ -2721,7 +2733,12 @@ function _verifUnverifiedIds() { async function verifApproveAll(btn) { const ids = _verifUnverifiedIds(); if (!ids.length) return; - if (!confirm(`Mark all ${ids.length} unverified entries as human-verified? The AcoustID scanner will skip them from now on.`)) return; + if (!await showConfirmDialog({ + title: 'Approve Unverified Files', + message: `Mark all ${ids.length} unverified entries as human-verified? The AcoustID scanner will skip them from now on.`, + confirmText: 'Approve All', + cancelText: 'Cancel', + })) return; if (btn) btn.disabled = true; let ok = 0; for (const id of ids) { @@ -2739,7 +2756,13 @@ async function verifApproveAll(btn) { async function verifDeleteAll(btn) { const ids = _verifUnverifiedIds(); if (!ids.length) return; - if (!confirm(`Delete ALL ${ids.length} unverified files from disk and remove their entries? This cannot be undone.`)) return; + if (!await showConfirmDialog({ + title: 'Delete Unverified Files', + message: `This permanently deletes all ${ids.length} unverified files from disk and removes their review entries. Cannot be undone.`, + confirmText: 'Delete All', + cancelText: 'Cancel', + destructive: true, + })) return; if (btn) btn.disabled = true; let ok = 0; for (const id of ids) { @@ -2760,7 +2783,12 @@ async function verifQuarApproveAll(btn) { showToast && showToast('No one-click-approvable entries (legacy sidecars need Recover)', 'info'); return; } - if (!confirm(`Approve and re-import all ${entries.length} quarantined files? They will be imported into the library marked human-verified.`)) return; + if (!await showConfirmDialog({ + title: 'Approve Quarantined Files', + message: `Approve and re-import all ${entries.length} quarantined files? They will be imported into the library marked human-verified.`, + confirmText: 'Approve & Import All', + cancelText: 'Cancel', + })) return; if (btn) btn.disabled = true; let ok = 0; for (const q of entries) { @@ -2779,7 +2807,13 @@ async function verifQuarApproveAll(btn) { async function verifQuarClearAll(btn) { if (!_verifQuarEntries.length) return; - if (!confirm(`Permanently delete ALL ${_verifQuarEntries.length} quarantined files? This cannot be undone.`)) return; + if (!await showConfirmDialog({ + title: 'Clear Quarantine', + message: `This permanently deletes all ${_verifQuarEntries.length} quarantined files and their metadata sidecars. Cannot be undone.`, + confirmText: 'Delete All', + cancelText: 'Cancel', + destructive: true, + })) return; if (btn) btn.disabled = true; try { const r = await fetch('/api/quarantine/clear', { method: 'POST' }); @@ -3635,4 +3669,3 @@ window.adlCancelRow = adlCancelRow; window.adlCancelAll = adlCancelAll; window.adlToggleBatchHistory = adlToggleBatchHistory; window.adlToggleBatchPanel = adlToggleBatchPanel; - From 17886477cb74f2aaffef3b1b1988a4281f0e0254 Mon Sep 17 00:00:00 2001 From: nick2000713 Date: Thu, 11 Jun 2026 17:08:20 +0200 Subject: [PATCH 14/14] remove docs from PR --- ...06-10-acoustid-verification-unification.md | 378 ------------------ ...coustid-verification-unification-design.md | 153 ------- 2 files changed, 531 deletions(-) delete mode 100644 docs/superpowers/plans/2026-06-10-acoustid-verification-unification.md delete mode 100644 docs/superpowers/specs/2026-06-10-acoustid-verification-unification-design.md diff --git a/docs/superpowers/plans/2026-06-10-acoustid-verification-unification.md b/docs/superpowers/plans/2026-06-10-acoustid-verification-unification.md deleted file mode 100644 index 5516bbc3..00000000 --- a/docs/superpowers/plans/2026-06-10-acoustid-verification-unification.md +++ /dev/null @@ -1,378 +0,0 @@ -# AcoustID Verification Unification + Status Tracking — Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Make import-time verification and the library AcoustID scan share ONE decision core, and persist a per-track verification status (DB + file tag) surfaced on the Downloads page. - -**Architecture:** Extract a pure `core/matching/audio_verification.py` (`normalize` + `evaluate`) built on the existing shared helpers (`artist_aliases`, `script_compat`, `acoustid_candidates`, `version_mismatch`). `acoustid_verification.verify_audio_file` (import) and `acoustid_scanner._scan_file` (scan) delegate the decision to it. A new `tracks.verification_status` column + `SOULSYNC_VERIFICATION` file tag record `verified` / `unverified` / `force_imported`; the Downloads page shows a badge. - -**Tech Stack:** Python 3.11, pytest (`.venv/bin/python -m pytest`), mutagen, SQLite, vanilla JS webui. - -**Spec:** `docs/superpowers/specs/2026-06-10-acoustid-verification-unification-design.md` - ---- - -## File Structure - -- **Create** `core/matching/audio_verification.py` — the shared `normalize()` + `evaluate()` decision core (pure, no I/O). Single source of truth for normalization, thresholds, alias-aware comparison, cross-script SKIP, version gate, duration guard. -- **Create** `core/matching/verification_status.py` — the status vocabulary (`VERIFIED`, `UNVERIFIED`, `FORCE_IMPORTED`) + `status_from_outcome(decision)` + `status_from_context(context)` mapping helpers. Tiny, pure, testable. -- **Modify** `core/acoustid_verification.py` — `_normalize`/`_similarity`/`_alias_aware_artist_sim`/`_find_best_title_artist_match` and the decision branches in `verify_audio_file` delegate to the core. Keep the import-facing wrapper (fingerprint lookup, MB alias provider, `VerificationResult`). -- **Modify** `core/repair_jobs/acoustid_scanner.py` — drop the private `_normalize` (line 485) and the inline decision; call `normalize` + `evaluate`. -- **Modify** `database/music_database.py` — additive migration: `tracks.verification_status TEXT`. -- **Modify** `core/tag_writer.py` — write `SOULSYNC_VERIFICATION` in `_write_vorbis`/`_write_id3`/`_write_mp4`; read it in `read_file_tags`. -- **Modify** `core/imports/pipeline.py` — compute + persist the status (DB + tag) after post-processing; stash on the download context. -- **Modify** `web_server.py` + `webui/static/downloads.js` + `webui/index.html` — surface the status badge. - ---- - -## Task 1: Core `normalize()` - -**Files:** -- Create: `core/matching/audio_verification.py` -- Test: `tests/matching/test_audio_verification_core.py` - -- [ ] **Step 1: Write the failing test** - -```python -from core.matching.audio_verification import normalize - -def test_normalize_strips_paren_bracket_angle_and_keeps_cjk(): - assert normalize("澤野弘之 ") == "澤野弘之" - assert normalize("Clarity (Live at X) [Remastered]") == "clarity" - assert normalize("Attack on Titan ") == "attack on titan" - -def test_normalize_strips_version_and_featuring(): - assert normalize("In My Feelings - Instrumental") == "in my feelings" - assert normalize("Song feat. Someone") == "song" -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `.venv/bin/python -m pytest tests/matching/test_audio_verification_core.py -q` -Expected: FAIL — `ModuleNotFoundError: core.matching.audio_verification`. - -- [ ] **Step 3: Write minimal implementation** - -Port the current (already-correct) body of `core/acoustid_verification.py::_normalize` -verbatim into the new module (it already strips `()`/`[]`/`<>`/version/featuring and -keeps CJK via `\w`). Add the thresholds as module constants for later tasks. - -```python -"""Shared audio-verification decision core (pure; no file/DB I/O). - -Single source of truth for normalization + the PASS/SKIP/FAIL decision used by -BOTH import-time verification (core/acoustid_verification.py) and the library -scan (core/repair_jobs/acoustid_scanner.py). -""" -import re -from difflib import SequenceMatcher - -MIN_ACOUSTID_SCORE = 0.80 -TITLE_MATCH_THRESHOLD = 0.70 -ARTIST_MATCH_THRESHOLD = 0.60 - -def normalize(text: str) -> str: - if not text: - return "" - s = text.lower().strip() - s = re.sub(r'\s*\([^)]*\)', '', s) - s = re.sub(r'\s*\[[^\]]*\]', '', s) - s = re.sub(r'\s*<[^>]*>', '', s) - s = re.sub(r'\s+(?:feat\.?|ft\.?|featuring)\s+.*$', '', s, flags=re.IGNORECASE) - s = re.sub(r'\s*-\s*(?:vocal|instrumental|acoustic|live|remix|cover|clean|explicit|radio\s*edit|original\s*mix|extended\s*mix|club\s*mix)\s*$', '', s, flags=re.IGNORECASE) - s = re.sub(r'\s*-\s*from\s+.+$', '', s, flags=re.IGNORECASE) - s = re.sub(r'[^\w\s]', '', s) - s = re.sub(r'\s+', ' ', s).strip() - return s - -def similarity(a: str, b: str) -> float: - na, nb = normalize(a), normalize(b) - if not na or not nb: - return 0.0 - if na == nb: - return 1.0 - return SequenceMatcher(None, na, nb).ratio() -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `.venv/bin/python -m pytest tests/matching/test_audio_verification_core.py -q` -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add core/matching/audio_verification.py tests/matching/test_audio_verification_core.py -git commit -m "feat(verification): shared normalize() core for import + scan" -``` - ---- - -## Task 2: Core `evaluate()` decision - -**Files:** -- Modify: `core/matching/audio_verification.py` -- Test: `tests/matching/test_audio_verification_core.py` - -`evaluate` reproduces the import-path decision (it is the richer of the two), so it -is behaviour-preserving for import and an upgrade for scan. Port the logic from -`acoustid_verification.verify_audio_file` Steps 4b–end (version gate via -`is_acceptable_version_mismatch`, alias-aware artist sim, secondary/scan match via -`find_matching_recording`, cross-script SKIP via `is_cross_script_mismatch`, -duration guard via `duration_mismatches_strongly`). - -- [ ] **Step 1: Write the failing tests** (the three real-world cases) - -```python -from core.matching.audio_verification import evaluate, Decision - -REC = lambda t, a, d=None: {"title": t, "artist": a, "duration": d} - -def test_cross_script_artist_with_vocal_credit_skips_not_fails(): - # Sawano / 澤野弘之 + IPA title -> SKIP, never FAIL - out = evaluate( - "Call Your Name", "Sawano Hiroyuki", - [REC("call your name", "澤野弘之 ")], - fingerprint_score=0.95, - aliases_provider=lambda: ["澤野弘之"], - ) - assert out.decision == Decision.SKIP - -def test_clean_match_passes(): - out = evaluate("Xl-Tt", "Sawano Hiroyuki", - [REC("xl-tt", "澤野弘之")], fingerprint_score=0.95, - aliases_provider=lambda: ["澤野弘之"]) - assert out.decision == Decision.PASS - -def test_genuine_wrong_song_fails(): - out = evaluate("Yellow", "Coldplay", - [REC("Rich Interlude", "Kendrick Lamar")], fingerprint_score=0.85) - assert out.decision == Decision.FAIL -``` - -- [ ] **Step 2: Run to verify failure** - -Run: `.venv/bin/python -m pytest tests/matching/test_audio_verification_core.py -k evaluate -q` -Expected: FAIL — `cannot import name 'evaluate'`. - -- [ ] **Step 3: Implement `evaluate` + `Decision` + `Outcome`** - -Add an `enum Decision {PASS, SKIP, FAIL}`, a dataclass `Outcome(decision, title_sim, -artist_sim, matched_title, matched_artist, reason)`, and `_alias_aware_artist_sim` -(ported from acoustid_verification). Port the decision sequence from -`verify_audio_file` lines 470–703 into `evaluate`, taking `recordings` + -`fingerprint_score` + `file_duration_s` + `aliases_provider` as params and -returning an `Outcome` instead of `(VerificationResult, msg)`. Reuse the existing -imports: `from core.matching.artist_aliases import artist_names_match, best_alias_match`, -`from core.matching.script_compat import is_cross_script_mismatch`, -`from core.matching.acoustid_candidates import find_matching_recording, duration_mismatches_strongly`, -`from core.matching.version_mismatch import is_acceptable_version_mismatch`, and -`MusicMatchingEngine().detect_version_type` for `_detect_title_version`. - -- [ ] **Step 4: Run to verify pass** - -Run: `.venv/bin/python -m pytest tests/matching/test_audio_verification_core.py -q` -Expected: PASS (all). - -- [ ] **Step 5: Commit** - -```bash -git add core/matching/audio_verification.py tests/matching/test_audio_verification_core.py -git commit -m "feat(verification): shared evaluate() PASS/SKIP/FAIL decision core" -``` - ---- - -## Task 3: Import path delegates to the core - -**Files:** -- Modify: `core/acoustid_verification.py` -- Test (regression): `tests/test_acoustid_skip_logic.py`, `tests/test_acoustid_version_mismatch.py`, `tests/test_acoustid_normalize_angle_annotations.py`, `tests/test_acoustid_error_reporting.py` - -- [ ] **Step 1: Run the existing suite to capture green baseline** - -Run: `.venv/bin/python -m pytest tests/test_acoustid_skip_logic.py tests/test_acoustid_version_mismatch.py tests/test_acoustid_normalize_angle_annotations.py tests/test_acoustid_error_reporting.py -q` -Expected: PASS (baseline before refactor). - -- [ ] **Step 2: Refactor** - -In `core/acoustid_verification.py`: replace the bodies of `_normalize`/`_similarity` -with re-exports from the core (`from core.matching.audio_verification import normalize as _normalize, similarity as _similarity`). Replace the decision block in -`verify_audio_file` (Steps 4b–end) with a single call to `evaluate(...)`, then map -`Outcome.decision` → `VerificationResult` (PASS→PASS, SKIP→SKIP, FAIL→FAIL) and pass -`Outcome.reason` through. Keep the existing fingerprint lookup, MB enrichment, and -`_resolve_expected_artist_aliases` thunk (pass it as `aliases_provider`). - -- [ ] **Step 3: Run regression suite** - -Run: same command as Step 1. -Expected: PASS (unchanged behaviour for import). - -- [ ] **Step 4: Commit** - -```bash -git add core/acoustid_verification.py -git commit -m "refactor(verification): import path delegates to shared core" -``` - ---- - -## Task 4: Scanner uses the core (gains alias bridge + cross-script SKIP) - -**Files:** -- Modify: `core/repair_jobs/acoustid_scanner.py` -- Test: `tests/test_acoustid_scanner.py` (regression) + new case - -- [ ] **Step 1: Write the failing test** (cross-script track must NOT create a finding) - -```python -# tests/test_acoustid_scanner_cross_script.py -def test_scanner_does_not_flag_cross_script_anime_ost(monkeypatch): - # Build a scan over one track expected "Call Your Name" / "Sawano Hiroyuki" - # whose fingerprint returns "澤野弘之 "; assert findings_created == 0. - ... -``` - -(Concrete construction mirrors `tests/test_acoustid_scanner.py` fixtures; assert the -mismatch branch is NOT reached.) - -- [ ] **Step 2: Run to verify failure** - -Run: `.venv/bin/python -m pytest tests/test_acoustid_scanner_cross_script.py -q` -Expected: FAIL — a finding IS created (current scanner strips non-ASCII, no alias). - -- [ ] **Step 3: Refactor scanner** - -Delete `_normalize` (line 485) and import `normalize` from the core. Replace the -`_scan_file` similarity + decision block (the `title_sim`/`artist_sim`/finding logic) -with a call to `evaluate(...)` passing `fp_result['recordings']`, `best_score`, -`file_duration_s`, and an `aliases_provider` (reuse the MB alias lookup, or pass the -DB-resolved aliases). Create a finding only when `Outcome.decision == Decision.FAIL`. - -- [ ] **Step 4: Run new + existing scanner tests** - -Run: `.venv/bin/python -m pytest tests/test_acoustid_scanner.py tests/test_acoustid_scanner_cross_script.py -q` -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add core/repair_jobs/acoustid_scanner.py tests/test_acoustid_scanner_cross_script.py -git commit -m "refactor(scanner): use shared verification core; stop false-flagging cross-script" -``` - ---- - -## Task 5: Status vocabulary + mapping - -**Files:** -- Create: `core/matching/verification_status.py` -- Test: `tests/matching/test_verification_status.py` - -- [ ] **Step 1: Failing test** - -```python -from core.matching.verification_status import ( - VERIFIED, UNVERIFIED, FORCE_IMPORTED, status_from_decision, status_from_context) -from core.matching.audio_verification import Decision - -def test_decision_maps_to_status(): - assert status_from_decision(Decision.PASS) == VERIFIED - assert status_from_decision(Decision.SKIP) == UNVERIFIED - -def test_force_import_context_wins(): - assert status_from_context({"_version_mismatch_fallback": "instrumental"}) == FORCE_IMPORTED - assert status_from_context({}) is None -``` - -- [ ] **Step 2: Run → fail.** `.venv/bin/python -m pytest tests/matching/test_verification_status.py -q` - -- [ ] **Step 3: Implement** the three string constants + the two pure mappers. - -- [ ] **Step 4: Run → pass.** - -- [ ] **Step 5: Commit** `feat(verification): status vocabulary + mappers`. - ---- - -## Task 6: DB migration `tracks.verification_status` - -**Files:** -- Modify: `database/music_database.py` -- Test: `tests/test_verification_status_migration.py` - -- [ ] **Step 1: Failing test** — open a fresh DB, assert `verification_status` is in `PRAGMA table_info(tracks)`. -- [ ] **Step 2: Run → fail.** -- [ ] **Step 3: Implement** following the existing additive pattern: - -```python -cursor.execute("PRAGMA table_info(tracks)") -cols = [r[1] for r in cursor.fetchall()] -if 'verification_status' not in cols: - cursor.execute("ALTER TABLE tracks ADD COLUMN verification_status TEXT") -``` -(Place beside the other `tracks` ADD COLUMN migrations, e.g. near line 2372.) - -- [ ] **Step 4: Run → pass.** -- [ ] **Step 5: Commit** `feat(db): add tracks.verification_status`. - ---- - -## Task 7: Write + read the `SOULSYNC_VERIFICATION` tag - -**Files:** -- Modify: `core/tag_writer.py` -- Test: `tests/test_verification_tag_roundtrip.py` - -- [ ] **Step 1: Failing test** — write a FLAC with `db_data={'verification_status': 'unverified', ...}`, read it back via `read_file_tags`, assert `tags['verification_status'] == 'unverified'`. (Build the FLAC with ffmpeg as in `/tmp` test harness; or reuse an existing tag-writer test fixture.) -- [ ] **Step 2: Run → fail.** -- [ ] **Step 3: Implement** — in `_write_vorbis` set `audio['SOULSYNC_VERIFICATION']=[status]`; in `_write_id3` add `TXXX(desc='SOULSYNC_VERIFICATION', text=[status])`; in `_write_mp4` set `----:com.soulsync:VERIFICATION`. Read all three back in `read_file_tags` into key `verification_status`. Only write when `db_data.get('verification_status')` is set (non-fatal on error). -- [ ] **Step 4: Run → pass.** -- [ ] **Step 5: Commit** `feat(tags): persist SOULSYNC_VERIFICATION tag`. - ---- - -## Task 8: Persist status at import (DB + tag + context) - -**Files:** -- Modify: `core/imports/pipeline.py` -- Test: `tests/imports/test_import_verification_status.py` - -- [ ] **Step 1: Failing test** — drive `post_process_matched_download` (or the helper that records the library row) with a PASS verification result and assert the track row's `verification_status == 'verified'`; with `context['_version_mismatch_fallback']` set assert `'force_imported'`. -- [ ] **Step 2: Run → fail.** -- [ ] **Step 3: Implement** — after verification, compute `status = status_from_context(context) or status_from_decision(verification_decision)`; thread it into `db_data` (so the tag write in Task 7 picks it up) and into the library-row write (`tracks.verification_status`); also stash `context['_verification_status'] = status` for the Downloads payload. -- [ ] **Step 4: Run → pass.** -- [ ] **Step 5: Commit** `feat(import): record verification status (db+tag+context)`. - ---- - -## Task 9: Downloads page badge - -**Files:** -- Modify: `web_server.py` (download-status payload), `webui/static/downloads.js`, `webui/index.html` (badge styles if needed) -- Test: manual + a small JS-free assertion if a payload test exists - -- [ ] **Step 1:** Add `verification_status` to the per-task download payload in `web_server.py` (read from `context['_verification_status']` / the task record). -- [ ] **Step 2:** In `downloads.js`, render a badge per completed row: ✓ verified / ⚠ unverified / ⚑ force-imported, keyed on `task.verification_status`. -- [ ] **Step 3:** Add minimal badge CSS in `index.html`/`style.css` mirroring existing status pills. -- [ ] **Step 4:** Manual verify: import one clean + one cross-script track; confirm ✓ and ⚠ badges. -- [ ] **Step 5: Commit** `feat(ui): show verification status badge on Downloads`. - ---- - -## Final verification - -- [ ] Run the full AcoustID + import suites: - `.venv/bin/python -m pytest tests/test_acoustid_*.py tests/matching/ tests/imports/ -q` - Expected: all PASS. -- [ ] Build + push image (per build-deploy loop) for live testing. - -## Self-review notes - -- Spec coverage: unify (Tasks 1–4), status values (Task 5), DB (6), tag (7), - import wiring (8), UI (9) — all spec sections mapped. -- Behaviour-preserving for import (Task 3 regression-gated); intentional change for - scan (Task 4 — the fix). -- Tasks 4, 8, 9 reference existing fixtures/APIs the executor must read live - (`test_acoustid_scanner.py` fixtures, the download payload shape, `downloads.js` - row render) — flagged as such rather than guessed. diff --git a/docs/superpowers/specs/2026-06-10-acoustid-verification-unification-design.md b/docs/superpowers/specs/2026-06-10-acoustid-verification-unification-design.md deleted file mode 100644 index 954564db..00000000 --- a/docs/superpowers/specs/2026-06-10-acoustid-verification-unification-design.md +++ /dev/null @@ -1,153 +0,0 @@ -# AcoustID Verification: Unify the Pipeline + Persist Verification Status - -Date: 2026-06-10 -Branch: `fix/import-folder-artist-override-optin` - -## Problem - -Audio verification logic is **duplicated** across two code paths that have -drifted apart, producing inconsistent results ("komische Fehler"): - -- **Import time** — `core/acoustid_verification.py` (`verify_audio_file`, - `_normalize` @ line 59, alias-aware artist sim, cross-script SKIP, version - gate, duration guard). Keeps CJK characters; strips `()`/`[]`/`<>` (after the - recent fix). -- **Library scan** — `core/repair_jobs/acoustid_scanner.py` (its OWN `_normalize` - @ line 485, which strips **all non-ASCII** — kanji/IPA vanish — and has NO - cross-script alias bridge / SKIP). A full library scan therefore false-flags - correct cross-script tracks (e.g. anime OSTs credited `澤野弘之 `). - -Two `_normalize`s + two decision paths = the same bug must be fixed twice and -they diverge. There is also no record of *how* a track passed verification, so a -force-imported or skipped-but-imported track looks identical to a cleanly -verified one. - -## Goals - -1. **One verification core** used by BOTH import and scan. Normalization, - alias-aware comparison, cross-script handling, version gate, duration guard, - and thresholds live in exactly one place. -2. **Persist a verification status** per track in the DB **and** as a file tag, - so it survives DB resets / file moves and can drive the UI. -3. **Surface the status on the Downloads page** as an info badge. - -## Consumer map (discovered 2026-06-10) - -In scope (call the new core): -- `core/imports/pipeline.py:340-373` — the only caller of - `AcoustIDVerification.verify_audio_file` (import path). -- `core/repair_jobs/acoustid_scanner.py` — the library-scan job. - -Shared helpers (stay shared, unchanged; the core builds on them): -- `core/matching/artist_aliases.py::artist_names_match` / `best_alias_match` -- `core/matching/script_compat.py::is_cross_script_mismatch` -- `core/matching/acoustid_candidates.py::find_matching_recording`, - `duration_mismatches_strongly` -- `core/matching/version_mismatch.py::is_acceptable_version_mismatch` - -Status hook: -- `core/imports/version_mismatch_fallback.py` sets - `context["_version_mismatch_fallback"]` when it force-accepts the best - quarantined candidate after retries are exhausted → maps to `force_imported`. - Dispatched from `core/downloads/task_worker.py` via `try_version_mismatch_fallback`. - -Deliberately OUT of scope (consume a shared helper but serve other purposes; -NOT merged): -- `core/repair_worker.py::_album_fill_artist_names_match` — Album-Completeness - auto-fill gate. -- `core/matching/album_context_title.py::_normalize` — trivial album-grouping - normalize. -- `core/downloads/task_worker.py`, `core/matching_engine.py` — pre-download - (Soulseek candidate) matching, not post-download verification. - -## Design - -### 1. Shared core — `core/matching/audio_verification.py` (new) - -The single home for the verification *decision*. Pure where possible (no file -I/O, no DB) so it is unit-testable in isolation. Callers keep their own I/O -(fingerprinting, quarantine, finding creation, tag writing). - -``` -def normalize(text: str) -> str - # lowercase; strip ()/[]/<> annotations; strip version (- Live, etc) + - # featuring tags; KEEP CJK (\w under unicode); collapse whitespace. - # Single source of truth — replaces acoustid_verification._normalize AND - # acoustid_scanner._normalize. - -def evaluate( - expected_title, expected_artist, recordings, *, - fingerprint_score, file_duration_s=None, aliases_provider=None, -) -> Outcome - # Outcome(decision, title_sim, artist_sim, matched_title, matched_artist, reason) - # decision in {PASS, SKIP, FAIL}. - # Encapsulates: alias-aware artist sim, version-mismatch gate, duration - # collision guard, cross-script SKIP, thresholds. Built on the shared helpers. -``` - -Thresholds (`TITLE_MATCH_THRESHOLD`, `ARTIST_MATCH_THRESHOLD`, `MIN_ACOUSTID_SCORE`) -move into the core as the single definition. - -Caller mapping: -- **Import** (`acoustid_verification.verify_audio_file`): PASS → import + - status `verified`; SKIP → import + status `unverified`; FAIL → quarantine. -- **Scan** (`acoustid_scanner._scan_file`): PASS/SKIP → no finding (optionally - refresh the stored status); FAIL → create `acoustid_mismatch` finding. - -`acoustid_verification.py` keeps `verify_audio_file` as the import-facing wrapper -(fingerprint lookup, MB enrichment, alias provider, returns `VerificationResult`) -but delegates the decision to `evaluate`. `acoustid_scanner.py` drops its private -`_normalize` and decision branches and calls `normalize` + `evaluate`. - -### 2. Verification status - -Values (the three the user selected; quarantined files are not imported, so they -carry no status): -- `verified` — clean AcoustID PASS. -- `unverified` — SKIP: cross-script / ambiguous / no AcoustID match. Imported, - not hard-confirmed. -- `force_imported` — accepted via `version_mismatch_fallback` after retries - exhausted. - -### 3. Storage — DB + tag - -- **DB:** new nullable column `tracks.verification_status TEXT` (idempotent - migration in `database/music_database.py`). Set at import; refreshed by the - scan. Mirrored onto the download record/context so the Downloads page can show - it before a library re-scan. -- **Tag:** `SOULSYNC_VERIFICATION=` written via `core/tag_writer.py` - (Vorbis comment / ID3 `TXXX`). Travels with the file; the scanner reads it and - may skip re-verifying an already-`verified` file (perf bonus) and to display. - -### 4. Downloads page UI - -A small, info-only badge per row sourced from the status: -- ✓ `verified` · ⚠ `unverified` · ⚑ `force_imported`. -Wired in `webui/static/downloads.js` (+ the download-status API payload). - -### 5. Testing (TDD) - -- Unit-test the core `normalize` (incl. `<>`/`()`/`[]`, CJK retention) and - `evaluate` (Sawano/IPA cross-script → SKIP not FAIL; vocal-credit artist → - alias 100%; version mismatch → FAIL; duration collision → no FAIL). -- Wire import + scanner to the core; their existing tests - (`test_acoustid_skip_logic`, `test_acoustid_scanner`, - `test_acoustid_version_mismatch`, `test_acoustid_error_reporting`, - `test_acoustid_normalize_angle_annotations`) act as regression. -- New tests for the status: force-import → `force_imported`; PASS → `verified`; - SKIP → `unverified`; tag written + read back; DB column populated. - -## Rollout / risk - -- Pure-core extraction is behaviour-preserving for the import path (existing - tests pin it); the scan path *changes* (gains alias bridge + cross-script SKIP) - — that is the intended fix. -- DB migration is additive (nullable column), safe on existing DBs. -- Tag writing reuses the existing `tag_writer` path; failures are non-fatal. -- Staged on the current branch with TDD; ships in the next image build. - -## Out of scope (now) - -Unifying the pre-download Soulseek matcher or the album-completeness gate. They -consume shared helpers but are different decisions; folding them in would widen -blast radius without serving this goal.