#758: a manual album match pins + locks the canonical version

Users manually match an album to the regular edition, but enrichment/
repair keeps treating it as the deluxe (missing songs, renumbered tracks).
Root cause: an album has TWO identities — the enrichment match
(spotify_album_id, which manual-match sets and the worker already honors)
and a SEPARATE canonical version pin (canonical_album_id, added by #777).
The canonical pin is what track-number repair / reorganize / missing-track
detection actually read, and library_manual_match never wrote it — so it
was resolved independently and landed on the deluxe edition.

(So #777 did NOT solve #758: it added canonical pinning, but manual
matches didn't write the pin.)

Fix: a manual ALBUM match on a canonical-recognised source now also pins
AND locks the canonical version to the chosen release:
- new canonical_locked column (same migration pattern as the other
  canonical cols).
- set_album_canonical(..., locked=False) gains an atomic WHERE-clause
  guard: an auto write can't overwrite a locked pin; a manual write
  (locked=True) always wins. get_album_canonical exposes `locked`.
- library_manual_match pins canonical for album matches via the pure
  should_pin_manual_canonical(entity_type, source).

The auto resolve job already skips already-pinned albums, so the lock is
protected on two fronts; the new guard also covers any future
re-resolution. A new manual match still overrides.

18 tests: the pure gate (+ a sync-invariant test vs _ALBUM_ID_COLUMNS)
and the DB lock seam (auto can't clobber a manual lock; manual overrides;
auto-over-auto still works). Additive — locked defaults False, so the
auto path is unchanged unless a manual lock exists. Full suite clean.

core/metadata/canonical_version.py

@@ -207,4 +207,24 @@ def pick_canonical_release(
     return best, best_score
 
 
-__all__ = ["score_release_against_files", "pick_canonical_release"]
+# Album sources the canonical system reads (mirror of
+# core.library_reorganize._ALBUM_ID_COLUMNS — a test pins them in sync). A manual
+# match on any of these should pin/lock the canonical version (#758); a match on
+# a source the canonical tools don't read (e.g. lastfm) has no version to pin.
+CANONICAL_ALBUM_SOURCES = frozenset({'spotify', 'itunes', 'deezer', 'discogs', 'hydrabase'})
+
+
+def should_pin_manual_canonical(entity_type: str, source: str) -> bool:
+    """Whether a manual match should also pin (and lock) the canonical album
+    version (#758). True only for an ALBUM match on a canonical-recognised
+    source — so the user's chosen edition becomes the authority every downstream
+    tool (track-number repair, reorganize, missing-tracks) reads, and the auto
+    resolve job won't override it.
+    """
+    return entity_type == 'album' and source in CANONICAL_ALBUM_SOURCES
+
+
+__all__ = [
+    "score_release_against_files", "pick_canonical_release",
+    "CANONICAL_ALBUM_SOURCES", "should_pin_manual_canonical",
+]

database/music_database.py

@@ -1019,6 +1019,10 @@ class MusicDatabase:
                 'canonical_album_id': 'TEXT DEFAULT NULL',
                 'canonical_score': 'REAL DEFAULT NULL',
                 'canonical_resolved_at': 'TIMESTAMP DEFAULT NULL',
+                # #758 — set when the user MANUALLY pins an album version. The
+                # auto resolve job (and any re-resolution) must never overwrite
+                # a locked pin, so a manual match stays put across cycles.
+                'canonical_locked': 'INTEGER DEFAULT 0',
             }
             for _col, _typedef in _canonical_cols.items():
                 if album_cols and _col not in album_cols:
@@ -1027,17 +1031,27 @@ 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:
+    def set_album_canonical(self, album_id, source: str, canonical_album_id: str,
+                            score: float, locked: bool = False) -> bool:
         """Persist the resolved canonical (source, album_id, score) for an album
-        (#765 Stage 2). Returns True if a row was updated."""
+        (#765 Stage 2). Returns True if a row was updated.
+
+        ``locked=True`` marks a MANUAL pin (#758): the user explicitly chose this
+        album version. A manual write always wins (overwrites any existing pin).
+        An AUTO write (``locked=False``, the resolve job) will NOT overwrite a
+        locked pin — the guard is in the WHERE clause so it's atomic.
+        """
         conn = self._get_connection()
         try:
             cursor = conn.cursor()
+            # Auto writes can't clobber a manual lock; manual writes always apply.
+            guard = "" if locked else " AND (canonical_locked IS NULL OR canonical_locked = 0)"
             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),
+                "canonical_score = ?, canonical_locked = ?, "
+                "canonical_resolved_at = CURRENT_TIMESTAMP "
+                f"WHERE id = ?{guard}",
+                (source, str(canonical_album_id), float(score), 1 if locked else 0, album_id),
             )
             conn.commit()
             return cursor.rowcount > 0
@@ -1048,15 +1062,16 @@ class MusicDatabase:
             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'."""
+        """Return ``{'source','album_id','score','resolved_at','locked'}`` for an
+        album's pinned canonical release, or ``None`` when unresolved (#765 Stage
+        2). ``locked`` is True for a manual pin (#758). 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 = ?",
+                "canonical_resolved_at, canonical_locked FROM albums WHERE id = ?",
                 (album_id,),
             )
             row = cursor.fetchone()
@@ -1067,6 +1082,7 @@ class MusicDatabase:
                 'album_id': row[1],
                 'score': row[2],
                 'resolved_at': row[3],
+                'locked': bool(row[4]),
             }
         except Exception as e:
             logger.error("Error reading album canonical for %s: %s", album_id, e)

tests/test_canonical_manual_lock.py

@@ -0,0 +1,99 @@
+"""#758 — a manual album match pins (and LOCKS) the canonical album version, so
+re-resolution / the auto canonical job can't drag it back to the deluxe edition.
+
+Two seams:
+  - should_pin_manual_canonical (pure): when a manual match should pin canonical.
+  - set_album_canonical / get_album_canonical (DB): the lock can't be overwritten
+    by an auto write, but a new manual write still wins.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+
+from core.metadata.canonical_version import (
+    CANONICAL_ALBUM_SOURCES,
+    should_pin_manual_canonical,
+)
+from database.music_database import MusicDatabase
+
+
+# ---------------------------------------------------------------------------
+# should_pin_manual_canonical — pure
+# ---------------------------------------------------------------------------
+
+@pytest.mark.parametrize('source', ['spotify', 'itunes', 'deezer', 'discogs', 'hydrabase'])
+def test_pins_album_on_recognised_source(source):
+    assert should_pin_manual_canonical('album', source) is True
+
+
+@pytest.mark.parametrize('entity', ['artist', 'track'])
+def test_does_not_pin_non_album(entity):
+    assert should_pin_manual_canonical(entity, 'spotify') is False
+
+
+@pytest.mark.parametrize('source', ['lastfm', 'genius', 'musicbrainz', 'audiodb', 'tidal'])
+def test_does_not_pin_source_canonical_cant_read(source):
+    # No album-version data the canonical tools read → nothing to pin.
+    assert should_pin_manual_canonical('album', source) is False
+
+
+def test_sources_stay_in_sync_with_album_id_columns():
+    # The set must mirror the canonical reader's column map; if a source is
+    # added there, this fails until CANONICAL_ALBUM_SOURCES is updated.
+    from core.library_reorganize import _ALBUM_ID_COLUMNS
+    assert CANONICAL_ALBUM_SOURCES == set(_ALBUM_ID_COLUMNS)
+
+
+# ---------------------------------------------------------------------------
+# set_album_canonical / get_album_canonical — the lock (DB)
+# ---------------------------------------------------------------------------
+
+def _insert_album(db, album_id):
+    conn = db._get_connection()
+    conn.execute("INSERT OR IGNORE INTO artists (id, name) VALUES ('ar1', 'Artist')")
+    conn.execute("INSERT INTO albums (id, artist_id, title) VALUES (?, 'ar1', 'Album')", (album_id,))
+    conn.commit()
+    conn.close()
+
+
+@pytest.fixture
+def db(tmp_path: Path) -> MusicDatabase:
+    d = MusicDatabase(database_path=str(tmp_path / "ml.db"))
+    _insert_album(d, 'al1')
+    return d
+
+
+def test_manual_lock_set_and_read(db):
+    assert db.set_album_canonical('al1', 'spotify', 'REG', 1.0, locked=True) is True
+    c = db.get_album_canonical('al1')
+    assert c['source'] == 'spotify' and c['album_id'] == 'REG' and c['locked'] is True
+
+
+def test_auto_cannot_overwrite_manual_lock(db):
+    db.set_album_canonical('al1', 'spotify', 'REG', 1.0, locked=True)
+    # The auto resolve job tries to re-pin the deluxe — must be refused.
+    assert db.set_album_canonical('al1', 'spotify', 'DELUXE', 0.9, locked=False) is False
+    c = db.get_album_canonical('al1')
+    assert c['album_id'] == 'REG' and c['locked'] is True  # unchanged
+
+
+def test_new_manual_match_overrides_existing_pin(db):
+    db.set_album_canonical('al1', 'spotify', 'OLD', 0.8, locked=False)  # auto pin
+    # User manually picks a different edition — manual always wins.
+    assert db.set_album_canonical('al1', 'itunes', 'NEW', 1.0, locked=True) is True
+    c = db.get_album_canonical('al1')
+    assert c['source'] == 'itunes' and c['album_id'] == 'NEW' and c['locked'] is True
+
+
+def test_auto_overwrites_auto(db):
+    db.set_album_canonical('al1', 'spotify', 'A', 0.8, locked=False)
+    assert db.set_album_canonical('al1', 'spotify', 'B', 0.9, locked=False) is True
+    assert db.get_album_canonical('al1')['album_id'] == 'B'
+
+
+def test_unresolved_album_returns_none(db):
+    _insert_album(db, 'al2')
+    assert db.get_album_canonical('al2') is None

web_server.py

@@ -11316,6 +11316,18 @@ def library_manual_match():
             if cursor.rowcount == 0:
                 return jsonify({"success": False, "error": "Entity not found"}), 404
 
+        # #758 — a manual ALBUM match also pins (and LOCKS) the canonical album
+        # version to the chosen release, so the auto resolve job and every tool
+        # that reads the canonical pin (track-number repair, reorganize,
+        # missing-tracks) honor the user's edition instead of re-resolving back
+        # to the deluxe. The lock survives future enrichment/resolution cycles.
+        try:
+            from core.metadata.canonical_version import should_pin_manual_canonical
+            if should_pin_manual_canonical(entity_type, service):
+                database.set_album_canonical(entity_id, service, service_id, 1.0, locked=True)
+        except Exception as e:
+            logger.warning("Manual canonical pin failed for album %s: %s", entity_id, e)
+
         # Re-fetch fresh data
         artist_id = data.get('artist_id', entity_id)
         if entity_type != 'artist':