From 8a22b3f8ba7a886b2098e0888bfd1ef7230a4e5b Mon Sep 17 00:00:00 2001 From: dev Date: Sun, 14 Jun 2026 23:45:55 +0200 Subject: [PATCH] fix: resolve relative library paths fully + run quality gate before AcoustID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1) _resolve_library_file_path now tries the FULL relative path (index 0) against each base dir, not just suffixes. The library scanner stores clean "Artist/Album/Track.flac" paths; skipping index 0 dropped the artist folder so the file never resolved — every quality-scanner probe failed ("20/20 could not be probed"). Now they resolve under the transfer/library dir. 2) Quality gate moved BEFORE the AcoustID check in post_process_matched_download. - A wrong-quality file is rejected without paying for an AcoustID fingerprint. - context['_audio_quality'] is set before either gate quarantines, so the real quality is recorded on the sidecar for EVERY quarantine trigger — it's known when reviewing/approving any quarantined file. - force_import still never fires on a quality mismatch (only AcoustID). normalize_import_context mutates in place, so the moved block keeps its context fields. New test pins the order + that AcoustID isn't run on a quality reject. Co-Authored-By: Claude Opus 4.8 --- core/imports/pipeline.py | 107 ++++++++++++++------------ tests/imports/test_import_pipeline.py | 42 ++++++++++ web_server.py | 7 +- 3 files changed, 104 insertions(+), 52 deletions(-) diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index 1a1967da..d21e826e 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -403,6 +403,62 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta _notify_download_completed(batch_id, task_id, success=False) return + # QUALITY GATE — runs BEFORE AcoustID so (1) a wrong-quality file is + # rejected without paying for an AcoustID fingerprint, and (2) the real + # audio quality is recorded on the context (→ quarantine sidecar) for + # EVERY trigger, so it's known when reviewing/approving any quarantined + # file. force_import still never fires on a quality mismatch. + context['_audio_quality'] = get_audio_quality_string(file_path) + if context['_audio_quality']: + logger.info(f"Audio quality detected: {context['_audio_quality']}") + + _skip_quality = _should_skip_quarantine_check(context, 'bit_depth') or \ + _should_skip_quarantine_check(context, 'quality') + rejection_reason = None if _skip_quality else check_quality_target(file_path, context) + if _skip_quality: + logger.info(f"[QualityGuard] Skipped (user approval) for {_basename}") + if rejection_reason: + try: + quarantine_path = move_to_quarantine( + file_path, + context, + rejection_reason, + automation_engine, + trigger='quality', + ) + _mark_task_quarantined(context, quarantine_path) + logger.info(f"File quarantined due to quality mismatch: {quarantine_path}") + except Exception as quarantine_error: + logger.error(f"Quarantine failed ({quarantine_error}), deleting file: {file_path}") + try: + os.remove(file_path) + except Exception as e: + logger.debug("delete quarantine fallback: %s", e) + + context['_bitdepth_rejected'] = True + task_id = context.get('task_id') + batch_id = context.get('batch_id') + with matched_context_lock: + matched_downloads_context.pop(context_key, None) + + # Try the next-best candidate before giving up — same pattern + # as AcoustID and integrity failures. + if _requeue_quarantined_task_for_retry(task_id, batch_id, 'quality'): + logger.info( + "Quality mismatch for task %s — retrying next-best candidate: %s", + task_id, rejection_reason, + ) + return + + if task_id: + with tasks_lock: + if task_id in download_tasks: + download_tasks[task_id]['status'] = 'failed' + download_tasks[task_id]['error_message'] = f"Quality filter: {rejection_reason}" + if task_id and batch_id: + _notify_download_completed(batch_id, task_id, success=False) + return + _skip_acoustid = _should_skip_quarantine_check(context, 'acoustid') if _skip_acoustid: logger.info(f"[AcoustID] Skipped (user approval) for {_basename}") @@ -627,57 +683,6 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta album_info.get('album_name', 'None'), ) - context['_audio_quality'] = get_audio_quality_string(file_path) - if context['_audio_quality']: - logger.info(f"Audio quality detected: {context['_audio_quality']}") - - _skip_quality = _should_skip_quarantine_check(context, 'bit_depth') or \ - _should_skip_quarantine_check(context, 'quality') - rejection_reason = None if _skip_quality else check_quality_target(file_path, context) - if _skip_quality: - logger.info(f"[QualityGuard] Skipped (user approval) for {_basename}") - if rejection_reason: - try: - quarantine_path = move_to_quarantine( - file_path, - context, - rejection_reason, - automation_engine, - trigger='quality', - ) - _mark_task_quarantined(context, quarantine_path) - logger.info(f"File quarantined due to quality mismatch: {quarantine_path}") - except Exception as quarantine_error: - logger.error(f"Quarantine failed ({quarantine_error}), deleting file: {file_path}") - try: - os.remove(file_path) - except Exception as e: - logger.debug("delete quarantine fallback: %s", e) - - context['_bitdepth_rejected'] = True - task_id = context.get('task_id') - batch_id = context.get('batch_id') - with matched_context_lock: - matched_downloads_context.pop(context_key, None) - - # Try the next-best candidate before giving up — same pattern - # as AcoustID and integrity failures. - if _requeue_quarantined_task_for_retry(task_id, batch_id, 'quality'): - logger.info( - "Quality mismatch for task %s — retrying next-best candidate: %s", - task_id, rejection_reason, - ) - return - - if task_id: - with tasks_lock: - if task_id in download_tasks: - download_tasks[task_id]['status'] = 'failed' - download_tasks[task_id]['error_message'] = f"Quality filter: {rejection_reason}" - if task_id and batch_id: - _notify_download_completed(batch_id, task_id, success=False) - return - file_ext = os.path.splitext(file_path)[1] clean_track_name = get_import_clean_title( context, diff --git a/tests/imports/test_import_pipeline.py b/tests/imports/test_import_pipeline.py index a8b550aa..06bffb6d 100644 --- a/tests/imports/test_import_pipeline.py +++ b/tests/imports/test_import_pipeline.py @@ -235,6 +235,48 @@ def test_post_process_matched_download_forwards_separate_metadata_runtime(tmp_pa # write to the task directly — it stashes on context and the wrapper applies it) # --------------------------------------------------------------------------- +def test_quality_gate_runs_before_acoustid(tmp_path, monkeypatch): + """The quality check must run BEFORE AcoustID: a wrong-quality file is + quarantined with trigger='quality' and AcoustID is never fingerprinted (so + quality is known on every quarantine entry, and no wasted AcoustID call).""" + src = tmp_path / "source.flac" + src.write_bytes(b"fLaC") + + # Reach the quality gate: bypass integrity + silence guards. + from core.imports.file_integrity import IntegrityResult + monkeypatch.setattr(import_pipeline, "check_audio_integrity", + lambda *_a, **_kw: IntegrityResult(ok=True, checks={})) + monkeypatch.setattr(import_pipeline, "detect_broken_audio", lambda *_a, **_kw: None) + + # Wrong quality → rejection. + monkeypatch.setattr(import_pipeline, "get_audio_quality_string", lambda fp: "FLAC 16bit/44.1kHz") + monkeypatch.setattr(import_pipeline, "check_quality_target", lambda fp, ctx: "Quality mismatch: FLAC 16bit") + + triggers = [] + monkeypatch.setattr(import_pipeline, "move_to_quarantine", + lambda fp, ctx, reason, eng, trigger=None: triggers.append(trigger) or "/q/x.flac.quarantined") + monkeypatch.setattr(import_pipeline, "_mark_task_quarantined", lambda *a, **k: None) + monkeypatch.setattr(import_pipeline, "_requeue_quarantined_task_for_retry", lambda *a, **k: False) + + # Spy: AcoustID must NOT be constructed when quality already rejected. + acoustid_constructed = [] + fake_mod = types.SimpleNamespace( + AcoustIDVerification=lambda *a, **k: acoustid_constructed.append(True), + VerificationResult=types.SimpleNamespace(FAIL="fail"), + ) + monkeypatch.setitem(sys.modules, "core.acoustid_verification", fake_mod) + + runtime = types.SimpleNamespace(automation_engine=None, on_download_completed=None, + web_scan_manager=None, repair_worker=None) + context = {"track_info": {}, "task_id": None, "batch_id": None} + + import_pipeline.post_process_matched_download("ctx", context, str(src), runtime) + + assert triggers == ["quality"] # quarantined for quality + assert acoustid_constructed == [] # AcoustID never ran + assert context.get("_audio_quality") == "FLAC 16bit/44.1kHz" # recorded for the sidecar + + def test_mark_task_quarantined_stashes_entry_id_when_task_id_absent(): ctx = {} # wrapper popped task_id before the inner pipeline ran import_pipeline._mark_task_quarantined(ctx, "/q/20260514_120000_song.flac.quarantined") diff --git a/web_server.py b/web_server.py index f68277ee..230f8461 100644 --- a/web_server.py +++ b/web_server.py @@ -11796,7 +11796,12 @@ def _resolve_library_file_path(file_path): for base_dir in [transfer_dir, download_dir] + list(library_dirs): if not base_dir or not os.path.isdir(base_dir): continue - for i in range(1, len(path_parts)): + # Start at index 0 so a clean relative path ("Artist/Album/Track.flac") + # is tried in FULL first — the library scanner stores exactly that, and + # skipping index 0 dropped the artist folder so it never matched. A + # Windows drive-letter part 0 ("E:") simply fails this attempt and falls + # through to the shorter suffixes below, so this is safe for both. + for i in range(0, len(path_parts)): found = find_on_disk(base_dir, path_parts[i:]) if found: return found