fix(library): use partial indexes for §6.9 remap lookup (#1105)

* fix(library): use partial indexes for §6.9 remap lookup

The delta-sync remap detection ran a single lookup with an OR across
content_hash and server_path. SQLite could not use the partial
idx_track_remap_hash / idx_track_remap_path indexes for it: a partial
index is only applied when the query's WHERE provably implies the index
predicate (… != ''), and an OR spanning two columns blocks the per-branch
index plan. The query degraded to a full track scan on every incoming
row → O(rows × catalog), causing multi-minute stalls on large libraries
(observed upsert_batch_remap exec_ms=162001 on a ~200k-track Navidrome
sync, which in turn blocked all other writers on the single write mutex).

Split it into two single-column lookups that each repeat the index
predicate so the planner picks the matching partial index (SEARCH, not
SCAN); hash is checked first, matching §6.9 strong-key priority. Adds an
EXPLAIN QUERY PLAN regression test asserting index usage.

* docs(changelog): note remap-lookup sync stall fix (PR #1105)

CHANGELOG.md

@@ -36,6 +36,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * On macOS, closing the window with the red close button always quit the app, even with "Minimize to Tray" enabled. The close button now respects the setting — with it on, the window hides to the tray instead of quitting, the same as the tray icon's "Hide".
 
+### Library sync stalling for many seconds on large Navidrome collections
+
+**By [@cucadmuh](https://github.com/cucadmuh), PR [#1105](https://github.com/Psychotoxical/psysonic/pull/1105)**
+
+* On large libraries (reported with ~200,000 tracks on Navidrome), background library sync could lock up database writes for minutes at a time — playback history, ratings and other saves piled up waiting behind it.
+* Root cause: the track id-remap step ran a database lookup that couldn't use its indexes and scanned the entire track table once per incoming track, so the cost grew with the square of the library size. The lookup now uses the proper indexes, bringing it back to a fast, near-instant operation.
+
 
 
 ## [1.48.0]

src-tauri/crates/psysonic-library/src/repos/track.rs

@@ -448,7 +448,10 @@ impl<'a> TrackRepository<'a> {
             let mut remapped: Vec<RemapEntry> = Vec::new();
             let mut upsert = tx.prepare_cached(UPSERT_SQL)?;
             let mut remap_lookup = if unstable_track_ids {
-                Some(tx.prepare_cached(REMAP_LOOKUP_SQL)?)
+                Some((
+                    tx.prepare_cached(REMAP_LOOKUP_BY_HASH_SQL)?,
+                    tx.prepare_cached(REMAP_LOOKUP_BY_PATH_SQL)?,
+                ))
             } else {
                 None
             };
@@ -459,11 +462,12 @@ impl<'a> TrackRepository<'a> {
                 // then do we retarget children to the new id, since
                 // child tables FK→track(server_id, id) and would refuse
                 // an UPDATE pointing at an id that doesn't exist yet.
-                let detected_old: Option<String> = if let Some(ref mut lookup) = remap_lookup {
-                    detect_remap_target_cached(lookup, r)?
-                } else {
-                    None
-                };
+                let detected_old: Option<String> =
+                    if let Some((ref mut by_hash, ref mut by_path)) = remap_lookup {
+                        detect_remap_target_cached(by_hash, by_path, r)?
+                    } else {
+                        None
+                    };
 
                 upsert.execute(params![
                     r.server_id,
@@ -543,38 +547,76 @@ impl<'a> TrackRepository<'a> {
     }
 }
 
-const REMAP_LOOKUP_SQL: &str = r#"
+// Two single-column lookups instead of one `OR` across `content_hash`
+// and `server_path`. The combined `OR` form could not use the partial
+// `idx_track_remap_hash` / `idx_track_remap_path` indexes — SQLite only
+// applies a partial index when the query's WHERE provably implies the
+// index predicate (`… != ''`), and an `OR` spanning two columns blocks
+// the per-branch index plan. The result was a full `track` scan per
+// incoming row → O(rows × catalog) on large libraries (observed:
+// `upsert_batch_remap exec_ms=162001` on a ~200k-track Navidrome sync).
+// Each statement below repeats the index predicate so the planner picks
+// the matching partial index (SEARCH, not SCAN); hash wins over path,
+// matching §6.9's strong-key priority.
+const REMAP_LOOKUP_BY_HASH_SQL: &str = r#"
 SELECT id FROM track
  WHERE server_id = ?1
    AND deleted = 0
-   AND id != ?2
-   AND (
-     (?3 IS NOT NULL AND content_hash = ?3)
-     OR (?4 IS NOT NULL AND server_path = ?4)
-   )
+   AND content_hash IS NOT NULL
+   AND content_hash != ''
+   AND content_hash = ?2
+   AND id != ?3
+ LIMIT 1
+"#;
+
+const REMAP_LOOKUP_BY_PATH_SQL: &str = r#"
+SELECT id FROM track
+ WHERE server_id = ?1
+   AND deleted = 0
+   AND server_path IS NOT NULL
+   AND server_path != ''
+   AND server_path = ?2
+   AND id != ?3
  LIMIT 1
 "#;
 
 /// Run the `SELECT old.id` half of §6.9 — returns `Some(old_id)` if a
 /// non-deleted row with a different id on this server matches the
-/// incoming row's `content_hash` or `server_path`.
+/// incoming row's `content_hash` or `server_path`. Hash is the stronger
+/// key, so it is checked first.
 fn detect_remap_target_cached(
-    lookup: &mut rusqlite::Statement<'_>,
+    by_hash: &mut rusqlite::Statement<'_>,
+    by_path: &mut rusqlite::Statement<'_>,
     incoming: &TrackRow,
 ) -> rusqlite::Result<Option<String>> {
     // Empty-string sentinels are *not* eligible — spec §6.9 explicitly
     // excludes them so the file-tree default never collides.
     let hash = incoming.content_hash.as_deref().filter(|s| !s.is_empty());
     let path = incoming.server_path.as_deref().filter(|s| !s.is_empty());
-    if hash.is_none() && path.is_none() {
-        return Ok(None);
+
+    if let Some(hash) = hash {
+        let old = by_hash
+            .query_row(params![incoming.server_id, hash, incoming.id], |row| {
+                row.get::<_, String>(0)
+            })
+            .optional()?;
+        if old.is_some() {
+            return Ok(old);
+        }
     }
-    lookup
-        .query_row(
-            params![incoming.server_id, incoming.id, hash, path],
-            |row| row.get::<_, String>(0),
-        )
-        .optional()
+
+    if let Some(path) = path {
+        let old = by_path
+            .query_row(params![incoming.server_id, path, incoming.id], |row| {
+                row.get::<_, String>(0)
+            })
+            .optional()?;
+        if old.is_some() {
+            return Ok(old);
+        }
+    }
+
+    Ok(None)
 }
 
 /// Run the §6.9 retarget half — UPDATE every FK-bound child to the
@@ -1247,6 +1289,48 @@ mod tests {
         assert_eq!(count, 2, "both rows kept; identity-less rows can't shadow");
     }
 
+    #[test]
+    fn remap_lookup_uses_partial_indexes_not_full_scan() {
+        // Regression: the §6.9 remap lookup must hit
+        // idx_track_remap_hash / idx_track_remap_path. The prior
+        // `OR`-based query fell back to a full `track` scan on every
+        // incoming row → O(rows × catalog) stalls on large libraries
+        // (`upsert_batch_remap exec_ms=162001` on a ~200k Navidrome sync).
+        let store = LibraryStore::open_in_memory();
+        let plan = |sql: &str| -> String {
+            store
+                .with_conn("misc", |c| {
+                    let mut stmt = c.prepare(&format!("EXPLAIN QUERY PLAN {sql}"))?;
+                    let rows: rusqlite::Result<Vec<String>> = stmt
+                        .query_map(params!["s1", "v", "id"], |r| r.get::<_, String>(3))?
+                        .collect();
+                    rows
+                })
+                .unwrap()
+                .join("\n")
+        };
+
+        let hash_plan = plan(REMAP_LOOKUP_BY_HASH_SQL);
+        assert!(
+            hash_plan.contains("idx_track_remap_hash"),
+            "hash lookup must use idx_track_remap_hash, got: {hash_plan}"
+        );
+        assert!(
+            !hash_plan.contains("SCAN"),
+            "hash lookup must not full-scan track, got: {hash_plan}"
+        );
+
+        let path_plan = plan(REMAP_LOOKUP_BY_PATH_SQL);
+        assert!(
+            path_plan.contains("idx_track_remap_path"),
+            "path lookup must use idx_track_remap_path, got: {path_plan}"
+        );
+        assert!(
+            !path_plan.contains("SCAN"),
+            "path lookup must not full-scan track, got: {path_plan}"
+        );
+    }
+
     #[test]
     fn remap_is_noop_when_new_id_matches_existing_id() {
         // Standard delta-sync: same id, same hash. Must not trigger