From 9626bb1ab46424811d42d2c23617b5e8ceb346e9 Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 26 Aug 2026 18:36:14 +0200 Subject: [PATCH 1/8] fix(duplicates): preserve intentional lossy companions --- core/library/duplicate_cleaner.py | 39 +++++++- core/library/duplicate_rules.py | 78 ++++++++++++++++ core/repair_jobs/duplicate_detector.py | 58 ++---------- tests/test_deleted_quarantine_mover_wiring.py | 93 ++++++++++++++++++- 4 files changed, 211 insertions(+), 57 deletions(-) create mode 100644 core/library/duplicate_rules.py diff --git a/core/library/duplicate_cleaner.py b/core/library/duplicate_cleaner.py index 47e048822..579cf38ae 100644 --- a/core/library/duplicate_cleaner.py +++ b/core/library/duplicate_cleaner.py @@ -9,7 +9,9 @@ from core.settings import config_manager from core.runtime_state import add_activity_item +from core.library.duplicate_rules import is_lossy_companion_pair, lossy_companion_exts from core.repair_jobs.base import skip_deleted_quarantine +from database.music_database import get_database logger = logging.getLogger(__name__) @@ -136,6 +138,19 @@ def _run_duplicate_cleaner(): '.mp3': 4, '.wma': 4 # Lower quality lossy } + # The lossy-copy feature deliberately writes a same-folder, + # same-stem MP3/Opus/M4A next to its lossless source. The catalogue + # detector already protects that pair; the filesystem cleaner must + # apply the same shared rule instead of quarantining the intended copy. + try: + database = get_database() + except Exception as e: # noqa: BLE001 - filesystem scan stays usable + logger.debug("lossy companion database unavailable: %s", e) + database = None + companion_exts = lossy_companion_exts( + config_manager, database, logger=logger, + ) + duplicates_found = 0 deleted_count = 0 space_freed = 0 @@ -146,9 +161,6 @@ def _run_duplicate_cleaner(): if len(file_versions) <= 1: continue - duplicates_found += len(file_versions) - 1 # Count all but the one we keep - logger.warning(f"[Duplicate Cleaner] Found {len(file_versions)} versions of '{filename}' in {directory}") - # Sort by priority: best format first, then largest size def sort_key(f): priority = format_priority.get(f['extension'], 999) @@ -157,12 +169,29 @@ def sort_key(f): sorted_versions = sorted(file_versions, key=sort_key) - # Keep the first one (best quality), delete the rest + # Keep the first one (best quality). Configured lossy + # companions of that lossless winner are intentional; only + # the remaining versions are actionable duplicates. best_version = sorted_versions[0] + duplicate_versions = [ + version for version in sorted_versions[1:] + if not is_lossy_companion_pair( + best_version['full_path'], version['full_path'], + companion_exts, + ) + ] + if not duplicate_versions: + continue + + duplicates_found += len(duplicate_versions) + logger.warning( + f"[Duplicate Cleaner] Found {len(duplicate_versions) + 1} " + f"versions of '{filename}' in {directory}" + ) logger.warning(f"[Duplicate Cleaner] Keeping: {os.path.basename(best_version['full_path'])} " f"({best_version['extension']}, {best_version['size']} bytes)") - for duplicate_file in sorted_versions[1:]: + for duplicate_file in duplicate_versions: try: # Move to deleted folder with relative path preserved relative_path = os.path.relpath(duplicate_file['full_path'], transfer_folder) diff --git a/core/library/duplicate_rules.py b/core/library/duplicate_rules.py new file mode 100644 index 000000000..c7c019a4c --- /dev/null +++ b/core/library/duplicate_rules.py @@ -0,0 +1,78 @@ +"""Shared rules for distinguishing duplicates from intentional lossy copies. + +Both the catalogue duplicate detector and the filesystem duplicate cleaner +compare files with the same stem. Keeping this exception in one place avoids +one tool protecting a configured lossy companion while the other quarantines +that exact same file. +""" + +from __future__ import annotations + +import os +from typing import Any + +from core.quality.lossless import is_lossless_format +from core.quality.source_map import format_from_extension + + +LOSSY_CODEC_EXTS = {"mp3": ".mp3", "opus": ".opus", "aac": ".m4a"} + + +def lossy_companion_exts(config_manager: Any = None, database: Any = None, + logger: Any = None) -> set[str]: + """Extensions intentionally written by any enabled lossy-copy setting. + + The global setting and every enabled quality profile count. Failures are + deliberately non-fatal: an unreadable setting must not make duplicate + detection itself fail, and an empty result preserves the historical + behaviour of treating same-stem cross-format files as duplicates. + """ + exts: set[str] = set() + try: + if config_manager and config_manager.get("lossy_copy.enabled", False): + codec = str(config_manager.get("lossy_copy.codec", "mp3")).lower() + exts.add(LOSSY_CODEC_EXTS.get(codec, ".mp3")) + except Exception as exc: # noqa: BLE001 - optional protection setting + if logger: + logger.debug("lossy companion config read failed: %s", exc) + + if database is None: + return exts + try: + conn = database._get_connection() + try: + rows = conn.execute( + "SELECT lossy_copy_codec FROM quality_profiles " + "WHERE lossy_copy_enabled = 1" + ).fetchall() + for (codec,) in rows: + exts.add(LOSSY_CODEC_EXTS.get(str(codec or "mp3").lower(), ".mp3")) + finally: + conn.close() + except Exception as exc: # noqa: BLE001 - old/missing schema is valid + if logger: + logger.debug("lossy companion profile read failed: %s", exc) + return exts + + +def is_lossy_companion_pair(path1: Any, path2: Any, + companion_exts: set[str] | frozenset[str]) -> bool: + """Whether two paths are one lossless source and its configured copy.""" + if not companion_exts: + return False + p1 = str(path1 or "").replace("\\", "/") + p2 = str(path2 or "").replace("\\", "/") + d1, b1 = os.path.split(p1) + d2, b2 = os.path.split(p2) + if d1.lower() != d2.lower(): + return False + s1, e1 = os.path.splitext(b1) + s2, e2 = os.path.splitext(b2) + if s1.lower() != s2.lower(): + return False + lossless1 = is_lossless_format(format_from_extension(e1.lstrip(".").lower())) + lossless2 = is_lossless_format(format_from_extension(e2.lstrip(".").lower())) + if lossless1 == lossless2: + return False + lossy_ext = e2 if lossless1 else e1 + return lossy_ext.lower() in companion_exts diff --git a/core/repair_jobs/duplicate_detector.py b/core/repair_jobs/duplicate_detector.py index 4e9c1a86d..0fab2de0e 100644 --- a/core/repair_jobs/duplicate_detector.py +++ b/core/repair_jobs/duplicate_detector.py @@ -5,8 +5,10 @@ from difflib import SequenceMatcher from core.imports.file_ops import _strip_slskd_dedup_suffix -from core.quality.lossless import is_lossless_format -from core.quality.source_map import format_from_extension +from core.library.duplicate_rules import ( + is_lossy_companion_pair as _is_lossy_companion_pair, + lossy_companion_exts, +) from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger @@ -330,36 +332,14 @@ def _scan_bucket( if context.update_progress and processed_holder['count'] % 200 == 0: context.update_progress(processed_holder['count'], total) - _LOSSY_CODEC_EXTS = {'mp3': '.mp3', 'opus': '.opus', 'aac': '.m4a'} - def _lossy_companion_exts(self, context: JobContext) -> set: """Extensions the lossy-copy feature writes next to lossless sources — from the global toggle and any quality profile that has it on. Empty set when nobody uses the feature, so nothing is ever skipped for users who don't.""" - exts = set() - try: - cfg = context.config_manager - if cfg and cfg.get('lossy_copy.enabled', False): - codec = str(cfg.get('lossy_copy.codec', 'mp3')).lower() - exts.add(self._LOSSY_CODEC_EXTS.get(codec, '.mp3')) - except Exception as e: - logger.debug("lossy companion config read failed: %s", e) - try: - conn = context.db._get_connection() - try: - cursor = conn.cursor() - cursor.execute( - "SELECT lossy_copy_codec FROM quality_profiles" - " WHERE lossy_copy_enabled = 1") - for (codec,) in cursor.fetchall(): - exts.add(self._LOSSY_CODEC_EXTS.get( - str(codec or 'mp3').lower(), '.mp3')) - finally: - conn.close() - except Exception as e: - logger.debug("lossy companion profile read failed: %s", e) - return exts + return lossy_companion_exts( + context.config_manager, context.db, logger=logger, + ) def _build_filename_buckets(self, *, buckets, found_groups): """Re-bucket all tracks by canonical filename stem. @@ -420,30 +400,6 @@ def _normalize(text: str) -> str: return ''.join(c for c in t if c.isalnum() or c in '() ').strip() -def _is_lossy_companion_pair(path1, path2, companion_exts) -> bool: - """True when the pair is a lossless file plus its intentional lossy copy: - same folder, same stem, one lossless / one lossy, and the lossy side's - extension is one the lossy-copy feature actually writes.""" - if not companion_exts: - return False - p1 = str(path1 or '').replace('\\', '/') - p2 = str(path2 or '').replace('\\', '/') - d1, b1 = os.path.split(p1) - d2, b2 = os.path.split(p2) - if d1.lower() != d2.lower(): - return False - s1, e1 = os.path.splitext(b1) - s2, e2 = os.path.splitext(b2) - if s1.lower() != s2.lower(): - return False - lossless1 = is_lossless_format(format_from_extension(e1.lstrip('.').lower())) - lossless2 = is_lossless_format(format_from_extension(e2.lstrip('.').lower())) - if lossless1 == lossless2: - return False - lossy_ext = e2 if lossless1 else e1 - return lossy_ext.lower() in companion_exts - - def _is_same_physical_file(p1, p2, dur1, dur2) -> bool: """Detect when two DB rows point at the same file mounted at different paths. diff --git a/tests/test_deleted_quarantine_mover_wiring.py b/tests/test_deleted_quarantine_mover_wiring.py index 781c18512..21fb2561b 100644 --- a/tests/test_deleted_quarantine_mover_wiring.py +++ b/tests/test_deleted_quarantine_mover_wiring.py @@ -2,6 +2,7 @@ duplicate cleaner worker against a tmp transfer folder.""" import os +import sqlite3 import pytest @@ -10,12 +11,18 @@ class _FakeConfig: - def __init__(self, transfer): + def __init__(self, transfer, *, lossy_enabled=False, lossy_codec='mp3'): self.transfer = transfer + self.lossy_enabled = lossy_enabled + self.lossy_codec = lossy_codec def get(self, key, default=None): if key == 'soulseek.transfer_path': return self.transfer + if key == 'lossy_copy.enabled': + return self.lossy_enabled + if key == 'lossy_copy.codec': + return self.lossy_codec return default @@ -25,6 +32,7 @@ def cleaner(tmp_path, monkeypatch): import threading dc.init(state, threading.Lock(), lambda p: p, None) monkeypatch.setattr(dc, 'config_manager', _FakeConfig(str(tmp_path))) + monkeypatch.setattr(dc, 'get_database', lambda: None) monkeypatch.setattr(dc, 'add_activity_item', lambda *a, **k: None) return state, str(tmp_path) @@ -53,3 +61,86 @@ def test_the_duplicate_cleaner_records_what_it_quarantines(cleaner): assert entry['source'] == 'duplicate-cleaner' assert entry['deleted_at'] is not None assert entry['original_path'] == os.path.join(transfer, 'Artist', 'Album', 'song.mp3') + + +def test_the_duplicate_cleaner_keeps_an_intentional_lossy_copy(cleaner, monkeypatch): + state, transfer = cleaner + monkeypatch.setattr( + dc, 'config_manager', + _FakeConfig(transfer, lossy_enabled=True, lossy_codec='mp3'), + ) + album = os.path.join(transfer, 'Artist', 'Album') + os.makedirs(album) + flac = os.path.join(album, 'song.flac') + mp3 = os.path.join(album, 'song.mp3') + with open(flac, 'wb') as handle: + handle.write(b'flac' * 100) + with open(mp3, 'wb') as handle: + handle.write(b'mp3') + + dc._run_duplicate_cleaner() + + assert os.path.isfile(flac) + assert os.path.isfile(mp3) + assert state['duplicates_found'] == 0 + assert state['deleted'] == 0 + assert list_entries(transfer)['count'] == 0 + + +def test_a_quality_profile_also_protects_its_lossy_copy(cleaner, monkeypatch, tmp_path): + state, transfer = cleaner + monkeypatch.setattr(dc, 'config_manager', _FakeConfig(transfer)) + db_path = tmp_path / 'profiles.db' + conn = sqlite3.connect(db_path) + conn.execute( + "CREATE TABLE quality_profiles (" + "lossy_copy_enabled INTEGER, lossy_copy_codec TEXT)" + ) + conn.execute("INSERT INTO quality_profiles VALUES (1, 'opus')") + conn.commit() + conn.close() + + class _Db: + def _get_connection(self): + return sqlite3.connect(db_path) + + monkeypatch.setattr(dc, 'get_database', _Db) + album = os.path.join(transfer, 'Artist', 'Album') + os.makedirs(album) + flac = os.path.join(album, 'song.flac') + opus = os.path.join(album, 'song.opus') + with open(flac, 'wb') as handle: + handle.write(b'flac' * 100) + with open(opus, 'wb') as handle: + handle.write(b'opus') + + dc._run_duplicate_cleaner() + + assert os.path.isfile(flac) + assert os.path.isfile(opus) + assert state['duplicates_found'] == 0 + assert state['deleted'] == 0 + + +def test_the_duplicate_cleaner_still_removes_a_real_cross_format_duplicate( + cleaner, monkeypatch): + state, transfer = cleaner + monkeypatch.setattr( + dc, 'config_manager', + _FakeConfig(transfer, lossy_enabled=True, lossy_codec='mp3'), + ) + album = os.path.join(transfer, 'Artist', 'Album') + os.makedirs(album) + flac = os.path.join(album, 'song.flac') + ogg = os.path.join(album, 'song.ogg') + with open(flac, 'wb') as handle: + handle.write(b'flac' * 100) + with open(ogg, 'wb') as handle: + handle.write(b'ogg') + + dc._run_duplicate_cleaner() + + assert os.path.isfile(flac) + assert not os.path.isfile(ogg) + assert state['duplicates_found'] == 1 + assert state['deleted'] == 1 From b85a12460de91ea24fee6a2af3c17bfa3f75a3d5 Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 26 Aug 2026 19:47:46 +0200 Subject: [PATCH 2/8] fix(quality): separate acquisition from retained output --- core/imports/pipeline.py | 72 ++++++-- core/imports/side_effects.py | 71 ++++++-- core/library/duplicate_rules.py | 31 ++++ core/quality/model.py | 29 +++ core/quality/retention.py | 114 ++++++++++++ core/repair_jobs/lossy_converter.py | 77 +++++--- core/repair_jobs/quality_upgrade.py | 26 ++- core/repair_jobs/quality_upgrade_scanner.py | 35 +++- core/repair_worker.py | 50 +++-- database/music_database.py | 9 +- tests/quality/test_retention_provenance.py | 172 ++++++++++++++++++ .../quality/test_wishlist_quality_columns.py | 11 ++ .../repair_jobs/test_lossy_converter_scan.py | 32 +++- tests/repair_jobs/test_quality_upgrade.py | 42 +++++ tests/test_duplicate_detector_cross_format.py | 11 ++ 15 files changed, 708 insertions(+), 74 deletions(-) create mode 100644 core/quality/retention.py create mode 100644 tests/quality/test_retention_provenance.py diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index 17aa8feb5..df3699f84 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -17,6 +17,7 @@ downsample_hires_flac, get_audio_quality_string, get_quality_tier_from_extension, + probe_audio_quality, safe_move_file, ) from core.imports.context import ( @@ -432,6 +433,58 @@ def _persist_verification_status(context, final_path): logger.debug(f"verification-status persist skipped: {_vs_err}") +def _apply_profile_output_transforms(final_path: str, context: dict, + profile: dict) -> str: + """Apply downsample/lossy retention while preserving acquisition truth.""" + acquired_quality = probe_audio_quality(final_path) + if acquired_quality is not None: + context['_acquired_audio_quality'] = acquired_quality.to_dict() + + downsampled_path = downsample_hires_flac( + final_path, context, enabled=profile.get('downsample_enabled')) + if downsampled_path: + final_path = downsampled_path + context['_final_processed_path'] = final_path + retained_quality = probe_audio_quality(final_path) + context.setdefault('_retention_transforms', []).append({ + 'type': 'downsample_hires_flac', + 'source_replaced': True, + 'target_bit_depth': 16, + 'target_sample_rate': 44100, + 'output_quality': retained_quality.to_dict() if retained_quality else None, + }) + + _persist_verification_status(context, final_path) + + lossy_path = create_lossy_copy(final_path, settings={ + 'enabled': profile.get('lossy_copy_enabled'), + 'codec': profile.get('lossy_copy_codec'), + 'bitrate': profile.get('lossy_copy_bitrate'), + 'delete_original': profile.get('lossy_copy_delete_original'), + } if profile else None) + if not lossy_path: + return final_path + + source_retained = os.path.isfile(final_path) + lossy_quality = probe_audio_quality(lossy_path) + context.setdefault('_retention_transforms', []).append({ + 'type': 'lossy_copy', + 'source_replaced': not source_retained, + 'codec': profile.get('lossy_copy_codec'), + 'bitrate': profile.get('lossy_copy_bitrate'), + 'output_quality': lossy_quality.to_dict() if lossy_quality else None, + }) + if source_retained: + companions = context.setdefault('_companion_file_paths', []) + if lossy_path not in companions: + companions.append(lossy_path) + context['_final_processed_path'] = final_path + return final_path + + context['_final_processed_path'] = lossy_path + return lossy_path + + def post_process_matched_download(context_key, context, file_path, runtime, metadata_runtime=None): on_download_completed = getattr(runtime, "on_download_completed", None) automation_engine = getattr(runtime, "automation_engine", None) @@ -1298,23 +1351,8 @@ def _notify_download_completed(batch_id, task_id, success=True): pp_logger.debug(f"ReplayGain analysis skipped: {rg_err}") _qp_post = _resolve_context_quality_profile(context) - downsampled_path = downsample_hires_flac( - final_path, context, - enabled=_qp_post.get('downsample_enabled')) - if downsampled_path: - final_path = downsampled_path - context['_final_processed_path'] = final_path - - _persist_verification_status(context, final_path) - - blasphemy_path = create_lossy_copy(final_path, settings={ - 'enabled': _qp_post.get('lossy_copy_enabled'), - 'codec': _qp_post.get('lossy_copy_codec'), - 'bitrate': _qp_post.get('lossy_copy_bitrate'), - 'delete_original': _qp_post.get('lossy_copy_delete_original'), - } if _qp_post else None) - if blasphemy_path: - context['_final_processed_path'] = blasphemy_path + final_path = _apply_profile_output_transforms( + final_path, context, _qp_post) downloads_path = docker_resolve_path(config_manager.get('soulseek.download_path', './downloads')) cleanup_empty_directories(downloads_path, file_path) diff --git a/core/imports/side_effects.py b/core/imports/side_effects.py index 8cff00a4b..976e99824 100644 --- a/core/imports/side_effects.py +++ b/core/imports/side_effects.py @@ -667,18 +667,31 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[ # Quality Upgrade Finder passes re-resolve the track against the # default profile instead of the one it was actually imported under. track_quality_profile_id = track_info.get("quality_profile_id") + from core.quality.retention import quality_json, transforms_json + from core.quality.model import AudioQuality + + acquired_value = context.get("_acquired_audio_quality") + try: + acquired_quality = AudioQuality.from_dict(acquired_value) if acquired_value else None + except (TypeError, ValueError): + acquired_quality = None + acquired_quality_json = quality_json(acquired_quality) + retention_json = transforms_json(context.get("_retention_transforms")) + try: + track_columns = { + column[1] for column in cursor.execute( + "PRAGMA table_info(tracks)").fetchall() + } + except Exception: # pragma is best-effort for non-SQLite test doubles + track_columns = set() + has_retention_columns = { + "acquired_quality_json", "retention_json" + }.issubset(track_columns) cursor.execute("SELECT id FROM tracks WHERE file_path = ?", (final_path,)) - if not cursor.fetchone(): - cursor.execute( - """ - INSERT INTO tracks (id, album_id, artist_id, title, track_number, - duration, file_path, bitrate, file_size, track_artist, - musicbrainz_recording_id, isrc, quality_profile_id, server_source, - created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) - """, - ( + existing_track = cursor.fetchone() + if not existing_track: + base_values = ( track_id, album_id, artist_id, @@ -692,8 +705,32 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[ track_mbid, track_isrc, track_quality_profile_id, - ), ) + if has_retention_columns: + cursor.execute( + """ + INSERT INTO tracks (id, album_id, artist_id, title, track_number, + duration, file_path, bitrate, file_size, track_artist, + musicbrainz_recording_id, isrc, quality_profile_id, + acquired_quality_json, retention_json, server_source, + created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, + 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + """, + base_values + (acquired_quality_json, retention_json), + ) + else: + cursor.execute( + """ + INSERT INTO tracks (id, album_id, artist_id, title, track_number, + duration, file_path, bitrate, file_size, track_artist, + musicbrainz_recording_id, isrc, quality_profile_id, server_source, + created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, + 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + """, + base_values, + ) track_source_col = source_columns.get("track") if track_source_col and track_source_id: try: @@ -709,6 +746,18 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[ ) except Exception as e: logger.debug("track source-id update failed: %s", e) + elif has_retention_columns: + # A repeated/import-resume write refreshes transformation + # provenance for the existing physical row. Never retain an + # old destructive-policy claim after a clean untransformed + # import of that same path. + cursor.execute( + """UPDATE tracks + SET acquired_quality_json=?, retention_json=?, + updated_at=CURRENT_TIMESTAMP + WHERE id=?""", + (acquired_quality_json, retention_json, existing_track[0]), + ) conn.commit() logger.info("[SoulSync Library] Added: %s / %s / %s", artist_name, album_name, track_name) diff --git a/core/library/duplicate_rules.py b/core/library/duplicate_rules.py index c7c019a4c..2c9c363ef 100644 --- a/core/library/duplicate_rules.py +++ b/core/library/duplicate_rules.py @@ -76,3 +76,34 @@ def is_lossy_companion_pair(path1: Any, path2: Any, return False lossy_ext = e2 if lossless1 else e1 return lossy_ext.lower() in companion_exts + + +def is_lossy_companion_file(path: Any, + companion_exts: set[str] | frozenset[str]) -> bool: + """Whether ``path`` is a configured lossy output beside a lossless source. + + This is the legacy-filesystem fallback used by scanners that walk files + with no catalogue row. Library v2 can replace it with explicit file-role + provenance, but Dev must infer the relationship without deleting or + flagging the user's intentional copy. + """ + candidate = os.path.abspath(str(path or "")) + stem, ext = os.path.splitext(os.path.basename(candidate)) + if ext.lower() not in companion_exts: + return False + folder = os.path.dirname(candidate) + try: + siblings = os.listdir(folder) + except OSError: + return False + for sibling_name in siblings: + sibling_stem, sibling_ext = os.path.splitext(sibling_name) + if sibling_stem.lower() != stem.lower() or sibling_ext.lower() == ext.lower(): + continue + sibling_path = os.path.join(folder, sibling_name) + if not os.path.isfile(sibling_path): + continue + sibling_format = format_from_extension(sibling_ext.lstrip(".").lower()) + if is_lossless_format(sibling_format): + return True + return False diff --git a/core/quality/model.py b/core/quality/model.py index 0cd466147..57af06c1d 100644 --- a/core/quality/model.py +++ b/core/quality/model.py @@ -116,6 +116,29 @@ def label(self) -> str: br = f" {self.bitrate}kbps" if self.bitrate else "" return f"{fmt}{br}" + def to_dict(self) -> dict: + """JSON-safe representation used by acquisition/retention provenance.""" + return { + key: value for key, value in { + "format": self.format, + "bitrate": self.bitrate, + "sample_rate": self.sample_rate, + "bit_depth": self.bit_depth, + }.items() if value is not None + } + + @classmethod + def from_dict(cls, value: dict) -> 'AudioQuality': + """Rebuild a quality descriptor from persisted provenance.""" + if not isinstance(value, dict) or not value.get("format"): + raise ValueError("audio quality needs a format") + return cls( + format=str(value["format"]), + bitrate=_optional_int(value.get("bitrate")), + sample_rate=_optional_int(value.get("sample_rate")), + bit_depth=_optional_int(value.get("bit_depth")), + ) + @classmethod def from_slskd_file(cls, file_data: dict, extension: str) -> 'AudioQuality': """Build from a raw slskd API file entry. @@ -142,6 +165,12 @@ def from_extension_and_bitrate(cls, extension: str, bitrate: Optional[int]) -> ' return cls(format=extension.lower().lstrip('.'), bitrate=bitrate) +def _optional_int(value) -> Optional[int]: + if value in (None, ""): + return None + return int(value) + + @dataclass class QualityTarget: """One ranked entry in the user's quality priority list.""" diff --git a/core/quality/retention.py b/core/quality/retention.py new file mode 100644 index 000000000..5d7461c2f --- /dev/null +++ b/core/quality/retention.py @@ -0,0 +1,114 @@ +"""Acquisition quality versus intentional retained-output transformations. + +A download can satisfy a quality profile and then deliberately be transformed: +hi-res FLAC may be downsampled to 16/44.1, or a lossless source may be replaced +by a lossy library copy. Upgrade decisions must remember the quality SoulSync +actually acquired or they will propose the same download forever. + +The persisted provenance is deliberately small and source agnostic. When it is +missing or malformed, callers fall back to the measured file quality so older +libraries never receive an unearned quality claim. +""" + +from __future__ import annotations + +import json +from typing import Any, Iterable, Optional + +from core.quality.model import AudioQuality, QualityTarget, rank_candidate +from core.quality.selection import quality_meets_profile + + +ACQUIRED_QUALITY_CONTEXT_KEY = "_acquired_audio_quality" +RETENTION_CONTEXT_KEY = "_retention_transforms" + + +def quality_json(quality: Optional[AudioQuality]) -> Optional[str]: + """Serialize an acquired quality, returning ``None`` when unavailable.""" + if quality is None: + return None + return json.dumps(quality.to_dict(), sort_keys=True, separators=(",", ":")) + + +def transforms_json(transforms: Any) -> Optional[str]: + """Serialize applied transform records; empty/non-list values stay NULL.""" + if not isinstance(transforms, list) or not transforms: + return None + return json.dumps(transforms, sort_keys=True, separators=(",", ":")) + + +def acquired_quality_from_json(value: Any) -> Optional[AudioQuality]: + """Parse trusted acquisition provenance, failing closed on old/bad data.""" + if not value: + return None + try: + data = json.loads(value) if isinstance(value, str) else value + return AudioQuality.from_dict(data) + except (TypeError, ValueError, json.JSONDecodeError): + return None + + +def has_destructive_retention(value: Any) -> bool: + """Whether provenance says the acquired representation was replaced.""" + if not value: + return False + try: + steps = json.loads(value) if isinstance(value, str) else value + except (TypeError, ValueError, json.JSONDecodeError): + return False + return isinstance(steps, list) and any( + isinstance(step, dict) and bool(step.get("source_replaced")) + for step in steps + ) + + +def evaluation_qualities( + measured: Optional[AudioQuality], + acquired_quality_json: Any = None, + retention_json: Any = None, +) -> list[AudioQuality]: + """Qualities that can honestly satisfy an upgrade policy. + + Measured retained quality always participates. Acquired quality only joins + it when explicit provenance proves a destructive, intentional transform. + This prevents a stale/random JSON value from suppressing real upgrades. + """ + values = [measured] if measured is not None else [] + if has_destructive_retention(retention_json): + acquired = acquired_quality_from_json(acquired_quality_json) + if acquired is not None and acquired.to_dict() not in [v.to_dict() for v in values]: + values.append(acquired) + return values + + +def best_quality_for_targets( + measured: Optional[AudioQuality], + targets: Iterable[QualityTarget], + *, + acquired_quality_json: Any = None, + retention_json: Any = None, +) -> Optional[AudioQuality]: + """Best honest quality representation for a ranked profile.""" + target_list = list(targets) + values = evaluation_qualities(measured, acquired_quality_json, retention_json) + if not values: + return None + return min(values, key=lambda quality: rank_candidate(quality, target_list)) + + +def retention_meets_profile( + measured: Optional[AudioQuality], + targets: Iterable[QualityTarget], + *, + cutoff_index: Optional[int] = None, + acquired_quality_json: Any = None, + retention_json: Any = None, +) -> bool: + """Apply an upgrade cutoff to measured + intentionally acquired quality.""" + target_list = list(targets) + values = evaluation_qualities(measured, acquired_quality_json, retention_json) + if not values: + return False + if cutoff_index is not None: + return any(rank_candidate(value, target_list)[0] <= cutoff_index for value in values) + return any(quality_meets_profile(value, target_list) for value in values) diff --git a/core/repair_jobs/lossy_converter.py b/core/repair_jobs/lossy_converter.py index 2d5b244e8..81e438c4e 100644 --- a/core/repair_jobs/lossy_converter.py +++ b/core/repair_jobs/lossy_converter.py @@ -16,6 +16,7 @@ ) from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob +from core.quality.selection import load_profile_by_id from utils.logging_config import get_logger logger = get_logger("repair_job.lossy_converter") @@ -27,6 +28,36 @@ } +def _profile_lossy_settings(context: JobContext, profile_id=None) -> dict: + """Resolve the live lossy policy for one track, with legacy fallback.""" + profile = None + try: + if profile_id: + profile = load_profile_by_id(profile_id) + elif context.db and hasattr(context.db, 'get_quality_profile'): + profile = context.db.get_quality_profile() + except Exception as exc: # noqa: BLE001 - legacy DB/config remains usable + logger.debug("Could not resolve quality profile %r: %s", profile_id, exc) + if isinstance(profile, dict) and 'lossy_copy_enabled' in profile: + return { + 'profile_id': profile.get('id'), + 'profile_name': profile.get('name') or profile.get('preset') or 'default', + 'enabled': bool(profile.get('lossy_copy_enabled')), + 'codec': str(profile.get('lossy_copy_codec') or 'mp3').lower(), + 'bitrate': str(profile.get('lossy_copy_bitrate') or '320'), + 'delete_original': bool(profile.get('lossy_copy_delete_original')), + } + cfg = context.config_manager + return { + 'profile_id': profile_id, + 'profile_name': 'legacy settings', + 'enabled': bool(cfg and cfg.get('lossy_copy.enabled', False)), + 'codec': str(cfg.get('lossy_copy.codec', 'mp3') if cfg else 'mp3').lower(), + 'bitrate': str(cfg.get('lossy_copy.bitrate', '320') if cfg else '320'), + 'delete_original': bool(cfg and cfg.get('lossy_copy.delete_original', False)), + } + + def _lossless_ext_where(col: str) -> str: """SQL pre-filter matching files whose extension *might* be lossless. The final decision (including ALAC-in-.m4a, which needs a codec probe) is made @@ -55,9 +86,9 @@ class LossyConverterJob(RepairJob): help_text = ( 'Scans your library for lossless files (FLAC/ALAC/WAV/AIFF/DSD) that don\'t already have a lossy copy ' '(MP3, Opus, or AAC) alongside them.\n\n' - 'Uses the codec setting from your Lossy Copy configuration on the Settings ' - 'page. Enable Lossy Copy in Settings first, then run this job to find FLAC ' - 'files missing a lossy copy.\n\n' + 'Uses each track\'s assigned Quality Profile, including its codec, bitrate, ' + 'and whether the lossless source should be retained. Enable Lossy Copy on ' + 'the relevant profile first.\n\n' 'Each finding can be fixed individually or in bulk — the fix action converts ' 'the lossless file using ffmpeg at your configured bitrate.\n\n' 'Requires ffmpeg to be installed.' @@ -65,9 +96,7 @@ class LossyConverterJob(RepairJob): icon = 'repair-icon-lossy' default_enabled = False default_interval_hours = 0 # Manual only - default_settings = { - 'delete_original': False, # Blasphemy Mode — delete FLAC after conversion - } + default_settings = {} auto_fix = False def scan(self, context: JobContext) -> JobResult: @@ -77,20 +106,6 @@ def scan(self, context: JobContext) -> JobResult: logger.warning("Config manager not available") return result - if not context.config_manager.get('lossy_copy.enabled', False): - if context.report_progress: - context.report_progress( - phase='Skipped — Lossy Copy not enabled in Settings', - log_line='Enable Lossy Copy in Settings before running this job', - log_type='warning' - ) - return result - - codec = context.config_manager.get('lossy_copy.codec', 'mp3').lower() - bitrate = context.config_manager.get('lossy_copy.bitrate', '320') - out_ext = CODEC_MAP.get(codec, '.mp3') - quality_label = f'{codec.upper()}-{bitrate}' - # Get all FLAC tracks from DB tracks = [] conn = None @@ -99,7 +114,8 @@ def scan(self, context: JobContext) -> JobResult: cursor = conn.cursor() cursor.execute(f""" SELECT t.id, t.title, ar.name, al.title, t.file_path, - al.thumb_url, ar.thumb_url, ar.id + al.thumb_url, ar.thumb_url, ar.id, + t.quality_profile_id FROM tracks t LEFT JOIN artists ar ON ar.id = t.artist_id LEFT JOIN albums al ON al.id = t.album_id @@ -120,7 +136,7 @@ def scan(self, context: JobContext) -> JobResult: context.update_progress(0, total) if context.report_progress: context.report_progress( - phase=f'Scanning {total} lossless files for missing {quality_label} copies...', + phase=f'Scanning {total} lossless files for profile-defined lossy copies...', total=total ) @@ -140,9 +156,19 @@ def scan(self, context: JobContext) -> JobResult: if i % 200 == 0 and context.wait_if_paused(): return result - track_id, title, artist_name, album_title, file_path, album_thumb, artist_thumb, artist_id = row + track_id, title, artist_name, album_title, file_path, album_thumb, artist_thumb, artist_id = row[:8] + profile_id = row[8] if len(row) > 8 else None result.scanned += 1 + policy = _profile_lossy_settings(context, profile_id) + if not policy['enabled']: + result.skipped += 1 + continue + codec = policy['codec'] + bitrate = policy['bitrate'] + out_ext = CODEC_MAP.get(codec, '.mp3') + quality_label = f'{codec.upper()}-{bitrate}' + if context.report_progress and i % 50 == 0: context.report_progress( scanned=i + 1, total=total, @@ -209,6 +235,9 @@ def scan(self, context: JobContext) -> JobResult: 'album_thumb_url': album_thumb or None, 'artist_thumb_url': artist_thumb or None, 'artist_id': artist_id, + 'quality_profile_id': policy.get('profile_id'), + 'quality_profile_name': policy.get('profile_name'), + 'delete_original': policy.get('delete_original', False), } ) if inserted: @@ -226,7 +255,7 @@ def scan(self, context: JobContext) -> JobResult: context.update_progress(total, total) if context.report_progress: - summary = f'Found {result.findings_created} lossless files without {quality_label} copies' + summary = f'Found {result.findings_created} lossless files without their profile-defined lossy copy' if skipped_missing: summary += f'; {skipped_missing} tracks could not be located on disk (skipped)' context.report_progress( diff --git a/core/repair_jobs/quality_upgrade.py b/core/repair_jobs/quality_upgrade.py index 4a7c89029..5b4a1e4b7 100644 --- a/core/repair_jobs/quality_upgrade.py +++ b/core/repair_jobs/quality_upgrade.py @@ -46,6 +46,7 @@ from core.imports.file_ops import probe_audio_quality from core.quality.model import rank_candidate from core.quality.selection import targets_from_profile, quality_meets_profile, load_profile_by_id +from core.quality.retention import acquired_quality_from_json, evaluation_qualities from utils.logging_config import get_logger logger = get_logger("repair_jobs.quality_upgrade") @@ -253,6 +254,7 @@ def _match_via_isrc(isrc: str, source_priority: List[str]) -> Tuple[Optional[Any 'id', 'title', 'file_path', 'bitrate', 'duration', 'artist_name', 'album_title', 'album_id', 'track_number', 'spotify_album_id', 'itunes_album_id', 'deezer_id', 'musicbrainz_release_id', 'audiodb_id', 'quality_profile_id', + 'acquired_quality_json', 'retention_json', ) # Human-readable note per match tier (search uses a confidence % instead). @@ -474,7 +476,8 @@ def _load_tracks(self, db: Any, scope: str) -> List[dict]: "SELECT t.id, t.title, t.file_path, t.bitrate, t.duration, " "a.name AS artist_name, al.title AS album_title, t.album_id, t.track_number, " "al.spotify_album_id, al.itunes_album_id, al.deezer_id, " - "al.musicbrainz_release_id, al.audiodb_id, t.quality_profile_id " + "al.musicbrainz_release_id, al.audiodb_id, t.quality_profile_id, " + "t.acquired_quality_json, t.retention_json " "FROM tracks t " "JOIN artists a ON t.artist_id = a.id " "JOIN albums al ON t.album_id = al.id " @@ -725,17 +728,27 @@ def _bundle_for_track(row_profile_id) -> Dict[str, Any]: context.update_progress(i + 1, total) continue - if not broken_reason and measured_aq is not None: + evaluation_values = evaluation_qualities( + measured_aq, + row.get('acquired_quality_json'), + row.get('retention_json'), + ) + if not broken_reason and evaluation_values: if cutoff_index is not None: # ranking-based: skip only if the file already sits at the # configured cutoff rank or better. Any lower rank triggers # a proposed upgrade. - idx, _ = rank_candidate(measured_aq, targets) - already_best = idx <= cutoff_index + already_best = any( + rank_candidate(value, targets)[0] <= cutoff_index + for value in evaluation_values + ) else: # default: skip if the file meets ANY configured target (i.e. # it's not below the acceptable floor). - already_best = quality_meets_profile(measured_aq, targets) + already_best = any( + quality_meets_profile(value, targets) + for value in evaluation_values + ) if already_best: result.skipped += 1 if context.update_progress and (i + 1) % 25 == 0: @@ -756,6 +769,8 @@ def _bundle_for_track(row_profile_id) -> Dict[str, Any]: return result current_label = measured_aq.label() if measured_aq is not None else 'broken/unreadable' + acquired_aq = acquired_quality_from_json(row.get('acquired_quality_json')) + acquired_label = acquired_aq.label() if acquired_aq is not None else None if broken_reason: current_label = f'{current_label} (broken: {broken_reason})' if measured_aq is not None else f'broken ({broken_reason})' if context.report_progress: @@ -864,6 +879,7 @@ def _bundle_for_track(row_profile_id) -> Dict[str, Any]: 'album_title': album_title, 'current_format': current_label, 'current_bitrate': bitrate, + 'acquired_quality': acquired_label, 'quality_profile_id': quality_profile_id, 'quality_profile_name': quality_profile_name, 'profile_config_fingerprint': config_fingerprint, diff --git a/core/repair_jobs/quality_upgrade_scanner.py b/core/repair_jobs/quality_upgrade_scanner.py index 00e70ffb5..d2ebc6380 100644 --- a/core/repair_jobs/quality_upgrade_scanner.py +++ b/core/repair_jobs/quality_upgrade_scanner.py @@ -28,6 +28,7 @@ # monkeypatch them the same way tests/repair_jobs/test_quality_upgrade.py does. from core.quality.model import rank_candidate from core.quality.selection import targets_from_profile, quality_meets_profile, load_profile_by_id +from core.quality.retention import acquired_quality_from_json, evaluation_qualities from utils.logging_config import get_logger logger = get_logger("repair_job.quality_upgrade") @@ -294,6 +295,13 @@ def _bundle_for(row_profile_id): deep_verify = settings.get('deep_audio_verify', False) probe_failed = 0 not_in_library = 0 + from core.library.duplicate_rules import ( + is_lossy_companion_file, + lossy_companion_exts, + ) + companion_exts = lossy_companion_exts( + context.config_manager, context.db, logger=logger, + ) for i, fpath in enumerate(audio_files): if context.check_stop(): return result @@ -306,6 +314,12 @@ def _bundle_for(row_profile_id): # the library, skip anything with no DB row BEFORE probing — no point # reading hundreds of orphan files. meta = self._match_db(fpath, db_index) + if meta is None and is_lossy_companion_file(fpath, companion_exts): + # A deliberately retained lossy derivative belongs to the + # lossless track beside it. It is not an orphan quality choice + # and must not generate a second, misleading upgrade finding. + result.skipped += 1 + continue if library_only and meta is None: not_in_library += 1 result.skipped += 1 @@ -364,6 +378,11 @@ def _bundle_for(row_profile_id): logger.debug("Probe failed for %s: %s", fname, e) aq = None + evaluation_values = evaluation_qualities( + aq, + meta.get('acquired_quality_json'), + meta.get('retention_json'), + ) if broken_reason: issue = 'broken_audio' current_label = aq.label() if aq is not None else 'unknown' @@ -372,10 +391,14 @@ def _bundle_for(row_profile_id): probe_failed += 1 result.skipped += 1 continue - elif cutoff_index is not None and rank_candidate(aq, targets)[0] > cutoff_index: + elif cutoff_index is not None and not any( + rank_candidate(value, targets)[0] <= cutoff_index + for value in evaluation_values): issue = 'below_profile' current_label = aq.label() - elif cutoff_index is None and not quality_meets_profile(aq, targets): + elif cutoff_index is None and not any( + quality_meets_profile(value, targets) + for value in evaluation_values): issue = 'below_profile' current_label = aq.label() else: @@ -413,6 +436,8 @@ def _bundle_for(row_profile_id): # the new one (same job_id + entity/file_path, any status). self._clear_stale_dismissed_finding(context.db, track_id, fpath) if context.create_finding: + acquired_aq = acquired_quality_from_json( + meta.get('acquired_quality_json')) inserted = context.create_finding( job_id=self.job_id, finding_type='quality_upgrade', @@ -430,6 +455,7 @@ def _bundle_for(row_profile_id): 'current_bitrate': aq.bitrate if aq is not None else None, 'current_sample_rate': aq.sample_rate if aq is not None else None, 'current_bit_depth': aq.bit_depth if aq is not None else None, + 'acquired_quality': acquired_aq.label() if acquired_aq else None, 'target_qualities': target_labels, 'expected_title': disp_title, 'expected_artist': disp_artist, @@ -529,7 +555,8 @@ def _build_db_suffix_index(self, context: JobContext) -> dict: COALESCE(NULLIF(t.track_artist, ''), ar.name) AS artist, t.file_path, t.track_number, al.title AS album_title, al.thumb_url, ar.thumb_url, - t.quality_profile_id, ar.id + t.quality_profile_id, ar.id, + t.acquired_quality_json, t.retention_json FROM tracks t LEFT JOIN artists ar ON ar.id = t.artist_id LEFT JOIN albums al ON al.id = t.album_id @@ -550,6 +577,8 @@ def _build_db_suffix_index(self, context: JobContext) -> dict: 'artist_thumb_url': row[7] or None, 'quality_profile_id': row[8], 'artist_id': row[9], + 'acquired_quality_json': row[10], + 'retention_json': row[11], } for depth in range(1, min(4, len(parts) + 1)): suffix = '/'.join(parts[-depth:]).lower() diff --git a/core/repair_worker.py b/core/repair_worker.py index 3cc728721..0a9b008dd 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -4932,12 +4932,30 @@ def _fix_missing_lossy_copy(self, entity_type, entity_id, file_path, details): if not file_path: return {'success': False, 'error': 'No file path associated with this finding'} - # Read fresh from current settings — not from finding details + # Read the track's assigned quality profile LIVE. Finding details only + # carry its id; codec/bitrate/delete-original may have changed since + # the scan. Fall back to legacy globals for old findings/installations. codec = 'mp3' bitrate = '320' - if self._config_manager: + delete_original = False + profile = None + profile_id = details.get('quality_profile_id') if isinstance(details, dict) else None + try: + from core.quality.selection import load_profile_by_id + profile = load_profile_by_id(profile_id) if profile_id else None + except Exception as e: + logger.debug("Could not resolve lossy-converter profile %r: %s", profile_id, e) + if isinstance(profile, dict) and 'lossy_copy_enabled' in profile: + if not profile.get('lossy_copy_enabled'): + return {'success': False, 'error': 'Lossy Copy is disabled for this track profile'} + codec = str(profile.get('lossy_copy_codec') or 'mp3').lower() + bitrate = str(profile.get('lossy_copy_bitrate') or '320') + delete_original = bool(profile.get('lossy_copy_delete_original')) + elif self._config_manager: codec = self._config_manager.get('lossy_copy.codec', 'mp3').lower() bitrate = self._config_manager.get('lossy_copy.bitrate', '320') + delete_original = bool( + self._config_manager.get('lossy_copy.delete_original', False)) # Opus max per-channel bitrate is 256kbps — cap to avoid encoding failures if codec == 'opus' and int(bitrate) > 256: bitrate = '256' @@ -4973,6 +4991,9 @@ def _fix_missing_lossy_copy(self, entity_type, entity_id, file_path, details): if not os.path.exists(resolved): return {'success': False, 'error': f'Source file not found: {file_path}'} + from core.imports.file_ops import probe_audio_quality + acquired_quality = probe_audio_quality(resolved) + out_path = os.path.splitext(resolved)[0] + out_ext # Safety invariant: ffmpeg runs with -y, so refuse to convert a file onto # itself (an .m4a ALAC source + AAC target shares the .m4a path) — that @@ -5063,13 +5084,6 @@ def _fix_missing_lossy_copy(self, entity_type, entity_id, file_path, details): except Exception as e: logger.debug("Failed to embed cover art in lossy copy: %s", e) - # Blasphemy Mode — uses the job's own setting, not the global lossy_copy one - delete_original = False - if self._config_manager: - job_settings = self._config_manager.get('repair.jobs.lossy_converter.settings', {}) - if isinstance(job_settings, dict): - delete_original = job_settings.get('delete_original', False) - if delete_original: try: from mutagen import File as MutagenFile @@ -5081,9 +5095,23 @@ def _fix_missing_lossy_copy(self, entity_type, entity_id, file_path, details): try: conn = self.db._get_connection() cursor = conn.cursor() + from core.quality.retention import quality_json, transforms_json + output_quality = probe_audio_quality(out_path) + retention_json = transforms_json([{ + 'type': 'lossy_copy', + 'source_replaced': True, + 'codec': codec, + 'bitrate': bitrate, + 'output_quality': ( + output_quality.to_dict() if output_quality else None), + }]) cursor.execute( - "UPDATE tracks SET file_path = ? WHERE id = ?", - (new_db_path, entity_id) + """UPDATE tracks + SET file_path=?, acquired_quality_json=?, + retention_json=?, updated_at=CURRENT_TIMESTAMP + WHERE id=?""", + (new_db_path, quality_json(acquired_quality), + retention_json, entity_id) ) conn.commit() conn.close() diff --git a/database/music_database.py b/database/music_database.py index c01bbfc97..f3a01d40d 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -426,6 +426,8 @@ def _initialize_database(self): bitrate INTEGER, file_size INTEGER, -- bytes; populated by deep scan from media-server API year INTEGER, -- per-track release year from file tags (albums.year is canonical) + acquired_quality_json TEXT, -- quality before intentional downsample/lossy retention + retention_json TEXT, -- JSON transform provenance for the retained representation created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, FOREIGN KEY (album_id) REFERENCES albums (id) ON DELETE CASCADE, @@ -1763,6 +1765,12 @@ def _ensure_library_quality_column(self, cursor): if cols and 'quality_profile_id' not in cols: cursor.execute("ALTER TABLE tracks ADD COLUMN quality_profile_id INTEGER DEFAULT NULL") logger.info("Added quality_profile_id column to tracks table (quality-profile pipeline)") + if cols and 'acquired_quality_json' not in cols: + cursor.execute("ALTER TABLE tracks ADD COLUMN acquired_quality_json TEXT DEFAULT NULL") + logger.info("Added acquired_quality_json column to tracks table (retention provenance)") + if cols and 'retention_json' not in cols: + cursor.execute("ALTER TABLE tracks ADD COLUMN retention_json TEXT DEFAULT NULL") + logger.info("Added retention_json column to tracks table (retention provenance)") except Exception as e: logger.error("Error adding library quality-profile column: %s", e) @@ -20547,4 +20555,3 @@ def close_database(): # Ignore threading errors during shutdown logger.debug("db instance close: %s", e) _database_instances.clear() - diff --git a/tests/quality/test_retention_provenance.py b/tests/quality/test_retention_provenance.py new file mode 100644 index 000000000..a84a91b2a --- /dev/null +++ b/tests/quality/test_retention_provenance.py @@ -0,0 +1,172 @@ +"""Intentional output transforms must not create endless quality upgrades.""" + +from __future__ import annotations + +import json + +from core.quality.model import AudioQuality, QualityTarget +from core.quality.retention import ( + acquired_quality_from_json, + evaluation_qualities, + quality_json, + retention_meets_profile, + transforms_json, +) + + +HIRES = QualityTarget( + label="FLAC 24-bit/96kHz", format="flac", bit_depth=24, + min_sample_rate=96_000, +) + + +def test_downsampled_file_uses_acquired_hires_quality_for_upgrade_policy(): + measured = AudioQuality(format="flac", bit_depth=16, sample_rate=44_100) + acquired = AudioQuality(format="flac", bit_depth=24, sample_rate=96_000) + retention = transforms_json([{ + "type": "downsample_hires_flac", "source_replaced": True, + }]) + + assert retention_meets_profile( + measured, [HIRES], acquired_quality_json=quality_json(acquired), + retention_json=retention, + ) is True + + +def test_unproven_acquired_quality_never_suppresses_a_real_upgrade(): + measured = AudioQuality(format="flac", bit_depth=16, sample_rate=44_100) + acquired = AudioQuality(format="flac", bit_depth=24, sample_rate=96_000) + + assert retention_meets_profile( + measured, [HIRES], acquired_quality_json=quality_json(acquired), + retention_json=None, + ) is False + assert evaluation_qualities(measured, "not-json", "not-json") == [measured] + + +def test_lossy_only_retention_can_satisfy_lossless_or_lossy_profile(): + measured = AudioQuality(format="mp3", bitrate=320) + acquired = AudioQuality(format="flac", bit_depth=24, sample_rate=96_000) + retention = transforms_json([{ + "type": "lossy_copy", "source_replaced": True, + }]) + + assert retention_meets_profile( + measured, [HIRES], acquired_quality_json=quality_json(acquired), + retention_json=retention, + ) is True + assert retention_meets_profile( + measured, [QualityTarget(label="MP3 320", format="mp3", min_bitrate=320)], + acquired_quality_json=quality_json(acquired), retention_json=retention, + ) is True + + +def test_quality_provenance_round_trip_is_stable_and_typed(): + quality = AudioQuality( + format="flac", bitrate=4608, sample_rate=96_000, bit_depth=24) + encoded = quality_json(quality) + + assert encoded == json.dumps( + quality.to_dict(), sort_keys=True, separators=(",", ":")) + assert acquired_quality_from_json(encoded) == quality + + +def test_pipeline_keeps_lossless_primary_when_lossy_companion_is_retained( + tmp_path, monkeypatch): + import core.imports.pipeline as pipeline + + source = tmp_path / "track.flac" + lossy = tmp_path / "track.opus" + source.write_bytes(b"lossless") + lossy.write_bytes(b"lossy") + qualities = iter([ + AudioQuality(format="flac", bit_depth=24, sample_rate=96_000), + AudioQuality(format="opus", bitrate=256), + ]) + monkeypatch.setattr(pipeline, "probe_audio_quality", lambda _path: next(qualities)) + monkeypatch.setattr(pipeline, "downsample_hires_flac", lambda *_a, **_k: None) + monkeypatch.setattr(pipeline, "create_lossy_copy", lambda *_a, **_k: str(lossy)) + monkeypatch.setattr(pipeline, "_persist_verification_status", lambda *_a: None) + context = {"_final_processed_path": str(source)} + + result = pipeline._apply_profile_output_transforms( + str(source), context, { + "lossy_copy_enabled": True, + "lossy_copy_codec": "opus", + "lossy_copy_bitrate": "256", + "lossy_copy_delete_original": False, + }) + + assert result == str(source) + assert context["_final_processed_path"] == str(source) + assert context["_companion_file_paths"] == [str(lossy)] + assert context["_retention_transforms"][-1]["source_replaced"] is False + + +def test_pipeline_records_destructive_lossy_retention(tmp_path, monkeypatch): + import core.imports.pipeline as pipeline + + source = tmp_path / "track.flac" + lossy = tmp_path / "track.mp3" + source.write_bytes(b"lossless") + lossy.write_bytes(b"lossy") + qualities = iter([ + AudioQuality(format="flac", bit_depth=24, sample_rate=96_000), + AudioQuality(format="mp3", bitrate=320), + ]) + + def _convert(*_args, **_kwargs): + source.unlink() + return str(lossy) + + monkeypatch.setattr(pipeline, "probe_audio_quality", lambda _path: next(qualities)) + monkeypatch.setattr(pipeline, "downsample_hires_flac", lambda *_a, **_k: None) + monkeypatch.setattr(pipeline, "create_lossy_copy", _convert) + monkeypatch.setattr(pipeline, "_persist_verification_status", lambda *_a: None) + context = {"_final_processed_path": str(source)} + + result = pipeline._apply_profile_output_transforms( + str(source), context, { + "lossy_copy_enabled": True, + "lossy_copy_codec": "mp3", + "lossy_copy_bitrate": "320", + "lossy_copy_delete_original": True, + }) + + assert result == str(lossy) + assert context["_final_processed_path"] == str(lossy) + assert context["_acquired_audio_quality"]["bit_depth"] == 24 + assert context["_retention_transforms"][-1]["source_replaced"] is True + + +def test_pipeline_records_hires_downsample_as_destructive_retention( + tmp_path, monkeypatch): + import core.imports.pipeline as pipeline + + source = tmp_path / "track.flac" + source.write_bytes(b"audio") + qualities = iter([ + AudioQuality(format="flac", bit_depth=24, sample_rate=96_000), + AudioQuality(format="flac", bit_depth=16, sample_rate=44_100), + ]) + monkeypatch.setattr(pipeline, "probe_audio_quality", lambda _path: next(qualities)) + monkeypatch.setattr( + pipeline, "downsample_hires_flac", lambda *_a, **_k: str(source)) + monkeypatch.setattr(pipeline, "create_lossy_copy", lambda *_a, **_k: None) + monkeypatch.setattr(pipeline, "_persist_verification_status", lambda *_a: None) + context = {"_final_processed_path": str(source)} + + result = pipeline._apply_profile_output_transforms( + str(source), context, {"downsample_enabled": True}) + + assert result == str(source) + assert context["_acquired_audio_quality"]["sample_rate"] == 96_000 + assert context["_retention_transforms"] == [{ + "type": "downsample_hires_flac", + "source_replaced": True, + "target_bit_depth": 16, + "target_sample_rate": 44_100, + "output_quality": { + "format": "flac", "bit_depth": 16, "sample_rate": 44_100, + }, + }] diff --git a/tests/quality/test_wishlist_quality_columns.py b/tests/quality/test_wishlist_quality_columns.py index 68d402cd5..b6651d082 100644 --- a/tests/quality/test_wishlist_quality_columns.py +++ b/tests/quality/test_wishlist_quality_columns.py @@ -139,3 +139,14 @@ def test_ensure_wishlist_quality_columns_drops_leftover_frozen_columns(db): assert "quality_profile_id" in cols_after finally: conn.close() + + +def test_library_tracks_have_retention_provenance_columns(db): + conn = db._get_connection() + try: + columns = {row[1] for row in conn.execute( + "PRAGMA table_info(tracks)").fetchall()} + finally: + conn.close() + + assert {"acquired_quality_json", "retention_json"} <= columns diff --git a/tests/repair_jobs/test_lossy_converter_scan.py b/tests/repair_jobs/test_lossy_converter_scan.py index ad95955b1..3bf6c9352 100644 --- a/tests/repair_jobs/test_lossy_converter_scan.py +++ b/tests/repair_jobs/test_lossy_converter_scan.py @@ -53,9 +53,9 @@ def _get_connection(self): return _FakeConn(self._rows) -def _row(track_id, title, path): +def _row(track_id, title, path, profile_id=None): # (t.id, t.title, ar.name, al.title, t.file_path, al.thumb_url, ar.thumb_url, ar.id) - return (track_id, title, "Artist", "Album", path, None, None, 42) + return (track_id, title, "Artist", "Album", path, None, None, 42, profile_id) def _context(rows, tmp_path: Path): @@ -130,3 +130,31 @@ def test_missing_on_disk_is_counted_and_surfaced_not_silently_dropped(tmp_path: completion = progress_lines[-1] assert "could not be located on disk" in completion assert "1 tracks" in completion + + +def test_each_track_uses_its_assigned_profile(monkeypatch, tmp_path: Path): + flac = tmp_path / "05 - Profile Track.flac" + flac.write_bytes(b"x") + monkeypatch.setattr( + "core.repair_jobs.lossy_converter.load_profile_by_id", + lambda profile_id: { + "id": profile_id, + "name": "Portable", + "lossy_copy_enabled": True, + "lossy_copy_codec": "mp3", + "lossy_copy_bitrate": "192", + "lossy_copy_delete_original": True, + }, + ) + ctx, findings, _ = _context( + [_row(5, "Profile Track", str(flac), profile_id=77)], tmp_path) + + result = LossyConverterJob().scan(ctx) + + assert result.findings_created == 1 + details = findings[0]["details"] + assert details["quality_profile_id"] == 77 + assert details["quality_profile_name"] == "Portable" + assert details["codec"] == "mp3" + assert details["bitrate"] == "192" + assert details["delete_original"] is True diff --git a/tests/repair_jobs/test_quality_upgrade.py b/tests/repair_jobs/test_quality_upgrade.py index c525e89f9..36346b1c1 100644 --- a/tests/repair_jobs/test_quality_upgrade.py +++ b/tests/repair_jobs/test_quality_upgrade.py @@ -349,6 +349,48 @@ def test_scan_with_empty_default_targets_still_processes_tracks(monkeypatch): assert findings == [] +def test_active_finder_does_not_loop_after_profile_downsampling(monkeypatch): + """A 24/96 acquisition intentionally retained as 16/44.1 still satisfies + the Hi-Res target that selected it; no metadata search should run again.""" + from core.quality.model import AudioQuality + from core.quality.retention import quality_json, transforms_json + + profile = { + 'id': 31, + 'name': 'Hi-Res acquisition / CD retention', + 'ranked_targets': [{ + 'label': 'FLAC 24-bit/96kHz', 'format': 'flac', + 'bit_depth': 24, 'min_sample_rate': 96_000, + }], + } + acquired = AudioQuality(format='flac', bit_depth=24, sample_rate=96_000) + row = _row(path='/music/downsampled.flac') + ( + None, None, None, None, None, 31, + quality_json(acquired), + transforms_json([{ + 'type': 'downsample_hires_flac', 'source_replaced': True, + }]), + ) + db = _FakeDB([row], profile) + monkeypatch.setattr(qu, 'resolve_library_file_path', lambda p, **kw: p) + monkeypatch.setattr( + qu, 'probe_audio_quality', + lambda _p: AudioQuality(format='flac', bit_depth=16, sample_rate=44_100), + ) + + def _no_search(*_a, **_kw): + raise AssertionError('retention provenance should suppress a repeat search') + + monkeypatch.setattr(qu, '_find_best_match', _no_search) + findings = [] + + result = qu.QualityUpgradeJob().scan(_ctx(db, findings)) + + assert result.scanned == 1 + assert result.skipped == 1 + assert findings == [] + + def test_dismissed_finding_is_cleared_and_reflagged_when_config_changed(monkeypatch): """A dismissed finding must not permanently block re-evaluation: if the profile/cutoff this track resolves to has genuinely changed since the diff --git a/tests/test_duplicate_detector_cross_format.py b/tests/test_duplicate_detector_cross_format.py index 1ad424af4..2a6909a53 100644 --- a/tests/test_duplicate_detector_cross_format.py +++ b/tests/test_duplicate_detector_cross_format.py @@ -15,6 +15,7 @@ _is_lossy_companion_pair, _normalize, ) +from core.library.duplicate_rules import is_lossy_companion_file def _track(track_id, *, title, artist="Double Duo", album="Crossword Puzzle", @@ -143,6 +144,16 @@ def test_empty_set_short_circuits(self): assert not _is_lossy_companion_pair( "/a/Song.flac", "/a/Song.mp3", frozenset()) + def test_filesystem_companion_fallback_requires_lossless_sibling(self, tmp_path): + flac = tmp_path / "Song.flac" + mp3 = tmp_path / "Song.mp3" + flac.write_bytes(b"lossless") + mp3.write_bytes(b"lossy") + + assert is_lossy_companion_file(mp3, frozenset({".mp3"})) is True + flac.unlink() + assert is_lossy_companion_file(mp3, frozenset({".mp3"})) is False + class TestCompanionExtsResolution: def test_reads_global_toggle_and_profiles(self, tmp_path): From dd2f8616ea6782fecb08796dcc91a898776f5f7d Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 26 Aug 2026 19:47:58 +0200 Subject: [PATCH 3/8] fix(ui): clarify acquisition and retention quality --- tests/test_quality_profile_preview_nudge.py | 9 +++++++++ webui/index.html | 22 +++++++++++++-------- webui/static/helper.js | 2 +- webui/static/settings.js | 9 +++++++-- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/tests/test_quality_profile_preview_nudge.py b/tests/test_quality_profile_preview_nudge.py index 75ed00f3f..01c245f0c 100644 --- a/tests/test_quality_profile_preview_nudge.py +++ b/tests/test_quality_profile_preview_nudge.py @@ -155,3 +155,12 @@ def test_the_banner_explains_intuitive_save_behaviour(index_html): banner = index_html[start:end] assert "Changes autosave to this profile" in banner assert "Save Settings saves it too" in banner + + +def test_conversion_copy_distinguishes_acquisition_from_retained_output( + settings_js, index_html): + assert "One profile, two decisions" in index_html + assert "acquired quality is remembered" in index_html + assert "lossless + ${codec} companion" in settings_js + assert "retain ${codec} only (acquisition remembered)" in settings_js + assert "Blasphemy Mode" not in index_html diff --git a/webui/index.html b/webui/index.html index 988c33bce..5d5073bbe 100644 --- a/webui/index.html +++ b/webui/index.html @@ -4426,9 +4426,10 @@

