diff --git a/core/metadata/canonical_resolver.py b/core/metadata/canonical_resolver.py new file mode 100644 index 00000000..b671a759 --- /dev/null +++ b/core/metadata/canonical_resolver.py @@ -0,0 +1,76 @@ +"""Resolve (and persist) the canonical release for an album — Stage 2 of #765. + +Stage 1 gave us the pure scorer (``core.metadata.canonical_version``). This +module turns it into an end-to-end resolver: gather the album's candidate +releases (one per metadata-source ID it has), score each against the on-disk +files, and return the best fit. Wiring (backfill job / enrichment hook) and the +DB store live alongside; the decision logic here is kept dependency-injected +(``fetch_tracklist`` is passed in) so it's fully unit-testable without live APIs +or real files. + +Still NO consumer reads the result in Stage 2 — populating the columns is +behavior-neutral. Stages 3-4 wire the Reorganizer and Track Number Repair to +read it. +""" + +from __future__ import annotations + +from typing import Any, Callable, Dict, List, Optional + +from core.metadata.canonical_version import pick_canonical_release + + +def resolve_canonical_for_album( + *, + album_source_ids: Dict[str, str], + file_tracks: List[Dict[str, Any]], + fetch_tracklist: Callable[[str, str], Optional[List[Dict[str, Any]]]], + source_priority: List[str], + min_score: float = 0.5, +) -> Optional[Dict[str, Any]]: + """Pick the canonical release for one album. + + ``album_source_ids``: ``{source: album_id}`` the album is linked to. + ``file_tracks``: on-disk track metadata (``{duration_ms, title}``). + ``fetch_tracklist(source, album_id)``: returns that release's tracklist (or + None/[] on miss); injected so callers supply ``get_album_tracks_for_source`` + while tests supply a fake. + ``source_priority``: order to build candidates in — ties break toward the + earlier (higher-priority) source, keeping the choice deterministic. + + Returns ``{'source', 'album_id', 'score'}`` for the best fit, or ``None`` + when there are no files, no resolvable candidates, or nothing clears + ``min_score`` (caller leaves the album unresolved → tools fall back).""" + if not file_tracks: + return None + + candidates: List[Dict[str, Any]] = [] + for source in source_priority: + album_id = album_source_ids.get(source) + if not album_id: + continue + try: + tracks = fetch_tracklist(source, str(album_id)) + except Exception: + tracks = None + if tracks: + candidates.append({ + 'source': source, + 'album_id': str(album_id), + 'tracks': tracks, + }) + + if not candidates: + return None + + best, score = pick_canonical_release(file_tracks, candidates, min_score=min_score) + if not best: + return None + return { + 'source': best['source'], + 'album_id': best['album_id'], + 'score': round(score, 4), + } + + +__all__ = ["resolve_canonical_for_album"] diff --git a/database/music_database.py b/database/music_database.py index 61847317..74fa5e13 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -974,6 +974,53 @@ class MusicDatabase: except Exception as e: logger.error("Error repairing core media schema columns: %s", e) + def set_album_canonical(self, album_id, source: str, canonical_album_id: str, score: float) -> bool: + """Persist the resolved canonical (source, album_id, score) for an album + (#765 Stage 2). Returns True if a row was updated.""" + conn = self._get_connection() + try: + cursor = conn.cursor() + cursor.execute( + "UPDATE albums SET canonical_source = ?, canonical_album_id = ?, " + "canonical_score = ?, canonical_resolved_at = CURRENT_TIMESTAMP " + "WHERE id = ?", + (source, str(canonical_album_id), float(score), album_id), + ) + conn.commit() + return cursor.rowcount > 0 + except Exception as e: + logger.error("Error setting album canonical for %s: %s", album_id, e) + return False + finally: + conn.close() + + def get_album_canonical(self, album_id) -> Optional[dict]: + """Return ``{'source','album_id','score','resolved_at'}`` for an album's + pinned canonical release, or ``None`` when unresolved (#765 Stage 2). + Consumers treat ``None`` as 'fall back to today's behavior'.""" + conn = self._get_connection() + try: + cursor = conn.cursor() + cursor.execute( + "SELECT canonical_source, canonical_album_id, canonical_score, " + "canonical_resolved_at FROM albums WHERE id = ?", + (album_id,), + ) + row = cursor.fetchone() + if not row or not row[0] or not row[1]: + return None + return { + 'source': row[0], + 'album_id': row[1], + 'score': row[2], + 'resolved_at': row[3], + } + except Exception as e: + logger.error("Error reading album canonical for %s: %s", album_id, e) + return None + finally: + conn.close() + def _add_mirrored_playlist_explored_column(self, cursor): """Add explored_at column to mirrored_playlists to persist explore badge.""" try: diff --git a/tests/test_canonical_db.py b/tests/test_canonical_db.py new file mode 100644 index 00000000..4bc0496f --- /dev/null +++ b/tests/test_canonical_db.py @@ -0,0 +1,54 @@ +"""DB persistence for canonical album version (#765 Stage 2).""" + +from __future__ import annotations + +from database.music_database import MusicDatabase + + +def _album(db, album_id="alb_evolve"): + # id columns are TEXT (GUID) post-migration, so insert explicit ids and a + # valid FK rather than relying on integer rowids. + conn = db._get_connection() + cur = conn.cursor() + cur.execute("INSERT INTO artists (id, name) VALUES ('art_id', 'Imagine Dragons')") + cur.execute( + "INSERT INTO albums (id, title, artist_id) VALUES (?, 'Evolve', 'art_id')", + (album_id,), + ) + conn.commit() + conn.close() + return album_id + + +def test_set_then_get_roundtrip(tmp_path): + db = MusicDatabase(str(tmp_path / "m.db")) + album_id = _album(db) + + assert db.get_album_canonical(album_id) is None # unresolved by default + + assert db.set_album_canonical(album_id, "spotify", "sp_evolve_123", 0.97) is True + got = db.get_album_canonical(album_id) + assert got["source"] == "spotify" + assert got["album_id"] == "sp_evolve_123" + assert abs(got["score"] - 0.97) < 1e-6 + assert got["resolved_at"] # timestamp populated + + +def test_get_unresolved_returns_none(tmp_path): + db = MusicDatabase(str(tmp_path / "m.db")) + album_id = _album(db) + assert db.get_album_canonical(album_id) is None + + +def test_set_overwrites_previous(tmp_path): + db = MusicDatabase(str(tmp_path / "m.db")) + album_id = _album(db) + db.set_album_canonical(album_id, "spotify", "old", 0.6) + db.set_album_canonical(album_id, "musicbrainz", "new", 0.95) + got = db.get_album_canonical(album_id) + assert got["source"] == "musicbrainz" and got["album_id"] == "new" + + +def test_set_on_missing_album_returns_false(tmp_path): + db = MusicDatabase(str(tmp_path / "m.db")) + assert db.set_album_canonical(999999, "spotify", "x", 0.9) is False diff --git a/tests/test_canonical_resolver.py b/tests/test_canonical_resolver.py new file mode 100644 index 00000000..2868a373 --- /dev/null +++ b/tests/test_canonical_resolver.py @@ -0,0 +1,108 @@ +"""Tests for resolve_canonical_for_album (#765 Stage 2 — injectable core).""" + +from __future__ import annotations + +from core.metadata.canonical_resolver import resolve_canonical_for_album + +STD = [{"duration_ms": 180_000 + i * 10_000, "title": f"Song {i+1}"} for i in range(11)] +DLX = STD + [{"duration_ms": 300_000 + i * 10_000, "title": f"Bonus {i+1}"} for i in range(6)] + +PRIORITY = ["spotify", "itunes", "deezer", "musicbrainz"] + + +def _fetcher(table): + """fetch_tracklist backed by a {(source, album_id): tracks} table.""" + def fetch(source, album_id): + return table.get((source, album_id)) + return fetch + + +def test_picks_source_whose_release_fits_the_files(): + files = list(STD) # user owns the 11-track standard + table = { + ("spotify", "sp_deluxe"): DLX, # spotify linked to deluxe (17) + ("musicbrainz", "mb_std"): STD, # musicbrainz has standard (11) + } + out = resolve_canonical_for_album( + album_source_ids={"spotify": "sp_deluxe", "musicbrainz": "mb_std"}, + file_tracks=files, + fetch_tracklist=_fetcher(table), + source_priority=PRIORITY, + ) + # Best FIT wins over priority — standard matches the files, deluxe doesn't. + assert out == {"source": "musicbrainz", "album_id": "mb_std", "score": out["score"]} + assert out["score"] > 0.9 + + +def test_priority_breaks_tie_between_equal_fits(): + files = list(STD) + table = {("spotify", "a"): STD, ("itunes", "b"): STD} # identical fit + out = resolve_canonical_for_album( + album_source_ids={"itunes": "b", "spotify": "a"}, + file_tracks=files, + fetch_tracklist=_fetcher(table), + source_priority=PRIORITY, # spotify before itunes + ) + assert out["source"] == "spotify" + + +def test_skips_sources_without_ids_or_failed_fetch(): + files = list(STD) + + def fetch(source, album_id): + if source == "spotify": + raise RuntimeError("API down") + if source == "deezer": + return STD + return None + + out = resolve_canonical_for_album( + album_source_ids={"spotify": "x", "deezer": "y"}, # no itunes id + file_tracks=files, + fetch_tracklist=fetch, + source_priority=PRIORITY, + ) + assert out["source"] == "deezer" + + +def test_none_when_no_candidates(): + out = resolve_canonical_for_album( + album_source_ids={}, + file_tracks=list(STD), + fetch_tracklist=_fetcher({}), + source_priority=PRIORITY, + ) + assert out is None + + +def test_none_when_no_files(): + out = resolve_canonical_for_album( + album_source_ids={"spotify": "a"}, + file_tracks=[], + fetch_tracklist=_fetcher({("spotify", "a"): STD}), + source_priority=PRIORITY, + ) + assert out is None + + +def test_none_when_below_floor(): + files = list(STD) # 11 tracks + # Only candidate is a wildly-wrong 3-track release. + table = {("spotify", "a"): [{"duration_ms": 60_000, "title": "X"}] * 3} + out = resolve_canonical_for_album( + album_source_ids={"spotify": "a"}, + file_tracks=files, + fetch_tracklist=_fetcher(table), + source_priority=PRIORITY, + ) + assert out is None + + +def test_score_is_rounded(): + out = resolve_canonical_for_album( + album_source_ids={"spotify": "a"}, + file_tracks=list(STD), + fetch_tracklist=_fetcher({("spotify", "a"): STD}), + source_priority=PRIORITY, + ) + assert out["score"] == round(out["score"], 4)