From 357f2373fa2071b9c1586dcc78580feefb7d1b37 Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:38:36 -0600 Subject: [PATCH] fix(library): omit remote albums without a server album ID Keep orphan Jellyfin and Subsonic tracks indexed without exposing internal grouping keys as server album IDs. Align album cards, artist counts and track navigation, and rebuild derived catalogs with schema 8. Regression fails on the previous projection (three cards for one real album) and passes with the fix for both providers. Catalog suite: 45 passed, 2 ignored scale benchmarks. cx --- .../qbz-local-catalog/src/bootstrap_tests.rs | 5 +- crates/qbz-local-catalog/src/catalog.rs | 27 +++- crates/qbz-local-catalog/src/schema.rs | 4 +- crates/qbz-local-catalog/src/tests.rs | 120 ++++++++++++++++++ 4 files changed, 148 insertions(+), 8 deletions(-) diff --git a/crates/qbz-local-catalog/src/bootstrap_tests.rs b/crates/qbz-local-catalog/src/bootstrap_tests.rs index 7efe8488b..1f4afc9c7 100644 --- a/crates/qbz-local-catalog/src/bootstrap_tests.rs +++ b/crates/qbz-local-catalog/src/bootstrap_tests.rs @@ -348,15 +348,16 @@ fn remote_collection_art_fills_missing_covers_without_replacing_track_art() { let conn = Connection::open(&path).unwrap(); conn.execute_batch( "ALTER TABLE remote_cache_tracks ADD COLUMN collection_artwork_token TEXT; + ALTER TABLE remote_cache_tracks ADD COLUMN album_id TEXT; DELETE FROM remote_cache_tracks;", ) .unwrap(); conn.execute( "INSERT INTO remote_cache_tracks (id,source,item_id,server_id,title,artist,album_artist,album, - duration_ms,artwork_token,collection_artwork_token,updated_at) + duration_ms,artwork_token,collection_artwork_token,updated_at,album_id) VALUES (1,?1,'track','server','Track','Artist','Artist','Album', - 180000,?2,?3,1)", + 180000,?2,?3,1,'album-id')", rusqlite::params![source, item_art, collection_art], ) .unwrap(); diff --git a/crates/qbz-local-catalog/src/catalog.rs b/crates/qbz-local-catalog/src/catalog.rs index eb7a5b659..66055692d 100644 --- a/crates/qbz-local-catalog/src/catalog.rs +++ b/crates/qbz-local-catalog/src/catalog.rs @@ -602,7 +602,9 @@ impl Catalog { LIMIT 1 ),'local'), COALESCE(( - SELECT scp.native_album_id FROM source_copies scp + SELECT CASE WHEN scp.source_kind IN ('jellyfin','subsonic') + THEN e.provider_release_id + ELSE scp.native_album_id END FROM source_copies scp WHERE scp.edition_id=e.edition_id ORDER BY scp.available DESC, CASE scp.source_kind @@ -673,7 +675,9 @@ impl Catalog { source_kind,artwork_source,artwork_token ) SELECT ac.artist_key,MIN(ac.display_name),ac.artist_key, - COUNT(DISTINCT CASE WHEN t.available=1 THEN sc.edition_id END), + COUNT(DISTINCT CASE WHEN t.available=1 + AND (am.source_kind NOT IN ('jellyfin','subsonic') OR am.native_album_id!='') + THEN sc.edition_id END), COUNT(DISTINCT CASE WHEN t.available=1 THEN ac.catalog_id END), COALESCE(MAX(t.available),0), CASE WHEN COUNT(DISTINCT t.source_kind)>1 THEN 'mixed' @@ -709,18 +713,22 @@ impl Catalog { FROM artist_identity_credits ac JOIN tracks t ON t.catalog_id=ac.catalog_id LEFT JOIN source_copies sc ON sc.source_copy_id=t.source_copy_id + LEFT JOIN albums_materialized am ON am.edition_id=sc.edition_id GROUP BY ac.artist_key; INSERT INTO artist_source_stats( artist_key,source_kind,source_instance,album_count,track_count,available ) SELECT ac.artist_key,t.source_kind,t.source_instance, - COUNT(DISTINCT CASE WHEN t.available=1 THEN sc.edition_id END), + COUNT(DISTINCT CASE WHEN t.available=1 + AND (am.source_kind NOT IN ('jellyfin','subsonic') OR am.native_album_id!='') + THEN sc.edition_id END), COUNT(DISTINCT CASE WHEN t.available=1 THEN ac.catalog_id END), COALESCE(MAX(t.available),0) FROM artist_identity_credits ac JOIN tracks t ON t.catalog_id=ac.catalog_id LEFT JOIN source_copies sc ON sc.source_copy_id=t.source_copy_id + LEFT JOIN albums_materialized am ON am.edition_id=sc.edition_id GROUP BY ac.artist_key,t.source_kind,t.source_instance; INSERT INTO artists_fts(artist_key,display_name) @@ -2070,7 +2078,9 @@ fn map_row(row: &Row<'_>) -> rusqlite::Result { const TRACK_COLUMNS: &str = " t.source_kind, t.source_instance, t.native_track_id, t.source_raw, t.local_track_id, t.local_path, - (SELECT NULLIF(sc.native_album_id,'') FROM source_copies sc + (SELECT CASE WHEN sc.source_kind IN ('jellyfin','subsonic') + THEN e.provider_release_id ELSE NULLIF(sc.native_album_id,'') END + FROM source_copies sc JOIN editions e ON e.edition_id=sc.edition_id WHERE sc.source_copy_id=t.source_copy_id), t.title, t.artist, t.album_artist, t.album, t.duration_ms, t.year, t.disc_number, t.track_number, t.format, t.bit_depth, t.sample_rate_hz, t.artwork_token, t.available, @@ -2128,6 +2138,12 @@ fn validate_albums_descriptor(descriptor: &QueryDescriptor) -> Result<()> { Ok(()) } +// Text-fallback groups keep orphan remote tracks indexed, but cannot be opened +// through the server's album endpoint. Only real server IDs may become cards. +// provider_release_id is normalized at ingestion, so whitespace IDs are empty. +const RESOLVABLE_ALBUM: &str = + "(am.source_kind NOT IN ('jellyfin','subsonic') OR am.native_album_id!='')"; + fn album_filter_parts( descriptor: &QueryDescriptor, cursor: Option<&AlbumCursor>, @@ -2137,7 +2153,7 @@ fn album_filter_parts( return Err(CatalogError::CursorDescriptorMismatch); } } - let mut predicates = Vec::new(); + let mut predicates = vec![RESOLVABLE_ALBUM.to_string()]; let mut params = Vec::new(); let from_sql = if descriptor.search().is_empty() { "FROM albums_materialized am".to_string() @@ -2586,6 +2602,7 @@ fn artist_album_filter( } } let mut predicates = vec![ + RESOLVABLE_ALBUM.to_string(), "am.available=1".to_string(), "EXISTS (SELECT 1 FROM edition_artists ea \ WHERE ea.edition_id=am.edition_id AND ea.artist_key=?)" diff --git a/crates/qbz-local-catalog/src/schema.rs b/crates/qbz-local-catalog/src/schema.rs index 93cbae50b..c6590cd97 100644 --- a/crates/qbz-local-catalog/src/schema.rs +++ b/crates/qbz-local-catalog/src/schema.rs @@ -11,7 +11,9 @@ use crate::{CatalogError, Result}; // Version 7 adds collection-art fallback to the remote projection. Rebuild // existing derived catalogs so coverless v6 rows are repaired even when the // authoritative remote cache has not changed. User caches remain untouched. -pub const SCHEMA_VERSION: u32 = 7; +// Version 8 distinguishes server album IDs from internal text-fallback groups. +// Rebuild album cards and artist counts without dropping orphan remote tracks. +pub const SCHEMA_VERSION: u32 = 8; pub const APPLICATION_ID: i64 = 0x5142_5A43; // "QBZC" pub(crate) fn configure(conn: &Connection) -> Result<()> { diff --git a/crates/qbz-local-catalog/src/tests.rs b/crates/qbz-local-catalog/src/tests.rs index 0ba01974e..1577029bc 100644 --- a/crates/qbz-local-catalog/src/tests.rs +++ b/crates/qbz-local-catalog/src/tests.rs @@ -70,6 +70,126 @@ fn projected(index: usize, source: SourceKind) -> ProjectedTrack { } } +#[test] +fn remote_tracks_without_album_ids_do_not_create_unresolvable_album_cards() { + for source in [SourceKind::Jellyfin, SourceKind::Subsonic] { + let mut catalog = Catalog::open_in_memory(1).unwrap(); + let mut orphan = projected(1, source); + orphan.native_album_id = None; + orphan.album.clear(); + orphan.available = true; + let mut named_orphan = orphan.clone(); + named_orphan.track_ref.native_id = "2".into(); + named_orphan.album = "Orphan title".into(); + named_orphan.native_album_id = Some(" ".into()); + let mut empty_id = named_orphan.clone(); + empty_id.track_ref.native_id = "4".into(); + empty_id.native_album_id = Some(String::new()); + let mut valid = orphan.clone(); + valid.track_ref.native_id = "3".into(); + valid.native_album_id = Some("real-album-id".into()); + valid.album = "Real album".into(); + let key = SourceKey { + source, + source_instance: orphan.track_ref.source_instance.clone(), + }; + let mut rows = vec![orphan.clone(), named_orphan, empty_id, valid]; + catalog + .apply_bootstrap_batch(&BootstrapBatch { + source: key.clone(), + snapshot_version: "missing-album-ids".into(), + expected_cursor: String::new(), + next_cursor: rows.len().to_string(), + tracks: rows.clone(), + complete: true, + }) + .unwrap(); + catalog.rebuild_materialized_views().unwrap(); + let descriptor = QueryDescriptor::albums(); + assert_eq!(catalog.count_tracks(&QueryDescriptor::tracks()).unwrap(), 4); + assert_eq!(catalog.count_albums(&descriptor).unwrap(), 1); + let resolved = catalog.resolve(&orphan.track_ref).unwrap().unwrap(); + assert_eq!(resolved.native_album_id, None); + let tracks = catalog + .query_tracks(&QueryDescriptor::tracks(), None, 10) + .unwrap(); + assert_eq!(tracks.rows.len(), 4); + assert_eq!( + tracks + .rows + .iter() + .filter(|row| row.native_album_id.is_some()) + .count(), + 1 + ); + let albums = catalog.query_albums(&descriptor, None, 1).unwrap(); + assert_eq!(albums.rows.len(), 1); + assert!(!albums.has_more); + assert_eq!(albums.rows[0].native_album_id, "real-album-id"); + let artist = normalize_artist_key(&orphan.artist); + for sources in [vec![], vec![key.clone()]] { + assert_eq!(catalog.count_artist_albums(&artist, &sources).unwrap(), 1); + let artist_albums = catalog + .query_artist_albums(&artist, &sources, None, 10) + .unwrap(); + assert_eq!(artist_albums.rows.len(), 1); + let artists = catalog + .query_artists( + &QueryDescriptor::artists().with_sources(sources.clone()), + None, + 10, + ) + .unwrap(); + assert_eq!(artists.rows[0].album_count, 1); + assert_eq!(artists.rows[0].track_count, 4); + assert_eq!( + catalog + .count_albums(&descriptor.clone().with_sources(sources)) + .unwrap(), + 1 + ); + } + assert_eq!( + catalog + .count_albums(&descriptor.with_search("Orphan title")) + .unwrap(), + 0 + ); + assert!(catalog.materialized_views_valid().unwrap()); + + // A later sync repairs the server metadata: the same track now has a + // real, navigable album and should reappear without manual cache deletion. + orphan.native_album_id = Some("repaired-album-id".into()); + orphan.album = "Repaired album".into(); + rows[0] = orphan.clone(); + catalog + .begin_reconciliation(&key, "repaired-album-id") + .unwrap(); + catalog + .apply_reconciliation_batch(&ReconciliationBatch { + source: key, + snapshot_version: "repaired-album-id".into(), + expected_cursor: String::new(), + next_cursor: rows.len().to_string(), + tracks: rows, + complete: true, + }) + .unwrap(); + catalog.rebuild_materialized_views().unwrap(); + assert_eq!(catalog.count_albums(&QueryDescriptor::albums()).unwrap(), 2); + assert_eq!(catalog.count_tracks(&QueryDescriptor::tracks()).unwrap(), 4); + assert_eq!( + catalog + .resolve(&orphan.track_ref) + .unwrap() + .unwrap() + .native_album_id, + Some("repaired-album-id".into()) + ); + assert!(catalog.materialized_views_valid().unwrap()); + } +} + fn insert_fixture(catalog: &mut Catalog, count: usize) { for start in (0..count).step_by(2_000) { let end = (start + 2_000).min(count);