From f9271c0cd8597c65bf636e7ff5656d2ad46de060 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 2 Jun 2026 11:53:45 -0700 Subject: [PATCH] =?UTF-8?q?Canonical=20album=20version=20=E2=80=94=20backf?= =?UTF-8?q?ill=20job=20(the=20opt-in=20activation)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The populate trigger that turns the (until now dormant) feature on. Until a user enables and runs this job, no album has a canonical -> both read sides (Stages 3-4) fall back -> zero behavior change. So the whole feature ships safely off. - core/repair_jobs/canonical_version_resolve.py — "Resolve Canonical Album Versions". Iterates the active server's albums, skips ones already pinned, and calls the tested resolve_and_store_canonical_for_album per album. Opt-in (default_enabled=False) and dry-run-by-default: resolving compares an album's candidate releases across sources (metadata-source API calls, once per album), so it's deliberately user-triggered. Dry run reports a finding per album it would pin; live mode stores. Registered in _JOB_MODULES. - core/metadata/canonical_resolver.py — resolve_and_store gains store=True; the job's dry run passes store=False to resolve-without-writing. Tests: tests/test_canonical_version_job.py (6) — registered, opt-in + dry-run defaults, live resolves+stores (auto_fixed), dry run creates findings without persisting, already-pinned albums skipped. Registry loads all 19 jobs cleanly. 145 tests across the full feature + reorganize/track-repair/DB regression pass. --- core/metadata/canonical_resolver.py | 8 +- core/repair_jobs/__init__.py | 1 + core/repair_jobs/canonical_version_resolve.py | 164 ++++++++++++++++++ tests/test_canonical_version_job.py | 103 +++++++++++ 4 files changed, 273 insertions(+), 3 deletions(-) create mode 100644 core/repair_jobs/canonical_version_resolve.py create mode 100644 tests/test_canonical_version_job.py diff --git a/core/metadata/canonical_resolver.py b/core/metadata/canonical_resolver.py index a240870a..15a1b279 100644 --- a/core/metadata/canonical_resolver.py +++ b/core/metadata/canonical_resolver.py @@ -110,10 +110,12 @@ def resolve_and_store_canonical_for_album( fetch_tracklist: Optional[Callable[[str, str], Any]] = None, source_priority: Optional[List[str]] = None, min_score: float = 0.5, + store: bool = True, ) -> Optional[Dict[str, Any]]: """Gather an album's source IDs + its tracks' (duration, title) from the DB, - resolve the best-fit canonical release, and persist it. Returns the stored - ``{source, album_id, score}`` or None when unresolved. + resolve the best-fit canonical release, and (when ``store``) persist it. + Returns the resolved ``{source, album_id, score}`` or None when unresolved. + ``store=False`` resolves without writing — used by the backfill job's dry run. Uses the SAME album/source-id loader the Reorganizer uses (``load_album_and_tracks`` + ``_extract_source_ids``) so the canonical is @@ -150,7 +152,7 @@ def resolve_and_store_canonical_for_album( source_priority=source_priority, min_score=min_score, ) - if result: + if result and store: db.set_album_canonical(album_id, result['source'], result['album_id'], result['score']) return result diff --git a/core/repair_jobs/__init__.py b/core/repair_jobs/__init__.py index c6f780e8..40fd7a04 100644 --- a/core/repair_jobs/__init__.py +++ b/core/repair_jobs/__init__.py @@ -44,6 +44,7 @@ _JOB_MODULES = [ 'core.repair_jobs.live_commentary_cleaner', 'core.repair_jobs.unknown_artist_fixer', 'core.repair_jobs.discography_backfill', + 'core.repair_jobs.canonical_version_resolve', ] diff --git a/core/repair_jobs/canonical_version_resolve.py b/core/repair_jobs/canonical_version_resolve.py new file mode 100644 index 00000000..1323eb11 --- /dev/null +++ b/core/repair_jobs/canonical_version_resolve.py @@ -0,0 +1,164 @@ +"""Resolve Canonical Album Versions — backfill job (#765 Stage 2 trigger). + +Pins each album's canonical release (best-fit to its files) so the Library +Reorganizer (Stage 3) and Track Number Repair (Stage 4) resolve the SAME +release and stop contradicting each other. The resolution logic lives in the +tested core.metadata.canonical_resolver; this job is the opt-in, rate-limited, +progress-reported bulk runner. + +Opt-in (``default_enabled = False``) because resolving compares an album's +candidate releases across sources, which costs metadata-source API calls — done +once per album, then stored. Albums that already have a canonical are skipped. +""" + +import os +from typing import Optional + +from core.metadata.canonical_resolver import resolve_and_store_canonical_for_album +from core.repair_jobs import register_job +from core.repair_jobs.base import JobContext, JobResult, RepairJob +from utils.logging_config import get_logger + +logger = get_logger("repair_job.canonical_version") + + +@register_job +class CanonicalVersionResolveJob(RepairJob): + job_id = 'canonical_version_resolve' + display_name = 'Resolve Canonical Album Versions' + description = ( + 'Pins the best-fit release per album (by track count + durations) so ' + 'reorganize and track-number repair agree (dry run by default)' + ) + help_text = ( + 'For each album, compares the releases its linked metadata sources point ' + 'at and pins the one that best matches the files you actually have ' + '(track count + durations + titles). The Library Reorganizer and Track ' + 'Number Repair then both use that pinned release, so they stop ' + 'contradicting each other (e.g. standard vs deluxe, or Spotify vs ' + 'MusicBrainz track numbering).\n\n' + 'In dry run mode (default) it reports what it would pin without saving. ' + 'Disable dry run to store the pins. Albums already pinned are skipped.\n\n' + 'Opt-in: resolving costs metadata-source API calls (once per album).' + ) + icon = 'repair-icon-tracknumber' + default_enabled = False + default_interval_hours = 168 # weekly, but disabled by default + default_settings = { + 'dry_run': True, + 'min_score': 0.5, + } + auto_fix = True + + def _get_settings(self, context: JobContext) -> dict: + merged = dict(self.default_settings) + if context.config_manager: + merged.update(context.config_manager.get(f'repair.jobs.{self.job_id}.settings', {}) or {}) + return merged + + def _load_album_ids(self, db, active_server: Optional[str]) -> list: + conn = None + try: + conn = db._get_connection() + cursor = conn.cursor() + if active_server: + cursor.execute( + "SELECT al.id, al.title FROM albums al WHERE al.server_source = ? ORDER BY al.id", + (active_server,), + ) + else: + cursor.execute("SELECT al.id, al.title FROM albums al ORDER BY al.id") + return [(row[0], row[1]) for row in cursor.fetchall()] + except Exception as e: + logger.error("Error loading albums for canonical resolve: %s", e) + return [] + finally: + if conn: + conn.close() + + def scan(self, context: JobContext) -> JobResult: + result = JobResult() + settings = self._get_settings(context) + dry_run = settings.get('dry_run', True) + min_score = settings.get('min_score', 0.5) + + active_server = None + if context.config_manager: + try: + active_server = context.config_manager.get_active_media_server() + except Exception as e: + logger.warning("Couldn't read active media server: %s", e) + + albums = self._load_album_ids(context.db, active_server) + total = len(albums) + if context.report_progress: + mode = 'DRY RUN' if dry_run else 'LIVE' + context.report_progress( + phase=f'Resolving canonical versions for {total} albums ({mode})...', + total=total, scanned=0, log_type='info', + ) + + for i, (album_id, album_title) in enumerate(albums): + if context.check_stop(): + return result + if i % 20 == 0 and context.wait_if_paused(): + return result + + # Skip albums already pinned — one-time cost per album. + try: + if context.db.get_album_canonical(album_id): + result.skipped += 1 + result.scanned += 1 + continue + except Exception: + pass + + try: + resolved = resolve_and_store_canonical_for_album( + context.db, album_id, min_score=min_score, store=not dry_run, + ) + except Exception as e: + logger.warning("Canonical resolve failed for album %s ('%s'): %s", + album_id, album_title, e) + result.errors += 1 + result.scanned += 1 + continue + + result.scanned += 1 + if resolved: + if dry_run and context.create_finding: + inserted = context.create_finding( + job_id=self.job_id, + finding_type='canonical_version', + severity='info', + entity_type='album', + entity_id=str(album_id), + file_path=None, + title=f'Would pin canonical: {album_title or album_id}', + description=( + f"Best-fit release: {resolved['source']} " + f"({resolved['album_id']}), score {resolved['score']}" + ), + details={'album_id': str(album_id), **resolved}, + ) + if inserted: + result.findings_created += 1 + else: + result.findings_skipped_dedup += 1 + elif not dry_run: + result.auto_fixed += 1 + + if context.report_progress and (i + 1) % 25 == 0: + context.report_progress(scanned=i + 1, total=total, + phase=f'Resolving ({i+1}/{total})...') + + return result + + def estimate_scope(self, context: JobContext) -> int: + active_server = None + if context.config_manager: + try: + active_server = context.config_manager.get_active_media_server() + except Exception: + pass + return len(self._load_album_ids(context.db, active_server)) diff --git a/tests/test_canonical_version_job.py b/tests/test_canonical_version_job.py new file mode 100644 index 00000000..32c20e3f --- /dev/null +++ b/tests/test_canonical_version_job.py @@ -0,0 +1,103 @@ +"""Backfill job: Resolve Canonical Album Versions (#765 Stage 2 trigger).""" + +from __future__ import annotations + +import types + +import core.repair_jobs.canonical_version_resolve as cvr +from core.repair_jobs import get_all_jobs +from core.repair_jobs.canonical_version_resolve import CanonicalVersionResolveJob +from database.music_database import MusicDatabase + + +def _ctx(db, findings): + return types.SimpleNamespace( + db=db, + config_manager=None, # -> active_server None -> all albums + check_stop=lambda: False, + wait_if_paused=lambda: False, + report_progress=None, + update_progress=None, + create_finding=lambda **kw: (findings.append(kw) or True), + ) + + +def _seed_two_albums(db): + conn = db._get_connection() + cur = conn.cursor() + cur.execute("INSERT INTO artists (id, name) VALUES ('art1', 'A')") + cur.execute("INSERT INTO albums (id, title, artist_id) VALUES ('alb1', 'Album One', 'art1')") + cur.execute("INSERT INTO albums (id, title, artist_id) VALUES ('alb2', 'Album Two', 'art1')") + conn.commit() + conn.close() + + +def _fake_resolver(monkeypatch): + def fake(db, album_id, *, min_score=0.5, store=True): + res = {"source": "spotify", "album_id": f"sp_{album_id}", "score": 0.9} + if store: + db.set_album_canonical(album_id, res["source"], res["album_id"], res["score"]) + return res + monkeypatch.setattr(cvr, "resolve_and_store_canonical_for_album", fake) + + +def test_job_is_registered(): + jobs = get_all_jobs() # {job_id: cls} + assert "canonical_version_resolve" in jobs + assert jobs["canonical_version_resolve"] is CanonicalVersionResolveJob + + +def test_job_is_opt_in_and_dry_run_by_default(): + assert CanonicalVersionResolveJob.default_enabled is False + assert CanonicalVersionResolveJob.default_settings["dry_run"] is True + + +def test_live_resolves_and_stores(tmp_path, monkeypatch): + db = MusicDatabase(str(tmp_path / "m.db")) + _seed_two_albums(db) + _fake_resolver(monkeypatch) + + findings = [] + ctx = _ctx(db, findings) + job = CanonicalVersionResolveJob() + # force live mode + monkeypatch.setattr(job, "_get_settings", lambda c: {"dry_run": False, "min_score": 0.5}) + + result = job.scan(ctx) + assert result.auto_fixed == 2 + assert db.get_album_canonical("alb1")["source"] == "spotify" + assert db.get_album_canonical("alb2")["album_id"] == "sp_alb2" + assert findings == [] # live mode writes, doesn't create findings + + +def test_dry_run_creates_findings_without_storing(tmp_path, monkeypatch): + db = MusicDatabase(str(tmp_path / "m.db")) + _seed_two_albums(db) + _fake_resolver(monkeypatch) + + findings = [] + ctx = _ctx(db, findings) + job = CanonicalVersionResolveJob() + monkeypatch.setattr(job, "_get_settings", lambda c: {"dry_run": True, "min_score": 0.5}) + + result = job.scan(ctx) + assert result.findings_created == 2 + assert len(findings) == 2 + # dry run must NOT persist + assert db.get_album_canonical("alb1") is None + + +def test_skips_already_pinned_albums(tmp_path, monkeypatch): + db = MusicDatabase(str(tmp_path / "m.db")) + _seed_two_albums(db) + db.set_album_canonical("alb1", "deezer", "dz_pinned", 0.8) # alb1 already pinned + _fake_resolver(monkeypatch) + + ctx = _ctx(db, []) + job = CanonicalVersionResolveJob() + monkeypatch.setattr(job, "_get_settings", lambda c: {"dry_run": False, "min_score": 0.5}) + + result = job.scan(ctx) + assert result.skipped == 1 # alb1 skipped + assert result.auto_fixed == 1 # only alb2 resolved + assert db.get_album_canonical("alb1")["album_id"] == "dz_pinned" # untouched