Quality

download the real file is verified against the same list. With fallback off, a track is left missing rather than accepting a quality below every target.

- Note: the Downsample hi-res option (under Lossy Copy) also - bypasses this gate — if off-list files keep slipping through with fallback off, check - that setting too. + Acquisition vs retained output: Downsample and Lossy Copy run only + after the downloaded file passes this gate. SoulSync remembers the acquired quality, + so intentionally retaining 16/44.1 or a lossy-only output does not make the same track + appear as an upgrade again.
@@ -4629,7 +4630,7 @@

Post-Download Conversion

i
- +
@@ -4641,7 +4642,7 @@

Post-Download Conversion

i
- +
diff --git a/webui/static/helper.js b/webui/static/helper.js index ec6b94f86..3ceacd9d6 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -1868,7 +1868,7 @@ const HELPER_CONTENT = { // Library — Lossy Copy '#lossy-copy-enabled': { title: 'Lossy Copy', - description: 'Create a lower-bitrate copy of every downloaded file alongside the original. Useful for syncing to mobile devices or bandwidth-limited streaming.', + description: 'Create a lower-bitrate derivative of downloaded lossless audio. If the source is kept, SoulSync treats both files as versions of one track; if it is deleted, the acquired quality is still remembered for upgrade decisions.', docsId: 'set-processing' }, diff --git a/webui/static/settings.js b/webui/static/settings.js index f20be1d14..035e74520 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -3007,8 +3007,13 @@ function qpProfileSummary(profile) { } if (profile.acoustid_required) parts.push('strict AcoustID'); if (profile.deep_audio_verify) parts.push('deep verify'); - if (profile.downsample_enabled) parts.push('downsample'); - if (profile.lossy_copy_enabled) parts.push(`lossy copy ${(profile.lossy_copy_codec || 'mp3').toUpperCase()}`); + if (profile.downsample_enabled) parts.push('retain CD-quality (acquisition remembered)'); + if (profile.lossy_copy_enabled) { + const codec = (profile.lossy_copy_codec || 'mp3').toUpperCase(); + parts.push(profile.lossy_copy_delete_original + ? `retain ${codec} only (acquisition remembered)` + : `lossless + ${codec} companion`); + } if (['until_cutoff', 'until_top'].includes(profile.upgrade_policy)) { const cutoffIndex = Math.min(Math.max(parseInt(profile.upgrade_cutoff_index || '0', 10) || 0, 0), Math.max(targets.length - 1, 0)); const cutoff = targets[cutoffIndex]?.label || 'top target'; From ebda7f301190ed3b795a7efdfb0bfefcc49f1f7e Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 26 Aug 2026 21:28:12 +0200 Subject: [PATCH 4/8] fix(duplicates): detect ALAC companions in M4A --- core/library/duplicate_rules.py | 11 +++++++--- tests/test_duplicate_detector_cross_format.py | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/core/library/duplicate_rules.py b/core/library/duplicate_rules.py index 2c9c363ef..017c585e9 100644 --- a/core/library/duplicate_rules.py +++ b/core/library/duplicate_rules.py @@ -11,7 +11,7 @@ import os from typing import Any -from core.quality.lossless import is_lossless_format +from core.quality.lossless import is_lossless_audio_path, is_lossless_format from core.quality.source_map import format_from_extension @@ -70,8 +70,13 @@ def is_lossy_companion_pair(path1: Any, path2: Any, s2, e2 = os.path.splitext(b2) if s1.lower() != s2.lower(): return False - lossless1 = is_lossless_format(format_from_extension(e1.lstrip(".").lower())) - lossless2 = is_lossless_format(format_from_extension(e2.lstrip(".").lower())) + # Extension-only classification is insufficient for MP4 containers: + # ``.m4a`` may contain either lossy AAC or lossless ALAC. Both duplicate + # callers operate on real files, so use the same codec probe as the import + # and converter paths before deciding which side is the source. + from core.imports.file_ops import m4a_codec + lossless1 = is_lossless_audio_path(p1, probe_codec=m4a_codec) + lossless2 = is_lossless_audio_path(p2, probe_codec=m4a_codec) if lossless1 == lossless2: return False lossy_ext = e2 if lossless1 else e1 diff --git a/tests/test_duplicate_detector_cross_format.py b/tests/test_duplicate_detector_cross_format.py index 2a6909a53..222854a41 100644 --- a/tests/test_duplicate_detector_cross_format.py +++ b/tests/test_duplicate_detector_cross_format.py @@ -140,6 +140,28 @@ def test_windows_paths_and_case(self): "C:\\Music\\Album\\Song.FLAC", "C:\\music\\album\\Song.Mp3", frozenset({'.mp3'})) + def test_alac_m4a_is_protected_with_mp3_or_opus(self, monkeypatch): + monkeypatch.setattr( + "core.imports.file_ops.m4a_codec", + lambda path: "alac" if str(path).lower().endswith(".m4a") else None, + ) + + for lossy_ext in (".mp3", ".opus"): + assert _is_lossy_companion_pair( + "/music/Album/Song.m4a", + f"/music/Album/Song{lossy_ext}", + frozenset({lossy_ext}), + ) + + def test_aac_m4a_is_not_treated_as_lossless_source(self, monkeypatch): + monkeypatch.setattr("core.imports.file_ops.m4a_codec", lambda _path: "aac") + + assert not _is_lossy_companion_pair( + "/music/Album/Song.m4a", + "/music/Album/Song.opus", + frozenset({".opus"}), + ) + def test_empty_set_short_circuits(self): assert not _is_lossy_companion_pair( "/a/Song.flac", "/a/Song.mp3", frozenset()) From 6e719cc47970e7601b333e7a1fd9f5c49f540ced Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 26 Aug 2026 21:30:46 +0200 Subject: [PATCH 5/8] fix(provenance): bridge retention through media scans --- core/imports/side_effects.py | 30 ++++++++++------ database/music_database.py | 22 +++++++++--- tests/imports/test_import_side_effects.py | 32 +++++++++++++++++ tests/test_provenance_id_persistence.py | 44 +++++++++++++++++++++++ 4 files changed, 113 insertions(+), 15 deletions(-) diff --git a/core/imports/side_effects.py b/core/imports/side_effects.py index 976e99824..32ff2a9e6 100644 --- a/core/imports/side_effects.py +++ b/core/imports/side_effects.py @@ -50,6 +50,22 @@ def _stable_soulsync_id(text: str) -> str: return str(abs(int(hashlib.md5(text.encode("utf-8", errors="replace")).hexdigest(), 16)) % (10 ** 9)) +def _retention_provenance_json(context: Dict[str, Any]) -> tuple[str | None, str | None]: + """Serialize acquisition/retention truth for either persistence path.""" + from core.quality.model import AudioQuality + from core.quality.retention import quality_json, transforms_json + + acquired_value = context.get("_acquired_audio_quality") + try: + acquired_quality = AudioQuality.from_dict(acquired_value) if acquired_value else None + except (TypeError, ValueError): + acquired_quality = None + return ( + quality_json(acquired_quality), + transforms_json(context.get("_retention_transforms")), + ) + + # Tiny SQL allowlist for the fill-empty helpers — prevents accidental # SQL injection through the f-string column-name interpolation. Only # columns the soulsync library write path ever updates are listed. @@ -348,6 +364,7 @@ def _embedded(*keys): audiodb_id = _embedded("AUDIODB_TRACK_ID") soul_id = _embedded("SOUL_ID") isrc = context.get("_isrc") + acquired_quality_json, retention_json = _retention_provenance_json(context) db = get_database() db.record_track_download( @@ -372,6 +389,8 @@ def _embedded(*keys): audiodb_id=audiodb_id, soul_id=soul_id, isrc=isrc, + acquired_quality_json=acquired_quality_json, + retention_json=retention_json, ) except Exception as e: logger.debug("record_download_provenance failed: %s", e) @@ -667,16 +686,7 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[ # Quality Upgrade Finder passes re-resolve the track against the # default profile instead of the one it was actually imported under. track_quality_profile_id = track_info.get("quality_profile_id") - from core.quality.retention import quality_json, transforms_json - from core.quality.model import AudioQuality - - acquired_value = context.get("_acquired_audio_quality") - try: - acquired_quality = AudioQuality.from_dict(acquired_value) if acquired_value else None - except (TypeError, ValueError): - acquired_quality = None - acquired_quality_json = quality_json(acquired_quality) - retention_json = transforms_json(context.get("_retention_transforms")) + acquired_quality_json, retention_json = _retention_provenance_json(context) try: track_columns = { column[1] for column in cursor.execute( diff --git a/database/music_database.py b/database/music_database.py index f3a01d40d..a231fc43f 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -2802,6 +2802,8 @@ def _add_discovery_tables(self, cursor): track_title TEXT, track_artist TEXT, track_album TEXT, + acquired_quality_json TEXT, + retention_json TEXT, status TEXT DEFAULT 'completed', created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP ) @@ -2836,6 +2838,9 @@ def _add_discovery_tables(self, cursor): added_external = True if added_external: logger.info(f"Added external-ID columns to track_downloads: {', '.join(external_id_cols)}") + for _col in ('acquired_quality_json', 'retention_json'): + if _col not in td_columns: + cursor.execute(f"ALTER TABLE track_downloads ADD COLUMN {_col} TEXT") cursor.execute("CREATE INDEX IF NOT EXISTS idx_td_spotify_id ON track_downloads (spotify_track_id)") cursor.execute("CREATE INDEX IF NOT EXISTS idx_td_itunes_id ON track_downloads (itunes_track_id)") cursor.execute("CREATE INDEX IF NOT EXISTS idx_td_deezer_id ON track_downloads (deezer_track_id)") @@ -16481,7 +16486,9 @@ def record_track_download(self, file_path: str, source_service: str, source_user musicbrainz_recording_id: Optional[str] = None, audiodb_id: Optional[str] = None, soul_id: Optional[str] = None, - isrc: Optional[str] = None) -> Optional[int]: + isrc: Optional[str] = None, + acquired_quality_json: Optional[str] = None, + retention_json: Optional[str] = None) -> Optional[int]: """Record a download with full source provenance. Returns the record ID. External-ID kwargs (spotify_track_id et al.) capture the metadata- @@ -16517,13 +16524,15 @@ def record_track_download(self, file_path: str, source_service: str, source_user source_size, audio_quality, track_title, track_artist, track_album, status, bit_depth, sample_rate, bitrate, spotify_track_id, itunes_track_id, deezer_track_id, tidal_track_id, - qobuz_track_id, musicbrainz_recording_id, audiodb_id, soul_id, isrc) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + qobuz_track_id, musicbrainz_recording_id, audiodb_id, soul_id, isrc, + acquired_quality_json, retention_json) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) """, (track_id, file_path, source_service, source_username, source_filename, source_size, audio_quality, track_title, track_artist, track_album, status, bit_depth, sample_rate, bitrate, spotify_track_id, itunes_track_id, deezer_track_id, tidal_track_id, - qobuz_track_id, musicbrainz_recording_id, audiodb_id, soul_id, isrc)) + qobuz_track_id, musicbrainz_recording_id, audiodb_id, soul_id, isrc, + acquired_quality_json, retention_json)) conn.commit() return cursor.lastrowid except Exception as e: @@ -16577,7 +16586,8 @@ def backfill_track_external_ids_from_provenance(self, track_id: str, file_path: number of columns updated. Called from ``insert_or_update_media_track`` immediately after the row is inserted/updated so freshly synced media-server rows pick up - whatever IDs SoulSync already knew at download time. + whatever identity and retention provenance SoulSync already knew at + download time. """ if not track_id or not file_path: return 0 @@ -16599,6 +16609,8 @@ def backfill_track_external_ids_from_provenance(self, track_id: str, file_path: 'audiodb_id': 'audiodb_id', 'soul_id': 'soul_id', 'isrc': 'isrc', + 'acquired_quality_json': 'acquired_quality_json', + 'retention_json': 'retention_json', } updates: Dict[str, str] = {} diff --git a/tests/imports/test_import_side_effects.py b/tests/imports/test_import_side_effects.py index 0be4b3780..9896330ed 100644 --- a/tests/imports/test_import_side_effects.py +++ b/tests/imports/test_import_side_effects.py @@ -1,3 +1,4 @@ +import json import os import sqlite3 from types import SimpleNamespace @@ -794,6 +795,37 @@ def record_track_download(self, **kwargs): assert captured.get("source_service") == "auto_import" +def test_download_provenance_carries_retention_policy_for_media_server(monkeypatch): + """The media-server path returns before the standalone track insert, so + track_downloads must carry the transform truth until the server scan adds + its tracks row.""" + captured = {} + + class _DBStub: + def record_track_download(self, **kwargs): + captured.update(kwargs) + + monkeypatch.setattr(side_effects, "get_database", lambda: _DBStub()) + context = { + "track_info": {"name": "Portable", "artists": [{"name": "Artist"}]}, + "_final_processed_path": "/library/Portable.mp3", + "_acquired_audio_quality": { + "format": "flac", "bit_depth": 24, "sample_rate": 96000, + }, + "_retention_transforms": [{ + "type": "lossy_copy", "source_replaced": True, + }], + } + + side_effects.record_download_provenance(context) + + assert json.loads(captured["acquired_quality_json"])["format"] == "flac" + assert json.loads(captured["retention_json"])[0] == { + "source_replaced": True, + "type": "lossy_copy", + } + + def test_is_active_media_server_ready_standalone_always_ready(monkeypatch): monkeypatch.setattr( side_effects, diff --git a/tests/test_provenance_id_persistence.py b/tests/test_provenance_id_persistence.py index 19b24b2af..dad0a8e66 100644 --- a/tests/test_provenance_id_persistence.py +++ b/tests/test_provenance_id_persistence.py @@ -73,6 +73,8 @@ def test_track_downloads_has_new_external_id_columns(self, db): assert 'audiodb_id' in cols assert 'soul_id' in cols assert 'isrc' in cols + assert 'acquired_quality_json' in cols + assert 'retention_json' in cols def test_track_downloads_has_external_id_indexes(self, db): conn = db._get_connection() @@ -144,6 +146,25 @@ def test_omitted_ids_persist_as_null(self, db): cursor.execute("SELECT spotify_track_id FROM track_downloads WHERE id = ?", (rec_id,)) assert cursor.fetchone()[0] is None + def test_persists_acquisition_and_retention_provenance(self, db): + rec_id = db.record_track_download( + file_path='/lib/Artist/Album/Track.mp3', + source_service='soulseek', source_username='user1', + source_filename='Track.flac', + acquired_quality_json='{"format":"flac","bit_depth":24}', + retention_json='[{"type":"lossy_copy","source_replaced":true}]', + ) + + conn = db._get_connection() + row = conn.execute( + "SELECT acquired_quality_json, retention_json FROM track_downloads " + "WHERE id = ?", (rec_id,), + ).fetchone() + assert tuple(row) == ( + '{"format":"flac","bit_depth":24}', + '[{"type":"lossy_copy","source_replaced":true}]', + ) + # --------------------------------------------------------------------------- # get_provenance_by_file_path @@ -273,6 +294,29 @@ def test_preserves_existing_ids(self, db): assert row[0] == 'sp-from-enrichment', "Existing spotify_track_id must be preserved" assert row[1] == 'dz1', "Empty deezer_id should be filled from provenance" + def test_copies_retention_provenance_to_media_server_track(self, db): + self._seed_artist_album_and_track(db, track_id='t1', file_path='/lib/Track.mp3') + db.record_track_download( + file_path='/app/Transfer/Track.mp3', + source_service='soulseek', source_username='u', + source_filename='Track.flac', + acquired_quality_json='{"format":"flac","bit_depth":24}', + retention_json='[{"type":"lossy_copy","source_replaced":true}]', + ) + + updated = db.backfill_track_external_ids_from_provenance( + 't1', '/lib/Track.mp3') + + assert updated > 0 + conn = db._get_connection() + row = conn.execute( + "SELECT acquired_quality_json, retention_json FROM tracks WHERE id='t1'" + ).fetchone() + assert tuple(row) == ( + '{"format":"flac","bit_depth":24}', + '[{"type":"lossy_copy","source_replaced":true}]', + ) + def test_returns_zero_when_no_provenance(self, db): self._seed_artist_album_and_track(db, track_id='t1', file_path='/lib/Track.mp3') # No record_track_download call — no provenance row exists From 6f2e97b332751059b7820b68ce0452aa308ccaca Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 26 Aug 2026 21:32:36 +0200 Subject: [PATCH 6/8] fix(repair): resolve unassigned profiles at apply time --- core/repair_jobs/lossy_converter.py | 6 +++- core/repair_worker.py | 5 ++- .../repair_jobs/test_lossy_converter_scan.py | 28 ++++++++++++++- tests/test_repair_worker_lossy_error.py | 34 +++++++++++++++++++ 4 files changed, 70 insertions(+), 3 deletions(-) diff --git a/core/repair_jobs/lossy_converter.py b/core/repair_jobs/lossy_converter.py index 81e438c4e..e144888ce 100644 --- a/core/repair_jobs/lossy_converter.py +++ b/core/repair_jobs/lossy_converter.py @@ -235,7 +235,11 @@ def scan(self, context: JobContext) -> JobResult: 'album_thumb_url': album_thumb or None, 'artist_thumb_url': artist_thumb or None, 'artist_id': artist_id, - 'quality_profile_id': policy.get('profile_id'), + # Preserve the track's assignment semantics, not + # the concrete default resolved for this scan. + # NULL means "follow the current default" and must + # remain live until the user applies the finding. + 'quality_profile_id': profile_id, 'quality_profile_name': policy.get('profile_name'), 'delete_original': policy.get('delete_original', False), } diff --git a/core/repair_worker.py b/core/repair_worker.py index 0a9b008dd..86bed8e18 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -4942,7 +4942,10 @@ def _fix_missing_lossy_copy(self, entity_type, entity_id, file_path, details): profile_id = details.get('quality_profile_id') if isinstance(details, dict) else None try: from core.quality.selection import load_profile_by_id - profile = load_profile_by_id(profile_id) if profile_id else None + # A NULL assignment deliberately means "use the current default". + # load_profile_by_id(None) performs that live resolution, so a + # default-profile change between scan and apply is respected. + profile = load_profile_by_id(profile_id) except Exception as e: logger.debug("Could not resolve lossy-converter profile %r: %s", profile_id, e) if isinstance(profile, dict) and 'lossy_copy_enabled' in profile: diff --git a/tests/repair_jobs/test_lossy_converter_scan.py b/tests/repair_jobs/test_lossy_converter_scan.py index 3bf6c9352..0f2018c57 100644 --- a/tests/repair_jobs/test_lossy_converter_scan.py +++ b/tests/repair_jobs/test_lossy_converter_scan.py @@ -46,12 +46,16 @@ def close(self): class _FakeDB: - def __init__(self, rows): + def __init__(self, rows, default_profile=None): self._rows = rows + self._default_profile = default_profile def _get_connection(self): return _FakeConn(self._rows) + def get_quality_profile(self): + return self._default_profile + def _row(track_id, title, path, profile_id=None): # (t.id, t.title, ar.name, al.title, t.file_path, al.thumb_url, ar.thumb_url, ar.id) @@ -158,3 +162,25 @@ def test_each_track_uses_its_assigned_profile(monkeypatch, tmp_path: Path): assert details["codec"] == "mp3" assert details["bitrate"] == "192" assert details["delete_original"] is True + + +def test_unassigned_track_preserves_live_default_pointer(tmp_path: Path): + flac = tmp_path / "06 - Default Track.flac" + flac.write_bytes(b"x") + ctx, findings, _ = _context( + [_row(6, "Default Track", str(flac), profile_id=None)], tmp_path) + ctx.db._default_profile = { + "id": 88, + "name": "Current Default", + "lossy_copy_enabled": True, + "lossy_copy_codec": "mp3", + "lossy_copy_bitrate": "192", + "lossy_copy_delete_original": True, + } + + result = LossyConverterJob().scan(ctx) + + assert result.findings_created == 1 + details = findings[0]["details"] + assert details["quality_profile_id"] is None + assert details["quality_profile_name"] == "Current Default" diff --git a/tests/test_repair_worker_lossy_error.py b/tests/test_repair_worker_lossy_error.py index 162ff6ae9..9070d8d5d 100644 --- a/tests/test_repair_worker_lossy_error.py +++ b/tests/test_repair_worker_lossy_error.py @@ -51,6 +51,15 @@ def test_fix_surfaces_real_ffmpeg_error_not_banner(tmp_path, monkeypatch): flac = tmp_path / "01 - Track.flac" flac.write_bytes(b"x") + monkeypatch.setattr( + "core.quality.selection.load_profile_by_id", + lambda _profile_id: { + "lossy_copy_enabled": True, + "lossy_copy_codec": "opus", + "lossy_copy_bitrate": "256", + "lossy_copy_delete_original": False, + }, + ) monkeypatch.setattr(shutil, "which", lambda _: "/fake/ffmpeg") monkeypatch.setattr( subprocess, "run", @@ -67,3 +76,28 @@ def test_fix_surfaces_real_ffmpeg_error_not_banner(tmp_path, monkeypatch): # ...banner absent. assert "ffmpeg version" not in err assert "configuration:" not in err + + +def test_fix_resolves_unassigned_track_against_live_default(tmp_path, monkeypatch): + resolved_ids = [] + + def _load(profile_id): + resolved_ids.append(profile_id) + return { + "id": 99, + "name": "New Default", + "lossy_copy_enabled": False, + } + + monkeypatch.setattr("core.quality.selection.load_profile_by_id", _load) + + result = _worker(tmp_path)._fix_missing_lossy_copy( + "track", "1", str(tmp_path / "track.flac"), + {"quality_profile_id": None}, + ) + + assert resolved_ids == [None] + assert result == { + "success": False, + "error": "Lossy Copy is disabled for this track profile", + } From a4320740c52e1d7a661ecb1fcb5a4a75b61d6f22 Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 26 Aug 2026 21:33:41 +0200 Subject: [PATCH 7/8] fix(quality): skip catalogued lossy companions --- core/repair_jobs/quality_upgrade_scanner.py | 7 +-- tests/repair_jobs/test_quality_upgrade.py | 50 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/core/repair_jobs/quality_upgrade_scanner.py b/core/repair_jobs/quality_upgrade_scanner.py index d2ebc6380..a2c53abae 100644 --- a/core/repair_jobs/quality_upgrade_scanner.py +++ b/core/repair_jobs/quality_upgrade_scanner.py @@ -314,10 +314,11 @@ def _bundle_for(row_profile_id): # the library, skip anything with no DB row BEFORE probing — no point # reading hundreds of orphan files. meta = self._match_db(fpath, db_index) - if meta is None and is_lossy_companion_file(fpath, companion_exts): + if is_lossy_companion_file(fpath, companion_exts): # A deliberately retained lossy derivative belongs to the - # lossless track beside it. It is not an orphan quality choice - # and must not generate a second, misleading upgrade finding. + # lossless track beside it. Media servers may catalogue both + # representations, so this applies regardless of whether the + # derivative itself has a DB row. result.skipped += 1 continue if library_only and meta is None: diff --git a/tests/repair_jobs/test_quality_upgrade.py b/tests/repair_jobs/test_quality_upgrade.py index 36346b1c1..ff3cc379d 100644 --- a/tests/repair_jobs/test_quality_upgrade.py +++ b/tests/repair_jobs/test_quality_upgrade.py @@ -767,6 +767,56 @@ def _setup_scanner_common(monkeypatch, tmp_path, targets, aq): monkeypatch.setattr('core.imports.file_ops.probe_audio_quality', lambda path: aq) +def test_scanner_skips_catalogued_lossy_companion(monkeypatch, tmp_path): + """Navidrome/Jellyfin may index both the source and its retained copy.""" + from core.quality.model import AudioQuality, QualityTarget + + flac = tmp_path / "song.flac" + mp3 = tmp_path / "song.mp3" + flac.write_bytes(b"lossless") + mp3.write_bytes(b"lossy") + target = QualityTarget(label="FLAC", format="flac") + profile = { + "id": 20, + "name": "Lossless with portable copy", + "ranked_targets": [target.to_dict()], + } + monkeypatch.setattr(qs, "targets_from_profile", lambda _profile: ([target], False)) + monkeypatch.setattr( + qs.QualityUpgradeScannerJob, + "_collect_music_dirs", + lambda self, context: [str(tmp_path)], + ) + monkeypatch.setattr( + qs.QualityUpgradeScannerJob, + "_build_db_suffix_index", + lambda self, context: { + "song.flac": {"track_id": 1, "title": "Song", "quality_profile_id": 20}, + "song.mp3": {"track_id": 2, "title": "Song", "quality_profile_id": 20}, + }, + ) + monkeypatch.setattr( + "core.library.duplicate_rules.lossy_companion_exts", + lambda *_args, **_kwargs: {".mp3"}, + ) + monkeypatch.setattr( + "core.imports.file_ops.probe_audio_quality", + lambda path: AudioQuality( + format="flac" if str(path).endswith(".flac") else "mp3", + bitrate=320, + ), + ) + monkeypatch.setattr("core.imports.silence.detect_broken_audio", lambda _path: None) + findings = [] + + result = qs.QualityUpgradeScannerJob().scan( + _scanner_ctx(_ScannerFakeDB(profile), tmp_path, findings)) + + assert findings == [] + assert result.scanned == 1 + assert result.skipped == 1 + + def test_scanner_dismissed_finding_stays_dismissed_when_config_unchanged(monkeypatch, tmp_path): """A track dismissed under the SAME profile/cutoff must not resurrect a finding on every re-run just because it still measures below profile.""" From 1431650906f0001d395123e5071eb112eada728b Mon Sep 17 00:00:00 2001 From: dev Date: Wed, 26 Aug 2026 21:34:31 +0200 Subject: [PATCH 8/8] fix(imports): probe explicit ALAC sources --- core/imports/file_ops.py | 9 +++++---- tests/imports/test_import_file_ops.py | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/core/imports/file_ops.py b/core/imports/file_ops.py index d91a68d68..1f8c0b2d0 100644 --- a/core/imports/file_ops.py +++ b/core/imports/file_ops.py @@ -568,14 +568,15 @@ def probe_audio_quality(file_path: str): sample_rate=audio.info.sample_rate, ) - if ext in ('m4a', 'aac', 'mp4'): + if ext in ('m4a', 'aac', 'mp4', 'alac'): from mutagen.mp4 import MP4 audio = MP4(file_path) # .m4a can carry AAC (lossy) OR ALAC (lossless) — only the real - # codec tells them apart, which is why extension-based classification - # defaults to 'aac' and we correct it here from the probed file. + # codec tells them apart. The explicit .alac extension is already + # unambiguous and is accepted by the lossy-copy path, so it must be + # probed here as ALAC as well to preserve acquisition provenance. codec = (getattr(audio.info, 'codec', '') or '').lower() - if 'alac' in codec: + if ext == 'alac' or 'alac' in codec: return AudioQuality( format='alac', bitrate=audio.info.bitrate // 1000 if audio.info.bitrate else None, diff --git a/tests/imports/test_import_file_ops.py b/tests/imports/test_import_file_ops.py index 2a74f503b..a7b89ee50 100644 --- a/tests/imports/test_import_file_ops.py +++ b/tests/imports/test_import_file_ops.py @@ -462,6 +462,31 @@ def __init__(self, _p): assert aq.bitrate == 160 +def test_probe_audio_quality_reads_explicit_alac_extension(tmp_path, monkeypatch): + path = tmp_path / "source.alac" + path.write_bytes(b"fake-alac") + + class _Info: + codec = "alac" + bitrate = 4_608_000 + sample_rate = 96_000 + bits_per_sample = 24 + + class _MP4: + def __init__(self, _path): + self.info = _Info() + + monkeypatch.setattr("mutagen.mp4.MP4", _MP4) + + aq = _fo.probe_audio_quality(str(path)) + + assert aq is not None + assert aq.format == "alac" + assert aq.bitrate == 4608 + assert aq.sample_rate == 96_000 + assert aq.bit_depth == 24 + + def test_opus_256_estimate_meets_opus_192_target(tmp_path, monkeypatch): path = tmp_path / "premium.opus" path.write_bytes(b"x" * 64_000) # 64 KB over 2s ≈ 256 kbps