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 <noreply@anthropic.com>
This commit is contained in:
dev
parent
8e6820dbdf
commit
9d1d09a571
14 changed files with 886 additions and 28 deletions
|
|
@ -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/<category>/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', '')
|
||||
|
|
|
|||
|
|
@ -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 '',
|
||||
|
|
|
|||
52
core/imports/folder_artist.py
Normal file
52
core/imports/folder_artist.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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("澤野弘之 <Vocal: MIKA KOBAYASHI>") == "澤野弘之"
|
||||
assert normalize("Clarity (Live at X) [Remastered]") == "clarity"
|
||||
assert normalize("Attack on Titan <TV Size>") == "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 / 澤野弘之 <Vocal: ...> + IPA title -> SKIP, never FAIL
|
||||
out = evaluate(
|
||||
"Call Your Name", "Sawano Hiroyuki",
|
||||
[REC("call your name", "澤野弘之 <Vocal: mpi & CASG>")],
|
||||
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 "澤野弘之 <Vocal: ...>"; 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.
|
||||
|
|
@ -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 `澤野弘之 <Vocal: …>`).
|
||||
|
||||
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=<status>` 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.
|
||||
85
tests/imports/test_folder_artist_override.py
Normal file
85
tests/imports/test_folder_artist_override.py
Normal file
|
|
@ -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
|
||||
36
tests/test_acoustid_normalize_angle_annotations.py
Normal file
36
tests/test_acoustid_normalize_angle_annotations.py
Normal file
|
|
@ -0,0 +1,36 @@
|
|||
"""`_normalize` must strip ``<...>`` annotations like the AcoustID/MusicBrainz
|
||||
vocalist credit ``澤野弘之 <Vocal: MIKA KOBAYASHI>``.
|
||||
|
||||
User report: a correct anime-OST track ("Attack on Titan" by "Sawano Hiroyuki")
|
||||
was false-quarantined. AcoustID returned the artist as
|
||||
``澤野弘之 <Vocal: MIKA KOBAYASHI>``. 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("澤野弘之 <Vocal: MIKA KOBAYASHI>") == "澤野弘之"
|
||||
|
||||
|
||||
def test_normalize_strips_angle_brackets_latin():
|
||||
assert _normalize("Attack on Titan <TV Size>") == "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("澤野弘之", "澤野弘之 <Vocal: MIKA KOBAYASHI>") == 1.0
|
||||
|
||||
|
||||
def test_normalize_keeps_plain_text_untouched():
|
||||
# Guard: no angle brackets -> unchanged behaviour.
|
||||
assert _normalize("Sawano Hiroyuki") == "sawano hiroyuki"
|
||||
|
|
@ -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 == []
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
</div>
|
||||
|
||||
<div class="form-group">
|
||||
<label class="checkbox-label">
|
||||
<input type="checkbox" id="import-folder-artist-override">
|
||||
Use the top Staging folder as the artist
|
||||
</label>
|
||||
</div>
|
||||
<div class="help-text">
|
||||
On by default (legacy behaviour). When enabled, files staged as
|
||||
<code>Artist/Album/…</code> (or <code>Artist/Albums/Album/…</code>) take the top
|
||||
folder as the album artist — handy for mixtapes/compilations whose embedded tags
|
||||
carry DJ names. <strong>Turn this off</strong> 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.
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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 += ' <span class="verif-badge verif-force" title="Force-imported: accepted as best available candidate after repeated mismatches (version-mismatch fallback). A library AcoustID scan reports these as informational.">⚑</span>';
|
||||
} else if (task.verification_status === 'unverified') {
|
||||
statusText += ' <span class="verif-badge verif-unverified" title="Imported but not hard-verified (AcoustID could not confirm — e.g. cross-script metadata or no fingerprint match).">⚠</span>';
|
||||
} else if (task.verification_status === 'verified') {
|
||||
statusText += ' <span class="verif-badge verif-ok" title="AcoustID verified: audio fingerprint matches the expected track.">✔</span>';
|
||||
}
|
||||
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'
|
||||
|
|
|
|||
|
|
@ -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: {
|
||||
|
|
|
|||
|
|
@ -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); }
|
||||
|
|
|
|||
Loading…
Reference in a new issue