From efe3895d5d8de8f7c7df769f553c67adc85b88f7 Mon Sep 17 00:00:00 2001
From: BoulderBadgeDad <batmanesignal@gmail.com>
Date: Sun, 31 May 2026 23:26:49 -0700
Subject: [PATCH] Fix: metadata cache tables silently missing after DB recovery
 (stale migration marker)

Nothing was landing in the metadata cache browser because the
metadata_cache_entities / metadata_cache_searches tables did not exist, so every
cache write no-op-ed. Root cause: _add_metadata_cache_tables short-circuited on a
marker-only guard (if the metadata_cache_v1 marker row exists, return). After a
DB corruption-recovery the small metadata table (with the marker) survived but
the large cache tables did not, so the stale marker permanently blocked the
idempotent CREATE TABLE IF NOT EXISTS and the cache was dead forever.

Guard now skips only when the marker is set AND the tables actually exist, so a
stale marker self-heals: the tables are re-created on the next init.

Tests: marker present but tables dropped -> re-created; marker + tables present
-> no-op (idempotent).
---
 database/music_database.py                  | 12 ++++-
 tests/test_metadata_cache_table_recreate.py | 54 +++++++++++++++++++++
 2 files changed, 64 insertions(+), 2 deletions(-)
 create mode 100644 tests/test_metadata_cache_table_recreate.py

diff --git a/database/music_database.py b/database/music_database.py
index 2855f4ed..85beef86 100644
--- a/database/music_database.py
+++ b/database/music_database.py
@@ -4267,9 +4267,17 @@ class MusicDatabase:
     def _add_metadata_cache_tables(self, cursor):
         """Create metadata_cache_entities and metadata_cache_searches tables for universal API response caching"""
         try:
+            # Skip only when the marker is set AND the tables actually exist.
+            # A marker-only guard is fragile: if the `metadata` table survives a
+            # corruption-recovery but the (large) cache tables don't, the stale
+            # marker would permanently short-circuit creation and the metadata
+            # cache would silently never work again (nothing lands in the
+            # browser). Verifying the table presence makes this self-heal.
+            cursor.execute("SELECT name FROM sqlite_master WHERE type='table' AND name='metadata_cache_entities'")
+            tables_present = cursor.fetchone() is not None
             cursor.execute("SELECT value FROM metadata WHERE key = 'metadata_cache_v1' LIMIT 1")
-            if cursor.fetchone():
-                return  # Already migrated
+            if cursor.fetchone() and tables_present:
+                return  # Already migrated and tables present
 
             logger.info("Creating metadata cache tables...")
 
diff --git a/tests/test_metadata_cache_table_recreate.py b/tests/test_metadata_cache_table_recreate.py
new file mode 100644
index 00000000..bb153b9b
--- /dev/null
+++ b/tests/test_metadata_cache_table_recreate.py
@@ -0,0 +1,54 @@
+"""Regression: metadata cache tables must self-heal when the migration marker
+is stale.
+
+After a DB corruption-recovery the `metadata` table (with the
+'metadata_cache_v1' marker) can survive while the large
+metadata_cache_entities/searches tables do not. A marker-only guard then
+permanently skips re-creating them, so the cache silently stops working and the
+browser shows nothing. _add_metadata_cache_tables must re-create the tables when
+the marker is present but the tables are gone.
+"""
+
+from __future__ import annotations
+
+from database.music_database import MusicDatabase
+
+
+def _tables(cur):
+    return {r[0] for r in cur.execute(
+        "SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'metadata_cache_%'"
+    ).fetchall()}
+
+
+def test_recreates_cache_tables_when_marker_stale(tmp_path):
+    db = MusicDatabase(str(tmp_path / "m.db"))
+    conn = db._get_connection()
+    cur = conn.cursor()
+
+    # Fresh DB: tables + marker present.
+    assert 'metadata_cache_entities' in _tables(cur)
+    assert cur.execute("SELECT value FROM metadata WHERE key='metadata_cache_v1'").fetchone()
+
+    # Simulate corruption-recovery: marker survives, cache tables don't.
+    cur.execute("DROP TABLE metadata_cache_entities")
+    cur.execute("DROP TABLE metadata_cache_searches")
+    conn.commit()
+    assert 'metadata_cache_entities' not in _tables(cur)
+    assert cur.execute("SELECT value FROM metadata WHERE key='metadata_cache_v1'").fetchone()  # stale marker
+
+    # Re-run the migration — must self-heal despite the stale marker.
+    db._add_metadata_cache_tables(cur)
+    conn.commit()
+    assert 'metadata_cache_entities' in _tables(cur)
+    assert 'metadata_cache_searches' in _tables(cur)
+
+
+def test_skips_when_marker_and_tables_both_present(tmp_path):
+    # Idempotent: a healthy DB shouldn't error on re-run.
+    db = MusicDatabase(str(tmp_path / "m2.db"))
+    conn = db._get_connection()
+    cur = conn.cursor()
+    before = _tables(cur)
+    db._add_metadata_cache_tables(cur)  # no-op fast path
+    conn.commit()
+    assert _tables(cur) == before