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); }