Fix import artist override and verification review
This commit is contained in:
parent
5896f2dcc6
commit
37ea6604c7
12 changed files with 326 additions and 71 deletions
|
|
@ -56,6 +56,11 @@
|
||||||
"playlist_path": "$playlist/$artist - $title"
|
"playlist_path": "$playlist/$artist - $title"
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
"import": {
|
||||||
|
"staging_path": "./Staging",
|
||||||
|
"replace_lower_quality": false,
|
||||||
|
"folder_artist_override": true
|
||||||
|
},
|
||||||
"lossy_copy": {
|
"lossy_copy": {
|
||||||
"enabled": false,
|
"enabled": false,
|
||||||
"bitrate": "320",
|
"bitrate": "320",
|
||||||
|
|
@ -67,4 +72,4 @@
|
||||||
"listenbrainz": {
|
"listenbrainz": {
|
||||||
"token": "LISTENBRAINZ_TOKEN"
|
"token": "LISTENBRAINZ_TOKEN"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -684,7 +684,13 @@ class ConfigManager:
|
||||||
},
|
},
|
||||||
"import": {
|
"import": {
|
||||||
"staging_path": "./Staging",
|
"staging_path": "./Staging",
|
||||||
"replace_lower_quality": False
|
"replace_lower_quality": False,
|
||||||
|
# Use the top Staging folder as the artist (Artist/Album layouts,
|
||||||
|
# mixtapes). On by default to preserve the long-standing import
|
||||||
|
# behaviour for existing users. Turn OFF if you stage a mixed pile
|
||||||
|
# of songs under one container folder, otherwise that folder's name
|
||||||
|
# overrides every metadata-identified artist (the "soulsync" case).
|
||||||
|
"folder_artist_override": True
|
||||||
},
|
},
|
||||||
"m3u_export": {
|
"m3u_export": {
|
||||||
"enabled": False,
|
"enabled": False,
|
||||||
|
|
|
||||||
|
|
@ -1577,15 +1577,12 @@ class AutoImportWorker:
|
||||||
album_name = identification.get('album_name', 'Unknown')
|
album_name = identification.get('album_name', 'Unknown')
|
||||||
image_url = identification.get('image_url', '')
|
image_url = identification.get('image_url', '')
|
||||||
|
|
||||||
# Parent folder artist override — OPT-IN via import.folder_artist_override
|
# Parent folder artist override via import.folder_artist_override.
|
||||||
# (default off). When enabled it uses the top Staging folder as the artist
|
# Default on to preserve the legacy Artist/Album staging behavior.
|
||||||
# for Artist/Album or Artist/<category>/Album layouts, which helps
|
# Users who stage mixed piles under one container folder can turn it off
|
||||||
# mixtapes/compilations whose embedded tags carry DJ names. Off by default
|
# to keep the metadata-identified artist.
|
||||||
# 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:
|
try:
|
||||||
if self._config_manager.get('import.folder_artist_override', False):
|
if self._config_manager.get('import.folder_artist_override', True):
|
||||||
staging_root = self._resolve_staging_path() or self.staging_path
|
staging_root = self._resolve_staging_path() or self.staging_path
|
||||||
rel_path = os.path.relpath(candidate.path, staging_root)
|
rel_path = os.path.relpath(candidate.path, staging_root)
|
||||||
folder_artist = resolve_folder_artist(rel_path, artist_name, enabled=True)
|
folder_artist = resolve_folder_artist(rel_path, artist_name, enabled=True)
|
||||||
|
|
|
||||||
|
|
@ -219,6 +219,7 @@ def list_quarantine_entries(quarantine_dir: str) -> List[Dict[str, Any]]:
|
||||||
"trigger": sidecar.get("trigger", "unknown"),
|
"trigger": sidecar.get("trigger", "unknown"),
|
||||||
"source_username": source_username,
|
"source_username": source_username,
|
||||||
"source_filename": source_filename,
|
"source_filename": source_filename,
|
||||||
|
"thumb_url": _extract_context_thumb(ctx),
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
@ -226,6 +227,51 @@ def list_quarantine_entries(quarantine_dir: str) -> List[Dict[str, Any]]:
|
||||||
return entries
|
return entries
|
||||||
|
|
||||||
|
|
||||||
|
def get_quarantine_entry_context(quarantine_dir: str, entry_id: str) -> Dict[str, Any]:
|
||||||
|
"""The sidecar's embedded pipeline ``context`` dict for one entry.
|
||||||
|
Returns {} for thin/legacy sidecars, missing entries or read errors."""
|
||||||
|
_, sidecar_path = _resolve_entry_paths(quarantine_dir, entry_id)
|
||||||
|
if not sidecar_path or not os.path.isfile(sidecar_path):
|
||||||
|
return {}
|
||||||
|
try:
|
||||||
|
with open(sidecar_path, encoding="utf-8") as f:
|
||||||
|
loaded = json.load(f)
|
||||||
|
ctx = loaded.get("context") if isinstance(loaded, dict) else None
|
||||||
|
return ctx if isinstance(ctx, dict) else {}
|
||||||
|
except Exception as exc:
|
||||||
|
logger.debug("quarantine context read failed for %s: %s", entry_id, exc)
|
||||||
|
return {}
|
||||||
|
|
||||||
|
|
||||||
|
def _extract_context_thumb(ctx: Dict[str, Any]) -> str:
|
||||||
|
"""Album-art URL from a sidecar's pipeline context — same lookup chain the
|
||||||
|
library-history recorder uses (album/spotify_album image, then album_info,
|
||||||
|
then the track_info's embedded album images). Empty string when absent."""
|
||||||
|
def _first_image(album: Any) -> str:
|
||||||
|
if not isinstance(album, dict):
|
||||||
|
return ""
|
||||||
|
url = album.get("image_url") or ""
|
||||||
|
if url:
|
||||||
|
return url
|
||||||
|
images = album.get("images") or []
|
||||||
|
if images and isinstance(images[0], dict):
|
||||||
|
return images[0].get("url", "") or ""
|
||||||
|
return ""
|
||||||
|
|
||||||
|
thumb = _first_image(ctx.get("album")) or _first_image(ctx.get("spotify_album"))
|
||||||
|
if not thumb:
|
||||||
|
album_info = ctx.get("album_info")
|
||||||
|
if isinstance(album_info, dict):
|
||||||
|
thumb = album_info.get("album_image_url", "") or ""
|
||||||
|
if not thumb:
|
||||||
|
ti = ctx.get("track_info")
|
||||||
|
if isinstance(ti, dict):
|
||||||
|
thumb = _first_image(ti.get("album"))
|
||||||
|
if not thumb:
|
||||||
|
thumb = ti.get("image_url", "") or ""
|
||||||
|
return thumb
|
||||||
|
|
||||||
|
|
||||||
def _resolve_entry_paths(quarantine_dir: str, entry_id: str) -> Tuple[Optional[str], Optional[str]]:
|
def _resolve_entry_paths(quarantine_dir: str, entry_id: str) -> Tuple[Optional[str], Optional[str]]:
|
||||||
"""Locate the `.quarantined` file + JSON sidecar for an entry id.
|
"""Locate the `.quarantined` file + JSON sidecar for an entry id.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -42,10 +42,17 @@ def status_from_acoustid_result(result_value):
|
||||||
def status_for_import(context: dict):
|
def status_for_import(context: dict):
|
||||||
"""Status for a just-imported file from its pipeline context.
|
"""Status for a just-imported file from its pipeline context.
|
||||||
|
|
||||||
The version-mismatch fallback flag wins: a force-accepted file is
|
Priority order:
|
||||||
``force_imported`` regardless of what the (earlier, failed) verification
|
1. A quarantine entry the user explicitly approved ("yes, import this
|
||||||
said about the candidate.
|
exact file") is a human decision — ``human_verified``, outranking
|
||||||
|
whatever the machine said about the candidate earlier.
|
||||||
|
2. The version-mismatch fallback flag: a force-accepted file is
|
||||||
|
``force_imported`` regardless of what the (earlier, failed)
|
||||||
|
verification said.
|
||||||
|
3. The AcoustID result of this pipeline run.
|
||||||
"""
|
"""
|
||||||
|
if context.get('_approved_quarantine_trigger'):
|
||||||
|
return HUMAN_VERIFIED
|
||||||
if context.get('_version_mismatch_fallback'):
|
if context.get('_version_mismatch_fallback'):
|
||||||
return FORCE_IMPORTED
|
return FORCE_IMPORTED
|
||||||
return status_from_acoustid_result(context.get('_acoustid_result'))
|
return status_from_acoustid_result(context.get('_acoustid_result'))
|
||||||
|
|
|
||||||
|
|
@ -53,10 +53,6 @@ class AcoustIDScannerJob(RepairJob):
|
||||||
'title_similarity': 0.70,
|
'title_similarity': 0.70,
|
||||||
'artist_similarity': 0.60,
|
'artist_similarity': 0.60,
|
||||||
'batch_size': 200,
|
'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
|
auto_fix = False # User chooses fix action per finding
|
||||||
|
|
||||||
|
|
@ -228,9 +224,11 @@ class AcoustIDScannerJob(RepairJob):
|
||||||
)
|
)
|
||||||
|
|
||||||
# Verification status from the embedded SOULSYNC_VERIFICATION tag.
|
# Verification status from the embedded SOULSYNC_VERIFICATION tag.
|
||||||
# force_imported = user accepted this file as best candidate after the
|
# Only a human decision short-circuits the scan: the user explicitly
|
||||||
# retry budget was exhausted — a mismatch here is EXPECTED. Either skip
|
# confirmed the file via the review queue. Everything else (verified /
|
||||||
# (job setting) or downgrade the finding to informational below.
|
# unverified / force_imported / untagged) is re-checked; force_imported
|
||||||
|
# mismatches are reported as informational below since a mismatch
|
||||||
|
# there is EXPECTED (the user accepted the best candidate).
|
||||||
file_verif_status = None
|
file_verif_status = None
|
||||||
try:
|
try:
|
||||||
from core.tag_writer import read_file_tags as _rft
|
from core.tag_writer import read_file_tags as _rft
|
||||||
|
|
@ -244,13 +242,6 @@ class AcoustIDScannerJob(RepairJob):
|
||||||
context.report_progress(
|
context.report_progress(
|
||||||
log_line=f'Skipped (human-verified): {fname}', log_type='skip')
|
log_line=f'Skipped (human-verified): {fname}', log_type='skip')
|
||||||
return
|
return
|
||||||
if file_verif_status == 'force_imported' and \
|
|
||||||
self._get_settings(context).get('skip_force_imported', False):
|
|
||||||
if context.report_progress:
|
|
||||||
context.report_progress(
|
|
||||||
log_line=f'Skipped (force-imported fallback): {fname}',
|
|
||||||
log_type='skip')
|
|
||||||
return
|
|
||||||
|
|
||||||
# Fingerprint-collision guard: when the TOP recording's length is wildly
|
# Fingerprint-collision guard: when the TOP recording's length is wildly
|
||||||
# different from the file, the fingerprint hit is a hash collision (the
|
# different from the file, the fingerprint hit is a hash collision (the
|
||||||
|
|
@ -289,17 +280,26 @@ class AcoustIDScannerJob(RepairJob):
|
||||||
aliases_provider=_aliases,
|
aliases_provider=_aliases,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Refresh the DB column from the file tag (the tag travels with the
|
# Persist the scan outcome so it feeds the same review pipeline as
|
||||||
# file and survives DB resets; the tracks row is the UI-facing cache).
|
# import-time verification: PASS backfills 'verified' on untagged or
|
||||||
if file_verif_status:
|
# previously-unverified files; SKIP (ambiguous / cross-script / no
|
||||||
try:
|
# hard confirmation) marks untagged files 'unverified' so they surface
|
||||||
conn = context.db._get_connection()
|
# in the Downloads-page review queue. force_imported is never blessed
|
||||||
conn.cursor().execute(
|
# here (normalize() strips version words, so an instrumental can PASS
|
||||||
"UPDATE tracks SET verification_status = ? WHERE id = ?",
|
# the title check) and 'verified' is never downgraded by a SKIP (the
|
||||||
(file_verif_status, track_id))
|
# import-time check ran with richer candidate metadata). FAIL keeps
|
||||||
getattr(conn, 'commit', lambda: None)()
|
# the finding flow below.
|
||||||
except Exception as e:
|
new_status = file_verif_status
|
||||||
logger.debug("verification_status refresh failed for %s: %s", track_id, e)
|
if outcome.decision == Decision.PASS and file_verif_status in (None, '', 'unverified'):
|
||||||
|
new_status = 'verified'
|
||||||
|
elif outcome.decision == Decision.SKIP and not file_verif_status:
|
||||||
|
new_status = 'unverified'
|
||||||
|
if new_status:
|
||||||
|
self._persist_status(
|
||||||
|
context, track_id, fpath,
|
||||||
|
(expected.get('file_path') or '').strip() or None,
|
||||||
|
new_status, write_tag=(new_status != file_verif_status),
|
||||||
|
expected=expected)
|
||||||
|
|
||||||
if outcome.decision != Decision.FAIL:
|
if outcome.decision != Decision.FAIL:
|
||||||
if context.report_progress:
|
if context.report_progress:
|
||||||
|
|
@ -362,6 +362,56 @@ class AcoustIDScannerJob(RepairJob):
|
||||||
else:
|
else:
|
||||||
result.findings_skipped_dedup += 1
|
result.findings_skipped_dedup += 1
|
||||||
|
|
||||||
|
def _persist_status(self, context, track_id, fpath, db_path, status, write_tag,
|
||||||
|
expected=None):
|
||||||
|
"""Persist a verification status to the file tag (durable, travels with
|
||||||
|
the file), the tracks row (UI cache) and any library_history rows for
|
||||||
|
this file (feeds the Unverified review queue on the Downloads page).
|
||||||
|
``db_path`` is the unresolved DB-side path — history rows may store
|
||||||
|
either form, so both are matched.
|
||||||
|
|
||||||
|
Files SoulSync never downloaded have no history row at all — for an
|
||||||
|
'unverified' outcome one is inserted (download_source 'acoustid_scan')
|
||||||
|
so EVERY scan-flagged file lands in the review queue, not just past
|
||||||
|
downloads. Re-scans then match this row via file_path (no duplicates).
|
||||||
|
"""
|
||||||
|
if not status:
|
||||||
|
return
|
||||||
|
if write_tag:
|
||||||
|
try:
|
||||||
|
from core.tag_writer import write_verification_status
|
||||||
|
write_verification_status(fpath, status)
|
||||||
|
except Exception as e:
|
||||||
|
logger.debug("verification tag write failed for %s: %s", fpath, e)
|
||||||
|
try:
|
||||||
|
conn = context.db._get_connection()
|
||||||
|
cur = conn.cursor()
|
||||||
|
cur.execute(
|
||||||
|
"UPDATE tracks SET verification_status = ? WHERE id = ?",
|
||||||
|
(status, track_id))
|
||||||
|
matched = 0
|
||||||
|
for p in {p for p in (fpath, db_path) if p}:
|
||||||
|
cur.execute(
|
||||||
|
"UPDATE library_history SET verification_status = ? WHERE file_path = ?",
|
||||||
|
(status, p))
|
||||||
|
matched += max(getattr(cur, 'rowcount', 0) or 0, 0)
|
||||||
|
if status == 'unverified' and matched == 0:
|
||||||
|
exp = expected or {}
|
||||||
|
cur.execute(
|
||||||
|
"""INSERT INTO library_history
|
||||||
|
(event_type, title, artist_name, album_name, file_path,
|
||||||
|
thumb_url, download_source, verification_status)
|
||||||
|
VALUES ('download', ?, ?, ?, ?, ?, 'acoustid_scan', ?)""",
|
||||||
|
(exp.get('title') or os.path.basename(fpath),
|
||||||
|
exp.get('artist') or None,
|
||||||
|
exp.get('album_title') or None,
|
||||||
|
db_path or fpath,
|
||||||
|
exp.get('album_thumb_url') or None,
|
||||||
|
status))
|
||||||
|
getattr(conn, 'commit', lambda: None)()
|
||||||
|
except Exception as e:
|
||||||
|
logger.debug("verification_status persist failed for %s: %s", track_id, e)
|
||||||
|
|
||||||
def _load_db_tracks(self, context: JobContext) -> dict:
|
def _load_db_tracks(self, context: JobContext) -> dict:
|
||||||
"""Load all tracks from DB keyed by track ID."""
|
"""Load all tracks from DB keyed by track ID."""
|
||||||
tracks = {}
|
tracks = {}
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
"""Folder-artist override is opt-in and never clobbers an identified artist.
|
"""Folder-artist override can be disabled to keep an identified artist.
|
||||||
|
|
||||||
Background
|
Background
|
||||||
----------
|
----------
|
||||||
|
|
@ -12,16 +12,15 @@ named ``soulsync`` therefore got the album-artist of *every* file forced to
|
||||||
with a Spotify/MusicBrainz id. Navidrome groups by album-artist, so 65 singles
|
with a Spotify/MusicBrainz id. Navidrome groups by album-artist, so 65 singles
|
||||||
collapsed under a bogus "soulsync" artist.
|
collapsed under a bogus "soulsync" artist.
|
||||||
|
|
||||||
``resolve_folder_artist`` is the extracted, pure decision. It must:
|
``resolve_folder_artist`` is the extracted, pure decision. It must keep the
|
||||||
- return ``None`` (i.e. keep the identified artist) when the feature is OFF,
|
identified artist when the feature is OFF, and reproduce the original
|
||||||
regardless of folder structure — this is the regression guard;
|
folder-derived artist when enabled.
|
||||||
- only when explicitly enabled, reproduce the original folder-derived artist.
|
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from core.imports.folder_artist import resolve_folder_artist
|
from core.imports.folder_artist import resolve_folder_artist
|
||||||
|
|
||||||
|
|
||||||
# --- OFF by default: never override (the bug fix) ---------------------------
|
# --- Disabled: never override -----------------------------------------------
|
||||||
|
|
||||||
def test_disabled_keeps_identified_artist_even_with_artist_album_structure():
|
def test_disabled_keeps_identified_artist_even_with_artist_album_structure():
|
||||||
# The 'soulsync' mass mis-file: identified artist is real, folder is a
|
# The 'soulsync' mass mis-file: identified artist is real, folder is a
|
||||||
|
|
@ -41,7 +40,7 @@ def test_disabled_returns_none_for_clean_artist_album_path():
|
||||||
) is None
|
) is None
|
||||||
|
|
||||||
|
|
||||||
# --- Enabled: original behaviour is available on request --------------------
|
# --- Enabled: original behaviour --------------------------------------------
|
||||||
|
|
||||||
def test_enabled_uses_top_folder_as_artist_when_it_differs():
|
def test_enabled_uses_top_folder_as_artist_when_it_differs():
|
||||||
assert resolve_folder_artist(
|
assert resolve_folder_artist(
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
"""Verification-status vocabulary + mapping (DB column / file tag / UI badge)."""
|
"""Verification-status vocabulary + mapping (DB column / file tag / UI badge)."""
|
||||||
|
|
||||||
from core.matching.verification_status import (
|
from core.matching.verification_status import (
|
||||||
VERIFIED, UNVERIFIED, FORCE_IMPORTED,
|
VERIFIED, UNVERIFIED, FORCE_IMPORTED, HUMAN_VERIFIED,
|
||||||
status_from_acoustid_result, status_for_import,
|
status_from_acoustid_result, status_for_import,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
@ -24,3 +24,13 @@ def test_status_for_import_falls_back_to_acoustid_result():
|
||||||
assert status_for_import({'_acoustid_result': 'pass'}) == VERIFIED
|
assert status_for_import({'_acoustid_result': 'pass'}) == VERIFIED
|
||||||
assert status_for_import({'_acoustid_result': 'skip'}) == UNVERIFIED
|
assert status_for_import({'_acoustid_result': 'skip'}) == UNVERIFIED
|
||||||
assert status_for_import({}) is None
|
assert status_for_import({}) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_approved_quarantine_import_is_human_verified():
|
||||||
|
# The user clicked Approve on a quarantine entry — an explicit human
|
||||||
|
# decision about THIS file. Outranks both the fallback flag and whatever
|
||||||
|
# the (earlier, failed) verification said.
|
||||||
|
ctx = {'_approved_quarantine_trigger': 'acoustid',
|
||||||
|
'_version_mismatch_fallback': 'instrumental',
|
||||||
|
'_acoustid_result': 'fail'}
|
||||||
|
assert status_for_import(ctx) == HUMAN_VERIFIED
|
||||||
|
|
|
||||||
|
|
@ -818,7 +818,7 @@ def test_scanner_does_not_flag_cross_script_when_alias_bridges(monkeypatch):
|
||||||
assert captured == [], f"cross-script track false-flagged: {captured}"
|
assert captured == [], f"cross-script track false-flagged: {captured}"
|
||||||
|
|
||||||
|
|
||||||
def _force_imported_scan(monkeypatch, *, skip_setting):
|
def _force_imported_scan(monkeypatch):
|
||||||
"""Drive a scan over a force-imported file whose fingerprint clearly
|
"""Drive a scan over a force-imported file whose fingerprint clearly
|
||||||
mismatches. Returns the captured findings."""
|
mismatches. Returns the captured findings."""
|
||||||
import core.repair_jobs.acoustid_scanner as scanner_mod
|
import core.repair_jobs.acoustid_scanner as scanner_mod
|
||||||
|
|
@ -829,10 +829,6 @@ def _force_imported_scan(monkeypatch, *, skip_setting):
|
||||||
lambda fpath: {'artist': None, 'verification_status': 'force_imported'},
|
lambda fpath: {'artist': None, 'verification_status': 'force_imported'},
|
||||||
)
|
)
|
||||||
job = AcoustIDScannerJob()
|
job = AcoustIDScannerJob()
|
||||||
if skip_setting:
|
|
||||||
monkeypatch.setattr(job, '_get_settings',
|
|
||||||
lambda ctx: {**job.default_settings,
|
|
||||||
'skip_force_imported': True})
|
|
||||||
captured = []
|
captured = []
|
||||||
context = _make_finding_capturing_context(
|
context = _make_finding_capturing_context(
|
||||||
track_row=("42", "Wanted Song", "Real Artist",
|
track_row=("42", "Wanted Song", "Real Artist",
|
||||||
|
|
@ -857,19 +853,14 @@ def _force_imported_scan(monkeypatch, *, skip_setting):
|
||||||
def test_force_imported_mismatch_is_reported_as_informational(monkeypatch):
|
def test_force_imported_mismatch_is_reported_as_informational(monkeypatch):
|
||||||
# The user opted into the fallback, so the scan must still TELL them the
|
# 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
|
# file is e.g. an instrumental — but as 'info', clearly marked, not as a
|
||||||
# red Wrong-download warning.
|
# red Wrong-download warning. Only human_verified short-circuits the scan.
|
||||||
captured = _force_imported_scan(monkeypatch, skip_setting=False)
|
captured = _force_imported_scan(monkeypatch)
|
||||||
assert len(captured) == 1
|
assert len(captured) == 1
|
||||||
assert captured[0]['severity'] == 'info'
|
assert captured[0]['severity'] == 'info'
|
||||||
assert captured[0]['details'].get('force_imported') is True
|
assert captured[0]['details'].get('force_imported') is True
|
||||||
assert 'Force-imported' in captured[0]['title']
|
assert 'Force-imported' in captured[0]['title']
|
||||||
|
|
||||||
|
|
||||||
def test_force_imported_can_be_skipped_via_setting(monkeypatch):
|
|
||||||
captured = _force_imported_scan(monkeypatch, skip_setting=True)
|
|
||||||
assert captured == []
|
|
||||||
|
|
||||||
|
|
||||||
def test_human_verified_files_are_never_scanned(monkeypatch):
|
def test_human_verified_files_are_never_scanned(monkeypatch):
|
||||||
import core.repair_jobs.acoustid_scanner as scanner_mod
|
import core.repair_jobs.acoustid_scanner as scanner_mod
|
||||||
monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases",
|
monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases",
|
||||||
|
|
@ -888,3 +879,79 @@ def test_human_verified_files_are_never_scanned(monkeypatch):
|
||||||
fake, context, JobResultStub(),
|
fake, context, JobResultStub(),
|
||||||
fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6)
|
fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6)
|
||||||
assert captured == []
|
assert captured == []
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Scan-outcome persistence — the scan feeds the same review pipeline as
|
||||||
|
# import-time verification (tag + tracks row + library_history rows).
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_artist):
|
||||||
|
"""Drive one _scan_file call and return (status_updates, tag_writes) where
|
||||||
|
status_updates is the list of (query, params) UPDATEs the scanner ran."""
|
||||||
|
import core.repair_jobs.acoustid_scanner as scanner_mod
|
||||||
|
monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases",
|
||||||
|
lambda name: [], raising=False)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
'core.tag_writer.read_file_tags',
|
||||||
|
lambda fpath: {'artist': None, 'verification_status': file_status})
|
||||||
|
tag_writes = []
|
||||||
|
monkeypatch.setattr(
|
||||||
|
'core.tag_writer.write_verification_status',
|
||||||
|
lambda fpath, status: tag_writes.append((fpath, status)) or True)
|
||||||
|
job = AcoustIDScannerJob()
|
||||||
|
captured = []
|
||||||
|
context = _make_finding_capturing_context(
|
||||||
|
track_row=("9", "Call Your Name", expected_artist,
|
||||||
|
"/music/cyn.flac", 1, "Album", None, None),
|
||||||
|
captured=captured)
|
||||||
|
fake = SimpleNamespace(fingerprint_and_lookup=lambda f: {
|
||||||
|
'best_score': 0.97,
|
||||||
|
'recordings': [{'title': 'Call Your Name', 'artist': aid_artist}]})
|
||||||
|
job._scan_file('/music/cyn.flac', '9',
|
||||||
|
{'title': 'Call Your Name', 'artist': expected_artist},
|
||||||
|
fake, context, JobResultStub(),
|
||||||
|
fp_threshold=0.85, title_threshold=0.85, artist_threshold=0.6)
|
||||||
|
conn = context.db._get_connection()
|
||||||
|
updates = [(q, p) for q, p in conn.cursor().executed
|
||||||
|
if 'verification_status' in q]
|
||||||
|
return updates, tag_writes, captured
|
||||||
|
|
||||||
|
|
||||||
|
def test_scan_pass_backfills_verified_status(monkeypatch):
|
||||||
|
# Untagged file + clean fingerprint PASS → the scan backfills 'verified'
|
||||||
|
# into the tag, the tracks row AND library_history (review-queue feed).
|
||||||
|
updates, tag_writes, captured = _run_persistence_scan(
|
||||||
|
monkeypatch, file_status=None,
|
||||||
|
aid_artist='Sawano Hiroyuki', expected_artist='Sawano Hiroyuki')
|
||||||
|
assert captured == []
|
||||||
|
assert tag_writes == [('/music/cyn.flac', 'verified')]
|
||||||
|
assert any('tracks' in q and p == ('verified', '9') for q, p in updates)
|
||||||
|
assert any('library_history' in q and p == ('verified', '/music/cyn.flac')
|
||||||
|
for q, p in updates)
|
||||||
|
|
||||||
|
|
||||||
|
def test_scan_skip_marks_untagged_file_unverified(monkeypatch):
|
||||||
|
# Title matches but the artist is ambiguous (cover/collab band?) → SKIP.
|
||||||
|
# An untagged file gets 'unverified' so it surfaces in the Downloads-page
|
||||||
|
# review queue instead of silently passing or being deleted.
|
||||||
|
updates, tag_writes, captured = _run_persistence_scan(
|
||||||
|
monkeypatch, file_status=None,
|
||||||
|
aid_artist='Mantilla', expected_artist='Metallica')
|
||||||
|
assert captured == []
|
||||||
|
assert tag_writes == [('/music/cyn.flac', 'unverified')]
|
||||||
|
assert any('tracks' in q and p == ('unverified', '9') for q, p in updates)
|
||||||
|
assert any('library_history' in q and p == ('unverified', '/music/cyn.flac')
|
||||||
|
for q, p in updates)
|
||||||
|
|
||||||
|
|
||||||
|
def test_scan_skip_does_not_downgrade_verified(monkeypatch):
|
||||||
|
# A SKIP must not downgrade an import-time 'verified' (that check ran with
|
||||||
|
# richer candidate metadata). Status is refreshed, tag untouched.
|
||||||
|
updates, tag_writes, captured = _run_persistence_scan(
|
||||||
|
monkeypatch, file_status='verified',
|
||||||
|
aid_artist='Mantilla', expected_artist='Metallica')
|
||||||
|
assert captured == []
|
||||||
|
assert tag_writes == []
|
||||||
|
assert any('tracks' in q and p == ('verified', '9') for q, p in updates)
|
||||||
|
|
|
||||||
|
|
@ -2390,7 +2390,7 @@
|
||||||
<button class="adl-pill" data-filter="queued" onclick="adlSetFilter('queued')">Queued</button>
|
<button class="adl-pill" data-filter="queued" onclick="adlSetFilter('queued')">Queued</button>
|
||||||
<button class="adl-pill" data-filter="completed" onclick="adlSetFilter('completed')">Completed</button>
|
<button class="adl-pill" data-filter="completed" onclick="adlSetFilter('completed')">Completed</button>
|
||||||
<button class="adl-pill" data-filter="failed" onclick="adlSetFilter('failed')">Failed</button>
|
<button class="adl-pill" data-filter="failed" onclick="adlSetFilter('failed')">Failed</button>
|
||||||
<button class="adl-pill" data-filter="unverified" onclick="adlSetFilter('unverified')" title="Completed downloads that were imported without hard AcoustID confirmation (unverified) or force-imported after repeated mismatches — review these.">⚠ Unverified</button>
|
<button class="adl-pill" data-filter="unverified" onclick="adlSetFilter('unverified')" title="Review queue: imported-but-unconfirmed downloads (unverified / force-imported) and quarantined files that were never imported.">⚠ Unverified/Quarantine</button>
|
||||||
</div>
|
</div>
|
||||||
<div style="display:flex;align-items:center;gap:10px;">
|
<div style="display:flex;align-items:center;gap:10px;">
|
||||||
<span class="adl-count" id="adl-count"></span>
|
<span class="adl-count" id="adl-count"></span>
|
||||||
|
|
|
||||||
|
|
@ -10864,6 +10864,26 @@ body.helper-mode-active #dashboard-activity-feed:hover {
|
||||||
border-color: rgba(239, 83, 80, 0.3);
|
border-color: rgba(239, 83, 80, 0.3);
|
||||||
color: #ef5350;
|
color: #ef5350;
|
||||||
}
|
}
|
||||||
|
.download-audit-hero-pill-verified {
|
||||||
|
background: rgba(74, 222, 128, 0.12);
|
||||||
|
border-color: rgba(74, 222, 128, 0.3);
|
||||||
|
color: #4ade80;
|
||||||
|
}
|
||||||
|
.download-audit-hero-pill-human_verified {
|
||||||
|
background: rgba(52, 152, 219, 0.12);
|
||||||
|
border-color: rgba(52, 152, 219, 0.35);
|
||||||
|
color: #3498db;
|
||||||
|
}
|
||||||
|
.download-audit-hero-pill-force_imported {
|
||||||
|
background: rgba(230, 126, 34, 0.12);
|
||||||
|
border-color: rgba(230, 126, 34, 0.35);
|
||||||
|
color: #e67e22;
|
||||||
|
}
|
||||||
|
.download-audit-hero-pill-unverified {
|
||||||
|
background: rgba(241, 196, 15, 0.12);
|
||||||
|
border-color: rgba(241, 196, 15, 0.35);
|
||||||
|
color: #f1c40f;
|
||||||
|
}
|
||||||
|
|
||||||
/* Tab bar */
|
/* Tab bar */
|
||||||
.download-audit-tabs {
|
.download-audit-tabs {
|
||||||
|
|
@ -67906,3 +67926,13 @@ body.em-scroll-lock { overflow: hidden; }
|
||||||
.verif-act-ok:hover { background: rgba(46,204,113,0.25); border-color: rgba(46,204,113,0.5); }
|
.verif-act-ok:hover { background: rgba(46,204,113,0.25); border-color: rgba(46,204,113,0.5); }
|
||||||
.verif-act-del:hover { background: rgba(231,76,60,0.25); border-color: rgba(231,76,60,0.5); }
|
.verif-act-del:hover { background: rgba(231,76,60,0.25); border-color: rgba(231,76,60,0.5); }
|
||||||
.verif-act-play.playing { background: rgba(var(--accent-rgb),0.3); }
|
.verif-act-play.playing { background: rgba(var(--accent-rgb),0.3); }
|
||||||
|
/* Review-queue row annotations (library-history-quarantine style) */
|
||||||
|
.verif-reason-badge { display: inline-block; font-size: 9.5px; font-weight: 600; letter-spacing: 0.04em; border: 1px solid; border-radius: 999px; padding: 1px 7px; line-height: 14px; white-space: nowrap; cursor: help; }
|
||||||
|
.verif-rb-unv { color: #f1c40f; border-color: rgba(241,196,15,0.5); }
|
||||||
|
.verif-rb-force { color: #ef5350; border-color: rgba(239,83,80,0.5); }
|
||||||
|
.verif-rb-int { color: #facc15; border-color: rgba(250,204,21,0.5); }
|
||||||
|
.verif-time { font-size: 11px; color: rgba(255,255,255,0.45); white-space: nowrap; }
|
||||||
|
.verif-quar-details { margin-top: 6px; font-size: 11.5px; color: rgba(255,255,255,0.6); line-height: 1.5; word-break: break-all; }
|
||||||
|
.verif-detail-label { color: rgba(255,255,255,0.4); margin-right: 4px; }
|
||||||
|
.verif-banner-spacer { flex: 1; }
|
||||||
|
.verif-bulk-danger { border-color: rgba(248,113,113,0.4) !important; color: #f87171 !important; }
|
||||||
|
|
|
||||||
|
|
@ -3794,8 +3794,10 @@ function openDownloadAuditModal(entry) {
|
||||||
|
|
||||||
// Live tag read for the Tags tab. Fire-and-forget on open so the
|
// Live tag read for the Tags tab. Fire-and-forget on open so the
|
||||||
// data is ready by the time the user switches to the Tags tab.
|
// data is ready by the time the user switches to the Tags tab.
|
||||||
if (entry.id != null) {
|
// Synthetic entries (quarantine review queue) have no history id but
|
||||||
fetchAndRenderEmbeddedTags(entry.id);
|
// carry their own tags URL in _file_tags_url.
|
||||||
|
if (entry.id != null || entry._file_tags_url) {
|
||||||
|
fetchAndRenderEmbeddedTags(entry.id, entry._file_tags_url);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -3809,11 +3811,9 @@ function switchAuditTab(tabName) {
|
||||||
if (body) body.innerHTML = renderDownloadAuditTabPanel(tabName, _downloadAuditEntry);
|
if (body) body.innerHTML = renderDownloadAuditTabPanel(tabName, _downloadAuditEntry);
|
||||||
// Re-fire the live fetch when switching to Tags (in case it
|
// Re-fire the live fetch when switching to Tags (in case it
|
||||||
// raced past first mount before the user got there).
|
// raced past first mount before the user got there).
|
||||||
if (tabName === 'tags' && _downloadAuditEntry.id != null) {
|
if ((tabName === 'tags' || tabName === 'lyrics') &&
|
||||||
fetchAndRenderEmbeddedTags(_downloadAuditEntry.id);
|
(_downloadAuditEntry.id != null || _downloadAuditEntry._file_tags_url)) {
|
||||||
}
|
fetchAndRenderEmbeddedTags(_downloadAuditEntry.id, _downloadAuditEntry._file_tags_url);
|
||||||
if (tabName === 'lyrics' && _downloadAuditEntry.id != null) {
|
|
||||||
fetchAndRenderEmbeddedTags(_downloadAuditEntry.id);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
window.switchAuditTab = switchAuditTab;
|
window.switchAuditTab = switchAuditTab;
|
||||||
|
|
@ -3851,7 +3851,14 @@ function renderDownloadAuditHero(entry) {
|
||||||
const pills = [];
|
const pills = [];
|
||||||
if (entry.download_source) pills.push(_auditHeroPill('source', entry.download_source));
|
if (entry.download_source) pills.push(_auditHeroPill('source', entry.download_source));
|
||||||
if (entry.quality) pills.push(_auditHeroPill('quality', entry.quality));
|
if (entry.quality) pills.push(_auditHeroPill('quality', entry.quality));
|
||||||
if (entry.acoustid_result) pills.push(_auditHeroPill('verify', formatAcoustidLabel(entry.acoustid_result), entry.acoustid_result));
|
const _vsLabels = { verified: 'Verified', unverified: 'Unverified', force_imported: 'Force-imported', human_verified: 'Human verified' };
|
||||||
|
if (entry.verification_status && _vsLabels[entry.verification_status]) {
|
||||||
|
pills.push(_auditHeroPill('verify', _vsLabels[entry.verification_status], entry.verification_status));
|
||||||
|
} else if (entry._quarantined) {
|
||||||
|
pills.push(_auditHeroPill('verify', 'Quarantined', 'fail'));
|
||||||
|
} else if (entry.acoustid_result) {
|
||||||
|
pills.push(_auditHeroPill('verify', formatAcoustidLabel(entry.acoustid_result), entry.acoustid_result));
|
||||||
|
}
|
||||||
|
|
||||||
return `
|
return `
|
||||||
${art}
|
${art}
|
||||||
|
|
@ -3952,7 +3959,7 @@ function renderEmbeddedTagsSection(entry) {
|
||||||
return renderDownloadAuditTagsTab(entry);
|
return renderDownloadAuditTagsTab(entry);
|
||||||
}
|
}
|
||||||
|
|
||||||
async function fetchAndRenderEmbeddedTags(historyId) {
|
async function fetchAndRenderEmbeddedTags(historyId, urlOverride) {
|
||||||
const tagsSlot = document.getElementById('download-audit-tags-body');
|
const tagsSlot = document.getElementById('download-audit-tags-body');
|
||||||
const lyricsSlot = document.getElementById('download-audit-lyrics-body');
|
const lyricsSlot = document.getElementById('download-audit-lyrics-body');
|
||||||
if (!tagsSlot && !lyricsSlot) return;
|
if (!tagsSlot && !lyricsSlot) return;
|
||||||
|
|
@ -3962,7 +3969,7 @@ async function fetchAndRenderEmbeddedTags(historyId) {
|
||||||
if (lyricsSlot) lyricsSlot.innerHTML = html;
|
if (lyricsSlot) lyricsSlot.innerHTML = html;
|
||||||
};
|
};
|
||||||
try {
|
try {
|
||||||
const resp = await fetch(`/api/library/history/${historyId}/file-tags`);
|
const resp = await fetch(urlOverride || `/api/library/history/${historyId}/file-tags`);
|
||||||
if (!resp.ok) { renderError(`Could not read file tags (HTTP ${resp.status}).`); return; }
|
if (!resp.ok) { renderError(`Could not read file tags (HTTP ${resp.status}).`); return; }
|
||||||
const data = await resp.json();
|
const data = await resp.json();
|
||||||
if (!data.success) { renderError(data.error || 'Could not read file tags.'); return; }
|
if (!data.success) { renderError(data.error || 'Could not read file tags.'); return; }
|
||||||
|
|
@ -4258,8 +4265,8 @@ function buildDownloadAuditSteps(entry) {
|
||||||
{
|
{
|
||||||
key: 'verify',
|
key: 'verify',
|
||||||
title: 'Verification',
|
title: 'Verification',
|
||||||
status: auditStatusFromAcoustid(entry.acoustid_result),
|
status: auditVerifyStatus(entry),
|
||||||
detail: buildAcoustidDetail(entry.acoustid_result),
|
detail: buildVerificationDetail(entry),
|
||||||
meta: [],
|
meta: [],
|
||||||
},
|
},
|
||||||
(() => {
|
(() => {
|
||||||
|
|
@ -4376,6 +4383,37 @@ function buildSourceMatchMeta(entry) {
|
||||||
return out;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function auditVerifyStatus(entry) {
|
||||||
|
// The persisted verification status is the final word — it captures
|
||||||
|
// outcomes the raw acoustid_result can't express (force-imported after
|
||||||
|
// N retries, human approval via the review queue).
|
||||||
|
if (entry._quarantined) return 'error';
|
||||||
|
if (entry.verification_status === 'human_verified') return 'complete';
|
||||||
|
if (entry.verification_status === 'verified') return 'complete';
|
||||||
|
if (entry.verification_status === 'force_imported') return 'partial';
|
||||||
|
if (entry.verification_status === 'unverified') return 'partial';
|
||||||
|
return auditStatusFromAcoustid(entry.acoustid_result);
|
||||||
|
}
|
||||||
|
|
||||||
|
function buildVerificationDetail(entry) {
|
||||||
|
if (entry._quarantined) {
|
||||||
|
return `Quarantined — the file was NOT imported. ${entry._quarantine_reason || ''}`.trim();
|
||||||
|
}
|
||||||
|
if (entry.verification_status === 'force_imported') {
|
||||||
|
return 'Force-imported: accepted as the best available candidate after the retry budget was exhausted (version-mismatch fallback). The AcoustID check was bypassed for this final re-import — earlier attempts had failed it.';
|
||||||
|
}
|
||||||
|
if (entry.verification_status === 'human_verified') {
|
||||||
|
return 'Human verified: you confirmed this file via the review queue. The AcoustID scanner skips it.';
|
||||||
|
}
|
||||||
|
if (entry.verification_status === 'unverified') {
|
||||||
|
return 'Imported but not hard-confirmed: AcoustID could not verify this file (ambiguous / cross-script metadata / no fingerprint match). Review it under Unverified/Quarantine on the Downloads page.';
|
||||||
|
}
|
||||||
|
if (entry.verification_status === 'verified') {
|
||||||
|
return 'AcoustID fingerprint matched the expected track.';
|
||||||
|
}
|
||||||
|
return buildAcoustidDetail(entry.acoustid_result);
|
||||||
|
}
|
||||||
|
|
||||||
function auditStatusFromAcoustid(result) {
|
function auditStatusFromAcoustid(result) {
|
||||||
if (!result) return 'unknown';
|
if (!result) return 'unknown';
|
||||||
if (result === 'pass') return 'complete';
|
if (result === 'pass') return 'complete';
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue