From 69dd4e1792321563de7342a5c0d2b4a7b3537bb4 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 11:51:43 -0700 Subject: [PATCH 01/42] Quality Upgrade Finder: new findings-based repair job (replaces auto-acting Quality Scanner) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old Quality Scanner tool judged quality by file EXTENSION only (a 128k and a 320k MP3 looked identical), ignored the bitrate-based quality profile, used min() of enabled tiers so the default profile flagged the ENTIRE non-lossless library, and auto-dumped every match into the wishlist with no review. This new repair job does it properly: - meets_preferred_quality(): pure, bitrate-AWARE decision honoring every enabled quality bucket (320 MP3 passes a FLAC+320+256 profile; 128 MP3 doesn't). Floor is the worst enabled bucket, not the best. - scans watchlist artists or whole library, finds below-quality tracks, matches a better version at scan time (reusing the existing tested match helpers), emits a FINDING showing the match + confidence. Off by default; nothing auto-queued. - _fix_quality_upgrade apply handler adds the matched track WITH album context to the wishlist — the user-approved version of what the old tool did silently. - Transcode/fake-lossless detection intentionally left to the existing Fake Lossless Detector job. 12 seam tests incl. a regression pinning the default-profile flooding bug. The old tool is still in place; removing it + rewiring its automation action is the next step. --- core/repair_jobs/__init__.py | 1 + core/repair_jobs/quality_upgrade.py | 396 ++++++++++++++++++++++ core/repair_worker.py | 31 ++ tests/repair_jobs/test_quality_upgrade.py | 221 ++++++++++++ 4 files changed, 649 insertions(+) create mode 100644 core/repair_jobs/quality_upgrade.py create mode 100644 tests/repair_jobs/test_quality_upgrade.py diff --git a/core/repair_jobs/__init__.py b/core/repair_jobs/__init__.py index fa95fc70..35a236bd 100644 --- a/core/repair_jobs/__init__.py +++ b/core/repair_jobs/__init__.py @@ -50,6 +50,7 @@ _JOB_MODULES = [ 'core.repair_jobs.discography_backfill', 'core.repair_jobs.canonical_version_resolve', 'core.repair_jobs.library_retag', + 'core.repair_jobs.quality_upgrade', ] diff --git a/core/repair_jobs/quality_upgrade.py b/core/repair_jobs/quality_upgrade.py new file mode 100644 index 00000000..c80a381f --- /dev/null +++ b/core/repair_jobs/quality_upgrade.py @@ -0,0 +1,396 @@ +"""Quality Upgrade Finder maintenance job. + +Replaces the old auto-acting "Quality Scanner" tool. That tool decided quality +purely by file EXTENSION (so a 128 kbps MP3 and a 320 kbps MP3 looked identical), +ignored the bitrate-based quality profile, and silently dumped every match +straight into the wishlist with no review — which, on the default profile, meant +flagging an entire non-lossless library at once. + +This job does it the way the rest of the app works: it SCANS (watchlist artists +or the whole library), judges each track against the user's quality profile using +BOTH format and bitrate, and for anything below the preferred quality it searches +the configured metadata source for a better version and emits a FINDING. Nothing +is queued until you review and Apply the finding — at which point the matched +track (carrying its album context) is added to the wishlist, exactly like every +other acquisition path. + +The quality decision (``meets_preferred_quality``) is a pure function so it can be +unit-tested without a database or network. Transcode/"fake lossless" detection is +intentionally NOT done here — that's the separate Fake Lossless Detector job. +""" + +from __future__ import annotations + +import os +import time +from typing import Any, Dict, List, Optional, Tuple + +from core.metadata.registry import get_client_for_source, get_primary_source, get_source_priority +from core.repair_jobs import register_job +from core.repair_jobs.base import JobContext, JobResult, RepairJob +# Reuse the (tested) provider search + result-normalization helpers from the old +# scanner module so matching stays a single source of truth. +from core.discovery.quality_scanner import ( + _extract_lookup_value, + _normalize_track_match, + _search_tracks_for_source, + _track_artist_names, + _track_name, +) +from utils.logging_config import get_logger + +logger = get_logger("repair_jobs.quality_upgrade") + + +# Quality ranks — higher is better. Lossless tops everything; lossy tiers fall out +# of bitrate. 0 means "below the lowest tracked tier / unknown". +RANK_LOSSLESS = 4 +RANK_320 = 3 +RANK_256 = 2 +RANK_192 = 1 +RANK_BELOW = 0 + +LOSSLESS_EXTENSIONS = {'.flac', '.alac', '.ape', '.wav', '.aiff', '.aif', '.dsf', '.dff', '.m4a'} +# NB: .m4a is ambiguous (ALAC vs AAC); we treat the *format* as lossy-capable and +# rely on bitrate below — a true ALAC .m4a reports a lossless-scale bitrate. + +# Quality-profile bucket key -> rank. +_PROFILE_KEY_RANK = { + 'flac': RANK_LOSSLESS, + 'mp3_320': RANK_320, + 'mp3_256': RANK_256, + 'mp3_192': RANK_192, +} + + +def _normalize_kbps(bitrate: Optional[int]) -> Optional[int]: + """Library bitrate may be stored in bps (e.g. 320000) or kbps (320). + Normalize to kbps. Returns None when unknown/zero.""" + if not bitrate: + return None + try: + b = int(bitrate) + except (TypeError, ValueError): + return None + if b <= 0: + return None + return b // 1000 if b > 4000 else b + + +def classify_track_quality(file_path: str, bitrate: Optional[int]) -> Optional[int]: + """Rank a file by format + bitrate. Returns a RANK_* value, or None when it + can't be judged (a lossy file with no known bitrate).""" + ext = os.path.splitext(file_path or '')[1].lower() + kbps = _normalize_kbps(bitrate) + + # Lossless containers: a real lossless file has a high bitrate; a low one is a + # lossy stream in a lossless container — but flagging that is the Fake Lossless + # Detector's job, so here we treat the lossless *format* as top rank. + if ext in {'.flac', '.alac', '.ape', '.wav', '.aiff', '.aif', '.dsf', '.dff'}: + return RANK_LOSSLESS + # .m4a / lossy: judge purely by bitrate. A lossless-scale bitrate (ALAC in m4a, + # or a mislabeled lossless) ranks as lossless. + if kbps is None: + return None + if kbps >= 800: + return RANK_LOSSLESS + if kbps >= 280: + return RANK_320 + if kbps >= 200: + return RANK_256 + if kbps >= 150: + return RANK_192 + return RANK_BELOW + + +def preferred_quality_floor(quality_profile: Dict[str, Any]) -> Optional[int]: + """The lowest acceptable quality rank from the profile's ENABLED buckets — the + floor a track must meet. Returns None when nothing is enabled (caller should + then flag nothing, rather than flagging everything).""" + qualities = (quality_profile or {}).get('qualities', {}) or {} + enabled_ranks = [ + _PROFILE_KEY_RANK[key] + for key, cfg in qualities.items() + if isinstance(cfg, dict) and cfg.get('enabled') and key in _PROFILE_KEY_RANK + ] + if not enabled_ranks: + return None + return min(enabled_ranks) + + +def meets_preferred_quality(file_path: str, bitrate: Optional[int], + quality_profile: Dict[str, Any]) -> bool: + """Pure decision: does this track already meet the user's preferred quality? + + A track meets quality when its format+bitrate rank is at least the profile's + floor (the worst quality the user still accepts). This honors a profile that + enables, say, FLAC *and* MP3-320: a 320 kbps MP3 passes, a 128 kbps MP3 does + not. With nothing enabled, everything passes (we never flag the whole library + on an empty profile).""" + floor = preferred_quality_floor(quality_profile) + if floor is None: + return True + + file_rank = classify_track_quality(file_path, bitrate) + if file_rank is None: + # Lossy file with unknown bitrate: only judgeable when the floor is + # lossless (then any lossy file is below it). Otherwise don't flag. + ext = os.path.splitext(file_path or '')[1].lower() + if floor == RANK_LOSSLESS and ext not in LOSSLESS_EXTENSIONS: + return False + return True + + return file_rank >= floor + + +def _rank_label(rank: Optional[int]) -> str: + return { + RANK_LOSSLESS: 'Lossless', RANK_320: 'MP3 320', RANK_256: 'MP3 256', + RANK_192: 'MP3 192', RANK_BELOW: 'low bitrate', + }.get(rank, 'unknown') + + +def _find_best_match(engine: Any, source_priority: List[str], title: str, artist: str, + album: str, min_confidence: float) -> Tuple[Optional[Any], float, Optional[str], bool]: + """Search the configured metadata sources for the best replacement match. + Returns (best_track, confidence, source, attempted_any_provider).""" + temp_track = type('TempTrack', (), {'name': title, 'artists': [artist], 'album': album})() + queries = engine.generate_download_queries(temp_track) + + best, best_conf, best_src = None, 0.0, None + attempted = False + for query in queries: + for source in source_priority: + client = get_client_for_source(source) + if not client or not hasattr(client, 'search_tracks'): + continue + attempted = True + matches = _search_tracks_for_source(source, query, limit=5, client=client) + time.sleep(0.5) # be gentle on metadata APIs + for cand in matches or []: + cand_artists = _track_artist_names(cand) + artist_conf = max( + (engine.similarity_score(engine.normalize_string(artist), + engine.normalize_string(n)) for n in cand_artists), + default=0.0, + ) + title_conf = engine.similarity_score( + engine.normalize_string(title), engine.normalize_string(_track_name(cand))) + conf = artist_conf * 0.5 + title_conf * 0.5 + album_type = _extract_lookup_value(cand, 'album_type', default='') or '' + if album_type == 'album': + conf += 0.02 + elif album_type == 'ep': + conf += 0.01 + if conf > best_conf and conf >= min_confidence: + best, best_conf, best_src = cand, conf, source + if best_conf >= 0.9: + break + if best_conf >= 0.9: + break + return best, best_conf, best_src, attempted + + +@register_job +class QualityUpgradeJob(RepairJob): + job_id = 'quality_upgrade' + display_name = 'Quality Upgrade Finder' + description = 'Finds library tracks below your preferred quality and proposes a better version' + help_text = ( + 'Scans your library (or just your watchlist artists) and compares each ' + "track against your Quality Profile using BOTH the file format and its " + 'bitrate — so a 128 kbps MP3 is no longer treated the same as a 320 kbps ' + 'one, and enabling MP3-320/256 in your profile actually counts.\n\n' + 'For every track below your preferred quality, it searches your configured ' + 'metadata source for a better version and creates a finding showing the ' + 'match and a confidence score. Nothing is queued automatically: applying a ' + 'finding adds that matched track — with its album context — to the wishlist, ' + 'the same as any other download.\n\n' + 'Settings:\n' + '- Scope: "watchlist" (watchlisted artists only) or "all" (whole library)\n' + '- Min confidence: minimum match confidence (0-1) to surface a finding\n\n' + 'Note: detecting fake/transcoded lossless files is handled by the separate ' + 'Fake Lossless Detector job.' + ) + icon = 'repair-icon-lossy' + default_enabled = False + default_interval_hours = 168 + default_settings = {'scope': 'watchlist', 'min_confidence': 0.7} + setting_options = {'scope': ['watchlist', 'all']} + auto_fix = False + + def _get_settings(self, context: JobContext) -> Dict[str, Any]: + cfg = context.config_manager + scope = 'watchlist' + min_conf = 0.7 + if cfg: + scope = cfg.get(self.get_config_key('settings.scope'), 'watchlist') or 'watchlist' + try: + min_conf = float(cfg.get(self.get_config_key('settings.min_confidence'), 0.7)) + except (TypeError, ValueError): + min_conf = 0.7 + return {'scope': scope, 'min_confidence': min_conf} + + def _load_tracks(self, db: Any, scope: str) -> List[tuple]: + conn = db._get_connection() + try: + base = ( + "SELECT t.id, t.title, t.file_path, t.bitrate, a.name AS artist_name, " + "al.title AS album_title, t.album_id " + "FROM tracks t " + "JOIN artists a ON t.artist_id = a.id " + "JOIN albums al ON t.album_id = al.id " + "WHERE t.file_path IS NOT NULL AND t.file_path != ''" + ) + if scope == 'watchlist': + artists = db.get_watchlist_artists(profile_id=1) + names = [getattr(ar, 'artist_name', None) for ar in artists] + names = [n for n in names if n] + if not names: + return [] + placeholders = ','.join('?' for _ in names) + rows = conn.execute( + base + f" AND a.name IN ({placeholders})", names).fetchall() + else: + rows = conn.execute(base).fetchall() + return rows + finally: + conn.close() + + def estimate_scope(self, context: JobContext) -> int: + try: + return len(self._load_tracks(context.db, self._get_settings(context)['scope'])) + except Exception: + return 0 + + def scan(self, context: JobContext) -> JobResult: + result = JobResult() + settings = self._get_settings(context) + scope = settings['scope'] + min_conf = settings['min_confidence'] + + db = context.db + quality_profile = db.get_quality_profile() + if preferred_quality_floor(quality_profile) is None: + logger.info("[Quality Upgrade] No quality buckets enabled in profile — nothing to flag") + return result + + try: + tracks = self._load_tracks(db, scope) + except Exception as e: + logger.error("[Quality Upgrade] Error loading tracks: %s", e, exc_info=True) + result.errors += 1 + return result + + total = len(tracks) + if context.update_progress: + context.update_progress(0, total) + if context.report_progress: + context.report_progress(phase=f'Checking quality on {total} tracks...', total=total) + + # Metadata source for matching — resolved lazily so we only fail if we + # actually find a low-quality track that needs a match. + engine = None + source_priority: List[str] = [] + + for i, row in enumerate(tracks): + if context.check_stop(): + return result + if i % 10 == 0 and context.wait_if_paused(): + return result + + track_id, title, file_path, bitrate, artist_name, album_title, album_id = ( + row[0], row[1], row[2], row[3], row[4], row[5], row[6]) + result.scanned += 1 + + if meets_preferred_quality(file_path, bitrate, quality_profile): + result.skipped += 1 + if context.update_progress and (i + 1) % 25 == 0: + context.update_progress(i + 1, total) + continue + + # Below preferred quality — find a better version to propose. + if engine is None: + from core.matching_engine import MusicMatchingEngine + engine = MusicMatchingEngine() + source_priority = get_source_priority(get_primary_source()) or [] + if not source_priority: + logger.warning("[Quality Upgrade] No metadata provider available — cannot propose upgrades") + return result + + if context.is_spotify_rate_limited(): + logger.info("[Quality Upgrade] Spotify rate-limited — stopping scan early") + return result + + current_rank = classify_track_quality(file_path, bitrate) + current_label = _rank_label(current_rank) + if context.report_progress: + context.report_progress( + scanned=i + 1, total=total, + log_line=f'Low quality ({current_label}): {artist_name} - {title}', + log_type='info') + + try: + best, conf, source, attempted = _find_best_match( + engine, source_priority, title, artist_name or '', album_title or '', min_conf) + except Exception as e: + logger.debug("[Quality Upgrade] Match error for %s - %s: %s", artist_name, title, e) + result.errors += 1 + continue + + if not attempted: + logger.warning("[Quality Upgrade] No metadata provider responded — stopping") + return result + if not best: + result.skipped += 1 + continue + + matched = _normalize_track_match(best, source or 'metadata') + # Carry album context: prefer the matched album, fall back to the + # library album the low-quality track came from. + alb = matched.get('album') + if (not isinstance(alb, dict) or not alb.get('name')) and album_title: + matched['album'] = {'name': album_title, 'images': (alb or {}).get('images', []) if isinstance(alb, dict) else []} + + if context.create_finding: + try: + inserted = context.create_finding( + job_id=self.job_id, + finding_type='quality_upgrade', + severity='info', + entity_type='track', + entity_id=str(track_id), + file_path=file_path, + title=f'Upgrade: {artist_name} - {title} ({current_label})', + description=( + f'"{title}" by {artist_name} is {current_label}, below your preferred ' + f'quality. Best match: "{_track_name(best)}" via {source} ' + f'(confidence {conf:.0%}). Apply to add it to the wishlist.'), + details={ + 'track_id': track_id, + 'track_title': title, + 'artist': artist_name, + 'album_id': album_id, + 'album_title': album_title, + 'current_format': current_label, + 'current_bitrate': bitrate, + 'match_confidence': conf, + 'provider': source, + 'matched_track_data': matched, + }) + if inserted: + result.findings_created += 1 + else: + result.findings_skipped_dedup += 1 + except Exception as e: + logger.debug("[Quality Upgrade] create finding failed for track %s: %s", track_id, e) + result.errors += 1 + + if context.update_progress and (i + 1) % 10 == 0: + context.update_progress(i + 1, total) + + if context.update_progress: + context.update_progress(total, total) + logger.info("[Quality Upgrade] %d scanned, %d upgrades found, %d met/skip", + result.scanned, result.findings_created, result.skipped) + return result diff --git a/core/repair_worker.py b/core/repair_worker.py index f57cba5d..519b04ed 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -981,6 +981,7 @@ class RepairWorker: 'acoustid_mismatch': self._fix_acoustid_mismatch, 'missing_discography_track': self._fix_discography_backfill, 'library_retag': self._fix_library_retag, + 'quality_upgrade': self._fix_quality_upgrade, } handler = handlers.get(finding_type) if not handler: @@ -1007,6 +1008,36 @@ class RepairWorker: except Exception as e: return {'success': False, 'error': str(e)} + def _fix_quality_upgrade(self, entity_type, entity_id, file_path, details): + """Add the matched higher-quality version to the wishlist (with album + context). Applying a Quality Upgrade finding is the user-approved step + that the old auto-acting Quality Scanner did without review.""" + track_data = details.get('matched_track_data') + if not track_data: + return {'success': False, 'error': 'No matched track in finding'} + try: + success = self.db.add_to_wishlist( + spotify_track_data=track_data, + failure_reason=f"Quality upgrade — current file is {details.get('current_format', 'low quality')}", + source_type='repair', + source_info={ + 'job': 'quality_upgrade', + 'original_file_path': file_path, + 'original_format': details.get('current_format'), + 'original_bitrate': details.get('current_bitrate'), + 'album_title': details.get('album_title'), + 'match_confidence': details.get('match_confidence'), + 'provider': details.get('provider'), + }, + ) + track_name = track_data.get('name', '?') + if success: + return {'success': True, 'action': 'added_to_wishlist', + 'message': f"Added '{track_name}' to wishlist for re-download"} + return {'success': False, 'error': f"Could not add '{track_name}' to wishlist (may already exist or be blocklisted)"} + except Exception as e: + return {'success': False, 'error': str(e)} + def _fix_dead_file(self, entity_type, entity_id, file_path, details): """Fix a dead file reference. Action depends on details['_fix_action']: 'redownload' (default) — add to wishlist + remove DB entry diff --git a/tests/repair_jobs/test_quality_upgrade.py b/tests/repair_jobs/test_quality_upgrade.py new file mode 100644 index 00000000..eb78a9bf --- /dev/null +++ b/tests/repair_jobs/test_quality_upgrade.py @@ -0,0 +1,221 @@ +"""Quality Upgrade Finder job — the findings-based replacement for the old +auto-acting Quality Scanner. + +The old tool judged quality by file EXTENSION only and used min() of the enabled +tiers, so with the default profile (FLAC + MP3-320 + MP3-256 enabled) it flagged +EVERY non-lossless file — a 320 kbps MP3 included — and dumped them all into the +wishlist with no review. These tests pin the corrected behavior: bitrate-aware, +honors every enabled bucket, and only proposes (findings) rather than auto-acting. +""" + +from __future__ import annotations + +import types + +import core.repair_jobs.quality_upgrade as qu +from core.repair_jobs.base import JobContext, JobResult + + +# Profiles ------------------------------------------------------------------ + +BALANCED = { # default: FLAC + MP3-320 + MP3-256 enabled, MP3-192 off + 'qualities': { + 'flac': {'enabled': True, 'min_kbps': 500}, + 'mp3_320': {'enabled': True, 'min_kbps': 280}, + 'mp3_256': {'enabled': True, 'min_kbps': 200}, + 'mp3_192': {'enabled': False, 'min_kbps': 150}, + } +} +LOSSLESS_ONLY = { + 'qualities': { + 'flac': {'enabled': True, 'min_kbps': 500}, + 'mp3_320': {'enabled': False, 'min_kbps': 280}, + 'mp3_256': {'enabled': False, 'min_kbps': 200}, + 'mp3_192': {'enabled': False, 'min_kbps': 150}, + } +} +NOTHING_ENABLED = {'qualities': {'flac': {'enabled': False}, 'mp3_320': {'enabled': False}}} + + +# --- pure quality decision ------------------------------------------------- + +def test_balanced_profile_accepts_320_mp3_REGRESSION(): + """The headline bug: with FLAC+320+256 enabled, a 320 kbps MP3 is acceptable. + The old min()-tier logic flagged it (and every other MP3) for re-download.""" + assert meets('song.mp3', 320, BALANCED) is True + + +def test_balanced_profile_accepts_256_mp3(): + assert meets('song.mp3', 256, BALANCED) is True + + +def test_balanced_profile_flags_low_bitrate_mp3(): + assert meets('song.mp3', 128, BALANCED) is False + assert meets('song.mp3', 192, BALANCED) is False # below the 256 floor + + +def test_flac_always_meets_when_flac_enabled(): + assert meets('song.flac', 900, BALANCED) is True + assert meets('song.flac', 900, LOSSLESS_ONLY) is True + + +def test_lossless_only_flags_every_lossy_regardless_of_bitrate(): + assert meets('song.mp3', 320, LOSSLESS_ONLY) is False + assert meets('song.m4a', 256, LOSSLESS_ONLY) is False + + +def test_nothing_enabled_flags_nothing(): + """Empty/disabled profile must NOT flag the whole library.""" + assert meets('song.mp3', 64, NOTHING_ENABLED) is True + + +def test_bitrate_in_bps_is_normalized(): + """Library bitrate stored as bps (320000) classifies the same as 320 kbps.""" + assert qu.classify_track_quality('song.mp3', 320000) == qu.RANK_320 + assert meets('song.mp3', 320000, BALANCED) is True + + +def test_unknown_lossy_bitrate_not_flagged_under_lossy_floor(): + """A lossy file with no bitrate can't be judged against a lossy floor → don't + flag (avoid false positives); but under a lossless floor it's clearly below.""" + assert meets('song.mp3', None, BALANCED) is True + assert meets('song.mp3', None, LOSSLESS_ONLY) is False + + +def test_floor_is_worst_enabled_not_best(): + # FLAC+320+256 enabled → floor is MP3-256 (rank 2), not FLAC. + assert qu.preferred_quality_floor(BALANCED) == qu.RANK_256 + assert qu.preferred_quality_floor(LOSSLESS_ONLY) == qu.RANK_LOSSLESS + assert qu.preferred_quality_floor(NOTHING_ENABLED) is None + + +def meets(path, bitrate, profile): + return qu.meets_preferred_quality(path, bitrate, profile) + + +# --- scan produces a finding (seam) ---------------------------------------- + +class _FakeConn: + def __init__(self, rows): + self._rows = rows + + def execute(self, *a, **k): + return self + + def fetchall(self): + return self._rows + + def close(self): + pass + + +class _FakeDB: + def __init__(self, rows, profile): + self._rows = rows + self._profile = profile + + def get_quality_profile(self): + return self._profile + + def _get_connection(self): + return _FakeConn(self._rows) + + def get_watchlist_artists(self, profile_id=1): + return [types.SimpleNamespace(artist_name='Artist A')] + + +def _ctx(db, findings): + return JobContext( + db=db, + transfer_folder='/tmp', + config_manager=None, + create_finding=lambda **kw: findings.append(kw) or True, + should_stop=lambda: False, + is_paused=lambda: False, + ) + + +def test_scan_creates_finding_for_low_quality_track(monkeypatch): + # One 128 kbps MP3 (below the balanced floor) for Artist A. + rows = [(1, 'Song One', '/music/a.mp3', 128, 'Artist A', 'Album X', 10)] + db = _FakeDB(rows, BALANCED) + + # Stub the metadata side so the test stays offline. + monkeypatch.setattr(qu, 'get_primary_source', lambda: 'spotify') + monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) + monkeypatch.setattr( + 'core.matching_engine.MusicMatchingEngine', + lambda: types.SimpleNamespace( + generate_download_queries=lambda t: ['q'], + similarity_score=lambda a, b: 1.0, + normalize_string=lambda s: s, + ), + ) + fake_match = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], + 'album': {'name': 'Album X', 'images': []}} + monkeypatch.setattr(qu, '_find_best_match', + lambda *a, **k: (fake_match, 0.95, 'spotify', True)) + monkeypatch.setattr(qu, '_normalize_track_match', lambda track, src: dict(fake_match)) + monkeypatch.setattr(qu, '_track_name', lambda t: 'Song One') + + findings = [] + job = qu.QualityUpgradeJob() + # default scope 'watchlist'; config_manager None → defaults used + result = job.scan(_ctx(db, findings)) + + assert result.findings_created == 1 + assert len(findings) == 1 + f = findings[0] + assert f['finding_type'] == 'quality_upgrade' + assert f['entity_id'] == '1' + # Album context + matched track carried for the apply step. + assert f['details']['matched_track_data']['id'] == 'sp1' + assert f['details']['album_title'] == 'Album X' + assert f['details']['provider'] == 'spotify' + + +def test_scan_skips_tracks_meeting_quality(monkeypatch): + # A 320 kbps MP3 meets the balanced profile → no finding, no metadata calls. + rows = [(2, 'Good Song', '/music/b.mp3', 320, 'Artist A', 'Album Y', 11)] + db = _FakeDB(rows, BALANCED) + + def _boom(*a, **k): # must never be called for an acceptable track + raise AssertionError("matching should not run for an acceptable track") + + monkeypatch.setattr(qu, '_find_best_match', _boom) + + findings = [] + result = qu.QualityUpgradeJob().scan(_ctx(db, findings)) + assert result.findings_created == 0 + assert result.skipped == 1 + assert findings == [] + + +# --- fix handler adds to wishlist ------------------------------------------ + +def test_fix_handler_adds_matched_track_to_wishlist(): + from core.repair_worker import RepairWorker + + captured = {} + + class _DB: + def add_to_wishlist(self, **kw): + captured.update(kw) + return True + + worker = object.__new__(RepairWorker) + worker.db = _DB() + + details = { + 'matched_track_data': {'id': 'sp1', 'name': 'Song One', + 'album': {'name': 'Album X'}}, + 'current_format': 'MP3 192', 'current_bitrate': 192, + 'album_title': 'Album X', 'provider': 'spotify', 'match_confidence': 0.9, + } + res = worker._fix_quality_upgrade('track', '1', '/music/a.mp3', details) + + assert res['success'] is True + assert captured['spotify_track_data']['id'] == 'sp1' + assert captured['source_type'] == 'repair' + assert captured['source_info']['job'] == 'quality_upgrade' + assert captured['source_info']['album_title'] == 'Album X' From b39386678264e073a292d260ddc542b71390a160 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 12:14:15 -0700 Subject: [PATCH 02/42] Remove old auto-acting Quality Scanner tool (replaced by Quality Upgrade Finder job) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 of the redesign. The tool that judged quality by extension and auto-dumped matches into the wishlist is gone; quality scanning is now the reviewed quality_upgrade repair job. Removed: - Frontend: Tools-page Quality Scanner card, its JS handlers/poller/socket listener, help tooltip + tour entry (webui index.html, core.js, helper.js, wishlist-tools.js). - Backend: /api/quality-scanner/{start,status,stop} endpoints, the in-memory state + executor + 1s socket broadcast, the QualityScannerDeps/run_quality_scanner shim. - core/discovery/quality_scanner.py: the auto-acting worker + deps class (the shared match/normalize helpers stay — the new job imports them). Rewired: - Automation 'start_quality_scan' action now triggers the quality_upgrade repair job via repair_worker.run_job_now() (AutomationDeps gains run_repair_job_now, drops the 4 scanner fields). Action block's vestigial scope field removed (scope lives in the job's settings now). NOTE: the 'quality_scan_completed' trigger no longer fires (the repair job doesn't emit it). - Updated all automation test _build_deps helpers + conftest tool-progress harness; deleted the obsolete worker test. 528 affected tests pass; 6123 collect cleanly. QUALITY_TIERS / _get_quality_tier_from_extension kept (used elsewhere). --- core/automation/blocks.py | 7 +- core/automation/deps.py | 8 +- core/automation/handlers/quality_scanner.py | 76 +-- core/automation/handlers/registration.py | 2 +- core/discovery/quality_scanner.py | 409 +-------------- tests/automation/test_handler_registration.py | 6 +- tests/automation/test_handlers_maintenance.py | 20 +- .../test_handlers_personalized_pipeline.py | 5 +- tests/automation/test_handlers_playlist.py | 5 +- tests/automation/test_handlers_simple.py | 5 +- .../test_playlist_pipeline_folder_mode.py | 5 +- tests/automation/test_progress_callbacks.py | 5 +- tests/conftest.py | 25 +- .../test_discovery_quality_scanner.py | 492 ------------------ tests/test_tool_progress_events.py | 42 +- web_server.py | 101 +--- webui/index.html | 43 -- webui/static/core.js | 2 - webui/static/helper.js | 13 +- webui/static/wishlist-tools.js | 184 +------ 20 files changed, 66 insertions(+), 1389 deletions(-) delete mode 100644 tests/discovery/test_discovery_quality_scanner.py diff --git a/core/automation/blocks.py b/core/automation/blocks.py index 2697af26..ecd88cd4 100644 --- a/core/automation/blocks.py +++ b/core/automation/blocks.py @@ -171,12 +171,7 @@ ACTIONS: list[dict] = [ {"type": "update_discovery_pool", "label": "Update Discovery", "icon": "compass", "description": "Refresh discovery pool with new tracks", "available": True}, {"type": "start_quality_scan", "label": "Run Quality Scan", "icon": "bar-chart", - "description": "Scan for low-quality audio files", "available": True, - "config_fields": [ - {"key": "scope", "type": "select", "label": "Scope", - "options": [{"value": "watchlist", "label": "Watchlist Artists"}, {"value": "library", "label": "Full Library"}], - "default": "watchlist"} - ]}, + "description": "Run the Quality Upgrade Finder (scope is set in Library Maintenance)", "available": True}, {"type": "backup_database", "label": "Backup Database", "icon": "save", "description": "Create timestamped database backup", "available": True}, {"type": "refresh_beatport_cache", "label": "Refresh Beatport Cache", "icon": "music", diff --git a/core/automation/deps.py b/core/automation/deps.py index 09c52a17..fb45bb42 100644 --- a/core/automation/deps.py +++ b/core/automation/deps.py @@ -123,10 +123,10 @@ class AutomationDeps: duplicate_cleaner_lock: Any duplicate_cleaner_executor: Any run_duplicate_cleaner: Callable[..., Any] - get_quality_scanner_state: Callable[[], dict] - quality_scanner_lock: Any - quality_scanner_executor: Any - run_quality_scanner: Callable[..., Any] + # Triggers a "Run Now" of a library-maintenance repair job by id (e.g. + # 'quality_upgrade'). Returns truthy if the job was queued. Replaces the old + # standalone quality-scanner executor/state (the scanner is now a repair job). + run_repair_job_now: Callable[[str], Any] # --- Download orchestrator + queue accessors --- download_orchestrator: Any diff --git a/core/automation/handlers/quality_scanner.py b/core/automation/handlers/quality_scanner.py index 69ec3f02..13f71ed9 100644 --- a/core/automation/handlers/quality_scanner.py +++ b/core/automation/handlers/quality_scanner.py @@ -1,83 +1,35 @@ """Automation handler: ``start_quality_scan`` action. -Lifted from ``web_server._register_automation_handlers`` (the -``_auto_start_quality_scan`` closure). Submits the quality scanner -to its executor with the configured scope (default: ``watchlist``) -then polls the shared state dict. +The quality scanner was redesigned from an auto-acting tool into the +``quality_upgrade`` library-maintenance repair job (findings-based, reviewed +before anything is wishlisted). This action now simply triggers a "Run Now" of +that job; its progress and findings surface in Library Maintenance. The action +name is kept so existing automation rules keep working. """ from __future__ import annotations -import time from typing import Any, Dict from core.automation.deps import AutomationDeps -_TIMEOUT_SECONDS = 7200 # 2 hours -_POLL_INTERVAL_SECONDS = 3 -_INITIAL_DELAY_SECONDS = 1 - - def auto_start_quality_scan(config: Dict[str, Any], deps: AutomationDeps) -> Dict[str, Any]: automation_id = config.get('_automation_id') - state = deps.get_quality_scanner_state() - if state.get('status') == 'running': - return {'status': 'skipped', 'reason': 'Quality scan already running'} - scope = config.get('scope', 'watchlist') - # Pre-set status before submit so the polling loop doesn't see a - # stale 'finished' from a previous run. - with deps.quality_scanner_lock: - state['status'] = 'running' - deps.quality_scanner_executor.submit(deps.run_quality_scanner, scope, deps.get_current_profile_id()) - deps.update_progress( - automation_id, log_line=f'Quality scan started (scope: {scope})', log_type='info', - ) - - # Monitor progress (max 2 hours). - time.sleep(_INITIAL_DELAY_SECONDS) - poll_start = time.time() - while time.time() - poll_start < _TIMEOUT_SECONDS: - time.sleep(_POLL_INTERVAL_SECONDS) - current_status = state.get('status', 'idle') - if current_status not in ('running',): - break + triggered = deps.run_repair_job_now('quality_upgrade') + if not triggered: deps.update_progress( - automation_id, - phase=state.get('phase', 'Scanning...'), - progress=state.get('progress', 0), - processed=state.get('processed', 0), - total=state.get('total', 0), - ) - else: - deps.update_progress( - automation_id, status='error', - phase='Timed out', log_line='Quality scan timed out after 2 hours', + automation_id, status='error', phase='Unavailable', + log_line='Quality Upgrade job could not be triggered (library worker unavailable)', log_type='error', ) - return {'status': 'error', 'reason': 'Timed out', '_manages_own_progress': True} + return {'status': 'error', 'reason': 'library worker unavailable', + '_manages_own_progress': True} - final_status = state.get('status', 'idle') - if final_status == 'error': - err = state.get('error_message', 'Unknown error') - deps.update_progress( - automation_id, status='error', progress=100, - phase='Error', log_line=err, log_type='error', - ) - return {'status': 'error', 'reason': err, '_manages_own_progress': True} - - issues = state.get('low_quality', 0) deps.update_progress( - automation_id, status='finished', progress=100, - phase='Complete', - log_line=f'Quality scan complete — {issues} issues found', + automation_id, status='finished', progress=100, phase='Triggered', + log_line='Quality Upgrade scan queued — findings appear in Library Maintenance', log_type='success', ) - return { - 'status': 'completed', 'scope': scope, '_manages_own_progress': True, - 'tracks_scanned': state.get('processed', 0), - 'quality_met': state.get('quality_met', 0), - 'low_quality': issues, - 'matched': state.get('matched', 0), - } + return {'status': 'completed', 'triggered': True, '_manages_own_progress': True} diff --git a/core/automation/handlers/registration.py b/core/automation/handlers/registration.py index 94f23db6..3afbe4df 100644 --- a/core/automation/handlers/registration.py +++ b/core/automation/handlers/registration.py @@ -136,7 +136,7 @@ def register_all(deps: AutomationDeps) -> None: engine.register_action_handler( 'start_quality_scan', lambda config: auto_start_quality_scan(config, deps), - lambda: deps.get_quality_scanner_state().get('status') == 'running', + lambda: False, # repair worker dedupes Run-Now requests itself ) engine.register_action_handler( 'backup_database', diff --git a/core/discovery/quality_scanner.py b/core/discovery/quality_scanner.py index 27457ca7..7c689a38 100644 --- a/core/discovery/quality_scanner.py +++ b/core/discovery/quality_scanner.py @@ -1,40 +1,23 @@ -"""Background worker for the library quality scanner. +"""Shared metadata match + result-normalization helpers for quality matching. -`run_quality_scanner(scope, profile_id, deps)` is the function the -quality-scanner endpoint kicks off in a thread to scan the library -for low-quality tracks (below the user's configured quality profile) -and add provider matches to the wishlist: +These were the matching guts of the old auto-acting quality-scanner worker (now +removed — quality scanning is the ``quality_upgrade`` library-maintenance repair +job in ``core/repair_jobs/quality_upgrade.py``). They're kept here as a single +source of truth and imported by that job: -1. Reset scanner state, load quality profile + minimum acceptable tier. -2. Load tracks from DB based on scope: - - 'watchlist' → tracks for watchlisted artists only. - - other → all library tracks. -3. For each track: - - Stop-request gate (state['status'] != 'running'). - - Quality-tier check via _get_quality_tier_from_extension(file_path). - - Skip tracks meeting standards (tier_num <= min_acceptable_tier). - - For low-quality tracks: matching_engine search query gen, score - candidates against the configured metadata source priority - (artist + title similarity, album-type bonus), pick best match >= - 0.7 confidence. - - On match: add normalized track data to wishlist via - `wishlist_service.add_track_to_wishlist` with - source_type='quality_scanner' and a source_context that captures - original file_path, format tier, bitrate, and match confidence. -4. After all tracks: status='finished', progress=100, activity feed - entry, emit `quality_scan_completed` event for automation engine. -5. On critical exception: status='error', error message captured. +- ``_search_tracks_for_source`` — query one metadata source's ``search_tracks``. +- ``_normalize_track_match`` / ``_normalize_track_album`` / ``_normalize_track_artists`` + — turn a provider track into the wishlist-ready dict (typed Album converters + with legacy duck-typed fallback). +- ``_track_name`` / ``_track_artist_names`` / ``_extract_lookup_value`` — accessors. """ from __future__ import annotations import logging -import time -from dataclasses import dataclass -from datetime import datetime from typing import Any, Callable, Dict, Optional -from core.metadata.registry import get_client_for_source, get_primary_source, get_source_priority +from core.metadata.registry import get_client_for_source from core.metadata.types import Album from core.wishlist.payloads import ensure_wishlist_track_format @@ -56,16 +39,6 @@ _TYPED_ALBUM_CONVERTERS: Dict[str, Callable[[Dict[str, Any]], Album]] = { } -@dataclass -class QualityScannerDeps: - """Bundle of cross-cutting deps the quality scanner needs.""" - quality_scanner_state: dict - quality_scanner_lock: Any # threading.Lock - QUALITY_TIERS: dict - matching_engine: Any - automation_engine: Any - get_quality_tier_from_extension: Callable - add_activity_item: Callable def _extract_lookup_value(value: Any, *names: str, default: Any = None) -> Any: @@ -300,363 +273,3 @@ def _search_tracks_for_source(source: str, query: str, limit: int = 5, client: A except Exception as exc: logger.debug("Could not search %s for %s: %s", source, query, exc) return [] - - -def run_quality_scanner(scope='watchlist', profile_id=1, deps: QualityScannerDeps = None): - """Main quality scanner worker function""" - from core.wishlist_service import get_wishlist_service - from database.music_database import MusicDatabase - - try: - with deps.quality_scanner_lock: - deps.quality_scanner_state["status"] = "running" - deps.quality_scanner_state["phase"] = "Initializing scan..." - deps.quality_scanner_state["progress"] = 0 - deps.quality_scanner_state["processed"] = 0 - deps.quality_scanner_state["total"] = 0 - deps.quality_scanner_state["quality_met"] = 0 - deps.quality_scanner_state["low_quality"] = 0 - deps.quality_scanner_state["matched"] = 0 - deps.quality_scanner_state["results"] = [] - deps.quality_scanner_state["error_message"] = "" - - logger.info(f"[Quality Scanner] Starting scan with scope: {scope}") - - # Get database instance - db = MusicDatabase() - - # Get quality profile to determine preferred quality - quality_profile = db.get_quality_profile() - preferred_qualities = quality_profile.get('qualities', {}) - - # Determine minimum acceptable tier based on enabled qualities - min_acceptable_tier = 999 - for quality_name, quality_config in preferred_qualities.items(): - if quality_config.get('enabled', False): - # Map quality profile names to tier names - tier_map = { - 'flac': 'lossless', - 'mp3_320': 'low_lossy', - 'mp3_256': 'low_lossy', - 'mp3_192': 'low_lossy' - } - tier_name = tier_map.get(quality_name) - if tier_name: - tier_num = deps.QUALITY_TIERS[tier_name]['tier'] - min_acceptable_tier = min(min_acceptable_tier, tier_num) - - logger.info(f"[Quality Scanner] Minimum acceptable tier: {min_acceptable_tier}") - - # Get tracks to scan based on scope - with deps.quality_scanner_lock: - deps.quality_scanner_state["phase"] = "Loading tracks from database..." - - if scope == 'watchlist': - # Get watchlist artists - watchlist_artists = db.get_watchlist_artists(profile_id=profile_id) - if not watchlist_artists: - with deps.quality_scanner_lock: - deps.quality_scanner_state["status"] = "finished" - deps.quality_scanner_state["phase"] = "No watchlist artists found" - deps.quality_scanner_state["error_message"] = "Please add artists to watchlist first" - logger.warning("[Quality Scanner] No watchlist artists found") - return - - # Get artist names from watchlist - artist_names = [artist.artist_name for artist in watchlist_artists] - logger.info(f"[Quality Scanner] Scanning {len(artist_names)} watchlist artists") - - # Get all tracks for these artists by name - conn = db._get_connection() - placeholders = ','.join(['?' for _ in artist_names]) - tracks_to_scan = conn.execute( - f"SELECT t.id, t.title, t.artist_id, t.album_id, t.file_path, t.bitrate, a.name as artist_name, al.title as album_title " - f"FROM tracks t " - f"JOIN artists a ON t.artist_id = a.id " - f"JOIN albums al ON t.album_id = al.id " - f"WHERE a.name IN ({placeholders}) AND t.file_path IS NOT NULL", - artist_names - ).fetchall() - conn.close() - else: - # Scan all library tracks - with deps.quality_scanner_lock: - deps.quality_scanner_state["phase"] = "Loading all library tracks..." - - conn = db._get_connection() - tracks_to_scan = conn.execute( - "SELECT t.id, t.title, t.artist_id, t.album_id, t.file_path, t.bitrate, a.name as artist_name, al.title as album_title " - "FROM tracks t " - "JOIN artists a ON t.artist_id = a.id " - "JOIN albums al ON t.album_id = al.id " - "WHERE t.file_path IS NOT NULL" - ).fetchall() - conn.close() - - total_tracks = len(tracks_to_scan) - logger.info(f"[Quality Scanner] Found {total_tracks} tracks to scan") - - with deps.quality_scanner_lock: - deps.quality_scanner_state["total"] = total_tracks - deps.quality_scanner_state["phase"] = f"Scanning {total_tracks} tracks..." - - source_priority = get_source_priority(get_primary_source()) - if not source_priority: - with deps.quality_scanner_lock: - deps.quality_scanner_state["status"] = "error" - deps.quality_scanner_state["phase"] = "No metadata provider available" - deps.quality_scanner_state["error_message"] = "No metadata provider is available for quality scanning" - logger.info("[Quality Scanner] No metadata provider available") - return - - logger.info("[Quality Scanner] Using metadata source priority: %s", source_priority) - - wishlist_service = get_wishlist_service() - add_to_wishlist = getattr(wishlist_service, 'add_track_to_wishlist', None) - if add_to_wishlist is None: - add_to_wishlist = getattr(wishlist_service, 'add_spotify_track_to_wishlist', None) - if add_to_wishlist is None: - raise AttributeError("Wishlist service does not expose an add-to-wishlist method") - - # Scan each track - for idx, track_row in enumerate(tracks_to_scan, 1): - # Check for stop request - if deps.quality_scanner_state.get('status') != 'running': - logger.info(f"[Quality Scanner] Stop requested, halting at track {idx}/{total_tracks}") - break - - try: - track_id, title, artist_id, album_id, file_path, bitrate, artist_name, album_title = track_row - - # Check quality tier - tier_name, tier_num = deps.get_quality_tier_from_extension(file_path) - - # Update progress - with deps.quality_scanner_lock: - deps.quality_scanner_state["processed"] = idx - deps.quality_scanner_state["progress"] = (idx / total_tracks) * 100 - deps.quality_scanner_state["phase"] = f"Scanning: {artist_name} - {title}" - - # Check if meets quality standards - if tier_num <= min_acceptable_tier: - # Quality met - with deps.quality_scanner_lock: - deps.quality_scanner_state["quality_met"] += 1 - continue - - # Low quality track found - with deps.quality_scanner_lock: - deps.quality_scanner_state["low_quality"] += 1 - - logger.info(f"[Quality Scanner] Low quality: {artist_name} - {title} ({tier_name}, {file_path})") - - # Attempt to match using the active metadata provider - matched = False - matched_track_data = None - best_source = None - attempted_any_provider = False - - try: - # Generate search queries using matching engine - temp_track = type('TempTrack', (), { - 'name': title, - 'artists': [artist_name], - 'album': album_title - })() - - search_queries = deps.matching_engine.generate_download_queries(temp_track) - logger.info(f"[Quality Scanner] Generated {len(search_queries)} search queries for {artist_name} - {title}") - - # Find best match using confidence scoring - best_match = None - best_confidence = 0.0 - min_confidence = 0.7 # Match existing standard - - for _query_idx, search_query in enumerate(search_queries): - try: - for source in source_priority: - client = get_client_for_source(source) - if not client or not hasattr(client, 'search_tracks'): - continue - - attempted_any_provider = True - provider_matches = _search_tracks_for_source(source, search_query, limit=5, client=client) - time.sleep(0.5) # Rate limit metadata API calls - - if not provider_matches: - continue - - # Score each result using matching engine - for provider_track in provider_matches: - try: - # Calculate artist confidence - artist_confidence = 0.0 - provider_artists = _track_artist_names(provider_track) - if provider_artists: - for result_artist in provider_artists: - artist_sim = deps.matching_engine.similarity_score( - deps.matching_engine.normalize_string(artist_name), - deps.matching_engine.normalize_string(result_artist) - ) - artist_confidence = max(artist_confidence, artist_sim) - - # Calculate title confidence - title_confidence = deps.matching_engine.similarity_score( - deps.matching_engine.normalize_string(title), - deps.matching_engine.normalize_string(_track_name(provider_track)) - ) - - # Combined confidence (50% artist + 50% title) - combined_confidence = (artist_confidence * 0.5 + title_confidence * 0.5) - - # Small bonus for album tracks over singles - _at = _extract_lookup_value(provider_track, 'album_type', default='') or '' - if _at == 'album': - combined_confidence += 0.02 - elif _at == 'ep': - combined_confidence += 0.01 - - candidate_artist = provider_artists[0] if provider_artists else 'Unknown Artist' - candidate_name = _track_name(provider_track) - logger.info( - f"[Quality Scanner] Candidate ({source}): '{candidate_artist}' - " - f"'{candidate_name}' (confidence: {combined_confidence:.3f})" - ) - - # Update best match if this is better - if combined_confidence > best_confidence and combined_confidence >= min_confidence: - best_confidence = combined_confidence - best_match = provider_track - best_source = source - logger.info( - f"[Quality Scanner] New best match ({source}): {candidate_artist} - " - f"{candidate_name} (confidence: {combined_confidence:.3f})" - ) - - except Exception as e: - logger.error(f"[Quality Scanner] Error scoring result: {e}") - continue - - # If we found a very high confidence match, stop searching this query - if best_confidence >= 0.9: - logger.info(f"[Quality Scanner] High confidence match found ({best_confidence:.3f}), stopping search") - break - - except Exception as e: - logger.debug(f"[Quality Scanner] Error searching with query '{search_query}': {e}") - continue - - if not attempted_any_provider: - with deps.quality_scanner_lock: - deps.quality_scanner_state["status"] = "error" - deps.quality_scanner_state["phase"] = "No metadata provider available" - deps.quality_scanner_state["error_message"] = "No metadata provider is available for quality scanning" - logger.info("[Quality Scanner] No metadata provider available") - return - - # Process best match - if best_match: - matched = True - final_artist = _track_artist_names(best_match)[0] if _track_artist_names(best_match) else 'Unknown Artist' - final_name = _track_name(best_match) - final_source = best_source or 'metadata' - logger.info( - f"[Quality Scanner] Final match ({final_source}): {final_artist} - " - f"{final_name} (confidence: {best_confidence:.3f})" - ) - - # Build normalized track data for wishlist - matched_track_data = _normalize_track_match(best_match, final_source) - - # Add to wishlist - source_context = { - 'quality_scanner': True, - 'original_file_path': file_path, - 'original_format': tier_name, - 'original_bitrate': bitrate, - 'match_confidence': best_confidence, - 'scan_date': datetime.now().isoformat() - } - - success = add_to_wishlist( - track_data=matched_track_data, - failure_reason=f"Low quality - {tier_name.replace('_', ' ').title()} format", - source_type='quality_scanner', - source_context=source_context, - profile_id=profile_id - ) - - if success: - with deps.quality_scanner_lock: - deps.quality_scanner_state["matched"] += 1 - logger.info(f"[Quality Scanner] Matched and added to wishlist: {artist_name} - {title}") - else: - logger.error(f"[Quality Scanner] Failed to add to wishlist: {artist_name} - {title}") - else: - logger.warning( - f"[Quality Scanner] No suitable metadata match found " - f"(best confidence: {best_confidence:.3f}, required: {min_confidence:.3f})" - ) - - except Exception as matching_error: - logger.error(f"[Quality Scanner] Matching error for {artist_name} - {title}: {matching_error}") - - # Store result - result_entry = { - 'track_id': track_id, - 'title': title, - 'artist': artist_name, - 'album': album_title, - 'file_path': file_path, - 'current_format': tier_name, - 'bitrate': bitrate, - 'matched': matched, - 'match_id': matched_track_data['id'] if matched_track_data else None, - 'provider': best_source if matched else None, - 'spotify_id': matched_track_data['id'] if matched_track_data else None, - } - - with deps.quality_scanner_lock: - deps.quality_scanner_state["results"].append(result_entry) - - if not matched: - logger.warning(f"[Quality Scanner] No metadata match found for: {artist_name} - {title}") - - except Exception as track_error: - logger.error(f"[Quality Scanner] Error processing track: {track_error}") - continue - - # Scan complete (don't overwrite if already stopped by user) - with deps.quality_scanner_lock: - was_stopped = deps.quality_scanner_state["status"] != "running" - deps.quality_scanner_state["status"] = "finished" - deps.quality_scanner_state["progress"] = 100 - if not was_stopped: - deps.quality_scanner_state["phase"] = "Scan complete" - - logger.info(f"[Quality Scanner] Scan {'stopped' if was_stopped else 'complete'}: {deps.quality_scanner_state['processed']} processed, " - f"{deps.quality_scanner_state['low_quality']} low quality, {deps.quality_scanner_state['matched']} matched to metadata providers") - - # Add activity - deps.add_activity_item("", "Quality Scan Complete", - f"{deps.quality_scanner_state['matched']} tracks added to wishlist", "Now") - - try: - if deps.automation_engine: - deps.automation_engine.emit('quality_scan_completed', { - 'quality_met': str(deps.quality_scanner_state.get('quality_met', 0)), - 'low_quality': str(deps.quality_scanner_state.get('low_quality', 0)), - 'total_scanned': str(deps.quality_scanner_state.get('processed', 0)), - }) - except Exception as e: - logger.debug("emit quality_scan_completed failed: %s", e) - - except Exception as e: - logger.error(f"[Quality Scanner] Critical error: {e}") - import traceback - traceback.print_exc() - - with deps.quality_scanner_lock: - deps.quality_scanner_state["status"] = "error" - deps.quality_scanner_state["error_message"] = str(e) - deps.quality_scanner_state["phase"] = f"Error: {str(e)}" diff --git a/tests/automation/test_handler_registration.py b/tests/automation/test_handler_registration.py index 357ebc06..7b23c67a 100644 --- a/tests/automation/test_handler_registration.py +++ b/tests/automation/test_handler_registration.py @@ -142,10 +142,7 @@ def _build_deps(engine, scan_mgr=None) -> AutomationDeps: duplicate_cleaner_lock=threading.Lock(), duplicate_cleaner_executor=None, run_duplicate_cleaner=lambda: None, - get_quality_scanner_state=lambda: {}, - quality_scanner_lock=threading.Lock(), - quality_scanner_executor=None, - run_quality_scanner=lambda *a, **k: None, + run_repair_job_now=lambda *a, **k: True, download_orchestrator=None, run_async=lambda coro: None, tasks_lock=threading.Lock(), @@ -360,7 +357,6 @@ class TestHandlerInvocation: **{f.name: getattr(deps, f.name) for f in deps.__dataclass_fields__.values()}, 'get_db_update_state': lambda: running_state, 'get_duplicate_cleaner_state': lambda: running_state, - 'get_quality_scanner_state': lambda: running_state, 'get_download_batches': lambda: active_batches, # forces clean_completed_downloads to skip 'get_database': lambda: _StubDB(), 'get_app': lambda: _StubApp(), diff --git a/tests/automation/test_handlers_maintenance.py b/tests/automation/test_handlers_maintenance.py index e9c62a05..36ca357b 100644 --- a/tests/automation/test_handlers_maintenance.py +++ b/tests/automation/test_handlers_maintenance.py @@ -89,10 +89,7 @@ def _build_deps(**overrides) -> AutomationDeps: duplicate_cleaner_lock=threading.Lock(), duplicate_cleaner_executor=None, run_duplicate_cleaner=lambda: None, - get_quality_scanner_state=lambda: {}, - quality_scanner_lock=threading.Lock(), - quality_scanner_executor=None, - run_quality_scanner=lambda *a, **k: None, + run_repair_job_now=lambda *a, **k: True, download_orchestrator=None, run_async=lambda coro: None, tasks_lock=threading.Lock(), @@ -185,11 +182,18 @@ class TestDuplicateCleaner: class TestQualityScanner: - def test_already_running_returns_skipped(self): - state = {'status': 'running'} - deps = _build_deps(get_quality_scanner_state=lambda: state) + def test_triggers_quality_upgrade_repair_job(self): + triggered = [] + deps = _build_deps(run_repair_job_now=lambda job_id: triggered.append(job_id) or True) result = auto_start_quality_scan({}, deps) - assert result == {'status': 'skipped', 'reason': 'Quality scan already running'} + assert triggered == ['quality_upgrade'] + assert result['status'] == 'completed' + assert result['triggered'] is True + + def test_error_when_worker_unavailable(self): + deps = _build_deps(run_repair_job_now=lambda job_id: None) + result = auto_start_quality_scan({}, deps) + assert result['status'] == 'error' # ─── clear_quarantine ──────────────────────────────────────────────── diff --git a/tests/automation/test_handlers_personalized_pipeline.py b/tests/automation/test_handlers_personalized_pipeline.py index af91bb03..019a2250 100644 --- a/tests/automation/test_handlers_personalized_pipeline.py +++ b/tests/automation/test_handlers_personalized_pipeline.py @@ -63,10 +63,7 @@ def _build_deps(**overrides) -> AutomationDeps: duplicate_cleaner_lock=threading.Lock(), duplicate_cleaner_executor=None, run_duplicate_cleaner=lambda: None, - get_quality_scanner_state=lambda: {}, - quality_scanner_lock=threading.Lock(), - quality_scanner_executor=None, - run_quality_scanner=lambda *a, **k: None, + run_repair_job_now=lambda *a, **k: True, download_orchestrator=None, run_async=lambda coro: None, tasks_lock=threading.Lock(), diff --git a/tests/automation/test_handlers_playlist.py b/tests/automation/test_handlers_playlist.py index e13179e3..a905c1e7 100644 --- a/tests/automation/test_handlers_playlist.py +++ b/tests/automation/test_handlers_playlist.py @@ -125,10 +125,7 @@ def _build_deps(**overrides) -> AutomationDeps: duplicate_cleaner_lock=threading.Lock(), duplicate_cleaner_executor=None, run_duplicate_cleaner=lambda: None, - get_quality_scanner_state=lambda: {}, - quality_scanner_lock=threading.Lock(), - quality_scanner_executor=None, - run_quality_scanner=lambda *a, **k: None, + run_repair_job_now=lambda *a, **k: True, download_orchestrator=None, run_async=lambda coro: None, tasks_lock=threading.Lock(), diff --git a/tests/automation/test_handlers_simple.py b/tests/automation/test_handlers_simple.py index 9320b96d..f34d3004 100644 --- a/tests/automation/test_handlers_simple.py +++ b/tests/automation/test_handlers_simple.py @@ -76,10 +76,7 @@ def _build_deps(**overrides: Any) -> AutomationDeps: duplicate_cleaner_lock=threading.Lock(), duplicate_cleaner_executor=None, run_duplicate_cleaner=lambda: None, - get_quality_scanner_state=lambda: {}, - quality_scanner_lock=threading.Lock(), - quality_scanner_executor=None, - run_quality_scanner=lambda *a, **k: None, + run_repair_job_now=lambda *a, **k: True, download_orchestrator=None, run_async=lambda coro: None, tasks_lock=threading.Lock(), diff --git a/tests/automation/test_playlist_pipeline_folder_mode.py b/tests/automation/test_playlist_pipeline_folder_mode.py index 9c412fce..e727ec95 100644 --- a/tests/automation/test_playlist_pipeline_folder_mode.py +++ b/tests/automation/test_playlist_pipeline_folder_mode.py @@ -41,10 +41,7 @@ def _minimal_deps(**overrides): duplicate_cleaner_lock=threading.Lock(), duplicate_cleaner_executor=None, run_duplicate_cleaner=lambda: None, - get_quality_scanner_state=lambda: {}, - quality_scanner_lock=threading.Lock(), - quality_scanner_executor=None, - run_quality_scanner=lambda *a, **k: None, + run_repair_job_now=lambda *a, **k: True, download_orchestrator=None, run_async=lambda coro: None, tasks_lock=threading.Lock(), diff --git a/tests/automation/test_progress_callbacks.py b/tests/automation/test_progress_callbacks.py index 16728255..cdceac05 100644 --- a/tests/automation/test_progress_callbacks.py +++ b/tests/automation/test_progress_callbacks.py @@ -62,10 +62,7 @@ def _build_deps(**overrides) -> AutomationDeps: duplicate_cleaner_lock=threading.Lock(), duplicate_cleaner_executor=None, run_duplicate_cleaner=lambda: None, - get_quality_scanner_state=lambda: {}, - quality_scanner_lock=threading.Lock(), - quality_scanner_executor=None, - run_quality_scanner=lambda *a, **k: None, + run_repair_job_now=lambda *a, **k: True, download_orchestrator=None, run_async=lambda coro: None, tasks_lock=threading.Lock(), diff --git a/tests/conftest.py b/tests/conftest.py index 75c3de68..288f439b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -113,12 +113,6 @@ _DEFAULT_STREAM_STATE = { "error_message": None, } -_DEFAULT_QUALITY_SCANNER_STATE = { - "status": "running", "phase": "Scanning...", "progress": 35, - "processed": 35, "total": 100, "quality_met": 30, - "low_quality": 5, "matched": 2, "error_message": "", "results": [], -} - _DEFAULT_DUPLICATE_CLEANER_STATE = { "status": "running", "phase": "Scanning...", "progress": 50, "files_scanned": 500, "total_files": 1000, "duplicates_found": 10, @@ -257,7 +251,6 @@ enrichment_status = copy.deepcopy(_DEFAULT_ENRICHMENT_STATUS) # Phase 4: Tool progress state stream_state = copy.deepcopy(_DEFAULT_STREAM_STATE) -quality_scanner_state = copy.deepcopy(_DEFAULT_QUALITY_SCANNER_STATE) duplicate_cleaner_state = copy.deepcopy(_DEFAULT_DUPLICATE_CLEANER_STATE) retag_state = copy.deepcopy(_DEFAULT_RETAG_STATE) db_update_state = copy.deepcopy(_DEFAULT_DB_UPDATE_STATE) @@ -350,13 +343,12 @@ ENRICHMENT_ENDPOINTS = { # Phase 4 helpers TOOL_NAMES = [ - 'stream', 'quality-scanner', 'duplicate-cleaner', + 'stream', 'duplicate-cleaner', 'retag', 'db-update', 'metadata', 'logs', ] TOOL_ENDPOINTS = { 'stream': '/api/stream/status', - 'quality-scanner': '/api/quality-scanner/status', 'duplicate-cleaner': '/api/duplicate-cleaner/status', 'retag': '/api/retag/status', 'db-update': '/api/database/update/status', @@ -374,10 +366,6 @@ def _build_stream_status(): } -def _build_quality_scanner_status(): - return dict(quality_scanner_state) - - def _build_duplicate_cleaner_status(): state_copy = duplicate_cleaner_state.copy() state_copy["space_freed_mb"] = duplicate_cleaner_state["space_freed"] / (1024 * 1024) @@ -419,7 +407,6 @@ def _build_tool_status(tool_name): """Dispatcher that returns the correct status payload for any tool.""" builders = { 'stream': _build_stream_status, - 'quality-scanner': _build_quality_scanner_status, 'duplicate-cleaner': _build_duplicate_cleaner_status, 'retag': _build_retag_status, 'db-update': _build_db_update_status, @@ -635,10 +622,6 @@ def test_app(): def stream_status_endpoint(): return jsonify(_build_stream_status()) - @app.route('/api/quality-scanner/status') - def quality_scanner_status_endpoint(): - return jsonify(_build_quality_scanner_status()) - @app.route('/api/duplicate-cleaner/status') def duplicate_cleaner_status_endpoint(): return jsonify(_build_duplicate_cleaner_status()) @@ -961,7 +944,6 @@ def shared_state(): 'enrichment_endpoints': ENRICHMENT_ENDPOINTS, # Phase 4 state 'stream_state': stream_state, - 'quality_scanner_state': quality_scanner_state, 'duplicate_cleaner_state': duplicate_cleaner_state, 'retag_state': retag_state, 'db_update_state': db_update_state, @@ -969,7 +951,6 @@ def shared_state(): 'logs_activities': logs_activities, 'build_tool_status': _build_tool_status, 'build_stream_status': _build_stream_status, - 'build_quality_scanner_status': _build_quality_scanner_status, 'build_duplicate_cleaner_status': _build_duplicate_cleaner_status, 'build_retag_status': _build_retag_status, 'build_db_update_status': _build_db_update_status, @@ -1019,8 +1000,6 @@ def reset_state(): # Phase 4 resets stream_state.clear() stream_state.update(copy.deepcopy(_DEFAULT_STREAM_STATE)) - quality_scanner_state.clear() - quality_scanner_state.update(copy.deepcopy(_DEFAULT_QUALITY_SCANNER_STATE)) duplicate_cleaner_state.clear() duplicate_cleaner_state.update(copy.deepcopy(_DEFAULT_DUPLICATE_CLEANER_STATE)) retag_state.clear() @@ -1061,8 +1040,6 @@ def reset_state(): enrichment_status.update(copy.deepcopy(_DEFAULT_ENRICHMENT_STATUS)) stream_state.clear() stream_state.update(copy.deepcopy(_DEFAULT_STREAM_STATE)) - quality_scanner_state.clear() - quality_scanner_state.update(copy.deepcopy(_DEFAULT_QUALITY_SCANNER_STATE)) duplicate_cleaner_state.clear() duplicate_cleaner_state.update(copy.deepcopy(_DEFAULT_DUPLICATE_CLEANER_STATE)) retag_state.clear() diff --git a/tests/discovery/test_discovery_quality_scanner.py b/tests/discovery/test_discovery_quality_scanner.py deleted file mode 100644 index f3395ad4..00000000 --- a/tests/discovery/test_discovery_quality_scanner.py +++ /dev/null @@ -1,492 +0,0 @@ -"""Tests for core/discovery/quality_scanner.py — library quality scanner.""" - -from __future__ import annotations - -import threading -from dataclasses import dataclass - -import pytest - -from core.discovery import quality_scanner as qs - - -# --------------------------------------------------------------------------- -# Fakes -# --------------------------------------------------------------------------- - -@dataclass -class _FakeSpotifyTrack: - id: str = 'spt-1' - name: str = 'Found' - artists: list = None - album: str = 'Found Album' - duration_ms: int = 200000 - popularity: int = 50 - preview_url: str = '' - external_urls: dict = None - album_type: str = 'album' - release_date: str = '2024-01-01' - - def __post_init__(self): - if self.artists is None: - self.artists = ['Found Artist'] - if self.external_urls is None: - self.external_urls = {} - - -class _FakeMetadataClient: - def __init__(self, results=None): - self._results = results if results is not None else [] - self.search_calls = [] - - def search_tracks(self, query, limit=5, allow_fallback=True): - self.search_calls.append((query, limit, allow_fallback)) - return self._results - - -_TEST_PRIMARY_SOURCE = 'spotify' -_TEST_SOURCE_CLIENTS = {} - - -@pytest.fixture(autouse=True) -def _patch_source_resolution(monkeypatch): - monkeypatch.setattr(qs, 'get_primary_source', lambda: _TEST_PRIMARY_SOURCE) - monkeypatch.setattr(qs, 'get_client_for_source', lambda source, **_kwargs: _TEST_SOURCE_CLIENTS.get(source)) - monkeypatch.setattr(qs.time, 'sleep', lambda *_args, **_kwargs: None) - yield - _TEST_SOURCE_CLIENTS.clear() - globals()['_TEST_PRIMARY_SOURCE'] = 'spotify' - - -class _FakeMatchingEngine: - def generate_download_queries(self, t): - return [f"{t.artists[0]} {t.name}"] - - def normalize_string(self, s): - return (s or '').lower().strip() - - def similarity_score(self, a, b): - if a == b: - return 1.0 - if not a or not b: - return 0.0 - return 0.95 if a in b or b in a else 0.0 - - -class _MultiQueryMatchingEngine(_FakeMatchingEngine): - def generate_download_queries(self, t): - return [ - f"{t.artists[0]} {t.name} first", - f"{t.artists[0]} {t.name} second", - ] - - -class _FakeAutomationEngine: - def __init__(self): - self.events = [] - - def emit(self, event_type, data): - self.events.append((event_type, data)) - - -class _FakeWishlistService: - def __init__(self): - self.added = [] - - def add_spotify_track_to_wishlist(self, **kwargs): - self.added.append(kwargs) - return True - - -class _FakeMusicDB: - def __init__(self, watchlist_artists=None, tracks=None, profile=None): - self._watchlist_artists = watchlist_artists if watchlist_artists is not None else [] - self._tracks = tracks if tracks is not None else [] - self._profile = profile or {'qualities': {'flac': {'enabled': True}}} - - def get_quality_profile(self): - return self._profile - - def get_watchlist_artists(self, profile_id=1): - return self._watchlist_artists - - def _get_connection(self): - rows = self._tracks - return _FakeConn(rows) - - -class _FakeConn: - def __init__(self, rows): - self._rows = rows - - def execute(self, query, params=None): - return _FakeCursor(self._rows) - - def close(self): - pass - - -class _FakeCursor: - def __init__(self, rows): - self._rows = rows - - def fetchall(self): - return self._rows - - -@dataclass -class _WatchlistArtist: - artist_name: str - - -def _build_deps( - *, - state=None, - source_clients=None, - primary_source='spotify', - quality_tier_result=('lossless', 1), - automation=None, -): - globals()['_TEST_PRIMARY_SOURCE'] = primary_source - _TEST_SOURCE_CLIENTS.clear() - if source_clients is not None: - _TEST_SOURCE_CLIENTS.update(source_clients) - elif primary_source: - _TEST_SOURCE_CLIENTS[primary_source] = _FakeMetadataClient(results=[]) - - deps = qs.QualityScannerDeps( - quality_scanner_state=state if state is not None else {}, - quality_scanner_lock=threading.Lock(), - QUALITY_TIERS={'lossless': {'tier': 1}, 'low_lossy': {'tier': 4}, 'lossy': {'tier': 3}}, - matching_engine=_FakeMatchingEngine(), - automation_engine=automation or _FakeAutomationEngine(), - get_quality_tier_from_extension=lambda fp: quality_tier_result, - add_activity_item=lambda *a, **kw: None, - ) - return deps - - -def _track_row(track_id=1, title='Track', artist_id=1, album_id=1, - file_path='/x.mp3', bitrate=128, artist_name='Artist', - album_title='Album'): - return (track_id, title, artist_id, album_id, file_path, bitrate, artist_name, album_title) - - -@pytest.fixture -def mock_db_and_wishlist(monkeypatch): - """Patches MusicDatabase and get_wishlist_service used inside the worker.""" - db = _FakeMusicDB() - ws = _FakeWishlistService() - monkeypatch.setattr('database.music_database.MusicDatabase', lambda: db) - monkeypatch.setattr('core.wishlist_service.get_wishlist_service', lambda: ws) - return db, ws - - -# --------------------------------------------------------------------------- -# State init + DB load -# --------------------------------------------------------------------------- - -def test_state_initialized_on_run(mock_db_and_wishlist): - """Scanner resets state to running with cleared counters.""" - db, _ = mock_db_and_wishlist - db._watchlist_artists = [] # no artists → exits early but after init - state = {} - deps = _build_deps(state=state) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert state['status'] == 'finished' # exited early since no artists - assert state['error_message'] == 'Please add artists to watchlist first' - - -def test_no_watchlist_artists_short_circuit(mock_db_and_wishlist): - """Scope=watchlist with no artists → status=finished, error message.""" - db, _ = mock_db_and_wishlist - db._watchlist_artists = [] - state = {} - deps = _build_deps(state=state) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert state['status'] == 'finished' - assert 'add artists' in state['error_message'] - - -# --------------------------------------------------------------------------- -# Provider availability gate -# --------------------------------------------------------------------------- - -def test_no_available_provider_marks_error(mock_db_and_wishlist): - """No available metadata providers → state['status']='error'.""" - db, _ = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('A')] - db._tracks = [_track_row()] - state = {} - deps = _build_deps(state=state, source_clients={}, quality_tier_result=('low_lossy', 4)) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert state['status'] == 'error' - assert 'metadata provider' in state['error_message'].lower() - - -# --------------------------------------------------------------------------- -# Quality tier check + skip -# --------------------------------------------------------------------------- - -def test_high_quality_tracks_skipped(mock_db_and_wishlist): - """Tracks meeting quality (tier_num <= min_acceptable) → quality_met += 1.""" - db, _ = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('A')] - db._tracks = [_track_row(file_path='/x.flac')] - state = {} - # Default min_acceptable is from {flac: enabled} → tier 1 (lossless) - # quality_tier_result=('lossless', 1) → 1 <= 1 → skip - deps = _build_deps(state=state, quality_tier_result=('lossless', 1)) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert state['quality_met'] == 1 - assert state['low_quality'] == 0 - - -def test_low_quality_tracks_attempted(mock_db_and_wishlist): - """Low-quality tracks (tier_num > min) trigger a metadata search.""" - db, _ = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('Artist')] - db._tracks = [_track_row(file_path='/x.mp3', artist_name='Artist', title='Track')] - state = {} - match = _FakeSpotifyTrack(name='Track', artists=['Artist']) - spotify_client = _FakeMetadataClient(results=[match]) - deps = _build_deps( - state=state, - quality_tier_result=('low_lossy', 4), - source_clients={'spotify': spotify_client}, - primary_source='spotify', - ) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert state['low_quality'] == 1 - assert spotify_client.search_calls - assert spotify_client.search_calls[0][2] is False - - -def test_client_lookup_happens_per_query(mock_db_and_wishlist, monkeypatch): - """Each generated query re-resolves the source client.""" - db, _ = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('Artist')] - db._tracks = [_track_row(file_path='/x.mp3', artist_name='Artist', title='Track')] - state = {} - spotify_client = _FakeMetadataClient(results=[]) - lookups = [] - - monkeypatch.setattr( - qs, - 'get_client_for_source', - lambda source, **_kwargs: (lookups.append(source), _TEST_SOURCE_CLIENTS.get(source))[1], - ) - - deps = _build_deps( - state=state, - quality_tier_result=('low_lossy', 4), - source_clients={'spotify': spotify_client}, - primary_source='spotify', - ) - deps.matching_engine = _MultiQueryMatchingEngine() - - qs.run_quality_scanner('watchlist', 1, deps) - - assert len(lookups) % 2 == 0 - midpoint = len(lookups) // 2 - assert lookups[:midpoint] == lookups[midpoint:] - assert len(spotify_client.search_calls) == 2 - - -def test_low_quality_tracks_follow_source_priority(mock_db_and_wishlist): - """Primary source is searched before Spotify.""" - db, ws = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('Artist')] - db._tracks = [_track_row(file_path='/x.mp3', artist_name='Artist', title='Track')] - state = {} - match = _FakeSpotifyTrack(name='Track', artists=['Artist']) - deezer_client = _FakeMetadataClient(results=[match]) - spotify_client = _FakeMetadataClient(results=[]) - deps = _build_deps( - state=state, - source_clients={'deezer': deezer_client, 'spotify': spotify_client}, - primary_source='deezer', - quality_tier_result=('low_lossy', 4), - ) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert state['matched'] == 1 - assert deezer_client.search_calls - assert spotify_client.search_calls == [] - assert len(ws.added) == 1 - add_args = ws.added[0] - assert add_args['track_data']['provider'] == 'deezer' - assert add_args['track_data']['source'] == 'deezer' - - -# --------------------------------------------------------------------------- -# Match → wishlist add -# --------------------------------------------------------------------------- - -def test_match_adds_to_wishlist(mock_db_and_wishlist): - """High-confidence match → wishlist_service.add_spotify_track_to_wishlist called.""" - db, ws = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('Artist')] - db._tracks = [_track_row(artist_name='Artist', title='Track', file_path='/x.mp3', bitrate=128)] - state = {} - match = _FakeSpotifyTrack(name='Track', artists=['Artist']) - deps = _build_deps( - state=state, - quality_tier_result=('low_lossy', 4), - source_clients={'spotify': _FakeMetadataClient(results=[match])}, - primary_source='spotify', - ) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert state['matched'] == 1 - assert len(ws.added) == 1 - add_args = ws.added[0] - assert add_args['source_type'] == 'quality_scanner' - assert add_args['source_context']['original_file_path'] == '/x.mp3' - - -def test_match_preserves_album_and_artist_images(mock_db_and_wishlist): - """Image metadata from the provider payload should survive the wishlist handoff.""" - db, ws = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('Artist')] - db._tracks = [_track_row(artist_name='Artist', title='Track', file_path='/x.mp3', bitrate=128)] - state = {} - match = { - 'id': 'sp-1', - 'name': 'Track', - 'artists': [{'name': 'Artist', 'image_url': 'https://example.test/artist.jpg'}], - 'album': 'Album', - 'image_url': 'https://example.test/cover.jpg', - 'duration_ms': 200000, - 'popularity': 50, - 'external_urls': {}, - 'album_type': 'album', - 'release_date': '2024-01-01', - } - deps = _build_deps( - state=state, - quality_tier_result=('low_lossy', 4), - source_clients={'spotify': _FakeMetadataClient(results=[match])}, - primary_source='spotify', - ) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert state['matched'] == 1 - assert len(ws.added) == 1 - add_args = ws.added[0] - assert add_args['track_data']['image_url'] == 'https://example.test/cover.jpg' - assert add_args['track_data']['album']['image_url'] == 'https://example.test/cover.jpg' - assert add_args['track_data']['album']['images'] == [{'url': 'https://example.test/cover.jpg'}] - assert add_args['track_data']['artists'][0]['image_url'] == 'https://example.test/artist.jpg' - - -def test_no_match_no_wishlist_add(mock_db_and_wishlist): - """No match found → no wishlist add, matched stays 0.""" - db, ws = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('A')] - db._tracks = [_track_row(artist_name='A', title='Z', file_path='/x.mp3')] - state = {} - # No spotify results → no match - deps = _build_deps( - state=state, - quality_tier_result=('low_lossy', 4), - source_clients={'spotify': _FakeMetadataClient(results=[])}, - primary_source='spotify', - ) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert state['matched'] == 0 - assert ws.added == [] - - -# --------------------------------------------------------------------------- -# Stop request gate -# --------------------------------------------------------------------------- - -def test_stop_request_halts_loop(mock_db_and_wishlist): - """Setting state['status'] != 'running' mid-loop halts processing.""" - db, _ = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('A')] - db._tracks = [_track_row(track_id=1), _track_row(track_id=2)] - state = {} - deps = _build_deps(state=state, quality_tier_result=('lossless', 1)) - - # Override get_quality_tier_from_extension to set stop after first track - call_count = [0] - - def stop_after_first(fp): - call_count[0] += 1 - if call_count[0] == 1: - # Set status to non-running BEFORE second track iter checks - with deps.quality_scanner_lock: - state['status'] = 'stopping' - return ('lossless', 1) - - deps.get_quality_tier_from_extension = stop_after_first - - qs.run_quality_scanner('watchlist', 1, deps) - - # Only first track processed - assert state['quality_met'] == 1 - - -# --------------------------------------------------------------------------- -# Completion -# --------------------------------------------------------------------------- - -def test_completion_marks_finished(mock_db_and_wishlist): - """All tracks processed → status='finished', progress=100.""" - db, _ = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('A')] - db._tracks = [_track_row()] - state = {} - deps = _build_deps(state=state) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert state['status'] == 'finished' - assert state['progress'] == 100 - - -def test_automation_event_emitted(mock_db_and_wishlist): - """Successful completion emits 'quality_scan_completed' on automation engine.""" - db, _ = mock_db_and_wishlist - db._watchlist_artists = [_WatchlistArtist('A')] - db._tracks = [_track_row()] - automation = _FakeAutomationEngine() - state = {} - deps = _build_deps(state=state, automation=automation) - - qs.run_quality_scanner('watchlist', 1, deps) - - assert any(name == 'quality_scan_completed' for name, _ in automation.events) - - -# --------------------------------------------------------------------------- -# All-library scope -# --------------------------------------------------------------------------- - -def test_scope_all_loads_all_tracks(mock_db_and_wishlist): - """scope != 'watchlist' loads all tracks (no watchlist filter).""" - db, _ = mock_db_and_wishlist - db._tracks = [_track_row(track_id=1), _track_row(track_id=2)] - state = {} - deps = _build_deps(state=state) - - qs.run_quality_scanner('all', 1, deps) - - assert state['total'] == 2 diff --git a/tests/test_tool_progress_events.py b/tests/test_tool_progress_events.py index d23dc765..d8424dc6 100644 --- a/tests/test_tool_progress_events.py +++ b/tests/test_tool_progress_events.py @@ -13,16 +13,15 @@ conftest is a different module instance. Use the ``shared_state`` fixture instea import pytest -# All 7 tool progress pollers +# Tool progress pollers TOOLS = [ - 'stream', 'quality-scanner', 'duplicate-cleaner', + 'stream', 'duplicate-cleaner', 'retag', 'db-update', 'metadata', 'logs', ] # Endpoint URLs keyed by tool name ENDPOINTS = { 'stream': '/api/stream/status', - 'quality-scanner': '/api/quality-scanner/status', 'duplicate-cleaner': '/api/duplicate-cleaner/status', 'retag': '/api/retag/status', 'db-update': '/api/database/update/status', @@ -63,27 +62,6 @@ class TestToolDataShape: assert 'error_message' in data assert isinstance(data['progress'], (int, float)) - def test_quality_scanner_shape(self, test_app, shared_state): - """Quality scanner has status, phase, progress, processed, total, quality_met.""" - app, socketio = test_app - client = socketio.test_client(app) - build = shared_state['build_quality_scanner_status'] - - socketio.emit('tool:quality-scanner', build()) - received = client.get_received() - events = [e for e in received if e['name'] == 'tool:quality-scanner'] - assert len(events) >= 1 - data = events[0]['args'][0] - - assert 'status' in data - assert 'phase' in data - assert 'progress' in data - assert 'processed' in data - assert 'total' in data - assert 'quality_met' in data - assert 'low_quality' in data - assert 'matched' in data - def test_duplicate_cleaner_shape(self, test_app, shared_state): """Duplicate cleaner has status, phase, progress, space_freed_mb.""" app, socketio = test_app @@ -255,11 +233,11 @@ class TestToolBackwardCompat: client2 = socketio.test_client(app) build = shared_state['build_tool_status'] - socketio.emit('tool:quality-scanner', build('quality-scanner')) + socketio.emit('tool:duplicate-cleaner', build('duplicate-cleaner')) for client in [client1, client2]: received = client.get_received() - events = [e for e in received if e['name'] == 'tool:quality-scanner'] + events = [e for e in received if e['name'] == 'tool:duplicate-cleaner'] assert len(events) >= 1 client1.disconnect() @@ -270,17 +248,15 @@ class TestToolBackwardCompat: app, socketio = test_app client = socketio.test_client(app) build = shared_state['build_tool_status'] - qs = shared_state['quality_scanner_state'] + dc = shared_state['duplicate_cleaner_state'] # Mutate state - qs['status'] = 'finished' - qs['progress'] = 100 - qs['processed'] = 100 + dc['status'] = 'finished' + dc['progress'] = 100 - socketio.emit('tool:quality-scanner', build('quality-scanner')) + socketio.emit('tool:duplicate-cleaner', build('duplicate-cleaner')) received = client.get_received() - events = [e for e in received if e['name'] == 'tool:quality-scanner'] + events = [e for e in received if e['name'] == 'tool:duplicate-cleaner'] data = events[-1]['args'][0] assert data['status'] == 'finished' assert data['progress'] == 100 - assert data['processed'] == 100 diff --git a/web_server.py b/web_server.py index 300c3a36..0196fe9b 100644 --- a/web_server.py +++ b/web_server.py @@ -990,21 +990,8 @@ def _set_db_update_automation_id(value): global _db_update_automation_id _db_update_automation_id = value -# Quality Scanner state -quality_scanner_state = { - "status": "idle", # idle, running, finished, error - "phase": "Ready to scan", - "progress": 0, - "processed": 0, - "total": 0, - "quality_met": 0, - "low_quality": 0, - "matched": 0, - "error_message": "", - "results": [], # List of low quality tracks with match status -} -quality_scanner_lock = threading.Lock() -quality_scanner_executor = ThreadPoolExecutor(max_workers=1, thread_name_prefix="QualityScanner") +# Quality scanning is now the 'quality_upgrade' library-maintenance repair job +# (core/repair_jobs/quality_upgrade.py) — no standalone state/executor here. # Duplicate Cleaner state duplicate_cleaner_state = { @@ -1253,10 +1240,7 @@ def _register_automation_handlers(): duplicate_cleaner_lock=duplicate_cleaner_lock, duplicate_cleaner_executor=duplicate_cleaner_executor, run_duplicate_cleaner=_run_duplicate_cleaner, - get_quality_scanner_state=lambda: quality_scanner_state, - quality_scanner_lock=quality_scanner_lock, - quality_scanner_executor=quality_scanner_executor, - run_quality_scanner=_run_quality_scanner, + run_repair_job_now=lambda job_id: repair_worker.run_job_now(job_id) if repair_worker else None, download_orchestrator=download_orchestrator, run_async=run_async, tasks_lock=tasks_lock, @@ -1859,7 +1843,6 @@ def _shutdown_runtime_components(): for executor, name in [ (stream_executor, "stream executor"), (db_update_executor, "db update executor"), - (quality_scanner_executor, "quality scanner executor"), (duplicate_cleaner_executor, "duplicate cleaner executor"), (sync_executor, "sync executor"), (missing_download_executor, "missing download executor"), @@ -17639,28 +17622,7 @@ def _get_quality_tier_from_extension(file_path): return ('unknown', 999) -# Quality scanner worker logic lives in core/discovery/quality_scanner.py. -from core.discovery import quality_scanner as _discovery_quality_scanner - - -def _build_quality_scanner_deps(): - """Build the QualityScannerDeps bundle from web_server.py globals on each call.""" - return _discovery_quality_scanner.QualityScannerDeps( - quality_scanner_state=quality_scanner_state, - quality_scanner_lock=quality_scanner_lock, - QUALITY_TIERS=QUALITY_TIERS, - matching_engine=matching_engine, - automation_engine=automation_engine, - get_quality_tier_from_extension=_get_quality_tier_from_extension, - add_activity_item=add_activity_item, - ) - - -def _run_quality_scanner(scope='watchlist', profile_id=1): - return _discovery_quality_scanner.run_quality_scanner( - scope, profile_id, _build_quality_scanner_deps() - ) - +# (Quality scanning moved to the 'quality_upgrade' library-maintenance repair job.) from core.library.duplicate_cleaner import ( _run_duplicate_cleaner, @@ -17673,55 +17635,6 @@ _init_duplicate_cleaner( engine=automation_engine, ) -@app.route('/api/quality-scanner/start', methods=['POST']) -def start_quality_scan(): - """Start the quality scanner""" - with quality_scanner_lock: - if quality_scanner_state["status"] == "running": - return jsonify({"success": False, "error": "A scan is already in progress"}), 409 - - data = request.get_json() or {} - scope = data.get('scope', 'watchlist') # 'watchlist' or 'all' - - logger.info(f"[Quality Scanner API] Starting scan with scope: {scope}") - - # Reset state - quality_scanner_state["status"] = "running" - quality_scanner_state["phase"] = "Initializing..." - quality_scanner_state["progress"] = 0 - quality_scanner_state["processed"] = 0 - quality_scanner_state["total"] = 0 - quality_scanner_state["quality_met"] = 0 - quality_scanner_state["low_quality"] = 0 - quality_scanner_state["matched"] = 0 - quality_scanner_state["results"] = [] - quality_scanner_state["error_message"] = "" - - # Submit worker (capture profile_id before thread) - scan_profile_id = get_current_profile_id() - quality_scanner_executor.submit(_run_quality_scanner, scope, scan_profile_id) - - add_activity_item("", "Quality Scan Started", f"Scanning {scope} tracks", "Now") - - return jsonify({"success": True, "message": "Quality scan started"}) - -@app.route('/api/quality-scanner/status', methods=['GET']) -def get_quality_scanner_status(): - """Get current quality scanner status""" - with quality_scanner_lock: - return jsonify(quality_scanner_state) - -@app.route('/api/quality-scanner/stop', methods=['POST']) -def stop_quality_scan(): - """Stop the quality scanner (sets a stop flag)""" - with quality_scanner_lock: - if quality_scanner_state["status"] == "running": - quality_scanner_state["status"] = "finished" - quality_scanner_state["phase"] = "Scan stopped by user" - return jsonify({"success": True, "message": "Stop request sent"}) - else: - return jsonify({"success": False, "error": "No scan is currently running"}), 404 - @app.route('/api/duplicate-cleaner/start', methods=['POST']) def start_duplicate_cleaner(): """Start the duplicate cleaner""" @@ -37341,12 +37254,6 @@ def _emit_tool_progress_loop(): # (which skipped HTTP polling while the socket was up) never learned # its stream was ready. Each client polls /api/stream/status instead, # which resolves its own session from the cookie. - # Quality Scanner - try: - with quality_scanner_lock: - socketio.emit('tool:quality-scanner', dict(quality_scanner_state)) - except Exception as e: - logger.debug(f"Error emitting quality scanner status: {e}") # Duplicate Cleaner (add computed space_freed_mb) try: with duplicate_cleaner_lock: diff --git a/webui/index.html b/webui/index.html index a93dd265..af40adfa 100644 --- a/webui/index.html +++ b/webui/index.html @@ -6831,49 +6831,6 @@ -
-
-

Quality Scanner

- -
-

Scan library for tracks below quality preferences

-
-
- Processed: - 0 -
-
- Quality Met: - 0 -
-
- Low Quality: - 0 -
-
- Matched: - 0 -
-
-
- - -
-
-

Ready to scan

-
-
-
-
-

0 / 0 tracks scanned - (0.0%)

-
-
-

Import IDs from File Tags

diff --git a/webui/static/core.js b/webui/static/core.js index ca175627..a6b0f0f9 100644 --- a/webui/static/core.js +++ b/webui/static/core.js @@ -29,7 +29,6 @@ let isSortReversed = false; let searchAbortController = null; let dbStatsInterval = null; let dbUpdateStatusInterval = null; -let qualityScannerStatusInterval = null; let duplicateCleanerStatusInterval = null; let wishlistCountInterval = null; let wishlistCountdownInterval = null; // Countdown timer for wishlist overview modal @@ -487,7 +486,6 @@ function initializeWebSocket() { // 'tool:stream' is intentionally NOT wired: stream state is per-listener // (session cookie), so the global broadcast could only carry the DEFAULT // session's eternal "stopped" — the player polls /api/stream/status instead. - socket.on('tool:quality-scanner', (data) => { if (_qaToolBusy(data)) qaSignal('tools'); updateQualityScanProgressFromData(data); }); socket.on('tool:duplicate-cleaner', (data) => { if (_qaToolBusy(data)) qaSignal('tools'); updateDuplicateCleanProgressFromData(data); }); socket.on('tool:db-update', (data) => { if (_qaToolBusy(data)) qaSignal('tools'); updateDbProgressFromData(data); }); socket.on('tool:metadata', (data) => { if (_qaToolBusy(data)) qaSignal('tools'); updateMetadataStatusFromData(data); }); diff --git a/webui/static/helper.js b/webui/static/helper.js index 60bba85a..b1168b2d 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -328,16 +328,6 @@ const HELPER_CONTENT = { ], docsId: 'dashboard' }, - '#quality-scanner-card': { - title: 'Quality Scanner', - description: 'Analyzes audio files for quality integrity. Calculates bitrate density to detect transcodes (e.g., an MP3 re-encoded as FLAC). Scope options: Full Library, New Only, or Single Artist.', - tips: [ - '"Quality Met" = file quality matches its format claims', - '"Low Quality" = suspicious file flagged for review', - 'Matched count shows tracks with verified metadata' - ], - docsId: 'dashboard' - }, '#duplicate-cleaner-card': { title: 'Duplicate Cleaner', description: 'Scans your library for duplicate tracks by comparing title, artist, album, and file characteristics. Reviews duplicates before taking any action.', @@ -2358,7 +2348,6 @@ const HELPER_TOURS = { // Tools — in page order { page: 'dashboard', selector: '#db-updater-card', title: 'Database Updater', description: 'Syncs your media server\'s library into SoulSync\'s database. Three modes: Incremental (fast, new content only), Full Refresh (rebuilds everything), Deep Scan (finds and removes stale entries).' }, { page: 'dashboard', selector: '#metadata-updater-card', title: 'Metadata Enrichment', description: 'Background workers that enrich your library from 9 services — Spotify, MusicBrainz, Deezer, Last.fm, iTunes, AudioDB, Genius, Tidal, Qobuz. Runs automatically at the configured interval.' }, - { page: 'dashboard', selector: '#quality-scanner-card', title: 'Quality Scanner', description: 'Analyzes audio files for quality integrity. Calculates bitrate density to detect transcodes (e.g., an MP3 re-encoded as FLAC). Scan by Full Library, New Only, or Single Artist.' }, { page: 'dashboard', selector: '#duplicate-cleaner-card', title: 'Duplicate Cleaner', description: 'Finds and removes duplicate tracks by comparing title, artist, album, and audio characteristics. Always reviews before deleting.' }, { page: 'dashboard', selector: '#discovery-pool-card', title: 'Discovery Pool', description: 'Tracks from similar artists found during watchlist scans. Matched tracks feed the Discover page playlists and genre browser. Fix failed matches manually.' }, { page: 'dashboard', selector: '#retag-tool-card', title: 'Retag Tool', description: 'Queue of tracks needing metadata corrections. When enrichment detects better tags than what\'s in your files, they appear here for batch review.' }, @@ -3386,7 +3375,7 @@ function _guessPageFromSelector(selector) { 'import': ['import-page-'], 'settings': ['settings-', 'stg-tab', 'api-service', 'server-toggle', 'save-button', 'spotify-client', 'soulseek-url', 'quality-profile'], 'issues': ['issues-'], - 'dashboard': ['dashboard-', 'service-card', 'watchlist-button', 'wishlist-button', 'db-updater', 'metadata-updater', 'quality-scanner', 'duplicate-cleaner', 'discovery-pool-card', 'retag-tool', 'media-scan', 'backup-manager', 'metadata-cache'], + 'dashboard': ['dashboard-', 'service-card', 'watchlist-button', 'wishlist-button', 'db-updater', 'metadata-updater', 'duplicate-cleaner', 'discovery-pool-card', 'retag-tool', 'media-scan', 'backup-manager', 'metadata-cache'], }; const selectorLower = selector.toLowerCase(); diff --git a/webui/static/wishlist-tools.js b/webui/static/wishlist-tools.js index 9cfed00f..b59ab783 100644 --- a/webui/static/wishlist-tools.js +++ b/webui/static/wishlist-tools.js @@ -2811,145 +2811,8 @@ function stopDbUpdatePolling() { } } -// =================================================================== -// QUALITY SCANNER TOOL -// =================================================================== - -async function handleQualityScanButtonClick() { - const button = document.getElementById('quality-scan-button'); - const currentAction = button.textContent; - - if (currentAction === 'Scan Library') { - const scopeSelect = document.getElementById('quality-scan-scope'); - const scope = scopeSelect.value; - - try { - button.disabled = true; - button.textContent = 'Starting...'; - const response = await fetch('/api/quality-scanner/start', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ scope: scope }) - }); - - if (response.ok) { - showToast('Quality scan started!', 'success'); - // Start polling immediately to get live status - checkAndUpdateQualityScanProgress(); - } else { - const errorData = await response.json(); - showToast(`Error: ${errorData.error}`, 'error'); - button.disabled = false; - button.textContent = 'Scan Library'; - } - } catch (error) { - showToast('Failed to start quality scan.', 'error'); - button.disabled = false; - button.textContent = 'Scan Library'; - } - - } else { // "Stop Scan" - try { - const response = await fetch('/api/quality-scanner/stop', { method: 'POST' }); - if (response.ok) { - showToast('Stop request sent.', 'info'); - } else { - showToast('Failed to send stop request.', 'error'); - } - } catch (error) { - showToast('Error sending stop request.', 'error'); - } - } -} - -async function checkAndUpdateQualityScanProgress() { - if (socketConnected) return; // WebSocket handles this - try { - const response = await fetch('/api/quality-scanner/status', { - signal: AbortSignal.timeout(10000) // 10 second timeout - }); - if (!response.ok) return; - - const state = await response.json(); - console.debug('🔍 Quality Scanner Status:', state.status, `${state.processed}/${state.total}`, `${state.progress.toFixed(1)}%`); - updateQualityScanProgressUI(state); - - // Start polling only if not already polling and status is running - if (state.status === 'running' && !qualityScannerStatusInterval) { - console.log('🔄 Starting quality scanner polling (1 second interval)'); - qualityScannerStatusInterval = setInterval(checkAndUpdateQualityScanProgress, 1000); - } - - } catch (error) { - console.warn('Could not fetch quality scanner status:', error); - // Don't stop polling on network errors - keep trying - } -} - -function updateQualityScanProgressFromData(data) { - const prev = _lastToolStatus['quality-scanner']; - _lastToolStatus['quality-scanner'] = data.status; - if (prev !== undefined && data.status === prev && data.status !== 'running') return; - updateQualityScanProgressUI(data); -} - -function updateQualityScanProgressUI(state) { - const button = document.getElementById('quality-scan-button'); - const phaseLabel = document.getElementById('quality-phase-label'); - const progressLabel = document.getElementById('quality-progress-label'); - const progressBar = document.getElementById('quality-progress-bar'); - const scopeSelect = document.getElementById('quality-scan-scope'); - - // Stats - const processedStat = document.getElementById('quality-stat-processed'); - const metStat = document.getElementById('quality-stat-met'); - const lowStat = document.getElementById('quality-stat-low'); - const matchedStat = document.getElementById('quality-stat-matched'); - - if (!button || !phaseLabel || !progressLabel || !progressBar || !scopeSelect) return; - - // Update stats - if (processedStat) processedStat.textContent = state.processed || 0; - if (metStat) metStat.textContent = state.quality_met || 0; - if (lowStat) lowStat.textContent = state.low_quality || 0; - if (matchedStat) matchedStat.textContent = state.matched || 0; - - if (state.status === 'running') { - button.textContent = 'Stop Scan'; - button.disabled = false; - scopeSelect.disabled = true; - - phaseLabel.textContent = state.phase || 'Scanning...'; - progressLabel.textContent = `${state.processed} / ${state.total} tracks scanned (${state.progress.toFixed(1)}%)`; - progressBar.style.width = `${state.progress}%`; - } else { // idle, finished, or error - stopQualityScannerPolling(); - button.textContent = 'Scan Library'; - button.disabled = false; - scopeSelect.disabled = false; - - if (state.status === 'error') { - phaseLabel.textContent = `Error: ${state.error_message}`; - progressBar.style.backgroundColor = '#ff4444'; // Red for error - } else { - phaseLabel.textContent = state.phase || 'Ready to scan'; - progressBar.style.backgroundColor = 'rgb(var(--accent-rgb))'; // Green for normal - } - - if (state.status === 'finished') { - // Show completion toast with results - showToast(`Scan complete! ${state.matched} tracks added to wishlist`, 'success'); - } - } -} - -function stopQualityScannerPolling() { - if (qualityScannerStatusInterval) { - console.log('⏹️ Stopping quality scanner polling'); - clearInterval(qualityScannerStatusInterval); - qualityScannerStatusInterval = null; - } -} +// (Quality Scanner tool removed — quality scanning is now the 'Quality Upgrade +// Finder' job in Library Maintenance / Tools → repair jobs.) // =================================================================== // IMPORT IDS FROM FILE TAGS (reconcile embedded provider IDs) @@ -5630,43 +5493,6 @@ const TOOL_HELP_CONTENT = {

Available for Plex and Jellyfin media servers. Each enrichment worker only runs if its service is authenticated.

` }, - 'quality-scanner': { - title: 'Quality Scanner', - content: ` -

What does this tool do?

-

The Quality Scanner identifies tracks in your library that don't meet your preferred quality settings and automatically matches them to Spotify to add to your wishlist for re-downloading.

- -

Scan Scope

-
    -
  • Watchlist Artists Only: Only scans tracks from artists you're watching. Faster and more focused.
  • -
  • All Library Tracks: Scans your entire music library. Comprehensive but takes longer.
  • -
- -

How it works

-
    -
  1. Scans tracks and checks file format against your quality preferences
  2. -
  3. Identifies tracks below your quality threshold (e.g., MP3 when you prefer FLAC)
  4. -
  5. Uses fuzzy matching to find the track on Spotify (70% confidence minimum)
  6. -
  7. Automatically adds matched tracks to your wishlist for re-download
  8. -
- -

Quality Tiers

-
    -
  • Tier 1 (Best): FLAC, WAV, ALAC, AIFF - Lossless formats
  • -
  • Tier 2: OPUS, OGG - High quality lossy
  • -
  • Tier 3: M4A, AAC - Standard lossy
  • -
  • Tier 4: MP3, WMA - Lower quality lossy
  • -
- -

Stats Explained

-
    -
  • Processed: Total tracks scanned so far
  • -
  • Quality Met: Tracks that meet your quality standards
  • -
  • Low Quality: Tracks below your quality threshold
  • -
  • Matched: Low quality tracks successfully matched to Spotify and added to wishlist
  • -
- ` - }, 'duplicate-cleaner': { title: 'Duplicate Cleaner', content: ` @@ -7566,11 +7392,6 @@ async function initializeToolsPage() { metadataButton._toolsWired = true; } - const qualityScanButton = document.getElementById('quality-scan-button'); - if (qualityScanButton && !qualityScanButton._toolsWired) { - qualityScanButton.addEventListener('click', handleQualityScanButtonClick); - qualityScanButton._toolsWired = true; - } const duplicateCleanButton = document.getElementById('duplicate-clean-button'); if (duplicateCleanButton && !duplicateCleanButton._toolsWired) { @@ -7615,7 +7436,6 @@ async function initializeToolsPage() { // Check for ongoing operations await checkAndUpdateDbProgress(); - await checkAndUpdateQualityScanProgress(); await checkAndUpdateDuplicateCleanProgress(); // Initialize library maintenance section From 3ea5b5181f3888fe8d3a5dd5090d438be2e85940 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 12:43:46 -0700 Subject: [PATCH 03/42] Quality Upgrade: ISRC-first exact matching using the IDs enrichment already embedded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The job was doing a blind fuzzy search for every low-quality track, ignoring that enrichment writes each track's ISRC + per-source IDs into the file tags. Now it reads the file's embedded ISRC and resolves the EXACT track via each source's 'isrc:' search (universal cross-source key), guarded by an ISRC-equality check so a source that ignores the syntax can't produce a false match — exact track, exact album context, one call. Falls back to the name/artist fuzzy search only for un-enriched tracks with no usable ISRC. Findings record matched_via=isrc|search. 4 new seam tests (guard accept/reject, ISRC-preferred-over-fuzzy, fuzzy fallback). --- core/repair_jobs/quality_upgrade.py | 105 +++++++++++++++++++--- tests/repair_jobs/test_quality_upgrade.py | 67 ++++++++++++++ 2 files changed, 159 insertions(+), 13 deletions(-) diff --git a/core/repair_jobs/quality_upgrade.py b/core/repair_jobs/quality_upgrade.py index c80a381f..993bfad9 100644 --- a/core/repair_jobs/quality_upgrade.py +++ b/core/repair_jobs/quality_upgrade.py @@ -37,6 +37,8 @@ from core.discovery.quality_scanner import ( _track_artist_names, _track_name, ) +from core.library.file_tags import read_embedded_tags +from core.library.path_resolver import resolve_library_file_path from utils.logging_config import get_logger logger = get_logger("repair_jobs.quality_upgrade") @@ -150,6 +152,69 @@ def _rank_label(rank: Optional[int]) -> str: }.get(rank, 'unknown') +def _norm_isrc(value: Any) -> str: + """Canonicalize an ISRC for comparison: uppercase, strip dashes/spaces.""" + if not value: + return '' + return str(value).upper().replace('-', '').replace(' ', '').strip() + + +def _read_track_isrc(file_path: str) -> str: + """Read the ISRC the enrichment pipeline embedded in the file's tags. + + Enrichment matches every track to the metadata sources and writes the IDs + (ISRC, per-source track IDs) into the file — so an already-enriched track + carries its exact identity. Returns '' when unreadable / not enriched.""" + resolved = resolve_library_file_path(file_path) if file_path else None + if not resolved and file_path and os.path.isfile(file_path): + resolved = file_path + if not resolved: + return '' + try: + info = read_embedded_tags(resolved) + except Exception: + return '' + if not info or not info.get('available'): + return '' + return _norm_isrc((info.get('tags') or {}).get('isrc')) + + +def _candidate_isrc(cand: Any) -> str: + """Pull an ISRC off a provider search result (Track / dict), checking the + common shapes: a flat ``isrc`` or a nested ``external_ids.isrc``.""" + direct = _extract_lookup_value(cand, 'isrc') + if direct: + return _norm_isrc(direct) + ext = _extract_lookup_value(cand, 'external_ids') + if isinstance(ext, dict): + return _norm_isrc(ext.get('isrc')) + return '' + + +def _match_via_isrc(isrc: str, source_priority: List[str]) -> Tuple[Optional[Any], Optional[str]]: + """Exact-match a track by its ISRC via each source's ``isrc:`` search. + + ISRC is the universal cross-source recording key, so this resolves the EXACT + track (with its real album) instead of fuzzy-matching by name. Guarded: only + a candidate whose own ISRC equals ours is accepted, so a source that ignores + the ``isrc:`` syntax and returns unrelated hits can't produce a false match. + Returns (track, source) or (None, None).""" + if not isrc: + return None, None + for source in source_priority: + client = get_client_for_source(source) + if not client or not hasattr(client, 'search_tracks'): + continue + try: + results = _search_tracks_for_source(source, f'isrc:{isrc}', limit=5, client=client) + except Exception: + results = [] + for cand in results or []: + if _candidate_isrc(cand) == isrc: + return cand, source + return None, None + + def _find_best_match(engine: Any, source_priority: List[str], title: str, artist: str, album: str, min_confidence: float) -> Tuple[Optional[Any], float, Optional[str], bool]: """Search the configured metadata sources for the best replacement match. @@ -201,11 +266,12 @@ class QualityUpgradeJob(RepairJob): "track against your Quality Profile using BOTH the file format and its " 'bitrate — so a 128 kbps MP3 is no longer treated the same as a 320 kbps ' 'one, and enabling MP3-320/256 in your profile actually counts.\n\n' - 'For every track below your preferred quality, it searches your configured ' - 'metadata source for a better version and creates a finding showing the ' - 'match and a confidence score. Nothing is queued automatically: applying a ' - 'finding adds that matched track — with its album context — to the wishlist, ' - 'the same as any other download.\n\n' + 'For every track below your preferred quality, it finds a better version and ' + 'creates a finding. If the track was enriched, it uses the ISRC embedded in ' + 'the file to resolve the EXACT track (and its album) — no guessing; otherwise ' + 'it falls back to a name/artist search with a confidence score. Nothing is ' + 'queued automatically: applying a finding adds that matched track — with its ' + 'album context — to the wishlist, the same as any other download.\n\n' 'Settings:\n' '- Scope: "watchlist" (watchlisted artists only) or "all" (whole library)\n' '- Min confidence: minimum match confidence (0-1) to surface a finding\n\n' @@ -330,13 +396,24 @@ class QualityUpgradeJob(RepairJob): log_line=f'Low quality ({current_label}): {artist_name} - {title}', log_type='info') - try: - best, conf, source, attempted = _find_best_match( - engine, source_priority, title, artist_name or '', album_title or '', min_conf) - except Exception as e: - logger.debug("[Quality Upgrade] Match error for %s - %s: %s", artist_name, title, e) - result.errors += 1 - continue + # Fast path: enrichment embeds the ISRC (and per-source track IDs) in + # the file's tags, so an already-enriched track carries its exact + # identity. Resolve the EXACT track by ISRC — no fuzzy matching, and + # the real album comes with it. Only fall back to name/artist search + # for tracks that were never enriched / have no usable ISRC. + matched_via = 'isrc' + best, source = _match_via_isrc(_read_track_isrc(file_path), source_priority) + conf, attempted = (1.0, True) if best else (0.0, False) + + if not best: + matched_via = 'search' + try: + best, conf, source, attempted = _find_best_match( + engine, source_priority, title, artist_name or '', album_title or '', min_conf) + except Exception as e: + logger.debug("[Quality Upgrade] Match error for %s - %s: %s", artist_name, title, e) + result.errors += 1 + continue if not attempted: logger.warning("[Quality Upgrade] No metadata provider responded — stopping") @@ -365,7 +442,8 @@ class QualityUpgradeJob(RepairJob): description=( f'"{title}" by {artist_name} is {current_label}, below your preferred ' f'quality. Best match: "{_track_name(best)}" via {source} ' - f'(confidence {conf:.0%}). Apply to add it to the wishlist.'), + f'({"exact ISRC match" if matched_via == "isrc" else f"confidence {conf:.0%}"}). ' + 'Apply to add it to the wishlist.'), details={ 'track_id': track_id, 'track_title': title, @@ -375,6 +453,7 @@ class QualityUpgradeJob(RepairJob): 'current_format': current_label, 'current_bitrate': bitrate, 'match_confidence': conf, + 'matched_via': matched_via, 'provider': source, 'matched_track_data': matched, }) diff --git a/tests/repair_jobs/test_quality_upgrade.py b/tests/repair_jobs/test_quality_upgrade.py index eb78a9bf..0640f843 100644 --- a/tests/repair_jobs/test_quality_upgrade.py +++ b/tests/repair_jobs/test_quality_upgrade.py @@ -174,6 +174,73 @@ def test_scan_creates_finding_for_low_quality_track(monkeypatch): assert f['details']['provider'] == 'spotify' +def test_match_via_isrc_accepts_exact_match(monkeypatch): + """The guard accepts only a candidate whose own ISRC equals ours (dash/case + insensitive), so it survives a source returning unrelated hits first.""" + monkeypatch.setattr(qu, 'get_client_for_source', + lambda src: types.SimpleNamespace(search_tracks=lambda *a, **k: [])) + monkeypatch.setattr(qu, '_search_tracks_for_source', lambda *a, **k: [ + {'id': 'x', 'name': 'Wrong', 'isrc': 'ZZISRC000000'}, + {'id': 'sp1', 'name': 'Right', 'isrc': 'US-RC1-76-07839'}, # dashed form + ]) + best, source = qu._match_via_isrc('USRC17607839', ['spotify']) + assert best['id'] == 'sp1' + assert source == 'spotify' + + +def test_match_via_isrc_rejects_all_mismatches(monkeypatch): + monkeypatch.setattr(qu, 'get_client_for_source', + lambda src: types.SimpleNamespace(search_tracks=lambda *a, **k: [])) + monkeypatch.setattr(qu, '_search_tracks_for_source', lambda *a, **k: [ + {'id': 'x', 'name': 'Wrong', 'external_ids': {'isrc': 'ZZISRC000000'}}, + ]) + assert qu._match_via_isrc('USRC17607839', ['spotify']) == (None, None) + + +def test_scan_prefers_isrc_exact_match_over_fuzzy(monkeypatch): + """When the file carries an ISRC and it resolves, use the exact match and do + NOT run the fuzzy search at all.""" + rows = [(1, 'Song One', '/music/a.mp3', 128, 'Artist A', 'Album X', 10)] + db = _FakeDB(rows, BALANCED) + monkeypatch.setattr(qu, 'get_primary_source', lambda: 'spotify') + monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) + monkeypatch.setattr('core.matching_engine.MusicMatchingEngine', lambda: types.SimpleNamespace()) + monkeypatch.setattr(qu, '_read_track_isrc', lambda fp: 'USRC17607839') + fake = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X'}} + monkeypatch.setattr(qu, '_match_via_isrc', lambda isrc, sp: (fake, 'spotify')) + monkeypatch.setattr(qu, '_normalize_track_match', lambda t, s: dict(fake)) + monkeypatch.setattr(qu, '_track_name', lambda t: 'Song One') + + def _boom(*a, **k): + raise AssertionError("fuzzy search must not run when an ISRC match exists") + monkeypatch.setattr(qu, '_find_best_match', _boom) + + findings = [] + result = qu.QualityUpgradeJob().scan(_ctx(db, findings)) + assert result.findings_created == 1 + assert findings[0]['details']['matched_via'] == 'isrc' + assert findings[0]['details']['match_confidence'] == 1.0 + + +def test_scan_falls_back_to_search_without_isrc(monkeypatch): + """No usable ISRC → fall back to fuzzy search.""" + rows = [(1, 'Song One', '/music/a.mp3', 128, 'Artist A', 'Album X', 10)] + db = _FakeDB(rows, BALANCED) + monkeypatch.setattr(qu, 'get_primary_source', lambda: 'spotify') + monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) + monkeypatch.setattr('core.matching_engine.MusicMatchingEngine', lambda: types.SimpleNamespace()) + monkeypatch.setattr(qu, '_read_track_isrc', lambda fp: '') # un-enriched + fake = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X'}} + monkeypatch.setattr(qu, '_find_best_match', lambda *a, **k: (fake, 0.88, 'spotify', True)) + monkeypatch.setattr(qu, '_normalize_track_match', lambda t, s: dict(fake)) + monkeypatch.setattr(qu, '_track_name', lambda t: 'Song One') + + findings = [] + result = qu.QualityUpgradeJob().scan(_ctx(db, findings)) + assert result.findings_created == 1 + assert findings[0]['details']['matched_via'] == 'search' + + def test_scan_skips_tracks_meeting_quality(monkeypatch): # A 320 kbps MP3 meets the balanced profile → no finding, no metadata calls. rows = [(2, 'Good Song', '/music/b.mp3', 320, 'Artist A', 'Album Y', 11)] From 777781db6a3bda0672afcf7d51e42e1690dc1dcd Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 13:00:16 -0700 Subject: [PATCH 04/42] Quality Upgrade: tiered structured matching (ISRC -> album->track -> artist+title) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the blind fuzzy search with a smart hierarchy that uses the data we already have, best identity first: 1. ISRC embedded in the file tags (enriched track) -> exact track. 2. Album -> track: use the album's stored source ID (albums.spotify_album_id / itunes_album_id / deezer_id / musicbrainz_release_id / audiodb_id) when the ALBUM is enriched (even if the track isn't); else find the album by searching 'artist album', then locate our track in that album's tracklist by normalized title (track_number breaks ties). Pins the exact album context. (artist->album->track) 3. Plain artist+title search with similarity scoring. (artist->track) — loosest. _load_tracks now returns dict rows (adds track_number + the album source-id columns). Findings record matched_via = isrc | album | search. All clients (spotify/deezer/itunes/discogs) expose search_albums + get_album_tracks with a uniform {'items': [...]} shape, so the album tier is source-agnostic. 26 repair tests pass (added album-tier + _find_track_in_album coverage). --- core/repair_jobs/quality_upgrade.py | 161 ++++++++++++++++++++-- tests/repair_jobs/test_quality_upgrade.py | 40 ++++++ 2 files changed, 186 insertions(+), 15 deletions(-) diff --git a/core/repair_jobs/quality_upgrade.py b/core/repair_jobs/quality_upgrade.py index 993bfad9..8f98f0fa 100644 --- a/core/repair_jobs/quality_upgrade.py +++ b/core/repair_jobs/quality_upgrade.py @@ -215,6 +215,109 @@ def _match_via_isrc(isrc: str, source_priority: List[str]) -> Tuple[Optional[Any return None, None +# Column order for the _load_tracks SELECT — rows come back as dicts keyed by these. +_TRACK_COLS = ( + 'id', 'title', 'file_path', 'bitrate', 'artist_name', 'album_title', 'album_id', + 'track_number', 'spotify_album_id', 'itunes_album_id', 'deezer_id', + 'musicbrainz_release_id', 'audiodb_id', +) + +# Human-readable note per match tier (search uses a confidence % instead). +_MATCH_NOTE = {'isrc': 'exact ISRC match', 'album': 'matched within album'} + +# Per-source column holding that source's album ID on the albums table. +_SOURCE_ALBUM_ID_COL = { + 'spotify': 'spotify_album_id', + 'itunes': 'itunes_album_id', + 'deezer': 'deezer_id', + 'musicbrainz': 'musicbrainz_release_id', + 'audiodb': 'audiodb_id', +} + + +def _norm_title(value: Any) -> str: + """Collapse a title to alphanumerics for tolerant comparison.""" + return ''.join(ch for ch in str(value or '').lower() if ch.isalnum()) + + +def _find_track_in_album(items: Any, title: str, track_number: Any, engine: Any) -> Optional[Any]: + """Pick the track in an album's tracklist that matches ours — exact normalized + title first (track_number breaks ties), then a high-similarity fuzzy fallback.""" + want = _norm_title(title) + exact = [] + best, best_score = None, 0.0 + for it in items or []: + it_name = _extract_lookup_value(it, 'name', 'title', default='') + if want and _norm_title(it_name) == want: + exact.append(it) + continue + if engine and it_name: + score = engine.similarity_score( + engine.normalize_string(title), engine.normalize_string(it_name)) + if score > best_score and score >= 0.85: + best, best_score = it, score + if exact: + if track_number: + for it in exact: + if _extract_lookup_value(it, 'track_number') == track_number: + return it + return exact[0] + return best + + +def _match_via_album(engine: Any, source_priority: List[str], artist: str, album_title: str, + title: str, track_number: Any, + stored_album_ids: Dict[str, str]) -> Tuple[Optional[Any], Optional[str]]: + """Structured artist → album → track match. For each source: use the album's + stored source ID if we already have it (enriched album), else find the album + by searching ``artist album``; then pull that album's tracklist and locate our + track in it. This pins the right album (exact context) without needing the + track itself to be enriched. Returns (track, source) or (None, None).""" + if not album_title: + return None, None + for source in source_priority: + client = get_client_for_source(source) + if not client or not hasattr(client, 'get_album_tracks'): + continue + + album_id = stored_album_ids.get(source) + album_name = album_title + if not album_id and hasattr(client, 'search_albums'): + try: + albums = client.search_albums(f'{artist} {album_title}'.strip(), limit=5) + except Exception: + albums = [] + best_alb, best_s = None, 0.0 + for alb in albums or []: + aname = _extract_lookup_value(alb, 'name', 'title', default='') + s = engine.similarity_score( + engine.normalize_string(album_title), engine.normalize_string(aname)) + if s > best_s and s >= 0.80: + best_alb, best_s = alb, s + if best_alb is not None: + album_id = _extract_lookup_value(best_alb, 'id') + album_name = _extract_lookup_value(best_alb, 'name', 'title', default=album_title) + if not album_id: + continue + + try: + resp = client.get_album_tracks(str(album_id)) + except Exception: + resp = None + items = resp.get('items') if isinstance(resp, dict) else None + match = _find_track_in_album(items, title, track_number, engine) + if match is None: + continue + # The album tracklist's tracks usually omit the album object — attach it so + # the wishlist add carries the correct album context. + if isinstance(match, dict): + alb = match.get('album') + if not isinstance(alb, dict) or not alb.get('name'): + match['album'] = {'name': album_name, 'images': []} + return match, source + return None, None + + def _find_best_match(engine: Any, source_priority: List[str], title: str, artist: str, album: str, min_confidence: float) -> Tuple[Optional[Any], float, Optional[str], bool]: """Search the configured metadata sources for the best replacement match. @@ -297,12 +400,14 @@ class QualityUpgradeJob(RepairJob): min_conf = 0.7 return {'scope': scope, 'min_confidence': min_conf} - def _load_tracks(self, db: Any, scope: str) -> List[tuple]: + def _load_tracks(self, db: Any, scope: str) -> List[dict]: conn = db._get_connection() try: base = ( "SELECT t.id, t.title, t.file_path, t.bitrate, a.name AS artist_name, " - "al.title AS album_title, t.album_id " + "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 " "FROM tracks t " "JOIN artists a ON t.artist_id = a.id " "JOIN albums al ON t.album_id = al.id " @@ -319,7 +424,7 @@ class QualityUpgradeJob(RepairJob): base + f" AND a.name IN ({placeholders})", names).fetchall() else: rows = conn.execute(base).fetchall() - return rows + return [dict(zip(_TRACK_COLS, r, strict=False)) for r in rows] finally: conn.close() @@ -365,8 +470,17 @@ class QualityUpgradeJob(RepairJob): if i % 10 == 0 and context.wait_if_paused(): return result - track_id, title, file_path, bitrate, artist_name, album_title, album_id = ( - row[0], row[1], row[2], row[3], row[4], row[5], row[6]) + track_id = row['id'] + title = row['title'] + file_path = row['file_path'] + bitrate = row['bitrate'] + artist_name = row['artist_name'] + album_title = row['album_title'] + album_id = row['album_id'] + track_number = row.get('track_number') + stored_album_ids = { + src: row[col] for src, col in _SOURCE_ALBUM_ID_COL.items() if row.get(col) + } result.scanned += 1 if meets_preferred_quality(file_path, bitrate, quality_profile): @@ -396,14 +510,31 @@ class QualityUpgradeJob(RepairJob): log_line=f'Low quality ({current_label}): {artist_name} - {title}', log_type='info') - # Fast path: enrichment embeds the ISRC (and per-source track IDs) in - # the file's tags, so an already-enriched track carries its exact - # identity. Resolve the EXACT track by ISRC — no fuzzy matching, and - # the real album comes with it. Only fall back to name/artist search - # for tracks that were never enriched / have no usable ISRC. + # Tiered match, best identity first, loosest last: + # 1. ISRC embedded in the file tags (enriched track) → EXACT track. + # 2. Album → track: use the album's stored source ID if we have it + # (enriched album), else find the album by search, then locate our + # track in its tracklist. Pins the right album even when the track + # itself isn't enriched. (artist → album → track) + # 3. Plain artist+title search with similarity scoring. (artist → track) + best, source, conf, attempted = None, None, 0.0, False + matched_via = 'isrc' best, source = _match_via_isrc(_read_track_isrc(file_path), source_priority) - conf, attempted = (1.0, True) if best else (0.0, False) + if best: + conf, attempted = 1.0, True + + if not best: + matched_via = 'album' + try: + best, source = _match_via_album( + engine, source_priority, artist_name or '', album_title or '', + title, track_number, stored_album_ids) + except Exception as e: + logger.debug("[Quality Upgrade] Album match error for %s - %s: %s", artist_name, title, e) + best = None + if best: + conf, attempted = 1.0, True if not best: matched_via = 'search' @@ -415,10 +546,10 @@ class QualityUpgradeJob(RepairJob): result.errors += 1 continue - if not attempted: - logger.warning("[Quality Upgrade] No metadata provider responded — stopping") - return result if not best: + if matched_via == 'search' and not attempted: + logger.warning("[Quality Upgrade] No metadata provider responded — stopping") + return result result.skipped += 1 continue @@ -442,7 +573,7 @@ class QualityUpgradeJob(RepairJob): description=( f'"{title}" by {artist_name} is {current_label}, below your preferred ' f'quality. Best match: "{_track_name(best)}" via {source} ' - f'({"exact ISRC match" if matched_via == "isrc" else f"confidence {conf:.0%}"}). ' + f'({_MATCH_NOTE.get(matched_via, "matched") if matched_via != "search" else f"confidence {conf:.0%}"}). ' 'Apply to add it to the wishlist.'), details={ 'track_id': track_id, diff --git a/tests/repair_jobs/test_quality_upgrade.py b/tests/repair_jobs/test_quality_upgrade.py index 0640f843..f4d8ab4c 100644 --- a/tests/repair_jobs/test_quality_upgrade.py +++ b/tests/repair_jobs/test_quality_upgrade.py @@ -153,6 +153,9 @@ def test_scan_creates_finding_for_low_quality_track(monkeypatch): ) fake_match = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X', 'images': []}} + # No ISRC / album hit → exercise the search tier. + monkeypatch.setattr(qu, '_read_track_isrc', lambda fp: '') + monkeypatch.setattr(qu, '_match_via_album', lambda *a, **k: (None, None)) monkeypatch.setattr(qu, '_find_best_match', lambda *a, **k: (fake_match, 0.95, 'spotify', True)) monkeypatch.setattr(qu, '_normalize_track_match', lambda track, src: dict(fake_match)) @@ -230,6 +233,7 @@ def test_scan_falls_back_to_search_without_isrc(monkeypatch): monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) monkeypatch.setattr('core.matching_engine.MusicMatchingEngine', lambda: types.SimpleNamespace()) monkeypatch.setattr(qu, '_read_track_isrc', lambda fp: '') # un-enriched + monkeypatch.setattr(qu, '_match_via_album', lambda *a, **k: (None, None)) # no album hit fake = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X'}} monkeypatch.setattr(qu, '_find_best_match', lambda *a, **k: (fake, 0.88, 'spotify', True)) monkeypatch.setattr(qu, '_normalize_track_match', lambda t, s: dict(fake)) @@ -241,6 +245,42 @@ def test_scan_falls_back_to_search_without_isrc(monkeypatch): assert findings[0]['details']['matched_via'] == 'search' +def test_scan_uses_album_tier_when_no_isrc(monkeypatch): + """No ISRC, but the album→track lookup resolves it → matched_via 'album', + and the fuzzy search is never reached.""" + rows = [(1, 'Song One', '/music/a.mp3', 128, 'Artist A', 'Album X', 10)] + db = _FakeDB(rows, BALANCED) + monkeypatch.setattr(qu, 'get_primary_source', lambda: 'spotify') + monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) + monkeypatch.setattr('core.matching_engine.MusicMatchingEngine', lambda: types.SimpleNamespace()) + monkeypatch.setattr(qu, '_read_track_isrc', lambda fp: '') + fake = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X'}} + monkeypatch.setattr(qu, '_match_via_album', lambda *a, **k: (fake, 'spotify')) + monkeypatch.setattr(qu, '_normalize_track_match', lambda t, s: dict(fake)) + monkeypatch.setattr(qu, '_track_name', lambda t: 'Song One') + + def _boom(*a, **k): + raise AssertionError("fuzzy search must not run when the album tier matches") + monkeypatch.setattr(qu, '_find_best_match', _boom) + + findings = [] + result = qu.QualityUpgradeJob().scan(_ctx(db, findings)) + assert result.findings_created == 1 + assert findings[0]['details']['matched_via'] == 'album' + assert findings[0]['details']['match_confidence'] == 1.0 + + +def test_find_track_in_album_exact_title_with_track_number(monkeypatch): + items = [ + {'id': 'a', 'name': 'Intro', 'track_number': 1}, + {'id': 'b', 'name': 'Karma Police', 'track_number': 6}, + {'id': 'c', 'name': 'Karma Police (Live)', 'track_number': 12}, + ] + eng = types.SimpleNamespace(similarity_score=lambda a, b: 0.0, normalize_string=lambda s: s) + got = qu._find_track_in_album(items, 'Karma Police', 6, eng) + assert got['id'] == 'b' + + def test_scan_skips_tracks_meeting_quality(monkeypatch): # A 320 kbps MP3 meets the balanced profile → no finding, no metadata calls. rows = [(2, 'Good Song', '/music/b.mp3', 320, 'Artist A', 'Album Y', 11)] From 030d9bf9ff05846180c910b901eda603d7f41b25 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 13:34:48 -0700 Subject: [PATCH 05/42] Quality Upgrade: best-in-class matching (direct track-ID tier, dedup-skip, duration guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four refinements on top of the tiered matcher: 1. Direct source track-ID tier (new top tier): enrichment writes each source's own track ID into the file tags (spotify_track_id/deezer_track_id/itunes_track_id/...). If we have the active source's track ID, fetch that exact track by ID via get_track_details — zero search. Tiers are now: track-ID -> ISRC -> album->track -> artist+title. _read_file_ids reads ISRC + all per-source IDs in one tag read. 2. Skip already-proposed tracks: a re-run loads existing finding entity_ids for the job and skips those tracks before any API call (pending stays deduped, dismissed stays dismissed) — re-runs are cheap. 3. Wrong-version guard: the fuzzy tiers (album-search + track search) reject a candidate whose length differs from ours by >5s (live/edit/remix with same title). _load_tracks now selects t.duration; exact tiers (track-ID/ISRC/stored-album-ID) skip the guard. 4. Tighter album matching: same-title cuts in an album are disambiguated by closest duration when track_number doesn't decide it. Findings record matched_via = track_id | isrc | album | search. 30 repair tests pass (added track-ID tier, duration guard, dedup-skip, and unit coverage). --- core/repair_jobs/quality_upgrade.py | 182 ++++++++++++++++++---- tests/repair_jobs/test_quality_upgrade.py | 147 ++++++++++++----- 2 files changed, 253 insertions(+), 76 deletions(-) diff --git a/core/repair_jobs/quality_upgrade.py b/core/repair_jobs/quality_upgrade.py index 8f98f0fa..f0ad8c53 100644 --- a/core/repair_jobs/quality_upgrade.py +++ b/core/repair_jobs/quality_upgrade.py @@ -64,6 +64,20 @@ _PROFILE_KEY_RANK = { 'mp3_192': RANK_192, } +# Per-source file-tag key holding that source's own track ID (written by enrichment). +_SOURCE_TRACK_ID_TAG = { + 'spotify': 'spotify_track_id', + 'deezer': 'deezer_track_id', + 'itunes': 'itunes_track_id', + 'audiodb': 'audiodb_track_id', + 'musicbrainz': 'musicbrainz_releasetrackid', + 'tidal': 'tidal_track_id', +} + +# Reject a fuzzy candidate whose length differs from ours by more than this (ms) — +# catches wrong versions (live/edit/remix) that share a title. Exact tiers skip it. +_DURATION_TOLERANCE_MS = 5000 + def _normalize_kbps(bitrate: Optional[int]) -> Optional[int]: """Library bitrate may be stored in bps (e.g. 320000) or kbps (320). @@ -159,24 +173,68 @@ def _norm_isrc(value: Any) -> str: return str(value).upper().replace('-', '').replace(' ', '').strip() -def _read_track_isrc(file_path: str) -> str: - """Read the ISRC the enrichment pipeline embedded in the file's tags. +def _read_file_ids(file_path: str) -> Dict[str, str]: + """Read the identifiers enrichment embedded in the file's tags. Enrichment matches every track to the metadata sources and writes the IDs - (ISRC, per-source track IDs) into the file — so an already-enriched track - carries its exact identity. Returns '' when unreadable / not enriched.""" + (ISRC + per-source track IDs) into the file — so an already-enriched track + carries its exact identity. Returns a dict with a normalized ``isrc`` plus any + ``_track_id`` tags present; empty dict when unreadable / not enriched.""" resolved = resolve_library_file_path(file_path) if file_path else None if not resolved and file_path and os.path.isfile(file_path): resolved = file_path if not resolved: - return '' + return {} try: info = read_embedded_tags(resolved) except Exception: - return '' + return {} if not info or not info.get('available'): - return '' - return _norm_isrc((info.get('tags') or {}).get('isrc')) + return {} + tags = info.get('tags') or {} + out: Dict[str, str] = {} + isrc = _norm_isrc(tags.get('isrc')) + if isrc: + out['isrc'] = isrc + for tag_key in set(_SOURCE_TRACK_ID_TAG.values()): + val = tags.get(tag_key) + if val: + out[tag_key] = str(val) + return out + + +def _duration_ok(want_ms: Any, got_ms: Any, tolerance_ms: int = _DURATION_TOLERANCE_MS) -> bool: + """Wrong-version guard: True when the candidate's length is within tolerance of + ours — or when either length is unknown (never reject on missing data).""" + try: + w, g = int(want_ms or 0), int(got_ms or 0) + except (TypeError, ValueError): + return True + if w <= 0 or g <= 0: + return True + return abs(w - g) <= tolerance_ms + + +def _match_via_track_id(file_ids: Dict[str, str], + source_priority: List[str]) -> Tuple[Optional[Any], Optional[str]]: + """Most-direct path: enrichment already wrote this track's per-source IDs into + the file. If we have the active source's own track ID, fetch that exact track by + ID — no search at all. Returns (track, source) or (None, None).""" + for source in source_priority: + tag_key = _SOURCE_TRACK_ID_TAG.get(source) + track_id = file_ids.get(tag_key) if tag_key else None + if not track_id: + continue + client = get_client_for_source(source) + if not client or not hasattr(client, 'get_track_details'): + continue + try: + track = client.get_track_details(str(track_id)) + except Exception: + track = None + if track: + return track, source + return None, None def _candidate_isrc(cand: Any) -> str: @@ -217,13 +275,16 @@ def _match_via_isrc(isrc: str, source_priority: List[str]) -> Tuple[Optional[Any # Column order for the _load_tracks SELECT — rows come back as dicts keyed by these. _TRACK_COLS = ( - 'id', 'title', 'file_path', 'bitrate', 'artist_name', 'album_title', 'album_id', - 'track_number', 'spotify_album_id', 'itunes_album_id', 'deezer_id', + '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', ) # Human-readable note per match tier (search uses a confidence % instead). -_MATCH_NOTE = {'isrc': 'exact ISRC match', 'album': 'matched within album'} +_MATCH_NOTE = { + 'track_id': 'exact track ID', 'isrc': 'exact ISRC match', + 'album': 'matched within album', +} # Per-source column holding that source's album ID on the albums table. _SOURCE_ALBUM_ID_COL = { @@ -240,9 +301,11 @@ def _norm_title(value: Any) -> str: return ''.join(ch for ch in str(value or '').lower() if ch.isalnum()) -def _find_track_in_album(items: Any, title: str, track_number: Any, engine: Any) -> Optional[Any]: +def _find_track_in_album(items: Any, title: str, track_number: Any, engine: Any, + want_duration_ms: Any = None) -> Optional[Any]: """Pick the track in an album's tracklist that matches ours — exact normalized - title first (track_number breaks ties), then a high-similarity fuzzy fallback.""" + title first (track_number then duration break ties), then a high-similarity + fuzzy fallback that respects the duration guard.""" want = _norm_title(title) exact = [] best, best_score = None, 0.0 @@ -252,6 +315,8 @@ def _find_track_in_album(items: Any, title: str, track_number: Any, engine: Any) exact.append(it) continue if engine and it_name: + if not _duration_ok(want_duration_ms, _extract_lookup_value(it, 'duration_ms', 'duration')): + continue score = engine.similarity_score( engine.normalize_string(title), engine.normalize_string(it_name)) if score > best_score and score >= 0.85: @@ -261,13 +326,17 @@ def _find_track_in_album(items: Any, title: str, track_number: Any, engine: Any) for it in exact: if _extract_lookup_value(it, 'track_number') == track_number: return it + # Multiple same-title cuts (e.g. album + live): prefer the closest length. + if want_duration_ms and len(exact) > 1: + exact.sort(key=lambda it: abs(int(want_duration_ms) - int( + _extract_lookup_value(it, 'duration_ms', 'duration', default=0) or 0))) return exact[0] return best def _match_via_album(engine: Any, source_priority: List[str], artist: str, album_title: str, - title: str, track_number: Any, - stored_album_ids: Dict[str, str]) -> Tuple[Optional[Any], Optional[str]]: + title: str, track_number: Any, stored_album_ids: Dict[str, str], + want_duration_ms: Any = None) -> Tuple[Optional[Any], Optional[str]]: """Structured artist → album → track match. For each source: use the album's stored source ID if we already have it (enriched album), else find the album by searching ``artist album``; then pull that album's tracklist and locate our @@ -305,7 +374,7 @@ def _match_via_album(engine: Any, source_priority: List[str], artist: str, album except Exception: resp = None items = resp.get('items') if isinstance(resp, dict) else None - match = _find_track_in_album(items, title, track_number, engine) + match = _find_track_in_album(items, title, track_number, engine, want_duration_ms) if match is None: continue # The album tracklist's tracks usually omit the album object — attach it so @@ -319,7 +388,8 @@ def _match_via_album(engine: Any, source_priority: List[str], artist: str, album def _find_best_match(engine: Any, source_priority: List[str], title: str, artist: str, - album: str, min_confidence: float) -> Tuple[Optional[Any], float, Optional[str], bool]: + album: str, min_confidence: float, + want_duration_ms: Any = None) -> Tuple[Optional[Any], float, Optional[str], bool]: """Search the configured metadata sources for the best replacement match. Returns (best_track, confidence, source, attempted_any_provider).""" temp_track = type('TempTrack', (), {'name': title, 'artists': [artist], 'album': album})() @@ -336,6 +406,10 @@ def _find_best_match(engine: Any, source_priority: List[str], title: str, artist matches = _search_tracks_for_source(source, query, limit=5, client=client) time.sleep(0.5) # be gentle on metadata APIs for cand in matches or []: + # Wrong-version guard: a candidate whose length is way off is a + # different cut (live/edit/remix) — reject before it can win. + if not _duration_ok(want_duration_ms, _extract_lookup_value(cand, 'duration_ms', 'duration')): + continue cand_artists = _track_artist_names(cand) artist_conf = max( (engine.similarity_score(engine.normalize_string(artist), @@ -369,12 +443,14 @@ class QualityUpgradeJob(RepairJob): "track against your Quality Profile using BOTH the file format and its " 'bitrate — so a 128 kbps MP3 is no longer treated the same as a 320 kbps ' 'one, and enabling MP3-320/256 in your profile actually counts.\n\n' - 'For every track below your preferred quality, it finds a better version and ' - 'creates a finding. If the track was enriched, it uses the ISRC embedded in ' - 'the file to resolve the EXACT track (and its album) — no guessing; otherwise ' - 'it falls back to a name/artist search with a confidence score. Nothing is ' - 'queued automatically: applying a finding adds that matched track — with its ' - 'album context — to the wishlist, the same as any other download.\n\n' + 'For every track below your preferred quality it resolves the exact better ' + 'version using the most precise identity available, in order: the source ' + "track ID enrichment wrote into the file → the file's ISRC → the album's " + 'tracklist (by stored album ID or album search) → a name/artist search. The ' + 'fuzzy steps also reject candidates whose length is off (wrong live/edit cut). ' + 'It skips tracks it already proposed, so re-runs are cheap. Nothing is queued ' + 'automatically: applying a finding adds that matched track — with its album ' + 'context — to the wishlist, the same as any other download.\n\n' 'Settings:\n' '- Scope: "watchlist" (watchlisted artists only) or "all" (whole library)\n' '- Min confidence: minimum match confidence (0-1) to surface a finding\n\n' @@ -404,8 +480,8 @@ class QualityUpgradeJob(RepairJob): conn = db._get_connection() try: base = ( - "SELECT t.id, t.title, t.file_path, t.bitrate, a.name AS artist_name, " - "al.title AS album_title, t.album_id, t.track_number, " + "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 " "FROM tracks t " @@ -428,6 +504,21 @@ class QualityUpgradeJob(RepairJob): finally: conn.close() + def _load_existing_finding_ids(self, db: Any) -> set: + """Track IDs that already have a finding for this job (any status). Lets a + re-run skip tracks we've already proposed/dismissed without re-hitting the + metadata API — pending stays deduped, and a dismissed track stays dismissed.""" + conn = db._get_connection() + try: + rows = conn.execute( + "SELECT entity_id FROM repair_findings WHERE job_id = ? AND entity_type = 'track'", + (self.job_id,)).fetchall() + return {str(r[0]) for r in rows if r and r[0] is not None} + except Exception: + return set() + finally: + conn.close() + def estimate_scope(self, context: JobContext) -> int: try: return len(self._load_tracks(context.db, self._get_settings(context)['scope'])) @@ -459,6 +550,10 @@ class QualityUpgradeJob(RepairJob): if context.report_progress: context.report_progress(phase=f'Checking quality on {total} tracks...', total=total) + # Tracks we've already proposed/dismissed — skip them so a re-run doesn't + # re-resolve the same tracks against the metadata API. + already_found = self._load_existing_finding_ids(db) + # Metadata source for matching — resolved lazily so we only fail if we # actually find a low-quality track that needs a match. engine = None @@ -474,6 +569,7 @@ class QualityUpgradeJob(RepairJob): title = row['title'] file_path = row['file_path'] bitrate = row['bitrate'] + duration_ms = row.get('duration') artist_name = row['artist_name'] album_title = row['album_title'] album_id = row['album_id'] @@ -483,6 +579,10 @@ class QualityUpgradeJob(RepairJob): } result.scanned += 1 + if str(track_id) in already_found: + result.findings_skipped_dedup += 1 + continue + if meets_preferred_quality(file_path, bitrate, quality_profile): result.skipped += 1 if context.update_progress and (i + 1) % 25 == 0: @@ -510,26 +610,39 @@ class QualityUpgradeJob(RepairJob): log_line=f'Low quality ({current_label}): {artist_name} - {title}', log_type='info') + # Read the identifiers enrichment embedded in the file once (ISRC + + # per-source track IDs), used by the two most-exact tiers below. + file_ids = _read_file_ids(file_path) + # Tiered match, best identity first, loosest last: - # 1. ISRC embedded in the file tags (enriched track) → EXACT track. - # 2. Album → track: use the album's stored source ID if we have it - # (enriched album), else find the album by search, then locate our - # track in its tracklist. Pins the right album even when the track - # itself isn't enriched. (artist → album → track) + # 0. The active source's OWN track ID, embedded in the file by + # enrichment → fetch that exact track by ID. No search at all. + # 1. ISRC (also in the tags) → exact track on any source. + # 2. Album → track: stored album source ID if we have it (enriched + # album), else find the album by search, then locate our track in + # its tracklist. Pins the right album even when the track itself + # isn't enriched. (artist → album → track) # 3. Plain artist+title search with similarity scoring. (artist → track) + # The fuzzy tiers (2-3) also apply a duration guard to reject wrong cuts. best, source, conf, attempted = None, None, 0.0, False - matched_via = 'isrc' - best, source = _match_via_isrc(_read_track_isrc(file_path), source_priority) + matched_via = 'track_id' + best, source = _match_via_track_id(file_ids, source_priority) if best: conf, attempted = 1.0, True + if not best: + matched_via = 'isrc' + best, source = _match_via_isrc(file_ids.get('isrc', ''), source_priority) + if best: + conf, attempted = 1.0, True + if not best: matched_via = 'album' try: best, source = _match_via_album( engine, source_priority, artist_name or '', album_title or '', - title, track_number, stored_album_ids) + title, track_number, stored_album_ids, duration_ms) except Exception as e: logger.debug("[Quality Upgrade] Album match error for %s - %s: %s", artist_name, title, e) best = None @@ -540,7 +653,8 @@ class QualityUpgradeJob(RepairJob): matched_via = 'search' try: best, conf, source, attempted = _find_best_match( - engine, source_priority, title, artist_name or '', album_title or '', min_conf) + engine, source_priority, title, artist_name or '', album_title or '', + min_conf, duration_ms) except Exception as e: logger.debug("[Quality Upgrade] Match error for %s - %s: %s", artist_name, title, e) result.errors += 1 diff --git a/tests/repair_jobs/test_quality_upgrade.py b/tests/repair_jobs/test_quality_upgrade.py index f4d8ab4c..d1ff35fa 100644 --- a/tests/repair_jobs/test_quality_upgrade.py +++ b/tests/repair_jobs/test_quality_upgrade.py @@ -96,13 +96,20 @@ def meets(path, bitrate, profile): # --- scan produces a finding (seam) ---------------------------------------- class _FakeConn: - def __init__(self, rows): + def __init__(self, rows, finding_ids=()): self._rows = rows + self._finding_ids = list(finding_ids) + self._sql = '' - def execute(self, *a, **k): + def execute(self, sql='', *a, **k): + self._sql = sql or '' return self def fetchall(self): + # The existing-findings query reads repair_findings; everything else is the + # track load. + if 'repair_findings' in self._sql: + return [(fid,) for fid in self._finding_ids] return self._rows def close(self): @@ -110,15 +117,16 @@ class _FakeConn: class _FakeDB: - def __init__(self, rows, profile): + def __init__(self, rows, profile, finding_ids=()): self._rows = rows self._profile = profile + self._finding_ids = finding_ids def get_quality_profile(self): return self._profile def _get_connection(self): - return _FakeConn(self._rows) + return _FakeConn(self._rows, self._finding_ids) def get_watchlist_artists(self, profile_id=1): return [types.SimpleNamespace(artist_name='Artist A')] @@ -135,12 +143,13 @@ def _ctx(db, findings): ) -def test_scan_creates_finding_for_low_quality_track(monkeypatch): - # One 128 kbps MP3 (below the balanced floor) for Artist A. - rows = [(1, 'Song One', '/music/a.mp3', 128, 'Artist A', 'Album X', 10)] - db = _FakeDB(rows, BALANCED) +def _row(track_id=1, title='Song One', path='/music/a.mp3', bitrate=128, duration=180000, + artist='Artist A', album='Album X', album_id=10, track_number=6): + """A track row in _TRACK_COLS order (album source-id columns default to None).""" + return (track_id, title, path, bitrate, duration, artist, album, album_id, track_number) - # Stub the metadata side so the test stays offline. + +def _stub_engine(monkeypatch): monkeypatch.setattr(qu, 'get_primary_source', lambda: 'spotify') monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) monkeypatch.setattr( @@ -151,10 +160,16 @@ def test_scan_creates_finding_for_low_quality_track(monkeypatch): normalize_string=lambda s: s, ), ) + + +def test_scan_creates_finding_for_low_quality_track(monkeypatch): + db = _FakeDB([_row(bitrate=128)], BALANCED) + _stub_engine(monkeypatch) fake_match = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X', 'images': []}} - # No ISRC / album hit → exercise the search tier. - monkeypatch.setattr(qu, '_read_track_isrc', lambda fp: '') + # No track-id / ISRC / album hit → exercise the search tier. + monkeypatch.setattr(qu, '_read_file_ids', lambda fp: {}) + monkeypatch.setattr(qu, '_match_via_track_id', lambda *a, **k: (None, None)) monkeypatch.setattr(qu, '_match_via_album', lambda *a, **k: (None, None)) monkeypatch.setattr(qu, '_find_best_match', lambda *a, **k: (fake_match, 0.95, 'spotify', True)) @@ -162,9 +177,7 @@ def test_scan_creates_finding_for_low_quality_track(monkeypatch): monkeypatch.setattr(qu, '_track_name', lambda t: 'Song One') findings = [] - job = qu.QualityUpgradeJob() - # default scope 'watchlist'; config_manager None → defaults used - result = job.scan(_ctx(db, findings)) + result = qu.QualityUpgradeJob().scan(_ctx(db, findings)) assert result.findings_created == 1 assert len(findings) == 1 @@ -177,6 +190,63 @@ def test_scan_creates_finding_for_low_quality_track(monkeypatch): assert f['details']['provider'] == 'spotify' +def test_match_via_track_id_fetches_exact_by_id(monkeypatch): + """Most-direct tier: a per-source track ID in the tags → get_track_details by ID.""" + track = {'id': 'sp9', 'name': 'Song One', 'album': {'name': 'Album X'}} + client = types.SimpleNamespace(get_track_details=lambda tid: track if tid == 'sp9' else None) + monkeypatch.setattr(qu, 'get_client_for_source', lambda src: client) + best, source = qu._match_via_track_id({'spotify_track_id': 'sp9'}, ['spotify']) + assert best['id'] == 'sp9' + assert source == 'spotify' + assert qu._match_via_track_id({}, ['spotify']) == (None, None) # no ID → nothing + + +def test_duration_ok_guard(): + assert qu._duration_ok(180000, 181000) is True # within 5s + assert qu._duration_ok(180000, 200000) is False # 20s off — wrong cut + assert qu._duration_ok(None, 200000) is True # unknown → lenient + assert qu._duration_ok(180000, 0) is True # unknown → lenient + + +def test_scan_prefers_track_id_tier(monkeypatch): + """The source's own track ID (from file tags) wins over every other tier.""" + db = _FakeDB([_row()], BALANCED) + _stub_engine(monkeypatch) + monkeypatch.setattr(qu, '_read_file_ids', lambda fp: {'spotify_track_id': 'sp9', 'isrc': 'X'}) + fake = {'id': 'sp9', 'name': 'Song One', 'album': {'name': 'Album X'}} + monkeypatch.setattr(qu, '_match_via_track_id', lambda ids, sp: (fake, 'spotify')) + monkeypatch.setattr(qu, '_normalize_track_match', lambda t, s: dict(fake)) + monkeypatch.setattr(qu, '_track_name', lambda t: 'Song One') + + def _boom(*a, **k): + raise AssertionError("no lower tier should run when the track-ID tier matches") + monkeypatch.setattr(qu, '_match_via_isrc', _boom) + monkeypatch.setattr(qu, '_match_via_album', _boom) + monkeypatch.setattr(qu, '_find_best_match', _boom) + + findings = [] + result = qu.QualityUpgradeJob().scan(_ctx(db, findings)) + assert result.findings_created == 1 + assert findings[0]['details']['matched_via'] == 'track_id' + + +def test_scan_skips_already_proposed_tracks(monkeypatch): + """A re-run must not re-resolve a track that already has a finding.""" + db = _FakeDB([_row(track_id=1)], BALANCED, finding_ids=['1']) + monkeypatch.setattr(qu, 'get_primary_source', lambda: 'spotify') + monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) + + def _boom(*a, **k): + raise AssertionError("no matching for an already-proposed track") + monkeypatch.setattr(qu, '_match_via_track_id', _boom) + monkeypatch.setattr(qu, '_find_best_match', _boom) + + findings = [] + result = qu.QualityUpgradeJob().scan(_ctx(db, findings)) + assert findings == [] + assert result.findings_skipped_dedup == 1 + + def test_match_via_isrc_accepts_exact_match(monkeypatch): """The guard accepts only a candidate whose own ISRC equals ours (dash/case insensitive), so it survives a source returning unrelated hits first.""" @@ -201,14 +271,12 @@ def test_match_via_isrc_rejects_all_mismatches(monkeypatch): def test_scan_prefers_isrc_exact_match_over_fuzzy(monkeypatch): - """When the file carries an ISRC and it resolves, use the exact match and do - NOT run the fuzzy search at all.""" - rows = [(1, 'Song One', '/music/a.mp3', 128, 'Artist A', 'Album X', 10)] - db = _FakeDB(rows, BALANCED) - monkeypatch.setattr(qu, 'get_primary_source', lambda: 'spotify') - monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) - monkeypatch.setattr('core.matching_engine.MusicMatchingEngine', lambda: types.SimpleNamespace()) - monkeypatch.setattr(qu, '_read_track_isrc', lambda fp: 'USRC17607839') + """No track-ID, but the file carries an ISRC that resolves → use the exact match + and do NOT run the album/search tiers.""" + db = _FakeDB([_row()], BALANCED) + _stub_engine(monkeypatch) + monkeypatch.setattr(qu, '_read_file_ids', lambda fp: {'isrc': 'USRC17607839'}) + monkeypatch.setattr(qu, '_match_via_track_id', lambda *a, **k: (None, None)) fake = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X'}} monkeypatch.setattr(qu, '_match_via_isrc', lambda isrc, sp: (fake, 'spotify')) monkeypatch.setattr(qu, '_normalize_track_match', lambda t, s: dict(fake)) @@ -225,15 +293,13 @@ def test_scan_prefers_isrc_exact_match_over_fuzzy(monkeypatch): assert findings[0]['details']['match_confidence'] == 1.0 -def test_scan_falls_back_to_search_without_isrc(monkeypatch): - """No usable ISRC → fall back to fuzzy search.""" - rows = [(1, 'Song One', '/music/a.mp3', 128, 'Artist A', 'Album X', 10)] - db = _FakeDB(rows, BALANCED) - monkeypatch.setattr(qu, 'get_primary_source', lambda: 'spotify') - monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) - monkeypatch.setattr('core.matching_engine.MusicMatchingEngine', lambda: types.SimpleNamespace()) - monkeypatch.setattr(qu, '_read_track_isrc', lambda fp: '') # un-enriched - monkeypatch.setattr(qu, '_match_via_album', lambda *a, **k: (None, None)) # no album hit +def test_scan_falls_back_to_search_without_ids(monkeypatch): + """No track-ID / ISRC / album hit → fall back to fuzzy search.""" + db = _FakeDB([_row()], BALANCED) + _stub_engine(monkeypatch) + monkeypatch.setattr(qu, '_read_file_ids', lambda fp: {}) # un-enriched + monkeypatch.setattr(qu, '_match_via_track_id', lambda *a, **k: (None, None)) + monkeypatch.setattr(qu, '_match_via_album', lambda *a, **k: (None, None)) fake = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X'}} monkeypatch.setattr(qu, '_find_best_match', lambda *a, **k: (fake, 0.88, 'spotify', True)) monkeypatch.setattr(qu, '_normalize_track_match', lambda t, s: dict(fake)) @@ -245,15 +311,13 @@ def test_scan_falls_back_to_search_without_isrc(monkeypatch): assert findings[0]['details']['matched_via'] == 'search' -def test_scan_uses_album_tier_when_no_isrc(monkeypatch): - """No ISRC, but the album→track lookup resolves it → matched_via 'album', - and the fuzzy search is never reached.""" - rows = [(1, 'Song One', '/music/a.mp3', 128, 'Artist A', 'Album X', 10)] - db = _FakeDB(rows, BALANCED) - monkeypatch.setattr(qu, 'get_primary_source', lambda: 'spotify') - monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) - monkeypatch.setattr('core.matching_engine.MusicMatchingEngine', lambda: types.SimpleNamespace()) - monkeypatch.setattr(qu, '_read_track_isrc', lambda fp: '') +def test_scan_uses_album_tier_when_no_ids(monkeypatch): + """No track-ID / ISRC, but the album→track lookup resolves it → matched_via + 'album', and the fuzzy search is never reached.""" + db = _FakeDB([_row()], BALANCED) + _stub_engine(monkeypatch) + monkeypatch.setattr(qu, '_read_file_ids', lambda fp: {}) + monkeypatch.setattr(qu, '_match_via_track_id', lambda *a, **k: (None, None)) fake = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X'}} monkeypatch.setattr(qu, '_match_via_album', lambda *a, **k: (fake, 'spotify')) monkeypatch.setattr(qu, '_normalize_track_match', lambda t, s: dict(fake)) @@ -283,8 +347,7 @@ def test_find_track_in_album_exact_title_with_track_number(monkeypatch): def test_scan_skips_tracks_meeting_quality(monkeypatch): # A 320 kbps MP3 meets the balanced profile → no finding, no metadata calls. - rows = [(2, 'Good Song', '/music/b.mp3', 320, 'Artist A', 'Album Y', 11)] - db = _FakeDB(rows, BALANCED) + db = _FakeDB([_row(track_id=2, title='Good Song', bitrate=320)], BALANCED) def _boom(*a, **k): # must never be called for an acceptable track raise AssertionError("matching should not run for an acceptable track") From 177a4d8d056ae1b91715f776e24f2678effc8c05 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 14:36:30 -0700 Subject: [PATCH 06/42] #868: disambiguate same-name artists by owned-catalog overlap during enrichment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enrichment matched artists by NAME ONLY (0.85 gate), so for a common name ('Rone' has ~5 artists) it stored whichever the source ranked first — often the wrong one, which then drove a wrong/sparse library 'Standard' discography while 'Enhanced' (the real owned albums) showed the full set. Fix — use the decisive signal the library already has (the albums you OWN): - worker_utils: pick_artist_by_catalog() + catalog_overlap_score() + owned_album_titles()/release_titles(). When 2+ candidates clear the name gate, fetch each one's catalog and choose the one overlapping the owned albums; falls back to the current best-by-name pick when there's nothing to disambiguate or no overlap (so the common single-candidate path makes no extra API calls). - Wired into Spotify (covers Spotify-Free, same client), iTunes, Deezer (now multi-candidate search_artists + get_artist_info store), and MusicBrainz (match_artist gains owned_titles; release-groups as the catalog). Re-match path (#868): - build_reset_query now also clears the stored source-ID column for artist/album item resets — previously a 're-match' only nulled match_status, so the worker's existing-id short-circuit re-confirmed the WRONG id and never re-resolved. Tracks excluded (ids live in tags, not a column). - MusicBrainz also self-corrects its 90-day name->mbid cache: match_artist bypasses a cached mbid whose catalog has ZERO overlap with the owned albums, so a re-match isn't blocked by a stale wrong cache entry. Tests: shared selector (9), per-worker disambiguation for all 4 sources + MB backward-compat + MB cache-revalidation (8), reset-clears-id (2). 99 worker/ enrichment tests green. --- core/deezer_worker.py | 25 ++- core/enrichment/unmatched.py | 17 +- core/itunes_worker.py | 34 +++- core/musicbrainz_service.py | 80 ++++++--- core/musicbrainz_worker.py | 5 +- core/spotify_worker.py | 30 ++-- core/worker_utils.py | 102 +++++++++++ tests/enrichment/test_unmatched.py | 18 ++ tests/test_artist_catalog_disambiguation.py | 98 +++++++++++ tests/test_worker_artist_disambiguation.py | 186 ++++++++++++++++++++ 10 files changed, 549 insertions(+), 46 deletions(-) create mode 100644 tests/test_artist_catalog_disambiguation.py create mode 100644 tests/test_worker_artist_disambiguation.py diff --git a/core/deezer_worker.py b/core/deezer_worker.py index 15441c1e..80647754 100644 --- a/core/deezer_worker.py +++ b/core/deezer_worker.py @@ -8,7 +8,15 @@ from datetime import datetime, timedelta from utils.logging_config import get_logger from database.music_database import MusicDatabase from core.deezer_client import DeezerClient -from core.worker_utils import accept_artist_match, interruptible_sleep, set_album_api_track_count +from core.worker_utils import ( + accept_artist_match, + artist_name_matches, + interruptible_sleep, + owned_album_titles, + pick_artist_by_catalog, + release_titles, + set_album_api_track_count, +) from core.enrichment.manual_match_honoring import honor_stored_match logger = get_logger("deezer_worker") @@ -401,7 +409,20 @@ class DeezerWorker: logger.debug(f"Preserving existing Deezer ID for artist '{artist_name}': {existing_id}") return - result = self.client.search_artist(artist_name) + # Multi-candidate search (was single search_artist) so same-name artists + # can be disambiguated: gate by name, then pick the one whose catalog + # overlaps the albums this library owns. + results = self.client.search_artists(artist_name, limit=5) + gated = [a for a in (results or []) if artist_name_matches(artist_name, getattr(a, 'name', ''))] + chosen, _overlap = pick_artist_by_catalog( + gated, + owned_album_titles(self.db, artist_id), + lambda a: release_titles(self.client.get_artist_albums_list(a.id)), + ) + + # search_artists returns lean Artist objects; fetch the full dict (same + # shape the old search_artist returned) for storage. + result = self.client.get_artist_info(chosen.id) if chosen else None if result: result_name = result.get('name', '') ok, reason = accept_artist_match( diff --git a/core/enrichment/unmatched.py b/core/enrichment/unmatched.py index 0d63ac79..7a574438 100644 --- a/core/enrichment/unmatched.py +++ b/core/enrichment/unmatched.py @@ -216,7 +216,22 @@ def build_reset_query( table = meta['table'] ms = match_status_column(service) la = last_attempted_column(service) - set_clause = f"SET {ms} = NULL, {la} = NULL" + set_parts = [f"{ms} = NULL", f"{la} = NULL"] + + # Also forget the stored source ID so re-matching actually RE-RESOLVES the + # entity. Without this, the worker hits its existing-id short-circuit, sees + # the old (possibly WRONG) id and just re-confirms it — which is why "click + # to rematch" never fixed a mis-matched same-name artist (#868). Tracks keep + # their ids in file tags rather than a column, so only artist/album clear one. + if entity_type in ('artist', 'album'): + try: + from core.source_ids import id_column + id_col = id_column(service, entity_type) + except Exception: + id_col = None + if id_col: + set_parts.append(f"{id_col} = NULL") + set_clause = "SET " + ", ".join(set_parts) if scope == 'item': if not entity_id: diff --git a/core/itunes_worker.py b/core/itunes_worker.py index 7e3e8a5d..50adba2a 100644 --- a/core/itunes_worker.py +++ b/core/itunes_worker.py @@ -9,7 +9,15 @@ from datetime import datetime, timedelta from utils.logging_config import get_logger from database.music_database import MusicDatabase from core.itunes_client import iTunesClient -from core.worker_utils import accept_artist_match, interruptible_sleep, set_album_api_track_count +from core.worker_utils import ( + accept_artist_match, + artist_name_matches, + interruptible_sleep, + owned_album_titles, + pick_artist_by_catalog, + release_titles, + set_album_api_track_count, +) from core.enrichment.manual_match_honoring import honor_stored_match logger = get_logger("itunes_worker") @@ -392,20 +400,30 @@ class iTunesWorker: logger.debug(f"No iTunes results for artist '{artist_name}'") return - for artist_obj in results: + # Candidates clearing the name gate (results are source-ranked, so [0] is + # the legacy "first passing" pick), then disambiguate same-name artists by + # which one's catalog overlaps the albums this library owns. + gated = [a for a in results if artist_name_matches(artist_name, a.name)] + chosen, _overlap = pick_artist_by_catalog( + gated, + owned_album_titles(self.db, artist_id), + lambda a: release_titles(self.client.get_artist_albums(a.id)), + ) + + if chosen: ok, reason = accept_artist_match( - self.db, 'itunes_artist_id', artist_obj.id, artist_id, - artist_name, artist_obj.name, + self.db, 'itunes_artist_id', chosen.id, artist_id, + artist_name, chosen.name, ) if ok: - if not self._is_itunes_id(artist_obj.id): - logger.warning(f"Rejecting non-iTunes ID '{artist_obj.id}' for artist '{artist_name}'") + if not self._is_itunes_id(chosen.id): + logger.warning(f"Rejecting non-iTunes ID '{chosen.id}' for artist '{artist_name}'") self._mark_status('artist', artist_id, 'error') self.stats['errors'] += 1 return - self._update_artist(artist_id, artist_obj) + self._update_artist(artist_id, chosen) self.stats['matched'] += 1 - logger.info(f"Matched artist '{artist_name}' -> iTunes ID: {artist_obj.id}") + logger.info(f"Matched artist '{artist_name}' -> iTunes ID: {chosen.id}") return self._mark_status('artist', artist_id, 'not_found') diff --git a/core/musicbrainz_service.py b/core/musicbrainz_service.py index 4508618d..a8a633dd 100644 --- a/core/musicbrainz_service.py +++ b/core/musicbrainz_service.py @@ -5,6 +5,7 @@ from datetime import datetime, timedelta from difflib import SequenceMatcher from utils.logging_config import get_logger from core.musicbrainz_client import MusicBrainzClient +from core.worker_utils import catalog_overlap_score, pick_artist_by_catalog from database.music_database import MusicDatabase logger = get_logger("musicbrainz_service") @@ -117,23 +118,51 @@ class MusicBrainzService: if conn: conn.close() - def match_artist(self, artist_name: str) -> Optional[Dict[str, Any]]: + def _candidate_release_titles(self, mbid: str) -> list: + """Release-group titles for a candidate MBID — the catalog side of + same-name artist disambiguation.""" + if not mbid: + return [] + try: + data = self.mb_client.get_artist(mbid, includes=['release-groups']) + except Exception: + return [] + groups = (data or {}).get('release-groups') or [] + return [g.get('title') for g in groups if isinstance(g, dict) and g.get('title')] + + def match_artist(self, artist_name: str, owned_titles: Optional[list] = None) -> Optional[Dict[str, Any]]: """ - Match an artist by name to MusicBrainz - + Match an artist by name to MusicBrainz. + + ``owned_titles`` — the library artist's owned album titles. When given and + more than one strong same-name candidate exists, the one whose release + groups overlap those owned titles is chosen (disambiguates the ~5 "Rone"s); + omitted → falls back to the highest-confidence candidate as before. + Returns: Dict with 'mbid', 'name', 'confidence' or None if no good match """ # Check cache first cached = self._check_cache('artist', artist_name) if cached: - logger.debug(f"Cache hit for artist '{artist_name}'") - return { - 'mbid': cached['musicbrainz_id'], - 'name': artist_name, - 'confidence': cached['confidence'], - 'cached': True - } + cached_mbid = cached.get('musicbrainz_id') + # Don't trust a cached mbid whose catalog has ZERO overlap with the + # albums this library owns — that's the wrong same-name artist (and a + # re-match would otherwise be blocked for up to the 90-day cache TTL, + # #868). Fall through to a fresh, disambiguated resolve in that case. + stale_wrong_match = bool( + cached_mbid and owned_titles + and catalog_overlap_score(owned_titles, self._candidate_release_titles(cached_mbid)) == 0 + ) + if not stale_wrong_match: + logger.debug(f"Cache hit for artist '{artist_name}'") + return { + 'mbid': cached_mbid, + 'name': artist_name, + 'confidence': cached['confidence'], + 'cached': True + } + logger.debug(f"Cached MB match for '{artist_name}' has no owned-catalog overlap — re-resolving") # Search MusicBrainz try: @@ -144,25 +173,30 @@ class MusicBrainzService: self._save_to_cache('artist', artist_name, None, None, None, 0) return None - # Find best match - best_match = None - best_confidence = 0 - + # Score every candidate (name similarity 60% + MB's own relevance 40%). + scored = [] for result in results: mb_name = result.get('name', '') mb_score = result.get('score', 0) # MusicBrainz search score - - # Calculate our own similarity similarity = self._calculate_similarity(artist_name, mb_name) - - # Combine MusicBrainz score with our similarity (weighted) # Cap at 100 to prevent edge cases where MB score > 100 confidence = min(100, int((similarity * 60) + (mb_score / 100 * 40))) - - if confidence > best_confidence: - best_confidence = confidence - best_match = result - + scored.append((confidence, result)) + scored.sort(key=lambda s: s[0], reverse=True) + + # Among the strong (>=70) candidates, disambiguate same-name artists by + # which one's release groups overlap the albums this library owns. + gated = [r for conf, r in scored if conf >= 70] + best_match = None + best_confidence = scored[0][0] if scored else 0 + if gated: + chosen, _overlap = pick_artist_by_catalog( + gated, owned_titles or [], + lambda r: self._candidate_release_titles(r.get('id')), + ) + best_match = chosen + best_confidence = next(conf for conf, r in scored if r is chosen) + # Only return matches with confidence >= 70% if best_match and best_confidence >= 70: mbid = best_match.get('id') diff --git a/core/musicbrainz_worker.py b/core/musicbrainz_worker.py index 8f7c1241..d197bf74 100644 --- a/core/musicbrainz_worker.py +++ b/core/musicbrainz_worker.py @@ -5,7 +5,7 @@ from datetime import datetime, timedelta from utils.logging_config import get_logger from database.music_database import MusicDatabase from core.musicbrainz_service import MusicBrainzService -from core.worker_utils import interruptible_sleep, source_id_conflict +from core.worker_utils import interruptible_sleep, owned_album_titles, source_id_conflict logger = get_logger("musicbrainz_worker") @@ -366,7 +366,8 @@ class MusicBrainzWorker: return if item_type == 'artist': - result = self.mb_service.match_artist(item_name) + result = self.mb_service.match_artist( + item_name, owned_titles=owned_album_titles(self.db, item_id)) mbid = result.get('mbid') if result else None # MB's combined score can match a weak name ("Grant" -> "Amy # Grant") when its own relevance rank is high. Guard against diff --git a/core/spotify_worker.py b/core/spotify_worker.py index ffc6b07d..0f2fcd54 100644 --- a/core/spotify_worker.py +++ b/core/spotify_worker.py @@ -12,6 +12,9 @@ from core.spotify_client import SpotifyClient, SpotifyRateLimitError from core.worker_utils import ( ARTIST_NAME_MATCH_THRESHOLD, interruptible_sleep, + owned_album_titles, + pick_artist_by_catalog, + release_titles, set_album_api_track_count, source_id_conflict, ) @@ -563,16 +566,23 @@ class SpotifyWorker: logger.debug(f"No Spotify results for artist '{artist_name}'") return - # Find best fuzzy match — score all candidates, pick highest above the - # (stricter, artist-specific) threshold so short-name false positives - # like "ODESZA"/"odessa" don't slip through. - best_obj = None - best_score = 0 - for artist_obj in results: - score = self._name_similarity(artist_name, artist_obj.name) - if score >= ARTIST_NAME_MATCH_THRESHOLD and score > best_score: - best_obj = artist_obj - best_score = score + # Candidates clearing the (stricter, artist-specific) name gate, best + # name-score first so [0] is the legacy "first/highest" pick. + scored = [ + (self._name_similarity(artist_name, a.name), a) + for a in results + ] + gated = [a for score, a in sorted(scored, key=lambda s: s[0], reverse=True) + if score >= ARTIST_NAME_MATCH_THRESHOLD] + + # Same-name disambiguation: when more than one "Rone" clears the gate, + # pick the one whose catalog overlaps the albums this library owns. + best_obj, _overlap = pick_artist_by_catalog( + gated, + owned_album_titles(self.db, artist_id), + lambda a: release_titles(self.client.get_artist_albums(a.id)), + ) + best_score = self._name_similarity(artist_name, best_obj.name) if best_obj else 0 if best_obj: if not self._is_spotify_id(best_obj.id): diff --git a/core/worker_utils.py b/core/worker_utils.py index 7a8b57cb..949d387b 100644 --- a/core/worker_utils.py +++ b/core/worker_utils.py @@ -101,6 +101,108 @@ def accept_artist_match(database, id_column: str, source_id, artist_id, return True, "" +# --- Same-name artist disambiguation by owned-catalog overlap ------------- +# The name gate (above) can't separate two artists who share a name ("Rone" has +# ~5). The decisive signal is the library itself: the user owns albums by the +# RIGHT one. So when several candidates clear the name gate, fetch each one's +# catalog and pick the one whose releases overlap the albums actually owned. + +def normalize_release_title(title: str) -> str: + """Collapse an album/release title for tolerant comparison — drop edition + suffixes ('(Deluxe)', ' - Remastered'), punctuation, and case.""" + t = (title or '').lower().strip() + t = re.sub(r'\s*\(.*?\)\s*', ' ', t) + t = re.sub(r'\s*\[.*?\]\s*', ' ', t) + t = re.sub(r'\s+[-–—]\s+.*$', '', t) + t = re.sub(r'[^\w\s]', '', t) + t = re.sub(r'\s+', ' ', t).strip() + return t + + +def catalog_overlap_score(owned_titles, candidate_titles, threshold: float = 0.85) -> int: + """How many OWNED album titles appear in the candidate's catalog (fuzzy, + edition-insensitive). The disambiguation signal — higher = better match.""" + owned = {normalize_release_title(t) for t in (owned_titles or []) if t} + owned.discard('') + cand = {normalize_release_title(t) for t in (candidate_titles or []) if t} + cand.discard('') + if not owned or not cand: + return 0 + score = 0 + for o in owned: + if o in cand or any(SequenceMatcher(None, o, c).ratio() >= threshold for c in cand): + score += 1 + return score + + +def pick_artist_by_catalog(candidates, owned_titles, fetch_titles) -> tuple: + """Choose, among same-name candidates, the one whose catalog best overlaps the + OWNED albums. Returns ``(chosen, overlap_score)``. + + ``candidates`` — artist objects already past the name gate (order = the + worker's existing best-by-name order; candidates[0] is the + current behavior's pick). + ``owned_titles`` — the library artist's owned album titles. + ``fetch_titles(candidate) -> list[str]`` — that candidate's album titles; + called ONLY when disambiguation is actually needed (2+ + candidates and we have owned albums), so the common + single-candidate path costs no extra API calls. + + Falls back to candidates[0] (unchanged behavior) when there's nothing to + disambiguate or no candidate overlaps the owned catalog. + """ + candidates = list(candidates or []) + if not candidates: + return None, 0 + if len(candidates) == 1: + return candidates[0], 0 + owned = [t for t in (owned_titles or []) if t] + if not owned: + return candidates[0], 0 + + best, best_score = None, 0 + for cand in candidates: + try: + titles = fetch_titles(cand) or [] + except Exception as exc: + logger.debug("catalog disambiguation: fetch_titles failed: %s", exc) + titles = [] + score = catalog_overlap_score(owned, titles) + if score > best_score: + best, best_score = cand, score + if best is not None and best_score > 0: + return best, best_score + return candidates[0], 0 # no overlap signal → keep the best-by-name pick + + +def owned_album_titles(database, artist_id) -> list: + """The album titles the library actually has for this artist — the ground + truth used to disambiguate same-name source artists.""" + try: + with database._get_connection() as conn: + rows = conn.execute( + "SELECT title FROM albums WHERE artist_id = ?", (artist_id,) + ).fetchall() + return [r[0] for r in rows if r and r[0]] + except Exception as exc: + logger.debug("owned_album_titles(%s) failed: %s", artist_id, exc) + return [] + + +def release_titles(albums) -> list: + """Extract titles from a list of album objects/dicts (handles ``.title``/ + ``.name`` / dict keys) — the candidate side of catalog disambiguation.""" + out = [] + for al in albums or []: + if isinstance(al, dict): + t = al.get('title') or al.get('name') + else: + t = getattr(al, 'title', None) or getattr(al, 'name', None) + if t: + out.append(t) + return out + + def interruptible_sleep(stop_event: threading.Event, seconds: float, step: float = 0.5) -> bool: """Sleep in chunks so shutdown can interrupt long waits.""" if seconds <= 0: diff --git a/tests/enrichment/test_unmatched.py b/tests/enrichment/test_unmatched.py index 1f749568..432fb21a 100644 --- a/tests/enrichment/test_unmatched.py +++ b/tests/enrichment/test_unmatched.py @@ -187,6 +187,24 @@ def test_reset_builder_nulls_status_not_just_attempted(): assert "WHERE spotify_match_status = 'not_found'" in sql +def test_reset_builder_also_clears_artist_source_id(): + # #868: a re-match must forget the stored id so the worker actually + # re-resolves (otherwise its existing-id short-circuit re-confirms the wrong + # same-name artist). + for service, col in [('spotify', 'spotify_artist_id'), ('itunes', 'itunes_artist_id'), + ('deezer', 'deezer_id'), ('musicbrainz', 'musicbrainz_id')]: + sql, _ = build_reset_query(service, 'artist', 'item', entity_id=5) + assert f'{col} = NULL' in sql, f'{service}: expected {col} cleared' + assert f'{service}_match_status = NULL' in sql + + +def test_reset_builder_does_not_clear_track_id(): + # Tracks have no source-id column (ids live in tags) — must not emit one. + sql, _ = build_reset_query('spotify', 'track', 'item', entity_id=5) + assert 'spotify_match_status = NULL' in sql + assert 'spotify_track_id = NULL' not in sql + + def test_reset_item_requeues_to_pending(db): n = db.reset_enrichment('spotify', 'artist', 'item', entity_id='a2') # was not_found assert n == 1 diff --git a/tests/test_artist_catalog_disambiguation.py b/tests/test_artist_catalog_disambiguation.py new file mode 100644 index 00000000..23d8a502 --- /dev/null +++ b/tests/test_artist_catalog_disambiguation.py @@ -0,0 +1,98 @@ +"""Same-name artist disambiguation by owned-catalog overlap (#868). + +Enrichment matched artists by NAME ONLY, so for a common name ("Rone" has ~5 +artists) it grabbed whichever the source ranked first — often the wrong one, +which then drove a wrong/sparse library discography. The fix: when several +candidates clear the name gate, pick the one whose catalog overlaps the albums +the user actually OWNS. These pin the source-agnostic selector. +""" + +from __future__ import annotations + +from core.worker_utils import ( + catalog_overlap_score, + normalize_release_title, + pick_artist_by_catalog, +) + + +# --- normalization --------------------------------------------------------- + +def test_normalize_strips_editions_and_punctuation(): + assert normalize_release_title('Tohu Bohu (Deluxe Edition)') == 'tohu bohu' + assert normalize_release_title('Mirapolis - Remastered') == 'mirapolis' + assert normalize_release_title('Room with a View [2020]') == 'room with a view' + assert normalize_release_title('') == '' + + +# --- overlap scoring ------------------------------------------------------- + +def test_overlap_counts_matching_owned_titles(): + owned = ['Tohu Bohu', 'Creatures', 'Mirapolis'] + cand = ['Tohu Bohu (Deluxe)', 'Creatures', 'Spanish Breakfast', 'Motion'] + assert catalog_overlap_score(owned, cand) == 2 # Tohu Bohu + Creatures + + +def test_overlap_zero_for_a_different_artists_catalog(): + owned = ['Tohu Bohu', 'Creatures', 'Mirapolis'] + cand = ['Some Other Record', 'Unrelated Album'] + assert catalog_overlap_score(owned, cand) == 0 + + +def test_overlap_zero_when_either_side_empty(): + assert catalog_overlap_score([], ['A']) == 0 + assert catalog_overlap_score(['A'], []) == 0 + + +# --- the selector ---------------------------------------------------------- + +def _cand(cid, titles): + return {'id': cid, '_titles': titles} + + +def _fetch(cand): + return cand['_titles'] + + +def test_single_candidate_returns_without_fetching(): + calls = [] + chosen, score = pick_artist_by_catalog( + [_cand('only', ['X'])], ['Tohu Bohu'], + lambda c: calls.append(c) or c['_titles']) + assert chosen['id'] == 'only' + assert calls == [] # never fetched — nothing to disambiguate + + +def test_no_owned_albums_keeps_name_order(): + calls = [] + chosen, score = pick_artist_by_catalog( + [_cand('first', ['A']), _cand('second', ['B'])], [], + lambda c: calls.append(c) or c['_titles']) + assert chosen['id'] == 'first' # candidates[0] — current behavior + assert calls == [] + + +def test_picks_the_candidate_overlapping_owned_catalog(): + # The WRONG Rone is ranked first; the right one overlaps the owned albums. + wrong = _cand('wrong', ['Rap Mixtape Vol 1', 'Some Single']) + right = _cand('right', ['Tohu Bohu', 'Creatures', 'Mirapolis']) + chosen, score = pick_artist_by_catalog( + [wrong, right], ['Tohu Bohu', 'Creatures', 'Spanish Breakfast'], _fetch) + assert chosen['id'] == 'right' + assert score == 2 + + +def test_no_overlap_anywhere_falls_back_to_first(): + a = _cand('a', ['Nope']); b = _cand('b', ['Also Nope']) + chosen, score = pick_artist_by_catalog([a, b], ['Tohu Bohu'], _fetch) + assert chosen['id'] == 'a' + assert score == 0 + + +def test_fetch_failure_is_tolerated(): + def _boom(_c): + raise RuntimeError('api down') + chosen, score = pick_artist_by_catalog( + [_cand('a', []), _cand('b', [])], ['Tohu Bohu'], _boom) + assert chosen['id'] == 'a' # both fail → fall back to first + assert score == 0 diff --git a/tests/test_worker_artist_disambiguation.py b/tests/test_worker_artist_disambiguation.py new file mode 100644 index 00000000..17d7bc93 --- /dev/null +++ b/tests/test_worker_artist_disambiguation.py @@ -0,0 +1,186 @@ +"""Per-worker same-name artist disambiguation (#868). + +Each enrichment worker, when several same-name candidates clear the name gate, +must pick the one whose catalog overlaps the albums the library actually owns — +not whichever the source ranked first. Covers Spotify (also the Spotify-Free +path, same client surface), iTunes, Deezer, and MusicBrainz. +""" + +from __future__ import annotations + +import types + + +# A query-aware fake DB: owned-albums query → owned titles; the source_id_conflict +# query (SELECT name FROM artists ...) → no conflict. +class _Cur: + def __init__(self, rows): + self._rows = rows + + def fetchall(self): + return self._rows + + +class _Conn: + def __init__(self, owned): + self._owned = owned + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def execute(self, sql, params=()): + if 'FROM albums' in sql: + return _Cur([(t,) for t in self._owned]) + return _Cur([]) # conflict check → none + + def cursor(self): + return self + + def commit(self): + pass + + def close(self): + pass + + +class _DB: + def __init__(self, owned): + self._owned = owned + + def _get_connection(self): + return _Conn(self._owned) + + +# The owned library: the RIGHT "Rone" made these. +OWNED = ['Tohu Bohu', 'Creatures', 'Mirapolis'] +WRONG = types.SimpleNamespace(id='wrong_rone', name='Rone') +RIGHT = types.SimpleNamespace(id='right_rone', name='Rone') +ALBUMS = { + 'wrong_rone': [{'title': 'Mixtape Vol 1'}, {'title': 'Random Single'}], + 'right_rone': [{'title': 'Tohu Bohu (Deluxe)'}, {'title': 'Creatures'}], +} + + +def test_spotify_picks_artist_overlapping_owned_catalog(): + from core.spotify_worker import SpotifyWorker + w = object.__new__(SpotifyWorker) + w.db = _DB(OWNED) + w.stats = {'matched': 0, 'not_found': 0, 'errors': 0} + w.client = types.SimpleNamespace( + search_artists=lambda name, limit=5: [WRONG, RIGHT], # wrong ranked first + get_artist_albums=lambda aid: ALBUMS.get(aid, []), + ) + captured = {} + w._get_existing_id = lambda *a: None + w._mark_status = lambda *a: None + w._name_similarity = lambda a, b: 1.0 # both "Rone" clear the gate + w._is_spotify_id = lambda i: True + w._update_artist = lambda artist_id, obj: captured.update(id=obj.id) + + w._process_artist({'id': 5, 'name': 'Rone'}) + assert captured.get('id') == 'right_rone' + assert w.stats['matched'] == 1 + + +def test_itunes_picks_artist_overlapping_owned_catalog(): + from core.itunes_worker import iTunesWorker + w = object.__new__(iTunesWorker) + w.db = _DB(OWNED) + w.stats = {'matched': 0, 'not_found': 0, 'errors': 0} + w.client = types.SimpleNamespace( + search_artists=lambda name, limit=5: [WRONG, RIGHT], + get_artist_albums=lambda aid: ALBUMS.get(aid, []), + ) + captured = {} + w._get_existing_id = lambda *a: None + w._mark_status = lambda *a: None + w._is_itunes_id = lambda i: True + w._update_artist = lambda artist_id, obj: captured.update(id=obj.id) + + w._process_artist({'id': 5, 'name': 'Rone'}) + assert captured.get('id') == 'right_rone' + assert w.stats['matched'] == 1 + + +def test_deezer_picks_artist_overlapping_owned_catalog(): + from core.deezer_worker import DeezerWorker + w = object.__new__(DeezerWorker) + w.db = _DB(OWNED) + w.stats = {'matched': 0, 'not_found': 0, 'errors': 0} + w.client = types.SimpleNamespace( + search_artists=lambda name, limit=5: [WRONG, RIGHT], + get_artist_albums_list=lambda aid: ALBUMS.get(aid, []), + get_artist_info=lambda aid: {'id': aid, 'name': 'Rone'}, + ) + captured = {} + w._get_existing_id = lambda *a: None + w._mark_status = lambda *a: None + w._update_artist = lambda artist_id, data: captured.update(id=data.get('id')) + + w._process_artist(5, 'Rone') + assert captured.get('id') == 'right_rone' + assert w.stats['matched'] == 1 + + +def _mb_service(owned_release_groups): + from core.musicbrainz_service import MusicBrainzService + s = object.__new__(MusicBrainzService) + s._check_cache = lambda *a, **k: None + s._save_to_cache = lambda *a, **k: None + s._calculate_similarity = lambda a, b: 1.0 + s.mb_client = types.SimpleNamespace( + search_artist=lambda name, limit=5: [ + {'id': 'wrong_mbid', 'name': 'Rone', 'score': 100}, # ranked first + {'id': 'right_mbid', 'name': 'Rone', 'score': 90}, + ], + get_artist=lambda mbid, includes=None: owned_release_groups.get(mbid), + ) + return s + + +def test_musicbrainz_picks_artist_overlapping_owned_catalog(): + rg = { + 'wrong_mbid': {'release-groups': [{'title': 'Other Stuff'}]}, + 'right_mbid': {'release-groups': [{'title': 'Tohu Bohu'}, {'title': 'Creatures'}]}, + } + result = _mb_service(rg).match_artist('Rone', owned_titles=OWNED) + assert result is not None + assert result['mbid'] == 'right_mbid' + + +def test_musicbrainz_without_owned_titles_keeps_legacy_behavior(): + """No owned titles → highest-confidence (name-order first) candidate, unchanged.""" + rg = {'wrong_mbid': {'release-groups': []}, 'right_mbid': {'release-groups': []}} + result = _mb_service(rg).match_artist('Rone') # no owned_titles + assert result['mbid'] == 'wrong_mbid' # the top-confidence pick + + +def test_musicbrainz_cache_hit_used_when_catalog_overlaps(): + """A cached mbid whose catalog overlaps the owned albums is trusted (no re-resolve).""" + rg = {'cached_mbid': {'release-groups': [{'title': 'Tohu Bohu'}, {'title': 'Creatures'}]}} + s = _mb_service(rg) + s._check_cache = lambda *a, **k: {'musicbrainz_id': 'cached_mbid', 'confidence': 95} + s.mb_client.search_artist = lambda *a, **k: (_ for _ in ()).throw( + AssertionError("must not re-search when the cached match fits the library")) + result = s.match_artist('Rone', owned_titles=OWNED) + assert result['mbid'] == 'cached_mbid' + assert result['cached'] is True + + +def test_musicbrainz_stale_cache_is_bypassed_and_re_resolved(): + """A cached mbid with ZERO owned-catalog overlap (wrong same-name artist) is + bypassed → fresh disambiguated resolve. This is what makes a re-match work for + MB despite the 90-day cache TTL (#868).""" + rg = { + 'cached_wrong': {'release-groups': [{'title': 'Unrelated'}]}, # the stale cache + 'wrong_mbid': {'release-groups': [{'title': 'Other Stuff'}]}, + 'right_mbid': {'release-groups': [{'title': 'Tohu Bohu'}, {'title': 'Creatures'}]}, + } + s = _mb_service(rg) + s._check_cache = lambda *a, **k: {'musicbrainz_id': 'cached_wrong', 'confidence': 95} + result = s.match_artist('Rone', owned_titles=OWNED) + assert result['mbid'] == 'right_mbid' # re-resolved + disambiguated + assert result.get('cached') is not True From 09b97c5f6395e73f625a16785a324fcd11ef5571 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 15:37:29 -0700 Subject: [PATCH 07/42] =?UTF-8?q?#870:=20Deezer=20ARL=20'resets=20itself'?= =?UTF-8?q?=20=E2=80=94=20test=20the=20SAVED=20token,=20not=20the=20redact?= =?UTF-8?q?ion=20mask?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Deezer ARL field round-trips a redaction sentinel for a saved-but-untouched secret (shown as dots). The save path already guards against the sentinel overwriting the real token (ConfigManager.set), so the ARL was never actually lost — but the connection TEST read the field value and sent the sentinel as the token, so Deezer returned USER_ID=0 ('Invalid ARL token') after navigating away and back. That false failure made it look like the ARL kept resetting. Fix: - ConfigManager.resolve_secret(key, posted): empty/sentinel posted value -> the stored value; a real string -> a genuine new secret. Reusable for any secret connection-test (single source of truth). - /api/deezer-download/test now resolves the effective ARL via resolve_secret, so an untouched field tests the stored token. - testDeezerDownloadConnection() strips the sentinel before sending (untouched -> empty -> backend uses the saved token). Seam/regression tests for resolve_secret (sentinel/empty/none -> stored, real -> passthrough, nothing stored -> empty). JS integrity 64 green. --- config/settings.py | 14 ++++++++++++ tests/test_resolve_secret.py | 43 ++++++++++++++++++++++++++++++++++++ web_server.py | 5 ++++- webui/static/settings.js | 12 +++++----- 4 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 tests/test_resolve_secret.py diff --git a/config/settings.py b/config/settings.py index f1482081..28d1908a 100644 --- a/config/settings.py +++ b/config/settings.py @@ -876,6 +876,20 @@ class ConfigManager: config[keys[-1]] = value self._save_config() + def resolve_secret(self, key: str, posted: Any) -> str: + """Resolve a secret value coming back from the settings UI. + + The UI renders a saved-but-untouched secret as the REDACTED_SENTINEL (shown + masked); empty or that sentinel means "use the stored value", while a real + string is a genuine new secret. A connection-test endpoint should test the + EFFECTIVE secret, not the mask — otherwise testing a saved-but-untouched + token sends the sentinel and the source rejects it (#870).""" + if isinstance(posted, str): + posted = posted.strip() + if not posted or posted == self.REDACTED_SENTINEL: + return self.get(key, '') or '' + return posted + def get_spotify_config(self) -> Dict[str, str]: return self.get('spotify', {}) diff --git a/tests/test_resolve_secret.py b/tests/test_resolve_secret.py new file mode 100644 index 00000000..3ff53a36 --- /dev/null +++ b/tests/test_resolve_secret.py @@ -0,0 +1,43 @@ +"""ConfigManager.resolve_secret — test the EFFECTIVE secret, not the mask (#870). + +The settings UI renders a saved-but-untouched secret as the redaction sentinel +(shown as dots). The Deezer ARL connection test was sending that sentinel as the +token, so Deezer rejected it ("Invalid ARL token — USER_ID=0") and it looked like +the ARL kept resetting — even though the saved value was fine. resolve_secret maps +an empty/sentinel posted value back to the stored token. +""" + +from __future__ import annotations + +from config.settings import ConfigManager + +S = ConfigManager.REDACTED_SENTINEL + + +def _cm(config_data): + cm = object.__new__(ConfigManager) # bypass __init__/DB — get() reads config_data + cm.config_data = config_data + return cm + + +def test_sentinel_resolves_to_stored(): + cm = _cm({'deezer_download': {'arl': 'real_arl_123'}}) + assert cm.resolve_secret('deezer_download.arl', S) == 'real_arl_123' + + +def test_empty_or_none_resolves_to_stored(): + cm = _cm({'deezer_download': {'arl': 'real_arl_123'}}) + assert cm.resolve_secret('deezer_download.arl', '') == 'real_arl_123' + assert cm.resolve_secret('deezer_download.arl', ' ') == 'real_arl_123' + assert cm.resolve_secret('deezer_download.arl', None) == 'real_arl_123' + + +def test_real_value_passes_through_trimmed(): + cm = _cm({'deezer_download': {'arl': 'old'}}) + assert cm.resolve_secret('deezer_download.arl', 'new_token') == 'new_token' + assert cm.resolve_secret('deezer_download.arl', ' spaced ') == 'spaced' + + +def test_sentinel_with_nothing_stored_returns_empty(): + cm = _cm({}) + assert cm.resolve_secret('deezer_download.arl', S) == '' diff --git a/web_server.py b/web_server.py index 0196fe9b..fd53c71a 100644 --- a/web_server.py +++ b/web_server.py @@ -21578,7 +21578,10 @@ def deezer_download_test(): """Test Deezer ARL token authentication.""" try: data = request.get_json() or {} - arl = data.get('arl', '') + # An empty/redaction-sentinel value means "test the SAVED token" — the + # settings field round-trips a mask for a saved-but-untouched secret, so + # testing it must use the stored ARL, not the mask (#870). + arl = config_manager.resolve_secret('deezer_download.arl', data.get('arl')) if not arl: return jsonify({'success': False, 'error': 'No ARL token provided'}) diff --git a/webui/static/settings.js b/webui/static/settings.js index af4bb3f7..1921839b 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -4305,13 +4305,11 @@ async function testDeezerDownloadConnection() { statusEl.textContent = 'Checking...'; statusEl.style.color = '#aaa'; try { - // Save the ARL first so the backend can use it - const arl = document.getElementById('deezer-download-arl')?.value || ''; - if (!arl) { - statusEl.textContent = 'No ARL token provided'; - statusEl.style.color = '#ff9800'; - return; - } + let arl = document.getElementById('deezer-download-arl')?.value || ''; + // An untouched field holds the redaction sentinel (a token IS saved) — + // send empty so the backend tests the SAVED token instead of the mask, + // which the source would reject (#870). + if (arl === REDACTED_SECRET_SENTINEL) arl = ''; const resp = await fetch('/api/deezer-download/test', { method: 'POST', headers: { 'Content-Type': 'application/json' }, From afa07690f5235f0b2746992984c58b048f6b995a Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 16:55:33 -0700 Subject: [PATCH 08/42] Find & Add: match a Spotify 'Title - Remix' query to the base-titled library track MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit wolf's report: Spotify shows 'Calma - Remix', Find & Add searches that literal string, but the library stores the track as just 'Calma' (only the 3:58 duration marks it the remix). The literal LIKE '%calma - remix%' misses, so it fell to the OR-fuzzy fallback which floods on the common word 'remix' (20 unrelated '... remix' hits). Dropping '- Remix' (searching 'Calma') finds it instantly. Fix: search_tracks (and api_search_tracks) now retry on the BASE title — the part before Spotify's ' - ' version separator — BEFORE the OR-fuzzy flood. So 'Calma - Remix' resolves to 'Calma' (or 'Calma (Remix)') and the noise fallback is never reached when the base matches. New core.text.title_match.base_title_before_dash (splits the first spaced ' - '; leaves bare hyphens like 'Up-Tight' alone). Tests: pure helper (3) + real-DB integration reproducing the Calma case, the parenthesized-remix variant, plain-title-unaffected, and no-flood (4). 64 search/match tests green. --- core/text/title_match.py | 18 +++++ database/music_database.py | 28 +++++++- tests/test_base_title_search_fallback.py | 88 ++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 tests/test_base_title_search_fallback.py diff --git a/core/text/title_match.py b/core/text/title_match.py index 85e1c10b..36818fcd 100644 --- a/core/text/title_match.py +++ b/core/text/title_match.py @@ -201,9 +201,27 @@ def numeric_tokens_differ(title_a: str, title_b: str) -> bool: return _digit_tokens(title_a) != _digit_tokens(title_b) +def base_title_before_dash(title: str) -> str: + """The base title before Spotify's ' - ' version separator. + + Spotify renders versions as 'Calma - Remix' / 'Song - Radio Edit' / + 'Track - Remastered 2019'. Libraries (and the files people actually have) + very often store just the base — 'Calma' — so a literal search for + 'Calma - Remix' finds nothing and the OR-fuzzy fallback then floods on the + common qualifier word ('remix' matches every remix). This returns the base + ('Calma') for a base-title search fallback. Splits on the FIRST ' - ' (the + spaced hyphen is Spotify's separator; a bare hyphen inside a word is left + alone). Returns the title unchanged when there's no separator.""" + if not title: + return title + idx = title.find(' - ') + return title[:idx].strip() if idx > 0 else title + + __all__ = [ "titles_plausibly_same", "strip_redundant_context_qualifiers", "strip_subtitle_qualifiers", "numeric_tokens_differ", + "base_title_before_dash", ] diff --git a/database/music_database.py b/database/music_database.py index 0d06a6db..1f6078fb 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -6887,11 +6887,26 @@ class MusicDatabase: # STRATEGY 1: Try basic SQL LIKE search first (fastest) basic_results = self._search_tracks_basic(cursor, title, artist, limit, server_source, rank_artist) - + if basic_results: logger.debug(f"Basic search found {len(basic_results)} results") return basic_results + # STRATEGY 1b: Spotify renders versions as "Title - Qualifier" + # ("Calma - Remix") but libraries usually store just the base + # ("Calma"), so the literal search misses. Retry on the base title + # BEFORE the OR-fuzzy fallback (which would flood on the common + # qualifier word — every "... remix" matches "remix"). #: Calma - Remix + if title: + from core.text.title_match import base_title_before_dash + base_title = base_title_before_dash(title) + if base_title and base_title != title: + base_results = self._search_tracks_basic( + cursor, base_title, artist, limit, server_source, rank_artist) + if base_results: + logger.debug("Base-title search matched '%s' via '%s'", title, base_title) + return base_results + # STRATEGY 2: Broader fuzzy search - splits into individual words with OR matching fuzzy_results = self._search_tracks_fuzzy_fallback(cursor, title, artist, limit, server_source) if fuzzy_results: @@ -6920,6 +6935,17 @@ class MusicDatabase: if basic_rows: return [dict(r) for r in basic_rows] + # Base-title fallback for Spotify "Title - Qualifier" forms (see + # search_tracks STRATEGY 1b) before the OR-fuzzy flood. + if title: + from core.text.title_match import base_title_before_dash + base_title = base_title_before_dash(title) + if base_title and base_title != title: + base_rows = self._search_tracks_basic_rows( + cursor, base_title, artist, limit, server_source) + if base_rows: + return [dict(r) for r in base_rows] + fuzzy_rows = self._search_tracks_fuzzy_rows(cursor, title, artist, limit, server_source) return [dict(r) for r in fuzzy_rows] except Exception as e: diff --git a/tests/test_base_title_search_fallback.py b/tests/test_base_title_search_fallback.py new file mode 100644 index 00000000..f9c2b936 --- /dev/null +++ b/tests/test_base_title_search_fallback.py @@ -0,0 +1,88 @@ +"""Find & Add: Spotify "Title - Qualifier" must find the base-titled library track. + +wolf's report: Spotify shows "Calma - Remix", Find & Add searches that literal +string, the library stores the track as just "Calma" (only the duration marks it +as the remix) → the literal search misses and the OR-fuzzy fallback floods 20 +unrelated "... remix" hits. Dropping "- Remix" (searching "Calma") finds it. + +Fix: search_tracks retries on the base title (before Spotify's " - " separator) +before the OR-fuzzy flood. +""" + +from __future__ import annotations + +import pytest + +from core.text.title_match import base_title_before_dash +from database.music_database import MusicDatabase + + +# --- pure helper ----------------------------------------------------------- + +def test_base_title_before_dash_strips_spotify_version_suffix(): + assert base_title_before_dash('Calma - Remix') == 'Calma' + assert base_title_before_dash('Closer - Radio Edit') == 'Closer' + assert base_title_before_dash('Crocodile Rock - Remastered 2014') == 'Crocodile Rock' + + +def test_base_title_before_dash_leaves_plain_titles_alone(): + assert base_title_before_dash('Tom Sawyer') == 'Tom Sawyer' + assert base_title_before_dash('21st Century Schizoid Man') == '21st Century Schizoid Man' + assert base_title_before_dash('Up-Tight') == 'Up-Tight' # bare hyphen, not a separator + assert base_title_before_dash('') == '' + + +def test_base_title_before_dash_splits_first_separator_only(): + assert base_title_before_dash('A - B - C') == 'A' + + +# --- integration: the real search path ------------------------------------- + +@pytest.fixture +def db(tmp_path): + return MusicDatabase(str(tmp_path / "music.db")) + + +def _insert(db, tid, title, artist_id, artist_name): + with db._get_connection() as conn: + conn.execute("INSERT OR IGNORE INTO artists (id, name) VALUES (?, ?)", (artist_id, artist_name)) + conn.execute("INSERT OR IGNORE INTO albums (id, title, artist_id) VALUES (?, ?, ?)", + (artist_id, "Alb", artist_id)) + conn.execute( + "INSERT INTO tracks (id, album_id, artist_id, title, track_number, duration, file_path) " + "VALUES (?, ?, ?, ?, 1, 238, ?)", + (tid, artist_id, artist_id, title, f"/m/{tid}.flac"), + ) + conn.commit() + + +def test_spotify_dash_remix_finds_base_titled_track(db): + # Library stores the remix as just "Calma" (the wolf case). + _insert(db, 1, "Calma", 1, "Pedro Capó") + results = db.search_tracks(title="Calma - Remix", rank_artist="Pedro Capó") + titles = [t.title for t in results] + assert "Calma" in titles, "base-title fallback should find 'Calma' for 'Calma - Remix'" + + +def test_spotify_dash_remix_finds_parenthesized_remix(db): + # …and still matches when the library DID label it "(Remix)". + _insert(db, 1, "Calma (Remix)", 1, "Pedro Capó") + results = db.search_tracks(title="Calma - Remix", rank_artist="Pedro Capó") + assert any("Calma" in t.title for t in results) + + +def test_plain_title_unaffected_uses_basic_search(db): + _insert(db, 1, "Tom Sawyer", 1, "Rush") + results = db.search_tracks(title="Tom Sawyer") + assert [t.title for t in results] == ["Tom Sawyer"] + + +def test_dash_query_does_not_flood_when_base_matches(db): + # The base-title retry must short-circuit BEFORE the OR-fuzzy flood, so an + # unrelated "... Remix" track doesn't drown the real one. + _insert(db, 1, "Calma", 1, "Pedro Capó") + _insert(db, 2, "Some Other Song (KAIZ Remix)", 2, "Someone Else") + results = db.search_tracks(title="Calma - Remix", rank_artist="Pedro Capó") + titles = [t.title for t in results] + assert "Calma" in titles + assert "Some Other Song (KAIZ Remix)" not in titles From 2905fe0853601534d0da84abd949e96c46f87f5f Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 15 Jun 2026 19:56:53 -0700 Subject: [PATCH 09/42] #880: retry 429 mid-walk when paginating Tidal Favorite Tracks (don't truncate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Favorites collection walker (_iter_collection_resource_ids) broke on ANY non-200 — including a transient 429. So a rate-limit mid-pagination silently truncated the collection: the log shows `status=429` then `Retrieved 98/100`, and the mirror saved 98 of a 524-track favorites list. The auto-sync cycle only "worked" because it dodged the 429. The regular-playlist paginator already retries 429; the collection walker didn't. Fix: retry the same cursor page with backoff (5/10/15/20s, 4 attempts) on 429, mirroring the playlist paginator; 401/403 still bail (+ reconnect flag), other non-200 still break. Regression tests: 429 mid-walk completes the full chain; exhausted retries return partial without hanging; 429 doesn't set reconnect. --- core/tidal_client.py | 26 ++++++++++++++ tests/test_tidal_collection_tracks.py | 49 +++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/core/tidal_client.py b/core/tidal_client.py index a18ab04f..4b542d3d 100644 --- a/core/tidal_client.py +++ b/core/tidal_client.py @@ -1655,6 +1655,8 @@ class TidalClient: ids: List[str] = [] next_path: Optional[str] = None + consecutive_429 = 0 + MAX_PAGE_RETRIES = 4 while True: if next_path: @@ -1687,6 +1689,28 @@ class TidalClient: logger.warning(f"Tidal collection page request failed: {e}") break + # Rate limited mid-walk → retry the SAME cursor page with backoff + # rather than silently truncating the collection. Without this a 429 + # on page ~5 capped a 513-track favorites list at ~98 (issue #880); + # the regular-playlist paginator already retries 429 the same way. + if resp.status_code == 429: + consecutive_429 += 1 + if consecutive_429 <= MAX_PAGE_RETRIES: + backoff = 5.0 * consecutive_429 # 5s, 10s, 15s, 20s + logger.warning( + f"Tidal collection {expected_type} rate limited (429) — " + f"retry {consecutive_429}/{MAX_PAGE_RETRIES} in {backoff}s " + f"({len(ids)} fetched so far)" + ) + time.sleep(backoff) + continue # next_path/cursor unchanged → re-request same page + logger.error( + f"Tidal collection {expected_type} still rate limited after " + f"{MAX_PAGE_RETRIES} retries — returning {len(ids)} IDs " + f"(PARTIAL: the collection may be larger)" + ) + break + if resp.status_code != 200: # 401/403 = scope/permission issue. Token predates the # `collection.read` scope expansion or the user revoked @@ -1704,6 +1728,8 @@ class TidalClient: ) break + consecutive_429 = 0 # a good page → reset the retry budget + try: data = resp.json() except ValueError as e: diff --git a/tests/test_tidal_collection_tracks.py b/tests/test_tidal_collection_tracks.py index b821a93c..88695e99 100644 --- a/tests/test_tidal_collection_tracks.py +++ b/tests/test_tidal_collection_tracks.py @@ -252,6 +252,55 @@ class TestIterCollectionTrackIds: assert ids == [] + def test_429_mid_walk_retries_and_completes(self): + """Regression for #880: a 429 mid-pagination must RETRY the same + cursor page (with backoff), not truncate the collection. Before the + fix a transient 429 on page ~5 capped a 513-track favorites list at + ~98 (the log showed `status=429` then `Retrieved 98/100`).""" + client = _make_authed_client() + # page1 (200) → 429 (transient) → page2 (200, end of chain) + responses = iter([ + _FakeResp(200, _PAGE_ONE), + _FakeResp(429, text=""), # rate limited — retry, don't truncate + _FakeResp(200, _PAGE_TWO), + ]) + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == ['1001', '1002', '1003', '1004', '1005'] # full chain, nothing dropped + + def test_429_does_not_set_reconnect_flag(self): + """A rate-limit is transient, NOT a scope problem — must not tell the + user to reconnect.""" + client = _make_authed_client() + responses = iter([_FakeResp(200, _PAGE_ONE), _FakeResp(429, text=""), _FakeResp(200, _PAGE_TWO)]) + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + client._iter_collection_track_ids() + + assert client.collection_needs_reconnect() is False + + def test_429_exhausts_retries_returns_partial(self): + """If the 429s never clear, give up after the retry budget and return + what we have (PARTIAL) rather than looping forever.""" + client = _make_authed_client() + + def gen(): + yield _FakeResp(200, _PAGE_ONE) + while True: + yield _FakeResp(429, text="") + + responses = gen() + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == ['1001', '1002', '1003'] # page 1 survives; no hang + # --------------------------------------------------------------------------- # get_collection_tracks_count From 02d6af29edc35a8826e88906970028b6af222cb8 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 15 Jun 2026 20:09:53 -0700 Subject: [PATCH 10/42] #879: a failed settings load must never overwrite the saved config Reported by @Lysticity: opening Settings reset the whole config to defaults. The chain: GET /api/settings 500s (their env: ConfigManager missing redacted_config) -> loadSettingsData() called response.json() WITHOUT checking response.ok, so the error body {"error": ...} was treated as settings -> every field populated as `settings.x?.y || ''` blanked to defaults -> autosave then wrote those defaults over the real config. Fix (settings.js): bail BEFORE touching any field when the response isn't ok / is an error body, set window._settingsLoadFailed, and guard BOTH save paths (debouncedAutoSaveSettings + saveSettings) on it. The flag clears on the next successful load. So any load failure (500, lock, network) now leaves the saved config untouched instead of wiping it. The redacted_config method exists in all 2.7.x source + on dev (their 500 looks like a stale/mismatched build), but the UI must not destroy config on ANY failed load. Regression test pins redacted_config stays a callable method on the class (its removal is exactly what 500s the endpoint). --- tests/test_settings_redaction.py | 17 +++++++++++++++++ webui/static/settings.js | 30 +++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/tests/test_settings_redaction.py b/tests/test_settings_redaction.py index 3aea7a2a..a27e4d25 100644 --- a/tests/test_settings_redaction.py +++ b/tests/test_settings_redaction.py @@ -24,6 +24,23 @@ def _cm(config_data): return cm +# ── #879: the GET /api/settings handler calls config_manager.redacted_config(). +# If that method is ever renamed/removed the endpoint 500s, the web UI treats the +# error body as settings, blanks the form to defaults, and autosaves over the +# user's real config. Pin the method's presence on the class so that can't ship. + +def test_redacted_config_is_a_method_on_the_class(): + assert callable(getattr(ConfigManager, 'redacted_config', None)), ( + "GET /api/settings depends on ConfigManager.redacted_config(); removing or " + "renaming it 500s the settings endpoint and the UI then wipes the config (#879)") + + +def test_redacted_config_callable_on_instance_returns_dict(): + cm = _cm({'spotify': {'client_secret': 'REAL'}}) + out = cm.redacted_config() + assert isinstance(out, dict) and out.get('spotify', {}).get('client_secret') == S + + # ── redacted_config: secrets out, everything else intact ──────────────────── def test_configured_secrets_are_masked(): diff --git a/webui/static/settings.js b/webui/static/settings.js index 1921839b..d00c79d9 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -45,6 +45,9 @@ function debouncedAutoSaveSettings() { // fields on load — those aren't user edits and must not trigger a full // save (which re-initializes every backend service client). if (window._suppressSettingsAutoSave) return; + // #879: never auto-save while the last settings load failed — the form is + // showing defaults, not the real config, so saving would wipe it. + if (window._settingsLoadFailed) return; // #827: the Logs tab has no savable settings — its live-viewer controls // (source picker, filters, auto-scroll) were tripping the auto-save and // flooding app.log with "Settings saved" lines, drowning out the logs the @@ -976,6 +979,18 @@ async function loadSettingsData() { const response = await fetch(API.settings); const settings = await response.json(); + // #879: a failed GET /api/settings returns an error body (e.g. {"error": + // "..."} on a 500), NOT real settings. Populating from it blanks every + // field to its default ('settings.spotify?.x || ""'), and the next + // (auto)save then overwrites the user's real config. Abort BEFORE + // touching any field and flag it so saves stay blocked until a good load. + if (!response.ok || !settings || typeof settings !== 'object' || settings.error) { + window._settingsLoadFailed = true; + throw new Error('settings load failed (HTTP ' + response.status + '): ' + + ((settings && settings.error) || 'unexpected response')); + } + window._settingsLoadFailed = false; // good load → saving is safe again + // Populate Spotify settings document.getElementById('spotify-client-id').value = settings.spotify?.client_id || ''; document.getElementById('spotify-client-secret').value = settings.spotify?.client_secret || ''; @@ -1461,7 +1476,10 @@ async function loadSettingsData() { } catch (error) { console.error('Error loading settings:', error); - showToast('Failed to load settings', 'error'); + // #879: any load failure → block saves so a blank/partial form can't be + // written over the real config. Cleared on the next successful load. + window._settingsLoadFailed = true; + showToast('Failed to load settings — reload the page before saving (your saved config is untouched)', 'error'); } } @@ -2861,6 +2879,16 @@ function _getTagConfig(path) { } async function saveSettings(quiet = false) { + // #879: refuse to save if the settings never loaded successfully — the form + // is showing defaults, not the user's real config, so saving would wipe it. + // Cleared automatically on the next successful load (reload the page). + if (window._settingsLoadFailed) { + if (!quiet && typeof showToast === 'function') { + showToast("Settings didn't load — reload the page before saving (your config is untouched)", 'error'); + } + return; + } + // Validate file organization templates before saving const validationErrors = validateFileOrganizationTemplates(); if (validationErrors.length > 0) { From f2f0f5d849f0c5f6588ade1f3c1b07915bece3cc Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 15 Jun 2026 21:16:18 -0700 Subject: [PATCH 11/42] Sidebar UI: frosted-glass header blur, centered nav badges, admin cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - .sidebar-header: real frosted-glass blur of content scrolling behind it — made the background translucent (was an opaque base layer), added backdrop-filter blur, and raised the header above the nav (z-index) so nav items actually sit in its backdrop. - .dl-nav-badge: vertically centered on the right (top:50% + translateY) instead of pinned to the top-right corner. - Removed border-top-right-radius from .sidebar and .sidebar-header (square top). - Hide the "My Accounts" + "My Settings" header buttons for admin profiles — both are inert for admin (every service is "Managed in Settings", and My Settings is an empty pointer note); kept for non-admins who get real UI. --- webui/static/init.js | 9 +++++++++ webui/static/style.css | 31 +++++++++++++++++++++---------- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/webui/static/init.js b/webui/static/init.js index d339cb18..a8a11b98 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -1116,6 +1116,15 @@ function updateProfileIndicator() { const statusSection = document.querySelector('.status-section--clickable'); if (statusSection) statusSection.classList.toggle('status-section--locked', !currentProfile.is_admin); + // My Accounts (per-profile streaming OAuth) and My Settings (per-profile + // server library) are inert for admin — admin uses the global app account + // for every service and the full Settings page. Hide both for admin; keep + // them for non-admins, who actually get a connect/library UI. + const myAccountsBtn = document.getElementById('my-accounts-btn'); + const personalSettingsBtn = document.getElementById('personal-settings-btn'); + if (myAccountsBtn) myAccountsBtn.style.display = currentProfile.is_admin ? 'none' : ''; + if (personalSettingsBtn) personalSettingsBtn.style.display = currentProfile.is_admin ? 'none' : ''; + indicator.onclick = async () => { const res = await fetch('/api/profiles'); const data = await res.json(); diff --git a/webui/static/style.css b/webui/static/style.css index d3a2d09c..608ee724 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -80,7 +80,6 @@ body { /* Soft translucent borders */ border-right: 1px solid rgba(255, 255, 255, 0.08); border-top: 1px solid rgba(255, 255, 255, 0.12); - border-top-right-radius: 24px; border-bottom-right-radius: 24px; /* Soft floating shadow with inner glow */ @@ -291,17 +290,16 @@ body.reduce-effects .sidebar::after { .sidebar-header { min-height: 115px; - /* Opaque base layered under the accent gradient so nav items scrolling - past the sticky header don't bleed through the translucent stops. */ + /* Translucent so the backdrop-filter blur below is visible — the dark tint + keeps the header readable while nav items scrolling behind it read as a + soft frosted blur instead of a sharp bleed-through. */ background: linear-gradient(180deg, - rgba(var(--accent-rgb), 0.14) 0%, - rgba(var(--accent-rgb), 0.08) 30%, - rgba(var(--accent-rgb), 0.03) 70%, - rgba(18, 18, 18, 1) 100%), - rgb(18, 18, 18); + rgba(var(--accent-rgb), 0.16) 0%, + rgba(var(--accent-rgb), 0.10) 30%, + rgba(24, 24, 24, 0.62) 70%, + rgba(18, 18, 18, 0.72) 100%); border-bottom: 1px solid rgba(255, 255, 255, 0.08); - border-top-right-radius: 20px; padding: 20px 24px; display: flex; flex-direction: column; @@ -309,9 +307,21 @@ body.reduce-effects .sidebar::after { gap: 8px; position: sticky; top: 0; + /* Above the nav (the sidebar gives its children z-index:1) so nav items + scrolling up sit BEHIND the header — i.e. in its backdrop, where the + blur below can actually act on them. Without this they paint in front + and backdrop-filter has nothing to blur. */ + z-index: 2; overflow: hidden; flex-shrink: 0; + /* Intense frosted-glass blur: nav items scrolling up behind the translucent + accent-gradient top now read as a soft blur instead of bleeding through + sharply. (Only visible where the background is translucent — the opaque + base toward the bottom still stops any bleed.) */ + backdrop-filter: blur(28px) saturate(1.3); + -webkit-backdrop-filter: blur(28px) saturate(1.3); + /* Subtle inner glow */ box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.1); } @@ -60149,8 +60159,9 @@ body.reduce-effects #page-particles-canvas { .dl-nav-badge { position: absolute; - top: 4px; + top: 50%; right: 8px; + transform: translateY(-50%); background: rgb(var(--accent-rgb)); color: #fff; font-size: 0.6rem; From e7814e0acf048851a2cd8516cdff1138e6561671 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 15 Jun 2026 21:30:55 -0700 Subject: [PATCH 12/42] #877: Download Discography filters mirror Artist Detail (fix dead EPs + add Live/Comp/Featured) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Download Discography modal exposed only Albums/EPs/Singles, its EPs toggle did nothing, and Live/Compilations/Featured were missing — so you couldn't fine-filter a bulk download the way Artist Detail lets you browse. Root cause: the modal's endpoint (/api/artist//discography) used the base get_artist_discography, which lumps EPs into singles, and the modal only read {albums, singles} — so the EPs bucket was always empty (dead toggle). It also had no content-type (Live/Compilation/Featured) classification at all. - Backend: the endpoint now uses get_artist_detail_discography — the SAME split Artist Detail uses — and returns a separate `eps` list. - Frontend: read `eps`; tag each card with data-is-live/compilation/featured via a new shared _classifyReleaseContent() (also adopted by the Artist Detail cards so the two can't drift); add Live/Compilations/Featured filter buttons; combined category+content filtering. The download payload is built from VISIBLE checked cards, so every toggle now actually changes what downloads. - Regression test: get_artist_detail_discography splits an EP into the eps bucket. --- tests/metadata/test_metadata_discography.py | 23 +++++++ web_server.py | 8 ++- webui/static/library.js | 68 +++++++++++++++------ 3 files changed, 81 insertions(+), 18 deletions(-) diff --git a/tests/metadata/test_metadata_discography.py b/tests/metadata/test_metadata_discography.py index 65ac0624..a5109dbb 100644 --- a/tests/metadata/test_metadata_discography.py +++ b/tests/metadata/test_metadata_discography.py @@ -731,3 +731,26 @@ def test_get_artist_discography_prefers_source_specific_artist_ids(monkeypatch): ) ] assert deezer.album_calls == [] + + +def test_get_artist_detail_discography_splits_eps_from_singles(monkeypatch): + """#877: get_artist_detail_discography must put EPs in their OWN bucket (not + lumped into singles). The Download Discography modal now reads this split, so + its EPs filter has cards to act on and stays in sync with Artist Detail.""" + spotify = _FakeSourceClient( + album_results=[ + _album("a1", "An Album", "2024-01-01", album_type="album"), + _album("e1", "An EP", "2024-02-01", album_type="ep"), + _album("s1", "A Single", "2024-03-01", album_type="single"), + ] + ) + clients = {"deezer": _FakeSourceClient(), "spotify": spotify, "itunes": _FakeSourceClient()} + monkeypatch.setattr(metadata_registry, "get_primary_source", lambda spotify_client_factory=None: "deezer") + monkeypatch.setattr(metadata_registry, "get_source_priority", lambda primary: [primary, "spotify", "itunes"]) + monkeypatch.setattr(metadata_registry, "get_client_for_source", lambda source, **kwargs: clients.get(source)) + + result = metadata_discography.get_artist_detail_discography("artist-1", "Artist One", MetadataLookupOptions()) + + assert [r["id"] for r in result["albums"]] == ["a1"] + assert [r["id"] for r in result["eps"]] == ["e1"] + assert [r["id"] for r in result["singles"]] == ["s1"] diff --git a/web_server.py b/web_server.py index fd53c71a..e53a2711 100644 --- a/web_server.py +++ b/web_server.py @@ -9114,7 +9114,11 @@ def get_artist_discography(artist_id): effective_override_source = 'spotify' from core.metadata.lookup import MetadataLookupOptions - from core.metadata_service import get_artist_discography as _get_artist_discography + # #877: use the artist-DETAIL discography so the Download Discography modal + # gets the SAME release-type split (albums / eps / singles) the Artist + # Detail view shows — EPs were being lumped into singles before, leaving + # the modal's EPs toggle dead. + from core.metadata.discography import get_artist_detail_discography as _get_artist_discography # Server-side per-source ID resolution. Look up the library row # by ANY of the IDs the frontend might send: library DB id, @@ -9192,6 +9196,7 @@ def get_artist_discography(artist_id): ) album_list = discography['albums'] + eps_list = discography.get('eps', []) singles_list = discography['singles'] active_source = discography['source'] source_priority = discography['source_priority'] @@ -9303,6 +9308,7 @@ def get_artist_discography(artist_id): return jsonify({ "albums": album_list, + "eps": eps_list, "singles": singles_list, "source": active_source or (source_priority[0] if source_priority else "unknown"), "artist_info": artist_info, diff --git a/webui/static/library.js b/webui/static/library.js index cd88e264..48f347f8 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -1891,16 +1891,12 @@ function createReleaseCard(release) { // Store mutable reference so stream updates propagate to click handler card._releaseData = release; - // Tag card for content-type filtering - const livePattern = /\b(live)\b|\(live[^)]*\)|\[live[^]]*\]/i; - const compilationPattern = /\b(greatest hits|best of|collection|anthology|essential)\b/i; - const featuredPattern = /\(?\bfeat\.?\s|\bft\.?\s|\bfeaturing\b/i; - const isLive = livePattern.test(release.title || '') || (release.album_type === 'compilation' && livePattern.test(release.title || '')); - const isCompilation = (release.album_type === 'compilation') || compilationPattern.test(release.title || ''); - const isFeatured = featuredPattern.test(release.title || ''); - card.setAttribute("data-is-live", isLive ? "true" : "false"); - card.setAttribute("data-is-compilation", isCompilation ? "true" : "false"); - card.setAttribute("data-is-featured", isFeatured ? "true" : "false"); + // Tag card for content-type filtering (shared classifier — #877, so Artist + // Detail and the Download Discography modal never drift apart). + const cc = _classifyReleaseContent(release); + card.setAttribute("data-is-live", cc.isLive ? "true" : "false"); + card.setAttribute("data-is-compilation", cc.isCompilation ? "true" : "false"); + card.setAttribute("data-is-featured", cc.isFeatured ? "true" : "false"); // Background image — use data-bg-src for IntersectionObserver lazy loading // (observeLazyBackgrounds is called by the caller after appending the grid). @@ -2518,8 +2514,8 @@ async function openDiscographyModal() { const data = await res.json(); if (!data.error) { - discography = { albums: data.albums || [], singles: data.singles || [] }; - if (discography.albums.length > 0 || discography.singles.length > 0) { + discography = { albums: data.albums || [], eps: data.eps || [], singles: data.singles || [] }; + if (discography.albums.length > 0 || discography.eps.length > 0 || discography.singles.length > 0) { artistsPageState.artistDiscography = discography; artistsPageState.sourceOverride = data.source || artistsPageState.sourceOverride || null; // Use metadata source ID for the modal (needed for download API calls) @@ -2569,6 +2565,9 @@ async function openDiscographyModal() { + + +
@@ -2605,21 +2604,36 @@ async function openDiscographyModal() { function _esc(s) { const d = document.createElement('div'); d.textContent = s; return d.innerHTML; } +// #877: single source of truth for content-type classification, shared by the +// Artist Detail cards and the Download Discography modal so they can't drift. +function _classifyReleaseContent(release) { + const t = (release && (release.title || release.name)) || ''; + const livePattern = /\b(live)\b|\(live[^)]*\)|\[live[^\]]*\]/i; + const compilationPattern = /\b(greatest hits|best of|collection|anthology|essential)\b/i; + const featuredPattern = /\(?\bfeat\.?\s|\bft\.?\s|\bfeaturing\b/i; + return { + isLive: livePattern.test(t), + isCompilation: (release && release.album_type === 'compilation') || compilationPattern.test(t), + isFeatured: featuredPattern.test(t), + }; +} + function _renderDiscogCard(release, index, completionData) { const comp = completionData?.albums?.find(c => c.id === release.id) || completionData?.singles?.find(c => c.id === release.id); const status = comp?.status || 'unknown'; const isOwned = status === 'completed'; const isPartial = status === 'partial' || status === 'nearly_complete'; const year = release.release_date ? release.release_date.substring(0, 4) : ''; - const tracks = release.total_tracks || 0; + const tracks = release.total_tracks || release.track_count || 0; const img = release.image_url || ''; + const cc = _classifyReleaseContent(release); const checked = !isOwned; const statusClass = isOwned ? 'owned' : isPartial ? 'partial' : ''; const statusIcon = isOwned ? '✓' : isPartial ? '◐' : ''; const albumName = release.name || release.title || ''; return ` -
+ +
+
+ + Priority: 1.5 +
+
+
+ +
+ + +
+
+
+ 128 kbps + - + 400 kbps +
+
+
+
+
diff --git a/webui/static/settings.js b/webui/static/settings.js index 0023acf8..abfa3e80 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -1934,7 +1934,7 @@ function populateQualityProfileUI(profile) { } // Populate each quality tier - const qualities = ['flac', 'mp3_320', 'mp3_256', 'mp3_192']; + const qualities = ['flac', 'aac', 'mp3_320', 'mp3_256', 'mp3_192']; qualities.forEach(quality => { const config = profile.qualities[quality]; if (config) { @@ -2128,15 +2128,18 @@ function collectQualityProfileFromUI() { fallback_enabled: document.getElementById('quality-fallback-enabled')?.checked ?? true }; - const qualities = ['flac', 'mp3_320', 'mp3_256', 'mp3_192']; + const qualities = ['flac', 'aac', 'mp3_320', 'mp3_256', 'mp3_192']; qualities.forEach((quality, index) => { const enabled = document.getElementById(`quality-${quality}-enabled`)?.checked || false; const minSlider = document.getElementById(`${quality}-min`); const maxSlider = document.getElementById(`${quality}-max`); - // Preserve priority from the currently loaded profile instead of using array order - const existingPriority = currentQualityProfile?.qualities?.[quality]?.priority ?? (index + 1); + // Preserve priority from the currently loaded profile instead of using array order. + // AAC's default is 1.5 (above MP3, below FLAC) — not index+1 — so an upgraded + // profile that never had an aac tier still ranks it correctly on first save. + const _defaultPriority = quality === 'aac' ? 1.5 : (index + 1); + const existingPriority = currentQualityProfile?.qualities?.[quality]?.priority ?? _defaultPriority; profile.qualities[quality] = { enabled: enabled, From b04010a0370d65967dc74c1595ee659a98d6ebc0 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 18 Jun 2026 14:02:09 -0700 Subject: [PATCH 29/42] #885: repair-job scheduling is timezone-independent (Australia/Sydney loop) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit paksenkin: TZ=Australia/Sydney made the Cache Maintenance job (and any repair job) run every ~5s. Root cause: finished_at is written by SQLite CURRENT_TIMESTAMP (always UTC) but the scheduler compared it against datetime.now() (naive LOCAL), so the local↔UTC offset leaked into the elapsed time. Sydney (+11) made every job look ~11h stale -> always due -> fired every poll; the Americas (behind UTC) deflated it and masked the bug (why New_York 'worked'). Fix: compare in UTC. now = datetime.now(timezone.utc), and a new _hours_since() helper parses the naive CURRENT_TIMESTAMP string AS UTC before subtracting — so the machine timezone never affects scheduling. 5 tests incl. the literal repro (a just-run job must not be due under Australia/Sydney) and a due-detection sanity check; 41 repair-worker tests pass, ruff clean. --- core/repair_worker.py | 23 ++++++++-- tests/test_repair_scheduler_tz.py | 73 +++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 tests/test_repair_scheduler_tz.py diff --git a/core/repair_worker.py b/core/repair_worker.py index 519b04ed..1394d8b4 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -17,7 +17,7 @@ import threading import time import uuid from difflib import SequenceMatcher -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from typing import Any, Dict, List, Optional, Tuple from core.metadata_service import ( @@ -585,13 +585,29 @@ class RepairWorker: logger.info("Repair worker thread finished") + @staticmethod + def _hours_since(finished_at_iso: str, now_utc: datetime) -> float: + """Hours between a stored ``finished_at`` and ``now_utc``, both in UTC. + + ``finished_at`` is written by SQLite's CURRENT_TIMESTAMP, which is ALWAYS + UTC (and naive). #885: the scheduler compared it against ``datetime.now()`` + (naive LOCAL), so the local↔UTC offset leaked into the elapsed time. For a + zone AHEAD of UTC (Australia/Sydney = +11) every job looked ~11h stale and + fired every poll; behind UTC (the Americas) it just waited too long. Parse + the naive timestamp AS UTC and subtract a UTC ``now`` so scheduling is + timezone-independent.""" + dt = datetime.fromisoformat(finished_at_iso) + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return (now_utc - dt).total_seconds() / 3600 + def _pick_next_job(self) -> Optional[str]: """Pick the next job to run based on staleness priority. Returns job_id of the stalest job whose interval has elapsed, or None if nothing is due. """ - now = datetime.now() + now = datetime.now(timezone.utc) best_job_id = None best_staleness = -1 @@ -613,8 +629,7 @@ class RepairWorker: continue try: - last_finished = datetime.fromisoformat(last_run['finished_at']) - elapsed_hours = (now - last_finished).total_seconds() / 3600 + elapsed_hours = self._hours_since(last_run['finished_at'], now) if elapsed_hours < interval_hours: continue # Not due yet diff --git a/tests/test_repair_scheduler_tz.py b/tests/test_repair_scheduler_tz.py new file mode 100644 index 00000000..ff869db4 --- /dev/null +++ b/tests/test_repair_scheduler_tz.py @@ -0,0 +1,73 @@ +"""#885: repair-job scheduling must be timezone-independent. + +`finished_at` is written by SQLite's CURRENT_TIMESTAMP (always UTC), but the +scheduler compared it against `datetime.now()` (naive LOCAL). With TZ=Australia/ +Sydney (UTC+11) every job looked ~11h stale and ran every poll; America/New_York +(behind UTC) masked it. The fix parses finished_at as UTC and compares against a +UTC now, so the machine timezone no longer leaks into elapsed time. +""" + +from __future__ import annotations + +import time +from datetime import datetime, timezone + +import pytest + +from core.repair_worker import RepairWorker + + +# ── pure helper ─────────────────────────────────────────────────────────────── +def test_hours_since_treats_naive_timestamp_as_utc(): + now = datetime(2026, 6, 18, 6, 0, 0, tzinfo=timezone.utc) + # SQLite CURRENT_TIMESTAMP style: UTC, no tz suffix. + assert RepairWorker._hours_since('2026-06-18 00:00:00', now) == pytest.approx(6.0) + + +def test_hours_since_handles_aware_timestamp(): + now = datetime(2026, 6, 18, 6, 0, 0, tzinfo=timezone.utc) + assert RepairWorker._hours_since('2026-06-18T00:00:00+00:00', now) == pytest.approx(6.0) + + +def test_hours_since_recent_is_near_zero(): + now = datetime(2026, 6, 18, 0, 0, 30, tzinfo=timezone.utc) + assert RepairWorker._hours_since('2026-06-18 00:00:00', now) == pytest.approx(30 / 3600, abs=1e-6) + + +# ── the #885 repro: a just-run job is never due, regardless of timezone ──────── +def _set_tz(monkeypatch, tz): + monkeypatch.setenv('TZ', tz) + try: + time.tzset() + except AttributeError: + pytest.skip('time.tzset() unavailable on this platform') + + +def test_just_run_job_not_due_under_any_timezone(monkeypatch): + w = RepairWorker.__new__(RepairWorker) + w._jobs = {'cache_evictor': object()} + monkeypatch.setattr(RepairWorker, 'get_job_config', + lambda self, jid: {'enabled': True, 'interval_hours': 6}) + # Job finished "now" in UTC (exactly how CURRENT_TIMESTAMP records it). + finished = datetime.now(timezone.utc).strftime('%Y-%m-%d %H:%M:%S') + monkeypatch.setattr(RepairWorker, '_get_last_run', + lambda self, jid: {'finished_at': finished}) + + # Australia/Sydney is the exact repro; check the Americas + UTC too. + for tz in ('Australia/Sydney', 'America/New_York', 'UTC'): + _set_tz(monkeypatch, tz) + assert w._pick_next_job() is None, f"just-run job wrongly due under TZ={tz}" + + +def test_stale_job_is_still_picked_under_sydney(monkeypatch): + # Sanity: a genuinely-overdue job IS picked (we didn't break due-detection). + w = RepairWorker.__new__(RepairWorker) + w._jobs = {'cache_evictor': object()} + monkeypatch.setattr(RepairWorker, 'get_job_config', + lambda self, jid: {'enabled': True, 'interval_hours': 6}) + # Finished ~10h ago in UTC. + old = datetime(2026, 1, 1, 0, 0, 0, tzinfo=timezone.utc).strftime('%Y-%m-%d %H:%M:%S') + monkeypatch.setattr(RepairWorker, '_get_last_run', + lambda self, jid: {'finished_at': old}) + _set_tz(monkeypatch, 'Australia/Sydney') + assert w._pick_next_job() == 'cache_evictor' From 70ea7eabf67c8b661b9e88ffe20d60132b41d54b Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 18 Jun 2026 15:05:14 -0700 Subject: [PATCH 30/42] Update style.css --- webui/static/style.css | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/webui/static/style.css b/webui/static/style.css index 6c344644..b8f35661 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -62563,6 +62563,10 @@ body[data-artist-source="source"] #artist-detail-page #library-artist-enhance-bt } #artist-detail-page .artist-detail-hero-bg { + /* Blurred artist-cover backdrop retired — Boulder didn't want the artist + image bleeding behind the header. The element stays in the DOM (JS still + sets its background-image) but is hidden; flip display back on to restore. */ + display: none; position: absolute; inset: -20px; background-size: cover; From dbd8278a144c379c0dba841d8e8ef32d82d94835 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 18 Jun 2026 15:15:41 -0700 Subject: [PATCH 31/42] #889 Phase 1: re-identify hint store (DB table + pure create/find/consume seam) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single-use, user-designated 'which release does this track belong to' answer. Written when the user picks a release in the Re-identify modal and the file is staged; the import flow will read it at the top of matching and consume it. - rematch_hints table (additive, IF NOT EXISTS + indexes) keyed on staged_path with content_hash as a rename-proof fallback. - core/imports/rematch_hints.py: pure DB seam over an injected cursor (create/find/consume/list) + a cheap size+head+tail file fingerprint. - exempt_dedup baked into the hint (a re-identify must bypass dedup-skip); replace_track_id carried for deferred post-success cleanup. Inert until wired (Phase 5) — nothing calls it yet. 9 seam tests. --- core/imports/rematch_hints.py | 226 ++++++++++++++++++++++++++++ database/music_database.py | 35 +++++ tests/imports/test_rematch_hints.py | 155 +++++++++++++++++++ 3 files changed, 416 insertions(+) create mode 100644 core/imports/rematch_hints.py create mode 100644 tests/imports/test_rematch_hints.py diff --git a/core/imports/rematch_hints.py b/core/imports/rematch_hints.py new file mode 100644 index 00000000..6804e788 --- /dev/null +++ b/core/imports/rematch_hints.py @@ -0,0 +1,226 @@ +"""Re-identify hints (#889) — a single-use, user-designated answer to "which +release does this already-imported track belong to". + +Flow: the user clicks *Re-identify* on a library track, searches a source, and +picks the exact release (single / EP / album) it should live under. We write a +**hint** here and stage the file for auto-import. The import flow then reads the +hint at the very TOP of matching — before any fuzzy tier — builds the match from +these exact IDs, and consumes the row. So the original ambiguity that mis-filed +the track (which release?) is gone: the user already answered it. + +Two safety properties live in the hint, not the import code: + +- ``replace_track_id`` — the library row to delete AFTER the re-import lands (so a + re-identify *replaces* rather than *duplicates*). Cleanup is deferred to success + so a failed import can never lose the file. +- ``exempt_dedup`` — always set: a re-identify is an explicit user action and must + not be silently dropped by the quality dedup-skip (which would otherwise see the + incoming file as a duplicate of the very row we're replacing). + +This module is pure DB mechanics over an injected ``cursor`` (sqlite3-style, +``?`` params) — no connection management, no app state — so the create / find / +consume seam is unit-tested against an in-memory DB with no live metadata client. +The binding is keyed on the staged path, with ``content_hash`` as a rename-proof +fallback in case the staging watcher normalizes the filename on ingest. +""" + +from __future__ import annotations + +import os +from dataclasses import dataclass +from typing import Any, Optional + +# Columns in INSERT/SELECT order — single source of truth so the dataclass, the +# write, and the read can't drift apart. +_FIELDS = ( + "staged_path", + "content_hash", + "source", + "isrc", + "track_id", + "album_id", + "artist_id", + "track_title", + "album_name", + "artist_name", + "album_type", + "track_number", + "disc_number", + "replace_track_id", + "exempt_dedup", +) + + +@dataclass +class RematchHint: + """One user-designated re-identify answer. ``id``/``status`` are set by the DB.""" + staged_path: str + source: str + content_hash: Optional[str] = None + isrc: Optional[str] = None + track_id: Optional[str] = None + album_id: Optional[str] = None + artist_id: Optional[str] = None + track_title: Optional[str] = None + album_name: Optional[str] = None + artist_name: Optional[str] = None + album_type: Optional[str] = None + track_number: Optional[int] = None + disc_number: Optional[int] = None + replace_track_id: Optional[int] = None + exempt_dedup: bool = True + id: Optional[int] = None + status: str = "pending" + + def _values(self) -> tuple: + return ( + self.staged_path, + self.content_hash, + self.source, + self.isrc, + self.track_id, + self.album_id, + self.artist_id, + self.track_title, + self.album_name, + self.artist_name, + self.album_type, + self.track_number, + self.disc_number, + self.replace_track_id, + 1 if self.exempt_dedup else 0, + ) + + +def _row_to_hint(row: Any) -> RematchHint: + """Map a sqlite3.Row (or any mapping/sequence-by-name) to a RematchHint.""" + def g(key, default=None): + try: + return row[key] + except (KeyError, IndexError, TypeError): + return default + return RematchHint( + id=g("id"), + staged_path=g("staged_path") or "", + content_hash=g("content_hash"), + source=g("source") or "", + isrc=g("isrc"), + track_id=g("track_id"), + album_id=g("album_id"), + artist_id=g("artist_id"), + track_title=g("track_title"), + album_name=g("album_name"), + artist_name=g("artist_name"), + album_type=g("album_type"), + track_number=g("track_number"), + disc_number=g("disc_number"), + replace_track_id=g("replace_track_id"), + exempt_dedup=bool(g("exempt_dedup", 1)), + status=g("status") or "pending", + ) + + +def create_hint(cursor: Any, hint: RematchHint) -> int: + """Insert a pending hint; return its new id. Caller owns commit.""" + placeholders = ", ".join("?" for _ in _FIELDS) + cursor.execute( + f"INSERT INTO rematch_hints ({', '.join(_FIELDS)}) VALUES ({placeholders})", + hint._values(), + ) + new_id = cursor.lastrowid + hint.id = new_id + return new_id + + +def find_hint_for_file( + cursor: Any, + staged_path: str, + content_hash: Optional[str] = None, +) -> Optional[RematchHint]: + """Return the newest PENDING hint for a staged file, or ``None``. + + Matched by exact ``staged_path`` first; if that misses and a ``content_hash`` + is given, fall back to it (covers a staging watcher that renamed the file on + ingest). Only ``status='pending'`` rows are returned, so a consumed hint is + never reused.""" + if staged_path: + cursor.execute( + "SELECT * FROM rematch_hints WHERE staged_path = ? AND status = 'pending' " + "ORDER BY id DESC LIMIT 1", + (staged_path,), + ) + row = cursor.fetchone() + if row is not None: + return _row_to_hint(row) + # Try by basename too — the watcher may move the file into a different dir. + base = os.path.basename(staged_path) + if base and base != staged_path: + cursor.execute( + "SELECT * FROM rematch_hints WHERE staged_path LIKE ? AND status = 'pending' " + "ORDER BY id DESC LIMIT 1", + ("%/" + base,), + ) + row = cursor.fetchone() + if row is not None: + return _row_to_hint(row) + + if content_hash: + cursor.execute( + "SELECT * FROM rematch_hints WHERE content_hash = ? AND status = 'pending' " + "ORDER BY id DESC LIMIT 1", + (content_hash,), + ) + row = cursor.fetchone() + if row is not None: + return _row_to_hint(row) + + return None + + +def consume_hint(cursor: Any, hint_id: int) -> None: + """Mark a hint consumed (single-use). Caller owns commit.""" + cursor.execute( + "UPDATE rematch_hints SET status = 'consumed', consumed_at = CURRENT_TIMESTAMP " + "WHERE id = ?", + (hint_id,), + ) + + +def list_pending_hints(cursor: Any) -> list: + """All pending hints (newest first) — for a 'pending re-identify' view and + orphan recovery when a staged file never imports.""" + cursor.execute("SELECT * FROM rematch_hints WHERE status = 'pending' ORDER BY id DESC") + return [_row_to_hint(r) for r in cursor.fetchall()] + + +def quick_file_signature(path: str, *, chunk: int = 65536) -> Optional[str]: + """A cheap, rename-proof content fingerprint: size + first/last chunk, hashed. + + Audio files are large, so a full hash is wasteful when we only need to re-bind + a hint to *this* file after a possible rename. Size + head + tail is plenty to + distinguish staged files in practice. Returns ``None`` if the file can't be + read (caller falls back to path-only binding).""" + import hashlib + + try: + size = os.path.getsize(path) + h = hashlib.sha256() + h.update(str(size).encode()) + with open(path, "rb") as f: + h.update(f.read(chunk)) + if size > chunk: + f.seek(max(0, size - chunk)) + h.update(f.read(chunk)) + return h.hexdigest() + except OSError: + return None + + +__all__ = [ + "RematchHint", + "create_hint", + "find_hint_for_file", + "consume_hint", + "list_pending_hints", + "quick_file_signature", +] diff --git a/database/music_database.py b/database/music_database.py index 7f50d875..8691e8d1 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -750,6 +750,41 @@ class MusicDatabase: cursor.execute("CREATE INDEX IF NOT EXISTS idx_aih_status ON auto_import_history (status)") cursor.execute("CREATE INDEX IF NOT EXISTS idx_aih_folder_hash ON auto_import_history (folder_hash)") + # Re-identify hints (#889) — a user-designated, single-use answer to "which + # release does this track belong to". Written when the user picks a release in + # the Re-identify modal and the file is staged for auto-import; the import flow + # reads the hint at the TOP of matching (keyed by staged path, content_hash as a + # rename-proof fallback), expedites the match to these exact IDs, then consumes + # the row. `replace_track_id` (when set) is the library row to delete AFTER the + # re-import lands; `exempt_dedup` is always 1 because a re-identify is an explicit + # user action that must not be silently dropped by the quality dedup-skip. + cursor.execute(""" + CREATE TABLE IF NOT EXISTS rematch_hints ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + staged_path TEXT NOT NULL, + content_hash TEXT, + source TEXT NOT NULL, + isrc TEXT, + track_id TEXT, + album_id TEXT, + artist_id TEXT, + track_title TEXT, + album_name TEXT, + artist_name TEXT, + album_type TEXT, + track_number INTEGER, + disc_number INTEGER, + replace_track_id INTEGER, + exempt_dedup INTEGER NOT NULL DEFAULT 1, + status TEXT NOT NULL DEFAULT 'pending', + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + consumed_at TIMESTAMP + ) + """) + cursor.execute("CREATE INDEX IF NOT EXISTS idx_rmh_staged_path ON rematch_hints (staged_path)") + cursor.execute("CREATE INDEX IF NOT EXISTS idx_rmh_content_hash ON rematch_hints (content_hash)") + cursor.execute("CREATE INDEX IF NOT EXISTS idx_rmh_status ON rematch_hints (status)") + # Sync history table — tracks the last 100 sync operations with cached context for re-trigger cursor.execute(""" CREATE TABLE IF NOT EXISTS sync_history ( diff --git a/tests/imports/test_rematch_hints.py b/tests/imports/test_rematch_hints.py new file mode 100644 index 00000000..6e1d08de --- /dev/null +++ b/tests/imports/test_rematch_hints.py @@ -0,0 +1,155 @@ +"""#889 Phase 1: the re-identify hint store — create / find / consume. + +The hint is the single-use, user-designated "which release" answer the import +flow reads at the top of matching. These lock down: a hint round-trips, it's +found by staged path, found by content_hash when the path missed (rename-proof), +found by basename when the dir changed, consumed exactly once, and that a +consumed hint is never handed back. +""" + +from __future__ import annotations + +import sqlite3 + +import pytest + +from core.imports.rematch_hints import ( + RematchHint, + consume_hint, + create_hint, + find_hint_for_file, + list_pending_hints, + quick_file_signature, +) + +# The slice of the real schema this module touches (kept in sync with +# database/music_database.py's rematch_hints CREATE). +_SCHEMA = """ +CREATE TABLE rematch_hints ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + staged_path TEXT NOT NULL, + content_hash TEXT, + source TEXT NOT NULL, + isrc TEXT, + track_id TEXT, + album_id TEXT, + artist_id TEXT, + track_title TEXT, + album_name TEXT, + artist_name TEXT, + album_type TEXT, + track_number INTEGER, + disc_number INTEGER, + replace_track_id INTEGER, + exempt_dedup INTEGER NOT NULL DEFAULT 1, + status TEXT NOT NULL DEFAULT 'pending', + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + consumed_at TIMESTAMP +) +""" + + +@pytest.fixture +def cur(): + conn = sqlite3.connect(":memory:") + conn.row_factory = sqlite3.Row + conn.executescript(_SCHEMA) + yield conn.cursor() + conn.close() + + +def _hint(**kw): + base = dict( + staged_path="/staging/Song.flac", + source="spotify", + isrc="USABC1234567", + track_id="trk_1", + album_id="alb_album1", + artist_id="art_1", + track_title="Song", + album_name="Album1", + artist_name="Artist", + album_type="album", + track_number=5, + disc_number=1, + replace_track_id=42, + ) + base.update(kw) + return RematchHint(**base) + + +def test_create_and_find_by_path_roundtrips(cur): + new_id = create_hint(cur, _hint()) + assert new_id > 0 + got = find_hint_for_file(cur, "/staging/Song.flac") + assert got is not None + assert got.id == new_id + assert got.album_id == "alb_album1" and got.album_type == "album" + assert got.isrc == "USABC1234567" + assert got.track_number == 5 and got.disc_number == 1 + assert got.replace_track_id == 42 + assert got.exempt_dedup is True # always set for a user-designated re-identify + assert got.status == "pending" + + +def test_find_by_content_hash_when_path_missed(cur): + create_hint(cur, _hint(content_hash="deadbeef")) + # Watcher renamed/moved the file → path lookup misses, hash rescues it. + assert find_hint_for_file(cur, "/totally/different.flac") is None + got = find_hint_for_file(cur, "/totally/different.flac", content_hash="deadbeef") + assert got is not None and got.album_name == "Album1" + + +def test_find_by_basename_when_dir_changed(cur): + create_hint(cur, _hint(staged_path="/staging/in/Song.flac")) + # Same filename, different directory (watcher moved it deeper). + got = find_hint_for_file(cur, "/staging/processing/Song.flac") + assert got is not None and got.track_id == "trk_1" + + +def test_consume_is_single_use(cur): + new_id = create_hint(cur, _hint()) + assert find_hint_for_file(cur, "/staging/Song.flac") is not None + consume_hint(cur, new_id) + # Consumed → never handed back, by path or by hash. + assert find_hint_for_file(cur, "/staging/Song.flac") is None + assert find_hint_for_file(cur, "/x", content_hash=None) is None + + +def test_list_pending_excludes_consumed(cur): + a = create_hint(cur, _hint(staged_path="/staging/A.flac")) + create_hint(cur, _hint(staged_path="/staging/B.flac")) + assert len(list_pending_hints(cur)) == 2 + consume_hint(cur, a) + pend = list_pending_hints(cur) + assert len(pend) == 1 and pend[0].staged_path == "/staging/B.flac" + + +def test_newest_pending_wins_on_duplicate_path(cur): + create_hint(cur, _hint(album_id="alb_old")) + create_hint(cur, _hint(album_id="alb_new")) # user re-picked for the same file + got = find_hint_for_file(cur, "/staging/Song.flac") + assert got.album_id == "alb_new" + + +def test_exempt_dedup_false_roundtrips(cur): + create_hint(cur, _hint(staged_path="/staging/Keep.flac", exempt_dedup=False)) + got = find_hint_for_file(cur, "/staging/Keep.flac") + assert got.exempt_dedup is False + + +# ── content fingerprint ─────────────────────────────────────────────────────── +def test_quick_file_signature_stable_and_distinct(tmp_path): + a = tmp_path / "a.bin" + b = tmp_path / "b.bin" + a.write_bytes(b"hello world" * 1000) + b.write_bytes(b"goodbye moon" * 1000) + sig_a1 = quick_file_signature(str(a)) + sig_a2 = quick_file_signature(str(a)) + sig_b = quick_file_signature(str(b)) + assert sig_a1 and sig_a1 == sig_a2 # stable + assert sig_a1 != sig_b # distinct content → distinct sig + + +def test_quick_file_signature_missing_file_is_none(): + assert quick_file_signature("/no/such/file.flac") is None From 08fb21fb1376b288e9d7de7c1f99905f2307151f Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 18 Jun 2026 15:25:37 -0700 Subject: [PATCH 32/42] =?UTF-8?q?#889=20Phase=202:=20import=20seam=20?= =?UTF-8?q?=E2=80=94=20hint=20short-circuits=20identification,=20replaces?= =?UTF-8?q?=20on=20success?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a staged single-file candidate carries a re-identify hint, the worker builds the identification straight from the user-chosen release (album_id/source) and skips the guessing tiers — so the ambiguity that mis-filed the track is gone. No hint → byte-identical to before (the lookup returns (None, None), fail-safe on any DB error). A hinted import auto-processes (explicit user choice), still gated on the global auto_process pref. After the re-import lands, _finalize_rematch_hint consumes the hint and (if replace was chosen) deletes the old row + file via delete_replaced_track — deferred to success so a failed import never loses the original. Safe by construction: unlink only when no surviving row references the file, and the modal never offers the track's current release so old path != new path. All hint logic lives in auto_import_worker.py + the pure rematch_hints helpers — pipeline.py / side_effects.py untouched. 18 tests; full auto-import suite green. --- core/auto_import_worker.py | 75 +++++++++- core/imports/rematch_hints.py | 61 ++++++++ tests/imports/test_rematch_hints_seam.py | 168 +++++++++++++++++++++++ 3 files changed, 301 insertions(+), 3 deletions(-) create mode 100644 tests/imports/test_rematch_hints_seam.py diff --git a/core/auto_import_worker.py b/core/auto_import_worker.py index 8bfbea34..c3f85603 100644 --- a/core/auto_import_worker.py +++ b/core/auto_import_worker.py @@ -660,8 +660,13 @@ class AutoImportWorker: auto_process = self._config_manager.get('auto_import.auto_process', True) try: - # Phase 3: Identify - identification = self._identify_folder(candidate) + # Phase 3: Identify. + # Re-identify (#889): if the user designated this exact file's release in + # the Re-identify modal, a hint short-circuits the guessing — we match + # straight against the chosen album. No hint → byte-identical to before. + rematch_hint, identification = self._resolve_rematch_hint(candidate) + if identification is None: + identification = self._identify_folder(candidate) if not identification: self._record_result(candidate, 'needs_identification', 0.0, error_message='Could not identify album from tags, folder name, or fingerprint') @@ -690,7 +695,10 @@ class AutoImportWorker: high_conf_matches = [m for m in match_result.get('matches', []) if m['confidence'] >= 0.8] has_strong_individual_matches = len(high_conf_matches) > 0 - if (confidence >= threshold or has_strong_individual_matches) and auto_process: + # A re-identify is an explicit user choice — let it auto-process like a + # strong match (still gated on the global auto_process preference). + if (confidence >= threshold or has_strong_individual_matches + or rematch_hint is not None) and auto_process: # Phase 5: Auto-process — insert an in-progress row # so the UI sees the import the moment it starts, # then update it with the final status when done. @@ -709,6 +717,11 @@ class AutoImportWorker: confidence = max(confidence, effective_conf) if success: self._bump_stat('auto_processed') + # Re-identify (#889): only NOW that the new home exists do we + # consume the hint and (if replace was chosen) delete the old + # row + file — so a failed import never loses the original. + if rematch_hint is not None: + self._finalize_rematch_hint(rematch_hint) else: self._bump_stat('failed') @@ -1003,6 +1016,62 @@ class AutoImportWorker: except Exception: return False + # ── Re-identify hints (#889) ── + + def _resolve_rematch_hint(self, candidate: 'FolderCandidate'): + """If this staged file carries a user-designated re-identify hint, return + ``(hint, identification)`` so matching skips the guessing tiers; otherwise + ``(None, None)`` and the caller falls back to normal identification. + + Fail-safe: ANY error (no table, DB hiccup) returns ``(None, None)`` so a + re-identify problem can never break ordinary auto-import. Only single-file + candidates are eligible — a re-identify always stages exactly one track.""" + try: + files = candidate.audio_files or [] + if len(files) != 1: + return None, None + from core.imports.rematch_hints import ( + build_identification_from_hint, + find_hint_for_file, + quick_file_signature, + ) + file_path = files[0] + sig = quick_file_signature(file_path) + conn = self.database._get_connection() + try: + cursor = conn.cursor() + hint = find_hint_for_file(cursor, file_path, sig) + finally: + conn.close() + if hint is None: + return None, None + logger.info("[Auto-Import] Re-identify hint for %s → %s '%s' (%s)", + candidate.name, hint.album_type or 'release', + hint.album_name or '?', hint.source) + return hint, build_identification_from_hint(hint) + except Exception as e: + logger.debug("[Auto-Import] rematch-hint lookup skipped: %s", e) + return None, None + + def _finalize_rematch_hint(self, hint) -> None: + """Post-success: delete the replaced library row + file (if the user chose + replace) and consume the hint so it's single-use. Best-effort — a cleanup + failure is logged, never raised, since the re-import already succeeded.""" + try: + from core.imports.rematch_hints import consume_hint, delete_replaced_track + conn = self.database._get_connection() + try: + cursor = conn.cursor() + removed = delete_replaced_track(cursor, hint.replace_track_id) + consume_hint(cursor, hint.id) + conn.commit() + finally: + conn.close() + if removed: + logger.info("[Auto-Import] Re-identify replaced old track — removed %s", removed) + except Exception as e: + logger.warning("[Auto-Import] rematch-hint finalize failed (import still OK): %s", e) + # ── Identification ── def _identify_folder(self, candidate: FolderCandidate) -> Optional[Dict]: diff --git a/core/imports/rematch_hints.py b/core/imports/rematch_hints.py index 6804e788..2a75e66a 100644 --- a/core/imports/rematch_hints.py +++ b/core/imports/rematch_hints.py @@ -193,6 +193,65 @@ def list_pending_hints(cursor: Any) -> list: return [_row_to_hint(r) for r in cursor.fetchall()] +def build_identification_from_hint(hint: RematchHint) -> dict: + """Turn a hint into the ``identification`` dict the auto-import matcher expects, + so a re-identify SKIPS the guessing tiers entirely and matches straight against + the user-chosen release. Mirrors the shape `_identify_folder` returns (album_id + / source / track_number drive the album fetch + file→track match).""" + return { + "album_id": hint.album_id or None, + "album_name": hint.album_name or hint.track_title or "", + "artist_name": hint.artist_name or "", + "artist_id": hint.artist_id or "", + "track_name": hint.track_title or "", + "track_id": hint.track_id or "", + "image_url": "", + "release_date": "", + "track_number": hint.track_number or 1, + "total_tracks": 1, + "source": hint.source, + "method": "rematch_hint", + "identification_confidence": 1.0, + "is_single": True, + "album_type": hint.album_type, + } + + +def delete_replaced_track(cursor: Any, replace_track_id: Any, *, unlink=os.remove) -> Optional[str]: + """Remove the OLD library row a re-identify replaces, and its file. + + Called only AFTER the re-import has landed the track at its new home, so the + original is never lost on failure. Safe by construction: the file is unlinked + only if it still exists and **no other track row references it** (guards against + yanking a file a different row legitimately points to). Returns the path it + removed, or ``None`` if there was nothing to do. ``unlink`` is injectable for + tests. Assumes the new home differs from the old — the Re-identify modal never + offers the track's current release as a target, so this holds.""" + if not replace_track_id: + return None + cursor.execute("SELECT file_path FROM tracks WHERE id = ?", (replace_track_id,)) + row = cursor.fetchone() + if row is None: + return None + old_path = (row["file_path"] if not isinstance(row, (tuple, list)) else row[0]) or "" + + cursor.execute("DELETE FROM tracks WHERE id = ?", (replace_track_id,)) + + if not old_path: + return None + # Only unlink if no surviving row still points at this file. + cursor.execute("SELECT 1 FROM tracks WHERE file_path = ? LIMIT 1", (old_path,)) + if cursor.fetchone() is not None: + return None + try: + if os.path.exists(old_path): + unlink(old_path) + return old_path + except OSError: + pass + return None + + def quick_file_signature(path: str, *, chunk: int = 65536) -> Optional[str]: """A cheap, rename-proof content fingerprint: size + first/last chunk, hashed. @@ -222,5 +281,7 @@ __all__ = [ "find_hint_for_file", "consume_hint", "list_pending_hints", + "build_identification_from_hint", + "delete_replaced_track", "quick_file_signature", ] diff --git a/tests/imports/test_rematch_hints_seam.py b/tests/imports/test_rematch_hints_seam.py new file mode 100644 index 00000000..58d479fd --- /dev/null +++ b/tests/imports/test_rematch_hints_seam.py @@ -0,0 +1,168 @@ +"""#889 Phase 2: the import seam — a hint short-circuits identification, and the +old library row is replaced only after the re-import succeeds. + +Two layers: + * pure helpers (build_identification_from_hint, delete_replaced_track) — exact + mapping + safe replacement against an in-memory DB, injectable unlink. + * the worker seam (_resolve_rematch_hint / _finalize_rematch_hint) — proves the + NO-HINT path is untouched, the hint path returns a ready identification, the + lookup is fail-safe, and finalize consumes + replaces. +""" + +from __future__ import annotations + +import sqlite3 +import types + +import pytest + +from core.auto_import_worker import AutoImportWorker, FolderCandidate +from core.imports.rematch_hints import ( + RematchHint, + build_identification_from_hint, + consume_hint, + create_hint, + delete_replaced_track, + find_hint_for_file, +) + +_SCHEMA = """ +CREATE TABLE rematch_hints ( + id INTEGER PRIMARY KEY AUTOINCREMENT, staged_path TEXT NOT NULL, content_hash TEXT, + source TEXT NOT NULL, isrc TEXT, track_id TEXT, album_id TEXT, artist_id TEXT, + track_title TEXT, album_name TEXT, artist_name TEXT, album_type TEXT, + track_number INTEGER, disc_number INTEGER, replace_track_id INTEGER, + exempt_dedup INTEGER NOT NULL DEFAULT 1, status TEXT NOT NULL DEFAULT 'pending', + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, consumed_at TIMESTAMP +); +CREATE TABLE tracks ( + id INTEGER PRIMARY KEY, album_id INTEGER, artist_id INTEGER, title TEXT, + track_number INTEGER, file_path TEXT +); +""" + + +@pytest.fixture +def conn(): + c = sqlite3.connect(":memory:") + c.row_factory = sqlite3.Row + c.executescript(_SCHEMA) + yield c + c.close() + + +def _hint(**kw): + base = dict(staged_path="/staging/Song.flac", source="spotify", album_id="alb_album1", + artist_id="art_1", track_id="trk_1", track_title="Song", album_name="Album1", + artist_name="Artist", album_type="album", track_number=5, disc_number=1) + base.update(kw) + return RematchHint(**base) + + +# ── pure: identification mapping ────────────────────────────────────────────── +def test_build_identification_maps_hint_fields(): + ident = build_identification_from_hint(_hint()) + assert ident["album_id"] == "alb_album1" + assert ident["source"] == "spotify" + assert ident["album_name"] == "Album1" + assert ident["artist_id"] == "art_1" + assert ident["track_number"] == 5 + assert ident["method"] == "rematch_hint" + assert ident["identification_confidence"] == 1.0 + assert ident["is_single"] is True + + +# ── pure: safe replacement ──────────────────────────────────────────────────── +def test_delete_replaced_track_removes_row_and_file(conn): + cur = conn.cursor() + cur.execute("INSERT INTO tracks (id, file_path) VALUES (7, '/lib/EP1/05 - Song.flac')") + removed = [] + out = delete_replaced_track(cur, 7, unlink=lambda p: removed.append(p)) + assert out == "/lib/EP1/05 - Song.flac" + assert removed == ["/lib/EP1/05 - Song.flac"] # file removed (we faked existence below) + cur.execute("SELECT 1 FROM tracks WHERE id = 7") + assert cur.fetchone() is None # row gone + + +def test_delete_replaced_track_keeps_file_if_another_row_points_at_it(conn): + cur = conn.cursor() + cur.execute("INSERT INTO tracks (id, file_path) VALUES (7, '/lib/shared.flac')") + cur.execute("INSERT INTO tracks (id, file_path) VALUES (8, '/lib/shared.flac')") + removed = [] + out = delete_replaced_track(cur, 7, unlink=lambda p: removed.append(p)) + assert out is None and removed == [] # row 8 still references it → no unlink + cur.execute("SELECT 1 FROM tracks WHERE id = 7") + assert cur.fetchone() is None # but row 7 still deleted + + +def test_delete_replaced_track_noops_on_missing_id(conn): + cur = conn.cursor() + assert delete_replaced_track(cur, None) is None + assert delete_replaced_track(cur, 999) is None # no such row + + +# patch os.path.exists so the unlink branch is reachable without real files +@pytest.fixture(autouse=True) +def _exists(monkeypatch): + monkeypatch.setattr("core.imports.rematch_hints.os.path.exists", lambda p: True) + + +# ── worker seam ─────────────────────────────────────────────────────────────── +def _worker(conn): + # Production hands out a FRESH connection per call (the worker closes it); + # here we share one in-memory DB, so proxy close() to a no-op. + w = AutoImportWorker.__new__(AutoImportWorker) + proxy = types.SimpleNamespace(cursor=conn.cursor, commit=conn.commit, close=lambda: None) + w.database = types.SimpleNamespace(_get_connection=lambda: proxy) + return w + + +def test_resolve_returns_none_when_no_hint(conn): + w = _worker(conn) + cand = FolderCandidate(path="/staging", name="Song", audio_files=["/staging/Song.flac"]) + assert w._resolve_rematch_hint(cand) == (None, None) # untouched → normal identify + + +def test_resolve_returns_identification_when_hinted(conn, monkeypatch): + # don't hash a real file + monkeypatch.setattr("core.imports.rematch_hints.quick_file_signature", lambda p: None) + create_hint(conn.cursor(), _hint(staged_path="/staging/Song.flac")) + conn.commit() + w = _worker(conn) + cand = FolderCandidate(path="/staging", name="Song", audio_files=["/staging/Song.flac"]) + hint, ident = w._resolve_rematch_hint(cand) + assert hint is not None and hint.album_id == "alb_album1" + assert ident["album_id"] == "alb_album1" and ident["method"] == "rematch_hint" + + +def test_resolve_ignores_multi_file_candidates(conn): + create_hint(conn.cursor(), _hint(staged_path="/staging/Song.flac")) + conn.commit() + w = _worker(conn) + cand = FolderCandidate(path="/staging", name="Album", + audio_files=["/staging/Song.flac", "/staging/Other.flac"]) + assert w._resolve_rematch_hint(cand) == (None, None) # re-identify is single-track only + + +def test_resolve_is_failsafe_on_db_error(): + w = AutoImportWorker.__new__(AutoImportWorker) + def _boom(): + raise RuntimeError("db down") + w.database = types.SimpleNamespace(_get_connection=_boom) + cand = FolderCandidate(path="/staging", name="Song", audio_files=["/staging/Song.flac"]) + assert w._resolve_rematch_hint(cand) == (None, None) # error never breaks auto-import + + +def test_finalize_consumes_and_replaces(conn, monkeypatch): + monkeypatch.setattr("core.imports.rematch_hints.quick_file_signature", lambda p: None) + cur = conn.cursor() + cur.execute("INSERT INTO tracks (id, file_path) VALUES (42, '/lib/EP1/05 - Song.flac')") + hid = create_hint(cur, _hint(replace_track_id=42)) + conn.commit() + w = _worker(conn) + hint = find_hint_for_file(conn.cursor(), "/staging/Song.flac") + w._finalize_rematch_hint(hint) + # old row deleted, hint consumed + cur.execute("SELECT 1 FROM tracks WHERE id = 42") + assert cur.fetchone() is None + assert find_hint_for_file(conn.cursor(), "/staging/Song.flac") is None # consumed From 3e554f8274263fde1486ee3e1f7c87fde7212668 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 18 Jun 2026 15:31:33 -0700 Subject: [PATCH 33/42] =?UTF-8?q?#889=20Phase=203:=20re-identify=20search?= =?UTF-8?q?=20=E2=80=94=20multi-source=20track=E2=86=92release=20lookup=20?= =?UTF-8?q?+=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Search any configured source (tabs, default active) and surface the SAME song across its collections (single/EP/album) so the user can pick which release a track should be filed under. - core/imports/rematch_search.py: pure normalize + injected client factory. search_release_candidates() → lightweight display rows from typed search_tracks (title/artist/release/type badge/year/count/art/isrc/track_id); resolve_hint_fields() runs ONCE on the picked row via get_track_details to pull the album_id (+ isrc/ track#/disc) the hint needs. infer_release_type() handles Spotify's missing 'EP' (multi-track 'single' → EP badge); filing is driven by real album_id, not the label. - GET /api/reidentify/sources (tabs) + GET /api/reidentify/search (rows). Graceful empty on dead source / blank query / client error — never raises. 14 tests. Inert until the modal (Phase 4) calls it. --- core/imports/rematch_search.py | 247 +++++++++++++++++++++++++++ tests/imports/test_rematch_search.py | 121 +++++++++++++ web_server.py | 41 +++++ 3 files changed, 409 insertions(+) create mode 100644 core/imports/rematch_search.py create mode 100644 tests/imports/test_rematch_search.py diff --git a/core/imports/rematch_search.py b/core/imports/rematch_search.py new file mode 100644 index 00000000..da32c373 --- /dev/null +++ b/core/imports/rematch_search.py @@ -0,0 +1,247 @@ +"""#889 Phase 3: search a metadata source for the releases a track appears on. + +The Re-identify modal lets the user search ANY configured source (tabs, defaulting +to the active one) and shows the SAME song across its different collections — +single / EP / album — so they can pick which release the track should be filed +under. Two steps, deliberately split: + + * ``search_release_candidates(source, query)`` — lightweight DISPLAY rows from the + normal typed ``search_tracks`` (title, artist, release name, type badge, year, + track count, art, ISRC, track_id). No album_id needed to draw the list. + * ``resolve_hint_fields(source, track_id)`` — runs ONCE, on the row the user + picks: ``get_track_details`` yields the album_id / isrc / track#/disc the hint + needs. We don't pay that lookup for every search result, only the chosen one. + +Pure normalization + injected client factory, so the search/normalize/resolve seam +is unit-tested with a fake client and no network. +""" + +from __future__ import annotations + +from typing import Any, Callable, Dict, List, Optional + + +def _get(obj: Any, key: str, default=None): + """Read ``key`` from either an object (attr) or a mapping (item).""" + if obj is None: + return default + if isinstance(obj, dict): + return obj.get(key, default) + return getattr(obj, key, default) + + +def _year(release_date: Any) -> Optional[str]: + s = str(release_date or "").strip() + return s[:4] if len(s) >= 4 and s[:4].isdigit() else None + + +def infer_release_type(album_type: Any, total_tracks: Any) -> str: + """Normalize a source's release type to one of album / ep / single / compilation. + + Sources disagree: Spotify has no 'EP' — EPs come back as ``album_type='single'`` + with several tracks; MusicBrainz/Deezer label EPs properly. So when a 'single' + carries more than a handful of tracks, call it an EP for the badge. The actual + filing is unaffected — that's driven by the real album_id, not this label.""" + t = str(album_type or "").strip().lower() + try: + n = int(total_tracks) if total_tracks is not None else 0 + except (TypeError, ValueError): + n = 0 + if t in ("compilation", "comp"): + return "compilation" + if t == "ep": + return "ep" + if t == "album": + return "album" # an explicit album stays an album; only 'single' gets promoted to EP + if t == "single": + # 1–3 tracks → single; 4+ → almost always an EP in practice. + return "ep" if n >= 4 else "single" + # Unknown type: infer purely from track count. + if n >= 7: + return "album" + if n >= 4: + return "ep" + if n >= 1: + return "single" + return t or "album" + + +def normalize_search_result(result: Any, source: str) -> Optional[Dict[str, Any]]: + """One typed search Track (or raw dict) → a display row, or ``None`` if it has + no usable id/title. ``album`` is just a name at search time; album_id is + resolved later for the picked row only.""" + track_id = _get(result, "id") or _get(result, "track_id") + title = _get(result, "name") or _get(result, "title") + if not track_id or not title: + return None + + artists = _get(result, "artists") + if isinstance(artists, list): + artist_name = ", ".join(str(_get(a, "name", a) if not isinstance(a, str) else a) for a in artists) + else: + artist_name = str(artists or _get(result, "artist") or "") + + album = _get(result, "album") + album_name = album if isinstance(album, str) else (_get(album, "name") or "") + raw_type = _get(result, "album_type") + total = _get(result, "total_tracks") + ext = _get(result, "external_ids") or {} + isrc = _get(result, "isrc") or (ext.get("isrc") if isinstance(ext, dict) else None) + + return { + "source": source, + "track_id": str(track_id), + "track_title": str(title), + "artist_name": artist_name, + "album_name": str(album_name or ""), + "album_type": infer_release_type(raw_type, total), + "raw_album_type": str(raw_type or ""), + "total_tracks": int(total) if isinstance(total, int) else None, + "year": _year(_get(result, "release_date")), + "image_url": _get(result, "image_url") or "", + "isrc": isrc or None, + } + + +def search_release_candidates( + source: str, + query: str, + *, + limit: int = 25, + client_factory: Optional[Callable[[str], Any]] = None, +) -> List[Dict[str, Any]]: + """Search ``source`` for tracks matching ``query`` → normalized display rows. + + Returns ``[]`` (never raises) when the source has no client or errors — the UI + just shows an empty tab. Rows keep duplicate releases; the UI groups them.""" + query = (query or "").strip() + if not query: + return [] + factory = client_factory or _default_client_factory + try: + client = factory(source) + except Exception: + client = None + if client is None or not hasattr(client, "search_tracks"): + return [] + try: + results = client.search_tracks(query, limit=limit) + except TypeError: + results = client.search_tracks(query) # clients with no limit kwarg + except Exception: + return [] + + rows: List[Dict[str, Any]] = [] + for r in results or []: + row = normalize_search_result(r, source) + if row is not None: + rows.append(row) + return rows + + +def resolve_hint_fields( + source: str, + track_id: str, + *, + client_factory: Optional[Callable[[str], Any]] = None, +) -> Optional[Dict[str, Any]]: + """Resolve the picked track to the fields a hint needs (album_id critically, + plus isrc / track# / disc# / album name+type). One lookup for one chosen row. + Returns ``None`` if it can't be resolved (caller surfaces an error).""" + factory = client_factory or _default_client_factory + try: + client = factory(source) + except Exception: + client = None + if client is None or not hasattr(client, "get_track_details"): + return None + try: + details = client.get_track_details(track_id) + except Exception: + return None + if not details: + return None + + album = _get(details, "album") or {} + album_id = _get(album, "id") if not isinstance(album, str) else None + album_name = _get(album, "name") if not isinstance(album, str) else album + album_type = _get(album, "album_type") or _get(details, "album_type") + total = _get(album, "total_tracks") or _get(details, "total_tracks") + + artists = _get(details, "artists") or [] + artist_id = None + artist_name = "" + if isinstance(artists, list) and artists: + artist_id = _get(artists[0], "id") + artist_name = ", ".join(str(_get(a, "name", a) if not isinstance(a, str) else a) for a in artists) + + ext = _get(details, "external_ids") or {} + isrc = _get(details, "isrc") or (ext.get("isrc") if isinstance(ext, dict) else None) + + if not album_id: + return None # without an album_id the import can't fetch the tracklist + + return { + "source": source, + "track_id": str(track_id), + "album_id": str(album_id), + "artist_id": str(artist_id) if artist_id else None, + "track_title": _get(details, "name") or _get(details, "title") or "", + "album_name": str(album_name or ""), + "artist_name": artist_name, + "album_type": infer_release_type(album_type, total), + "track_number": _get(details, "track_number"), + "disc_number": _get(details, "disc_number") or 1, + "isrc": isrc or None, + } + + +def _default_client_factory(source: str): + from core.metadata.registry import get_client_for_source + return get_client_for_source(source) + + +def available_sources() -> List[Dict[str, Any]]: + """The source tabs for the modal: every metadata source with a live client, + the primary one flagged ``active`` so the UI selects it by default.""" + from core.metadata.registry import ( + METADATA_SOURCE_PRIORITY, + get_client_for_source, + get_primary_source, + ) + + try: + primary = get_primary_source() + except Exception: + primary = None + + out: List[Dict[str, Any]] = [] + seen = set() + for src in METADATA_SOURCE_PRIORITY: + if src in seen: + continue + seen.add(src) + try: + client = get_client_for_source(src) + except Exception: + client = None + if client is None or not hasattr(client, "search_tracks"): + continue + out.append({ + "source": src, + "label": src.replace("_", " ").title(), + "active": src == primary, + }) + # Guarantee the primary is selectable + first even if priority ordering missed it. + if primary and not any(s["active"] for s in out): + out.insert(0, {"source": primary, "label": primary.replace("_", " ").title(), "active": True}) + return out + + +__all__ = [ + "infer_release_type", + "normalize_search_result", + "search_release_candidates", + "resolve_hint_fields", + "available_sources", +] diff --git a/tests/imports/test_rematch_search.py b/tests/imports/test_rematch_search.py new file mode 100644 index 00000000..dd962bbe --- /dev/null +++ b/tests/imports/test_rematch_search.py @@ -0,0 +1,121 @@ +"""#889 Phase 3: re-identify search — normalize results across sources, infer the +release-type badge, and resolve the picked row's album_id. + +Locks down: same song surfaces as multiple rows (single/EP/album), the EP +inference from a multi-track 'single', graceful empty on a dead source, and that +resolve_hint_fields pulls album_id (and refuses a result without one). +""" + +from __future__ import annotations + +import types + +from core.imports.rematch_search import ( + available_sources, + infer_release_type, + normalize_search_result, + resolve_hint_fields, + search_release_candidates, +) + + +# typed-Track-ish object (mirrors core.metadata.types.Track attrs the modal reads) +def _track(tid, title, album, album_type, total, isrc=None, year="2020"): + return types.SimpleNamespace( + id=tid, name=title, artists=["Artist"], album=album, + album_type=album_type, total_tracks=total, release_date=year + "-01-01", + image_url="http://img/" + tid, isrc=isrc, external_ids={}, + ) + + +# ── release-type inference ──────────────────────────────────────────────────── +def test_infer_album_stays_album(): + assert infer_release_type("album", 12) == "album" + + +def test_infer_single_one_track_is_single(): + assert infer_release_type("single", 1) == "single" + + +def test_infer_multitrack_single_promoted_to_ep(): + # Spotify labels EPs as album_type='single' — promote on track count. + assert infer_release_type("single", 5) == "ep" + + +def test_infer_compilation(): + assert infer_release_type("compilation", 40) == "compilation" + + +def test_infer_unknown_falls_back_to_count(): + assert infer_release_type(None, 10) == "album" + assert infer_release_type("", 4) == "ep" + assert infer_release_type(None, 1) == "single" + + +# ── normalization ───────────────────────────────────────────────────────────── +def test_normalize_builds_display_row(): + row = normalize_search_result(_track("t1", "Song", "Album1", "album", 12, isrc="US1234567890"), "spotify") + assert row["track_id"] == "t1" + assert row["album_name"] == "Album1" and row["album_type"] == "album" + assert row["artist_name"] == "Artist" + assert row["year"] == "2020" and row["isrc"] == "US1234567890" + + +def test_normalize_skips_result_without_id_or_title(): + assert normalize_search_result(types.SimpleNamespace(id="", name="X"), "spotify") is None + assert normalize_search_result(types.SimpleNamespace(id="t", name=""), "spotify") is None + + +def test_same_song_multiple_collections(): + """The headline case: one song, three releases, three distinct rows + badges.""" + results = [ + _track("t_alb", "Song", "Album1", "album", 12), + _track("t_ep", "Song", "EP1", "single", 5), # multi-track single → EP + _track("t_sgl", "Song", "Song (Single)", "single", 1), + ] + client = types.SimpleNamespace(search_tracks=lambda q, limit=25: results) + rows = search_release_candidates("spotify", "Song", client_factory=lambda s: client) + badges = {r["album_name"]: r["album_type"] for r in rows} + assert badges == {"Album1": "album", "EP1": "ep", "Song (Single)": "single"} + + +def test_search_empty_on_missing_client(): + assert search_release_candidates("spotify", "x", client_factory=lambda s: None) == [] + + +def test_search_empty_on_blank_query(): + called = [] + search_release_candidates("spotify", " ", client_factory=lambda s: called.append(1)) + assert called == [] # never even fetches a client for an empty query + + +def test_search_swallows_client_error(): + def boom(q, limit=25): + raise RuntimeError("rate limited") + client = types.SimpleNamespace(search_tracks=boom) + assert search_release_candidates("spotify", "x", client_factory=lambda s: client) == [] + + +# ── resolve on select ───────────────────────────────────────────────────────── +def test_resolve_pulls_album_id_and_fields(): + details = { + "name": "Song", "track_number": 5, "disc_number": 1, "isrc": "US1234567890", + "album": {"id": "alb_album1", "name": "Album1", "album_type": "album", "total_tracks": 12}, + "artists": [{"id": "art_1", "name": "Artist"}], + } + client = types.SimpleNamespace(get_track_details=lambda tid: details) + out = resolve_hint_fields("spotify", "t_alb", client_factory=lambda s: client) + assert out["album_id"] == "alb_album1" + assert out["artist_id"] == "art_1" + assert out["track_number"] == 5 and out["disc_number"] == 1 + assert out["album_type"] == "album" and out["isrc"] == "US1234567890" + + +def test_resolve_refuses_result_without_album_id(): + details = {"name": "Song", "album": {"name": "NoId Album"}} # no album id + client = types.SimpleNamespace(get_track_details=lambda tid: details) + assert resolve_hint_fields("spotify", "t", client_factory=lambda s: client) is None + + +def test_resolve_none_on_missing_client(): + assert resolve_hint_fields("spotify", "t", client_factory=lambda s: None) is None diff --git a/web_server.py b/web_server.py index 6d9cdb3d..d5234210 100644 --- a/web_server.py +++ b/web_server.py @@ -10044,6 +10044,47 @@ def get_artist_enhanced_detail(artist_id): except Exception as e: return jsonify({"success": False, "error": str(e)}), 500 + +# ── Re-identify an imported track (#889) ── +@app.route('/api/reidentify/sources', methods=['GET']) +def reidentify_sources(): + """Source tabs for the Re-identify modal — every metadata source with a live + client, the active one flagged so the UI selects it by default.""" + try: + from core.imports.rematch_search import available_sources + return jsonify({"success": True, "sources": available_sources()}) + except Exception as e: + return jsonify({"success": False, "error": str(e), "sources": []}), 500 + + +@app.route('/api/reidentify/search', methods=['GET']) +def reidentify_search(): + """Search one metadata source for the releases a track appears on. + + Query params: ``source`` (defaults to the active source), ``q`` (the query), + ``limit``. Returns display rows — the SAME song across single/EP/album, each + with a type badge — for the user to pick. album_id is resolved later, only for + the chosen row.""" + try: + from core.imports.rematch_search import available_sources, search_release_candidates + query = (request.args.get('q') or '').strip() + if not query: + return jsonify({"success": True, "results": []}) + source = (request.args.get('source') or '').strip() + if not source: + actives = [s for s in available_sources() if s.get('active')] + source = actives[0]['source'] if actives else 'spotify' + try: + limit = max(1, min(50, int(request.args.get('limit', 25)))) + except (TypeError, ValueError): + limit = 25 + rows = search_release_candidates(source, query, limit=limit) + return jsonify({"success": True, "source": source, "results": rows}) + except Exception as e: + logger.error(f"Re-identify search error: {e}") + return jsonify({"success": False, "error": str(e), "results": []}), 500 + + @app.route('/api/library/artist//quality-analysis') def get_artist_quality_analysis(artist_id): """Analyze track quality for an artist — returns tier classification for each track.""" From f4c16ecc22b7392b5843292999976561bdd599d7 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 18 Jun 2026 15:37:56 -0700 Subject: [PATCH 34/42] #889 Phase 4: the Re-identify modal + apply backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The showpiece: a focused 'which release does this track belong to?' chooser. Source tabs (default active), pre-seeded search, the same song surfaced across single/EP/album with color-coded type badges, ISRC-ranked, replace-original toggle (on by default). Glassy panel, blurred hero art, shimmer/spinner states, hover-lift result cards — matched to the app's modal language. Backend: - core/imports/rematch_apply.py: pure staged_destination + build_reidentify_hint, injectable stage_file_for_reidentify (COPIES the file, never moves — original safe until re-import succeeds). 6 tests. - POST /api/reidentify/apply (admin-only): resolve_hint_fields → stage file → create_hint → nudge the worker. Replace deletes the old row only on success. Frontend: modal markup (index.html), full stylesheet (style.css), and the openReidentifyModal/search/select/confirm flow (library.js). Not yet reachable from a button — Phase 5 wires it. --- core/imports/rematch_apply.py | 92 +++++++++++++ tests/imports/test_rematch_apply.py | 71 ++++++++++ web_server.py | 78 +++++++++++ webui/index.html | 42 ++++++ webui/static/library.js | 195 ++++++++++++++++++++++++++++ webui/static/style.css | 187 ++++++++++++++++++++++++++ 6 files changed, 665 insertions(+) create mode 100644 core/imports/rematch_apply.py create mode 100644 tests/imports/test_rematch_apply.py diff --git a/core/imports/rematch_apply.py b/core/imports/rematch_apply.py new file mode 100644 index 00000000..33bcb8c6 --- /dev/null +++ b/core/imports/rematch_apply.py @@ -0,0 +1,92 @@ +"""#889 Phase 4/5: apply a re-identify — stage the library file + write the hint. + +When the user confirms a release in the Re-identify modal, we: + 1. COPY (never move) the track's library file into the auto-import staging folder, + so the original is untouched until the re-import succeeds, + 2. fingerprint the staged copy (rename-proof binding), and + 3. write a single-use hint carrying the chosen release's IDs (+ ``replace_track_id`` + when 'replace original' is ticked). + +The auto-import worker then picks the staged file up, finds the hint, and re-imports +it against the user-chosen release (Phase 2). The pieces here are split so the +naming + hint construction are pure/unit-tested and the actual copy is injectable. +""" + +from __future__ import annotations + +import os +import shutil +from typing import Any, Callable, Dict, Optional + +from core.imports.paths import sanitize_filename +from core.imports.rematch_hints import RematchHint, quick_file_signature + + +def staged_destination(staging_dir: str, real_path: str, library_track_id: Any) -> str: + """Where the staged copy lands: a single loose file in the staging ROOT (so the + worker treats it as a single-track candidate), named to keep the extension and + be unique + traceable to the track it re-identifies. The filename is cosmetic — + matching is driven by the hint, not the name.""" + base = os.path.basename(real_path) + stem, ext = os.path.splitext(base) + safe_stem = sanitize_filename(stem).strip() or "track" + name = f"{safe_stem} [reid-{library_track_id}]{ext}" + return os.path.join(staging_dir, name) + + +def stage_file_for_reidentify( + real_path: str, + staging_dir: str, + library_track_id: Any, + *, + copy_fn: Callable[[str, str], object] = shutil.copy2, + signature_fn: Callable[[str], Optional[str]] = quick_file_signature, +) -> Dict[str, Any]: + """Copy the library file into staging and fingerprint the copy. Returns + ``{staged_path, content_hash}``. Raises ``FileNotFoundError`` if the source is + gone (caller surfaces a clear error rather than writing a dangling hint).""" + if not real_path or not os.path.isfile(real_path): + raise FileNotFoundError(real_path or "(empty path)") + os.makedirs(staging_dir, exist_ok=True) + dest = staged_destination(staging_dir, real_path, library_track_id) + copy_fn(real_path, dest) + return {"staged_path": dest, "content_hash": signature_fn(dest)} + + +def build_reidentify_hint( + library_track_id: Any, + hint_fields: Dict[str, Any], + staged_path: str, + content_hash: Optional[str], + *, + replace: bool, +) -> RematchHint: + """Pure: assemble the RematchHint from the resolved release fields + staging + info. ``replace_track_id`` is the library row to delete on success, but only + when 'replace original' was ticked. ``exempt_dedup`` is always True — a + re-identify is explicit and must bypass dedup-skip.""" + return RematchHint( + staged_path=staged_path, + content_hash=content_hash, + source=hint_fields.get("source") or "", + isrc=hint_fields.get("isrc"), + track_id=hint_fields.get("track_id"), + album_id=hint_fields.get("album_id"), + artist_id=hint_fields.get("artist_id"), + track_title=hint_fields.get("track_title"), + album_name=hint_fields.get("album_name"), + artist_name=hint_fields.get("artist_name"), + album_type=hint_fields.get("album_type"), + track_number=hint_fields.get("track_number"), + disc_number=hint_fields.get("disc_number"), + replace_track_id=(int(library_track_id) if replace and str(library_track_id).isdigit() else + (library_track_id if replace else None)), + exempt_dedup=True, + ) + + +__all__ = [ + "staged_destination", + "stage_file_for_reidentify", + "build_reidentify_hint", +] diff --git a/tests/imports/test_rematch_apply.py b/tests/imports/test_rematch_apply.py new file mode 100644 index 00000000..20c3fd3e --- /dev/null +++ b/tests/imports/test_rematch_apply.py @@ -0,0 +1,71 @@ +"""#889 Phase 4/5: apply a re-identify — stage the file (copy, not move) + build +the hint. Locks down: the original is never touched, the staged name is unique + +keeps the extension, the hint carries the chosen release, and replace_track_id is +set ONLY when 'replace' is ticked. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from core.imports.rematch_apply import ( + build_reidentify_hint, + stage_file_for_reidentify, + staged_destination, +) + +_FIELDS = { + "source": "spotify", "track_id": "trk_1", "album_id": "alb_album1", + "artist_id": "art_1", "track_title": "Song", "album_name": "Album1", + "artist_name": "Artist", "album_type": "album", "track_number": 5, + "disc_number": 1, "isrc": "US1234567890", +} + + +def test_staged_destination_keeps_ext_and_is_traceable(): + dest = staged_destination("/staging", "/lib/EP1/05 - Song.flac", 42) + assert dest.endswith(".flac") + assert "[reid-42]" in dest # traceable to the track + unique per track + assert dest.startswith("/staging/") # loose file in staging root → single candidate + + +def test_stage_copies_not_moves(tmp_path: Path): + src = tmp_path / "lib" / "EP1" / "05 - Song.flac" + src.parent.mkdir(parents=True) + src.write_bytes(b"audio-bytes") + staging = tmp_path / "Staging" + + out = stage_file_for_reidentify(str(src), str(staging), 42, + signature_fn=lambda p: "sig123") + staged = Path(out["staged_path"]) + assert staged.is_file() and staged.read_bytes() == b"audio-bytes" + assert src.is_file() # ORIGINAL untouched (copy, never move) + assert out["content_hash"] == "sig123" + + +def test_stage_missing_source_raises(tmp_path: Path): + with pytest.raises(FileNotFoundError): + stage_file_for_reidentify(str(tmp_path / "gone.flac"), str(tmp_path / "S"), 1) + + +def test_build_hint_sets_replace_when_ticked(): + h = build_reidentify_hint(42, _FIELDS, "/staging/x.flac", "sig", replace=True) + assert h.replace_track_id == 42 + assert h.album_id == "alb_album1" and h.source == "spotify" + assert h.track_number == 5 and h.isrc == "US1234567890" + assert h.exempt_dedup is True + assert h.staged_path == "/staging/x.flac" and h.content_hash == "sig" + + +def test_build_hint_no_replace_when_unticked(): + h = build_reidentify_hint(42, _FIELDS, "/staging/x.flac", "sig", replace=False) + assert h.replace_track_id is None # keep original → no deletion + assert h.exempt_dedup is True # still bypasses dedup-skip (explicit action) + + +def test_build_hint_handles_non_numeric_track_id(): + # Jellyfin-style GUID track ids must still round-trip as replace target. + h = build_reidentify_hint("abc-guid", _FIELDS, "/s/x.flac", None, replace=True) + assert h.replace_track_id == "abc-guid" diff --git a/web_server.py b/web_server.py index d5234210..2cea211d 100644 --- a/web_server.py +++ b/web_server.py @@ -10085,6 +10085,84 @@ def reidentify_search(): return jsonify({"success": False, "error": str(e), "results": []}), 500 +@app.route('/api/reidentify/apply', methods=['POST']) +def reidentify_apply(): + """Apply a re-identify: stage the track's library file + write a single-use hint + so the auto-import worker re-files it under the chosen release (Phase 2). + + Body: ``{library_track_id, source, track_id, replace}``. Admin-only (mutates the + library). COPIES the file — the original is removed only after the re-import + succeeds, and only when ``replace`` is true.""" + try: + database = get_database() + pid = get_current_profile_id() + prof = database.get_profile(pid) if pid else None + if not prof or not prof.get('is_admin'): + return jsonify({"success": False, "error": "Admin only"}), 403 + + data = request.get_json(silent=True) or {} + library_track_id = data.get('library_track_id') + source = (data.get('source') or '').strip() + track_id = (data.get('track_id') or '').strip() + replace = bool(data.get('replace', True)) + if not library_track_id or not source or not track_id: + return jsonify({"success": False, "error": "library_track_id, source and track_id are required"}), 400 + + from core.imports.rematch_search import resolve_hint_fields + from core.imports.rematch_apply import stage_file_for_reidentify, build_reidentify_hint + from core.imports.rematch_hints import create_hint + + # 1) Resolve the picked release → the IDs the hint needs (album_id critically). + hint_fields = resolve_hint_fields(source, track_id) + if not hint_fields: + return jsonify({"success": False, "error": "Could not resolve the selected release (no album id)"}), 400 + + # 2) Locate the library file for this track. + conn = database._get_connection() + try: + cur = conn.cursor() + cur.execute("SELECT file_path FROM tracks WHERE id = ?", (str(library_track_id),)) + row = cur.fetchone() + finally: + conn.close() + if not row or not row['file_path']: + return jsonify({"success": False, "error": "Library track has no file on disk"}), 404 + real_path = _resolve_library_file_path(row['file_path']) or row['file_path'] + + # 3) Copy into staging + fingerprint the copy. + staging_dir = docker_resolve_path(config_manager.get('import.staging_path', './Staging')) + staged = stage_file_for_reidentify(real_path, staging_dir, library_track_id) + + # 4) Persist the single-use hint. + hint = build_reidentify_hint(library_track_id, hint_fields, + staged['staged_path'], staged['content_hash'], replace=replace) + conn = database._get_connection() + try: + cur = conn.cursor() + hint_id = create_hint(cur, hint) + conn.commit() + finally: + conn.close() + + # 5) Nudge the worker so it doesn't wait for the next timer tick. + try: + if auto_import_worker is not None: + auto_import_worker.trigger_scan() + except Exception as _e: + logger.debug("Re-identify: scan nudge failed (worker will catch it on its timer): %s", _e) + + logger.info("[Re-identify] staged track %s → %s '%s' (%s), replace=%s", + library_track_id, hint.album_type or 'release', hint.album_name or '?', + source, replace) + return jsonify({"success": True, "hint_id": hint_id, "staged_path": staged['staged_path'], + "album_name": hint.album_name, "album_type": hint.album_type}) + except FileNotFoundError as e: + return jsonify({"success": False, "error": f"Source file not found: {e}"}), 404 + except Exception as e: + logger.error(f"Re-identify apply error: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + @app.route('/api/library/artist//quality-analysis') def get_artist_quality_analysis(artist_id): """Analyze track quality for an artist — returns tier classification for each track.""" diff --git a/webui/index.html b/webui/index.html index 127eb692..6db10878 100644 --- a/webui/index.html +++ b/webui/index.html @@ -7743,6 +7743,48 @@
+ + +