Merge pull request #509 from Nezreka/fix/hifi-instances-table-missing

Fix "no such table: hifi_instances" via defensive lazy-create

database/music_database.py

@@ -11996,10 +11996,33 @@ class MusicDatabase:
 
     # ===================== HiFi Instances =====================
 
+    def _ensure_hifi_instances_table(self, cursor) -> None:
+        """Defensive lazy-create. Issue #503: some users hit a "no such
+        table: hifi_instances" error when adding a HiFi instance even
+        though ``_initialize_database`` runs ``CREATE TABLE IF NOT EXISTS``
+        on every boot. Root cause: the bulk init runs every CREATE +
+        every migration inside one transaction, so if any later migration
+        step throws on the user's specific DB shape, the whole batch
+        rolls back (Python's sqlite3 module doesn't autocommit DDL by
+        default) and ``hifi_instances`` never lands. This helper ensures
+        the table exists immediately before every operation that touches
+        it — idempotent, costs one PRAGMA-level no-op when the table is
+        already present, and fully recovers from a broken init."""
+        cursor.execute("""
+            CREATE TABLE IF NOT EXISTS hifi_instances (
+                id INTEGER PRIMARY KEY AUTOINCREMENT,
+                url TEXT NOT NULL UNIQUE,
+                priority INTEGER NOT NULL DEFAULT 0,
+                enabled INTEGER NOT NULL DEFAULT 1,
+                created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
+            )
+        """)
+
     def get_hifi_instances(self) -> List[Dict[str, Any]]:
         """Get all enabled HiFi instances ordered by priority."""
         conn = self._get_connection()
         cursor = conn.cursor()
+        self._ensure_hifi_instances_table(cursor)
         cursor.execute("SELECT url, priority, enabled FROM hifi_instances WHERE enabled = 1 ORDER BY priority ASC, id ASC")
         return [dict(row) for row in cursor.fetchall()]
 
@@ -12007,6 +12030,7 @@ class MusicDatabase:
         """Get all HiFi instances (including disabled) ordered by priority."""
         conn = self._get_connection()
         cursor = conn.cursor()
+        self._ensure_hifi_instances_table(cursor)
         cursor.execute("SELECT url, priority, enabled FROM hifi_instances ORDER BY priority ASC, id ASC")
         return [dict(row) for row in cursor.fetchall()]
 
@@ -12014,6 +12038,7 @@ class MusicDatabase:
         """Add a new HiFi instance."""
         conn = self._get_connection()
         cursor = conn.cursor()
