Canonical album version — Stage 2 (core): resolver + persistence (dormant)
Turns the Stage-1 scorer into an end-to-end resolver + persists the result.
Still DORMANT — no consumer reads it yet, so zero behavior change.
- core/metadata/canonical_resolver.py — resolve_canonical_for_album(): builds
candidate releases from the album's per-source IDs (in source-priority order),
fetches each tracklist via an INJECTED fetch_tracklist (so it's unit-testable
without live APIs), scores them with pick_canonical_release, and returns the
best-fit {source, album_id, score}. Skips sources with no id / failed fetch;
returns None when there are no files, no candidates, or nothing clears the
confidence floor.
- database/music_database.py — set_album_canonical() / get_album_canonical()
write/read the Stage-1 columns. get returns None when unresolved, which every
consumer will treat as "fall back to today's behavior".
Tests: tests/test_canonical_resolver.py (7) — best-fit beats priority, priority
breaks true ties, skips missing-id/failed-fetch sources, None on
no-candidates/no-files/below-floor, score rounding. tests/test_canonical_db.py
(4) — set/get round-trip incl. timestamp, unresolved -> None, overwrite,
missing-album -> False. 34 canonical + DB-migration tests pass.
Remaining for Stage 2 (the trigger): read on-disk file durations/titles for an
album, gather its source IDs, call the resolver, store — wired via a backfill
repair job + an enrichment hook. Then Stages 3-4 wire the Reorganizer and Track
Number Repair to READ the pinned canonical.
This commit is contained in:
parent
818c4f0bff
commit
f37bc34082
4 changed files with 285 additions and 0 deletions
76
core/metadata/canonical_resolver.py
Normal file
76
core/metadata/canonical_resolver.py
Normal file
|
|
@ -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"]
|
||||||
|
|
@ -974,6 +974,53 @@ class MusicDatabase:
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error("Error repairing core media schema columns: %s", 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):
|
def _add_mirrored_playlist_explored_column(self, cursor):
|
||||||
"""Add explored_at column to mirrored_playlists to persist explore badge."""
|
"""Add explored_at column to mirrored_playlists to persist explore badge."""
|
||||||
try:
|
try:
|
||||||
|
|
|
||||||
54
tests/test_canonical_db.py
Normal file
54
tests/test_canonical_db.py
Normal file
|
|
@ -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
|
||||||
108
tests/test_canonical_resolver.py
Normal file
108
tests/test_canonical_resolver.py
Normal file
|
|
@ -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)
|
||||||
Loading…
Reference in a new issue