From a42cf3b8657988b1865dce38142f40f6681415ac Mon Sep 17 00:00:00 2001 From: dev Date: Sat, 27 Jun 2026 19:22:36 +0200 Subject: [PATCH] downloads(#934): instant startup reconcile of stuck-'unverified' history from tracks truth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complements the AcoustID-scan-time heal: re-links library_history rows still showing 'unverified' to the verified/human_verified status their file already carries in the tracks table — matching exact path AND basename, so a file that moved (media-server import / reorganize) heals even though the stored history path is frozen. Upgrade-only and non-destructive (no deletes, no bulk migration). Why this is needed on top of the scan-time fix: - It clears the EXISTING backlog (e.g. 5551 rows) on the next restart with NO re-fingerprinting and no AcoustID API calls — the file's status is already in tracks from the prior scan; this just propagates it to the frozen history row. - It covers human_verified files, which the AcoustID scan skips entirely (file_verif_status == 'human_verified' returns early), so their stale history rows would otherwise never heal. Runs once on DB init (cheap, idempotent). 5 real-sqlite tests. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LWJk7EuM7YktQeNyqQwTZY --- database/music_database.py | 57 +++++++++ tests/test_unverified_history_reconcile.py | 140 +++++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 tests/test_unverified_history_reconcile.py diff --git a/database/music_database.py b/database/music_database.py index dd37aee3..07a85687 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -1000,6 +1000,10 @@ class MusicDatabase: self._init_manual_library_match_table() self._backfill_mirrored_track_source_ids() + # Self-heal the Unverified review queue: lift history rows stuck at + # 'unverified' whose file has since been verified (issue #934). Cheap, + # idempotent (only touches rows that need it), so it's safe every boot. + self.reconcile_unverified_history_from_tracks() def _backfill_mirrored_track_source_ids(self) -> int: """One-time, idempotent: assign a stable source_track_id to mirrored tracks @@ -13857,6 +13861,59 @@ class MusicDatabase: logger.error("Error querying unverified library history: %s", e) return [] + def reconcile_unverified_history_from_tracks(self) -> int: + """Heal library_history rows stuck at 'unverified' whose underlying file + has since been confirmed in the tracks table (AcoustID scan PASS or a + human decision). Matches by exact path AND basename — the same physical + file keeps its filename across path-form differences (relative vs + absolute, library moved/reorganized, different mount), which is why an + exact-path-only heal left thousands of already-verified files showing as + Unverified (issue #934). + + Upgrade-only and non-destructive: it only lifts 'unverified' rows to the + confirmed status, never downgrades and never deletes. Returns the number + of rows healed. Genuinely-unverified rows and orphans (no matching + track) are left untouched. + """ + healed = 0 + try: + conn = self._get_connection() + cursor = conn.cursor() + rank = {'verified': 1, 'human_verified': 2} + by_path = {} + by_base = {} + cursor.execute( + "SELECT file_path, verification_status FROM tracks " + "WHERE verification_status IN ('verified', 'human_verified') " + "AND file_path IS NOT NULL AND file_path != ''") + for fp, st in cursor.fetchall(): + if not fp: + continue + by_path[fp] = st + base = os.path.basename(fp) + if base and rank.get(st, 0) >= rank.get(by_base.get(base), 0): + by_base[base] = st + updates = [] + cursor.execute( + "SELECT id, file_path FROM library_history " + "WHERE verification_status = 'unverified' " + "AND file_path IS NOT NULL AND file_path != ''") + for rid, fp in cursor.fetchall(): + target = by_path.get(fp) or by_base.get(os.path.basename(fp or '')) + if target: + updates.append((target, rid)) + for status, rid in updates: + cursor.execute( + "UPDATE library_history SET verification_status = ? WHERE id = ?", + (status, rid)) + healed += 1 + if healed: + conn.commit() + logger.info("Reconciled %d unverified history rows from tracks truth", healed) + except Exception as e: + logger.error("Error reconciling unverified history: %s", e) + return healed + def get_library_history_stats(self): """Return counts per event_type and per download_source.""" try: diff --git a/tests/test_unverified_history_reconcile.py b/tests/test_unverified_history_reconcile.py new file mode 100644 index 00000000..e614b4d3 --- /dev/null +++ b/tests/test_unverified_history_reconcile.py @@ -0,0 +1,140 @@ +"""Issue #934 — one-time reconcile that clears the existing backlog of +``library_history`` rows stuck at 'unverified' even though the file has since +been verified (by an AcoustID scan, or human-confirmed). Heals from the +``tracks`` truth, matching exact path AND basename (so a reorganized/moved file +heals too), upgrade-only. Never deletes anything.""" + +import sqlite3 +import sys +import types + +if "spotipy" not in sys.modules: # match the suite's lightweight stubs + spotipy = types.ModuleType("spotipy") + spotipy.Spotify = object + oauth2 = types.ModuleType("spotipy.oauth2") + oauth2.SpotifyOAuth = object + oauth2.SpotifyClientCredentials = object + spotipy.oauth2 = oauth2 + sys.modules["spotipy"] = spotipy + sys.modules["spotipy.oauth2"] = oauth2 + +if "config.settings" not in sys.modules: + config_pkg = types.ModuleType("config") + settings_mod = types.ModuleType("config.settings") + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + def get_active_media_server(self): + return "primary" + + settings_mod.config_manager = _DummyConfigManager() + config_pkg.settings = settings_mod + sys.modules["config"] = config_pkg + sys.modules["config.settings"] = settings_mod + +from database.music_database import MusicDatabase # noqa: E402 + + +class _NonClosingConn: + def __init__(self, real): + self._real = real + + def cursor(self): + return self._real.cursor() + + def commit(self): + return self._real.commit() + + def close(self): + pass + + def __enter__(self): + return self + + def __exit__(self, *args): + pass + + +class _InMemoryDB(MusicDatabase): + def __init__(self): + self._conn = sqlite3.connect(":memory:") + self._conn.row_factory = sqlite3.Row + self._conn.execute( + "CREATE TABLE tracks (id INTEGER PRIMARY KEY, file_path TEXT, " + "verification_status TEXT)" + ) + self._conn.execute( + "CREATE TABLE library_history (" + "id INTEGER PRIMARY KEY AUTOINCREMENT, event_type TEXT, title TEXT, " + "artist_name TEXT, album_name TEXT, file_path TEXT, " + "download_source TEXT, verification_status TEXT, " + "created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP)" + ) + + def _get_connection(self): + return _NonClosingConn(self._conn) + + def _add_track(self, tid, path, status): + self._conn.execute( + "INSERT INTO tracks (id, file_path, verification_status) VALUES (?,?,?)", + (tid, path, status)) + self._conn.commit() + + def _add_history(self, path, status, title="Song"): + self._conn.execute( + "INSERT INTO library_history (event_type, title, file_path, " + "verification_status) VALUES ('download', ?, ?, ?)", + (title, path, status)) + self._conn.commit() + + def _status_of(self, hid): + return self._conn.execute( + "SELECT verification_status FROM library_history WHERE id = ?", (hid,) + ).fetchone()[0] + + +def test_reconcile_heals_exact_path_match(): + db = _InMemoryDB() + db._add_track(1, "/lib/A/01 - Song.flac", "verified") + db._add_history("/lib/A/01 - Song.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 1 + assert db._status_of(1) == "verified" + + +def test_reconcile_heals_by_basename_when_path_form_differs(): + db = _InMemoryDB() + db._add_track(1, "/library/Artist/Album/01 - Song.flac", "verified") + # History stored the transfer-folder path; basename still matches. + db._add_history("/transfer/Artist - Album/01 - Song.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 1 + assert db._status_of(1) == "verified" + + +def test_reconcile_propagates_human_verified(): + db = _InMemoryDB() + db._add_track(1, "/lib/01 - Song.flac", "human_verified") + db._add_history("/lib/01 - Song.flac", "unverified") + db.reconcile_unverified_history_from_tracks() + assert db._status_of(1) == "human_verified" + + +def test_reconcile_leaves_genuinely_unverified_rows(): + db = _InMemoryDB() + db._add_track(1, "/lib/01 - Song.flac", "unverified") # track itself unconfirmed + db._add_history("/lib/01 - Song.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 0 + assert db._status_of(1) == "unverified" + + +def test_reconcile_leaves_orphans_untouched(): + db = _InMemoryDB() + # No track references this file at all (deleted / re-downloaded elsewhere). + db._add_history("/lib/gone.flac", "unverified") + healed = db.reconcile_unverified_history_from_tracks() + assert healed == 0 + assert db._status_of(1) == "unverified"