+        self._ensure_hifi_instances_table(cursor)
         cursor.execute(
             "INSERT OR IGNORE INTO hifi_instances (url, priority, enabled) VALUES (?, ?, 1)",
             (url, priority)
@@ -12025,6 +12050,7 @@ class MusicDatabase:
         """Remove a HiFi instance."""
         conn = self._get_connection()
         cursor = conn.cursor()
+        self._ensure_hifi_instances_table(cursor)
         cursor.execute("DELETE FROM hifi_instances WHERE url = ?", (url,))
         conn.commit()
         return cursor.rowcount > 0
@@ -12033,6 +12059,7 @@ class MusicDatabase:
         """Enable or disable a HiFi instance."""
         conn = self._get_connection()
         cursor = conn.cursor()
+        self._ensure_hifi_instances_table(cursor)
         cursor.execute("UPDATE hifi_instances SET enabled = ? WHERE url = ?", (1 if enabled else 0, url))
         conn.commit()
         return cursor.rowcount > 0
@@ -12045,6 +12072,7 @@ class MusicDatabase:
             return True
         conn = self._get_connection()
         cursor = conn.cursor()
+        self._ensure_hifi_instances_table(cursor)
         placeholders = ",".join("?" for _ in urls)
         cursor.execute(
             f"SELECT url FROM hifi_instances WHERE url IN ({placeholders})",
@@ -12063,6 +12091,7 @@ class MusicDatabase:
         """Insert default instances if the table is empty."""
         conn = self._get_connection()
         cursor = conn.cursor()
+        self._ensure_hifi_instances_table(cursor)
         cursor.execute("SELECT COUNT(*) as cnt FROM hifi_instances")
         count = cursor.fetchone()['cnt']
         if count == 0:

tests/test_hifi_instance_methods.py

@@ -306,13 +306,17 @@ def test_seed_hifi_instances_does_not_duplicate_on_reseed(db):
     assert len(rows) == 1
 
 
-# ── error propagation ────────────────────────────────────────────────────
-# These methods now let DB errors bubble up so the route layer turns them
-# into a 500 — the user sees a real failure instead of a phantom empty state.
+# ── lazy-create recovery (issue #503) ────────────────────────────────────
+# When the bulk ``_initialize_database`` rolls back due to a brittle
+# migration step, ``hifi_instances`` would normally be missing on the
+# user's DB. The CRUD methods now ensure the table exists immediately
+# before operating, so the operation succeeds rather than throwing
+# "no such table: hifi_instances".
 
 
 def _db_without_hifi_table():
-    """Returns a MusicDatabase with NO hifi_instances table."""
+    """Returns a MusicDatabase with NO hifi_instances table — simulates
+    the broken-init state where ``_initialize_database`` rolled back."""
     conn = sqlite3.connect(":memory:")
     conn.row_factory = sqlite3.Row
 
@@ -326,43 +330,78 @@ def _db_without_hifi_table():
     return _NoTableDB()
 
 
-def test_get_hifi_instances_propagates_db_errors():
+def _hifi_table_exists(db):
+    cur = db._conn.cursor()
+    cur.execute("SELECT name FROM sqlite_master WHERE type='table' AND name='hifi_instances'")
+    return cur.fetchone() is not None
+
+
+def test_add_hifi_instance_creates_table_when_missing():
+    """Pin issue #503 fix: add_hifi_instance must lazily create the
+    table when init failed to land it. Pre-fix this raised
+    ``no such table: hifi_instances`` and the user couldn't add any
+    HiFi instances at all."""
     db = _db_without_hifi_table()
-    with pytest.raises(sqlite3.OperationalError):
-        db.get_hifi_instances()
+    assert not _hifi_table_exists(db)
+
+    result = db.add_hifi_instance("http://recovery.com", priority=0)
+
+    assert result is True
+    assert _hifi_table_exists(db)
+    rows = db.get_all_hifi_instances()
+    assert len(rows) == 1
+    assert rows[0]["url"] == "http://recovery.com"
 
 
-def test_get_all_hifi_instances_propagates_db_errors():
+def test_get_hifi_instances_creates_table_when_missing():
+    """Empty result instead of OperationalError — read methods
+    self-heal too."""
     db = _db_without_hifi_table()
-    with pytest.raises(sqlite3.OperationalError):
-        db.get_all_hifi_instances()
+    assert not _hifi_table_exists(db)
+
+    rows = db.get_hifi_instances()
+
+    assert rows == []
+    assert _hifi_table_exists(db)
 
 
-def test_add_hifi_instance_propagates_db_errors():
+def test_get_all_hifi_instances_creates_table_when_missing():
     db = _db_without_hifi_table()
-    with pytest.raises(sqlite3.OperationalError):
-        db.add_hifi_instance("http://x.com")
+    rows = db.get_all_hifi_instances()
+    assert rows == []
+    assert _hifi_table_exists(db)
 
 
-def test_remove_hifi_instance_propagates_db_errors():
+def test_remove_hifi_instance_creates_table_when_missing():
+    """Removing from a missing table is a no-op (returns False) —
+    doesn't crash."""
     db = _db_without_hifi_table()
-    with pytest.raises(sqlite3.OperationalError):
-        db.remove_hifi_instance("http://x.com")
+    result = db.remove_hifi_instance("http://x.com")
+    assert result is False
+    assert _hifi_table_exists(db)
 
 
-def test_toggle_hifi_instance_propagates_db_errors():
+def test_toggle_hifi_instance_creates_table_when_missing():
     db = _db_without_hifi_table()
-    with pytest.raises(sqlite3.OperationalError):
-        db.toggle_hifi_instance("http://x.com", enabled=True)
+    result = db.toggle_hifi_instance("http://x.com", enabled=True)
+    assert result is False  # Nothing to toggle
+    assert _hifi_table_exists(db)
 
 
-def test_reorder_hifi_instances_propagates_db_errors():
+def test_reorder_hifi_instances_creates_table_when_missing():
+    """Reorder with missing entries returns False (the existing
+    contract — caller-supplied URLs not present)."""
     db = _db_without_hifi_table()
-    with pytest.raises(sqlite3.OperationalError):
-        db.reorder_hifi_instances(["http://x.com"])
+    result = db.reorder_hifi_instances(["http://x.com"])
+    assert result is False  # No matching rows
+    assert _hifi_table_exists(db)
 
 
-def test_seed_hifi_instances_propagates_db_errors():
+def test_seed_hifi_instances_creates_table_when_missing():
+    """Seeding into a missing table works — table is created, then
+    defaults inserted."""
     db = _db_without_hifi_table()
-    with pytest.raises(sqlite3.OperationalError):
-        db.seed_hifi_instances(["http://x.com"])
+    db.seed_hifi_instances(["http://default-a.com", "http://default-b.com"])
+    rows = db.get_all_hifi_instances()
+    assert len(rows) == 2
+    assert _hifi_table_exists(db)

webui/static/helper.js

@@ -3432,6 +3432,7 @@ const WHATS_NEW = {
     '2.4.2': [
         // --- post-2.4.1 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
         { date: 'Unreleased — 2.4.2 dev cycle' },
+        { title: 'Fix: "no such table: hifi_instances" When Adding HiFi Instance', desc: 'github issue #503 (hadshaw21): adding a hifi instance via downloader settings popped up `no such table: hifi_instances` even though the connection test and "check all instances" both worked. root cause: `_initialize_database` runs every CREATE TABLE + every migration step inside one sqlite transaction. python\'s sqlite3 module doesn\'t autocommit DDL by default, so if any later migration step throws on a user\'s specific DB shape (e.g. an old volume from a prior soulsync version with quirky schema state), the WHOLE batch rolls back — including the hifi_instances CREATE that ran successfully. user\'s next boot retries init, hits the same migration failure, rolls back again. table never lands. fix: defensive lazy-create. every hifi_instances CRUD method now runs `CREATE TABLE IF NOT EXISTS hifi_instances (...)` immediately before its operation. idempotent — costs one PRAGMA-level no-op when the table is already present, fully recovers from a broken init. read methods (`get_hifi_instances`, `get_all_hifi_instances`) now return empty instead of raising when init failed. write methods (`add`, `remove`, `toggle`, `reorder`, `seed`) work end-to-end. doesn\'t paper over the underlying init issue (still worth tracking down which migration breaks for which users) but makes hifi instance management self-healing. 7 new tests pin the lazy-create behavior — every method works against a DB that\'s missing the table.', page: 'settings' },
         { title: 'Plex: "All Libraries (Combined)" Mode', desc: 'github issue #505 (popebruhlxix): users with multiple plex music libraries (e.g. one per plex home user) only saw one library inside soulsync because the connection settings forced you to pick a single library section. now there\'s a new "all libraries (combined)" option in settings → connections → plex → music library dropdown. picking it flips the plex client into a server-wide read mode where every read method (`get_all_artists` / `get_all_album_ids` / `search_tracks` / `get_library_stats` / etc) dispatches through `server.library.search(libtype=...)` instead of querying a single section. one api call, plex handles the aggregation. cross-section dedup applied at the listing layer — same-name artists across sections collapse to a canonical entry (the one with more tracks), so plex home families with overlapping music tastes don\'t see "drake" twice. removal-detection id enumeration stays raw on purpose — deduping there would falsely prune tracks linked to non-canonical ratingKeys. write methods (genre / poster / metadata updates) are unaffected and operate on plex objects via ratingKey directly — write-back targets one section\'s copy of an artist if it exists in multiple, document and revisit if it matters. trigger_library_scan + is_library_scanning fan out across every music section in the new mode. backward compatible — existing users with a real library name saved see no behavior change. the "all libraries" option only appears in the dropdown when more than one music library exists on the server. 29 new tests pin both modes (single-section preserved, all-libraries dispatches through server-wide search, dedup keeps canonical, id enumeration stays raw).', page: 'settings' },
         { title: 'Fix: Download Discography Showed Wrong Artist\'s Albums', desc: 'clicked download discography on 50 cent → modal showed young hot rod\'s albums. clicked weird al → modal showed the beatles. cause: the endpoint received whichever single artist id the frontend happened to pick (spotify or itunes or deezer or library db id) and dispatched it as-is to whichever source it queried. when the picked id didn\'t match the queried source\'s id format, lookup either returned wrong-artist results (numeric collisions — db id 194687 was a real deezer artist for someone else) or fell back to a fuzzy name search that picked a wrong artist. the per-source id dispatch mechanism (`MetadataLookupOptions.artist_source_ids`) already existed and the watchlist scanner already used it; the on-demand discography endpoint just wasn\'t wired to it. fixed: when the url artist_id matches a library row by ANY stored id (db id, spotify_artist_id, itunes_artist_id, deezer_id, musicbrainz_id), backend pulls every stored provider id and dispatches the right id to each source. each source gets its OWN stored id regardless of what the url carries. when the url id is a non-library source-native id and the row lookup misses entirely, behavior is identical to before (single-id fallback). also fixed two log-namespace bugs: enhance quality and multi-source search were writing through `getLogger(__name__)` which resolves to `core.artists.quality` / `core.metadata.multi_source_search` — neither under the soulsync handler — so every diagnostic line was silently dropped. switched both to `get_logger()` from utils.logging_config so they actually land in app.log.', page: 'library' },
         { title: 'Enhance Quality: Direct ID Lookup Like Download Discography', desc: 'two related fixes. (1) discord report: enhance quality on an artist with neither spotify nor deezer connected added tracks as "unknown artist - unknown album - unknown track". enhance was running a single-source itunes fallback chain that returned junk matches with empty fields, while track redownload had been doing parallel multi-source search the whole time. extracted that search into `core/metadata/multi_source_search.py` and pointed both flows at it. (2) followup: enhance was still using fuzzy text search even when the library track had a stored source ID (spotify_track_id / deezer_id / itunes_track_id / soul_id) on the row, which meant tracks with messy tags ("Title (Live)", featured artists in the artist field, etc.) failed to match even though a perfect ID was sitting right there. download discography never had this problem because it resolves albums by stable ID, not by name. enhance now does the same: for every source you have configured, if the track has a stored ID for that source, it calls `get_track_details(id)` directly — no fuzzy matching. preferred source (your configured primary) is tried first so a deezer-primary user gets deezer payloads on the wishlist entry. text search is only the fallback now (kicks in for tracks with no stored IDs). also fixed the modal toast that lied "matching tracks to spotify..." regardless of which sources were actually being queried.', page: 'library' },
@@ -3777,6 +3778,18 @@ const WHATS_NEW = {
 // Section shape: { title, description, features: [bullet strings],
 //                  usage_note?: 'optional hint shown at the bottom' }
 const VERSION_MODAL_SECTIONS = [
+    {
+        title: "HiFi Instance Add No Longer Errors With \"no such table\"",
+        description: "github issue #503 (hadshaw21): adding a hifi instance from downloader settings popped up `no such table: hifi_instances` even when the connection test passed.",
+        features: [
+            "• root cause: the bulk db init runs every CREATE TABLE + every migration step inside one sqlite transaction — python\'s sqlite3 module doesn\'t autocommit DDL, so if any later migration throws on your DB shape, the WHOLE batch rolls back including the hifi_instances create that ran successfully",
+            "• fix: defensive lazy-create — every hifi_instances CRUD method now runs `CREATE TABLE IF NOT EXISTS` right before its operation",
+            "• idempotent — one no-op cost when the table is already there, fully self-heals when it isn\'t",
+            "• read methods return empty instead of raising; write methods work end-to-end",
+            "• doesn\'t paper over the underlying init issue (still worth tracking which migration breaks for which users) but makes hifi instance management work regardless",
+        ],
+        usage_note: "no settings to change — applies on next click of \"add\" in hifi instance settings",
+    },
     {
         title: "Plex: Combine All Music Libraries Into One",
         description: "github issue #505 (popebruhlxix): users with multiple plex music libraries (e.g. one per plex home user) only saw one library inside soulsync because settings forced you to pick a single library section.",