diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index fc5bdf24..369b4d66 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -62,6 +62,7 @@ from core.runtime_state import ( ) from core.metadata.artwork import download_cover_art from core.metadata.common import wipe_source_tags +from core.imports.tag_policy import should_wipe_tags_on_enhancement_failure from core.metadata.enrichment import enhance_file_metadata from core.imports.paths import ( build_final_path_for_track, @@ -590,7 +591,13 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta except Exception as meta_err: import traceback pp_logger.info(f"[inner] Metadata enhancement FAILED for {context_key}: {meta_err}\n{traceback.format_exc()}") - wipe_source_tags(file_path) + if should_wipe_tags_on_enhancement_failure(has_clean_metadata): + wipe_source_tags(file_path) + else: + logger.warning( + "[Metadata] Enhancement failed but import has clean/matched metadata — " + "preserving the file's existing tags (not wiping): %s", + os.path.basename(file_path)) logger.info(f"Moving '{os.path.basename(file_path)}' to '{final_path}'") safe_move_file(file_path, final_path) @@ -785,7 +792,13 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta except Exception as meta_err: import traceback pp_logger.info(f"[inner] Metadata enhancement FAILED for {context_key}: {meta_err}\n{traceback.format_exc()}") - wipe_source_tags(file_path) + if should_wipe_tags_on_enhancement_failure(has_clean_metadata): + wipe_source_tags(file_path) + else: + logger.warning( + "[Metadata] Enhancement failed but import has clean/matched metadata — " + "preserving the file's existing tags (not wiping): %s", + os.path.basename(file_path)) _enhance_source_info = context.get('track_info', {}).get('source_info') or {} if isinstance(_enhance_source_info, str): diff --git a/core/imports/tag_policy.py b/core/imports/tag_policy.py new file mode 100644 index 00000000..3983b416 --- /dev/null +++ b/core/imports/tag_policy.py @@ -0,0 +1,23 @@ +"""Tag-preservation policy for the import pipeline. + +Tiny, pure, and deliberately its own seam: it encodes one rule that, when it +was wrong, silently destroyed users' metadata (#804). Keeping it here with a +regression test stops anyone from re-introducing the unconditional wipe. +""" + +from __future__ import annotations + + +def should_wipe_tags_on_enhancement_failure(has_clean_metadata: bool) -> bool: + """Whether to strip the file's tags after metadata enhancement raised. + + Enhancement throwing means NO new tags were written, so wiping just + destroys whatever the file already had. For a clean/matched import that's + catastrophic — #804: already-tagged files (Bruno Mars, Coldplay) got + blanked into an "Unknown Artist" folder by a transient enhancement error. + + So: only wipe for UNMATCHED downloads (no clean/matched metadata), where + the tags are likely source junk anyway. NEVER wipe a clean/matched import — + preserve the user's existing tags. + """ + return not bool(has_clean_metadata) diff --git a/tests/test_import_tag_preservation.py b/tests/test_import_tag_preservation.py new file mode 100644 index 00000000..0ca0f308 --- /dev/null +++ b/tests/test_import_tag_preservation.py @@ -0,0 +1,22 @@ +"""Regression: a metadata-enhancement failure must NOT wipe a clean/matched +import's tags (#804 — already-tagged files were blanked into Unknown Artist). +""" + +from __future__ import annotations + +from core.imports.tag_policy import should_wipe_tags_on_enhancement_failure + + +def test_clean_matched_import_is_never_wiped_on_failure(): + # The #804 case: matched import (clean metadata) → preserve existing tags. + assert should_wipe_tags_on_enhancement_failure(has_clean_metadata=True) is False + + +def test_unmatched_download_still_strips_junk_on_failure(): + # Unchanged behavior for unmatched downloads (likely junk source tags). + assert should_wipe_tags_on_enhancement_failure(has_clean_metadata=False) is True + + +def test_falsey_values_treated_as_unmatched(): + assert should_wipe_tags_on_enhancement_failure(None) is True + assert should_wipe_tags_on_enhancement_failure(0) is True