From 6bbafb4f693ce11d2de17363d68f6031372407db Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:11:53 -0600 Subject: [PATCH] fix(library): collapse repeated artist credit families Derive one canonical artist identity when an album-artist prefix repeats with multiple bullet-separated contributors, while preserving source-faithful credits and one-off collaborations. Rebuild schema-v4 catalogs side-by-side so existing installations receive the corrected artist rail without a rescan or in-place database migration. cx --- .../qbz-local-catalog/src/bootstrap_tests.rs | 44 +++++- crates/qbz-local-catalog/src/catalog.rs | 146 ++++++++++++++++-- crates/qbz-local-catalog/src/schema.rs | 22 ++- crates/qbz-local-catalog/src/tests.rs | 102 +++++++++++- 4 files changed, 300 insertions(+), 14 deletions(-) diff --git a/crates/qbz-local-catalog/src/bootstrap_tests.rs b/crates/qbz-local-catalog/src/bootstrap_tests.rs index 574d93473..803a76d0b 100644 --- a/crates/qbz-local-catalog/src/bootstrap_tests.rs +++ b/crates/qbz-local-catalog/src/bootstrap_tests.rs @@ -8,9 +8,9 @@ use tempfile::tempdir; use crate::{ bootstrap_legacy_caches, bootstrap_legacy_caches_at_with_progress, ActiveCatalog, - BootstrapBatch, BootstrapLayout, BootstrapOutcome, Catalog, CatalogError, FallbackReason, - LegacyLocations, PreflightReport, ProjectedTrack, QueryDescriptor, SourceKey, SourceKind, - SourceProbe, TrackRef, BOOTSTRAP_BATCH_ROWS, + BootstrapBatch, BootstrapLayout, BootstrapManifest, BootstrapOutcome, Catalog, CatalogError, + FallbackReason, LegacyLocations, PreflightReport, ProjectedTrack, QueryDescriptor, SourceKey, + SourceKind, SourceProbe, TrackRef, BOOTSTRAP_BATCH_ROWS, SCHEMA_VERSION, }; fn source(kind: SourceKind, instance: &str) -> SourceKey { @@ -357,6 +357,44 @@ fn legacy_fixture_bootstraps_all_sources_read_only_and_is_idempotent() { assert!(!BootstrapLayout::new(temp.path()).building_path(2).exists()); } +#[test] +fn obsolete_catalog_schema_rebuilds_side_by_side_without_touching_sources() { + let temp = tempdir().unwrap(); + let source_path = temp.path().join("library.db"); + create_local_fixture(&source_path); + let source_bytes = fs::metadata(&source_path).unwrap().len(); + let layout = BootstrapLayout::new(temp.path()); + let obsolete_generation = 4; + let obsolete_bytes = b"obsolete derived catalog"; + fs::write(layout.generation_path(obsolete_generation), obsolete_bytes).unwrap(); + layout + .write_manifest(&BootstrapManifest { + manifest_version: 1, + schema_version: SCHEMA_VERSION - 1, + active_generation: obsolete_generation, + previous_generation: None, + activated_at_unix_ms: 1, + }) + .unwrap(); + + let outcome = bootstrap_legacy_caches(temp.path(), &AtomicBool::new(false)).unwrap(); + assert!(matches!( + outcome, + BootstrapOutcome::Activated { + generation: 5, + track_count: 2, + .. + } + )); + assert_eq!(fs::read(layout.generation_path(4)).unwrap(), obsolete_bytes); + assert_eq!(fs::metadata(source_path).unwrap().len(), source_bytes); + let ActiveCatalog::Ready { catalog, manifest } = layout.open_active() else { + panic!("rebuilt catalog did not activate") + }; + assert_eq!(manifest.schema_version, SCHEMA_VERSION); + assert_eq!(catalog.stats().unwrap().track_count, 2); +} + #[test] fn split_profile_and_global_cache_locations_build_one_user_catalog() { let temp = tempdir().unwrap(); diff --git a/crates/qbz-local-catalog/src/catalog.rs b/crates/qbz-local-catalog/src/catalog.rs index 293c2310b..3e9515bc9 100644 --- a/crates/qbz-local-catalog/src/catalog.rs +++ b/crates/qbz-local-catalog/src/catalog.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::path::Path; use std::time::{Duration, Instant}; @@ -19,6 +19,13 @@ use crate::schema; use crate::{CatalogError, Result, SCHEMA_VERSION}; const MAX_PAGE_SIZE: usize = 500; +const ALBUM_ARTIST_FAMILY_SEPARATOR: &str = " • "; + +#[derive(Default)] +struct AlbumArtistFamilyEvidence { + display_counts: HashMap, + suffixes: HashSet, +} pub struct Catalog { conn: Connection, @@ -540,6 +547,7 @@ impl Catalog { pub(crate) fn rebuild_materialized_views(&mut self) -> Result<()> { let tx = self.conn.transaction()?; + rebuild_artist_identities(&tx)?; tx.execute_batch( "DELETE FROM artists_fts; DELETE FROM artist_source_stats; @@ -549,7 +557,7 @@ impl Catalog { INSERT INTO edition_artists(edition_id,artist_key,role) SELECT DISTINCT sc.edition_id,ac.artist_key,ac.role - FROM artist_credits ac + FROM artist_identity_credits ac JOIN tracks t ON t.catalog_id=ac.catalog_id JOIN source_copies sc ON sc.source_copy_id=t.source_copy_id; @@ -597,7 +605,7 @@ impl Catalog { ),''), COALESCE(( SELECT group_concat(DISTINCT ac.display_name) - FROM artist_credits ac + FROM artist_identity_credits ac JOIN tracks ta ON ta.catalog_id=ac.catalog_id JOIN source_copies sca ON sca.source_copy_id=ta.source_copy_id WHERE sca.edition_id=e.edition_id @@ -658,7 +666,7 @@ impl Catalog { ELSE MIN(t.source_kind) END, COALESCE(( SELECT t2.source_kind - FROM artist_credits ac2 + FROM artist_identity_credits ac2 JOIN tracks t2 ON t2.catalog_id=ac2.catalog_id WHERE ac2.artist_key=ac.artist_key AND t2.available=1 @@ -672,7 +680,7 @@ impl Catalog { ),''), COALESCE(( SELECT t2.artwork_token - FROM artist_credits ac2 + FROM artist_identity_credits ac2 JOIN tracks t2 ON t2.catalog_id=ac2.catalog_id WHERE ac2.artist_key=ac.artist_key AND t2.available=1 @@ -684,7 +692,7 @@ impl Catalog { WHEN 'performer' THEN 3 ELSE 4 END, t2.catalog_id LIMIT 1 ),'') - FROM artist_credits ac + 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 GROUP BY ac.artist_key; @@ -696,7 +704,7 @@ impl Catalog { COUNT(DISTINCT CASE WHEN t.available=1 THEN sc.edition_id END), COUNT(DISTINCT CASE WHEN t.available=1 THEN ac.catalog_id END), COALESCE(MAX(t.available),0) - FROM artist_credits ac + 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 GROUP BY ac.artist_key,t.source_kind,t.source_instance; @@ -715,7 +723,7 @@ impl Catalog { (SELECT COUNT(*) FROM tracks), (SELECT COALESCE(SUM(track_count),0) FROM albums_materialized), (SELECT COUNT(*) FROM tracks WHERE source_copy_id IS NULL), - (SELECT COUNT(DISTINCT artist_key) FROM artist_credits), + (SELECT COUNT(DISTINCT artist_key) FROM artist_identity_credits), (SELECT COUNT(*) FROM artists_materialized), (SELECT COUNT(*) FROM artists_materialized am WHERE NOT EXISTS (SELECT 1 FROM artist_source_stats ast @@ -1189,6 +1197,119 @@ impl Catalog { } } +/// Collapse a repeated `collection • contributor` album-artist family into +/// one collection identity while retaining every track/contributor credit. +/// +/// A single bullet-bearing name is left intact because it can be a deliberate +/// collaboration or stage name. Two distinct suffixes sharing a normalized +/// prefix provide corpus-level evidence that the source encoded a family of +/// album credits rather than separate root artists. Only the derived credit +/// relation is rebuilt; raw credits plus track and album presentation metadata +/// remain byte-for-byte source-authored for the next incremental generation. +fn rebuild_artist_identities(tx: &rusqlite::Transaction<'_>) -> Result { + let mut families = HashMap::::new(); + { + let mut statement = tx.prepare( + "SELECT display_name,COUNT(*) + FROM artist_credits + WHERE role='album_artist' AND instr(display_name,?1)>0 + GROUP BY display_name + ORDER BY display_name", + )?; + let rows = statement.query_map(params![ALBUM_ARTIST_FAMILY_SEPARATOR], |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, i64>(1)?.max(0) as u64, + )) + })?; + for row in rows { + let (display_name, count) = row?; + let Some((prefix, suffix)) = split_album_artist_family(&display_name) else { + continue; + }; + let family_key = normalize_artist_key(prefix); + let suffix_key = normalize_artist_key(suffix); + if family_key.is_empty() || suffix_key.is_empty() { + continue; + } + let evidence = families.entry(family_key).or_default(); + evidence.suffixes.insert(suffix_key); + *evidence + .display_counts + .entry(prefix.to_string()) + .or_default() += count; + } + } + + let mut aliases = Vec::new(); + for (family_key, evidence) in &families { + if evidence.suffixes.len() < 2 { + continue; + } + let canonical_display = { + let mut displays = evidence.display_counts.iter().collect::>(); + displays.sort_by(|(left_name, left_count), (right_name, right_count)| { + right_count + .cmp(left_count) + .then_with(|| left_name.cmp(right_name)) + }); + displays + .into_iter() + .next() + .map(|(display_name, _)| (*display_name).clone()) + .unwrap_or_default() + }; + for prefix in evidence.display_counts.keys() { + aliases.push(( + prefix.clone(), + family_key.clone(), + canonical_display.clone(), + )); + } + } + aliases.sort(); + + tx.execute("DELETE FROM artist_identity_credits", [])?; + tx.execute( + "INSERT OR IGNORE INTO artist_identity_credits( + catalog_id,artist_key,display_name,role,ordinal + ) SELECT catalog_id,artist_key,display_name,role,ordinal + FROM artist_credits", + [], + )?; + let mut rewritten = 0; + for (prefix, family_key, canonical_display) in aliases { + rewritten += tx.execute( + "DELETE FROM artist_identity_credits + WHERE role='album_artist' AND instr(display_name,?1)>0 + AND trim(substr(display_name,1,instr(display_name,?1)-1))=?2", + params![ALBUM_ARTIST_FAMILY_SEPARATOR, prefix], + )?; + tx.execute( + "INSERT OR IGNORE INTO artist_identity_credits( + catalog_id,artist_key,display_name,role,ordinal + ) SELECT catalog_id,?1,?2,role,ordinal + FROM artist_credits + WHERE role='album_artist' AND instr(display_name,?3)>0 + AND trim(substr(display_name,1,instr(display_name,?3)-1))=?4", + params![ + family_key, + canonical_display, + ALBUM_ARTIST_FAMILY_SEPARATOR, + prefix + ], + )?; + } + Ok(rewritten) +} + +fn split_album_artist_family(display_name: &str) -> Option<(&str, &str)> { + let (prefix, suffix) = display_name.split_once(ALBUM_ARTIST_FAMILY_SEPARATOR)?; + let prefix = prefix.trim(); + let suffix = suffix.trim(); + (!prefix.is_empty() && !suffix.is_empty()).then_some((prefix, suffix)) +} + fn validate_ref(track_ref: &TrackRef) -> Result<()> { if track_ref.source_instance.trim().is_empty() { return Err(CatalogError::InvalidInput( @@ -2423,7 +2544,14 @@ pub(crate) fn descriptor_key(descriptor: &QueryDescriptor) -> String { for format in descriptor.formats() { push_key_part(&mut key, format); } - push_key_part(&mut key, if descriptor.other_formats() { "other" } else { "" }); + push_key_part( + &mut key, + if descriptor.other_formats() { + "other" + } else { + "" + }, + ); for tier in descriptor.quality_tiers() { push_key_part(&mut key, tier); } diff --git a/crates/qbz-local-catalog/src/schema.rs b/crates/qbz-local-catalog/src/schema.rs index 88ae0ca60..9eaffdd15 100644 --- a/crates/qbz-local-catalog/src/schema.rs +++ b/crates/qbz-local-catalog/src/schema.rs @@ -2,7 +2,10 @@ use rusqlite::{params, Connection}; use crate::{CatalogError, Result}; -pub const SCHEMA_VERSION: u32 = 4; +// Version 5 changes the derived artist-identity projection. Existing v4 +// catalogs are rebuilt side-by-side from their authoritative caches; no user +// library database is migrated in place. +pub const SCHEMA_VERSION: u32 = 5; pub const APPLICATION_ID: i64 = 0x5142_5A43; // "QBZC" pub(crate) fn configure(conn: &Connection) -> Result<()> { @@ -263,6 +266,23 @@ CREATE TABLE IF NOT EXISTS artist_credits ( CREATE INDEX IF NOT EXISTS idx_artist_credits_artist ON artist_credits(artist_key, role, catalog_id); +-- Canonical identities are rebuilt from the source-faithful credit relation +-- before every materialization. Keeping both relations lets later projection +-- generations re-evaluate corpus-level aliases instead of losing evidence. +CREATE TABLE IF NOT EXISTS artist_identity_credits ( + catalog_id INTEGER NOT NULL REFERENCES tracks(catalog_id) ON DELETE CASCADE, + artist_key TEXT NOT NULL, + display_name TEXT NOT NULL, + role TEXT NOT NULL CHECK ( + role IN ('track_artist','album_artist','composer','performer','featured') + ), + ordinal INTEGER NOT NULL DEFAULT 0, + PRIMARY KEY (catalog_id, role, ordinal, artist_key) +) STRICT; + +CREATE INDEX IF NOT EXISTS idx_artist_identity_credits_artist + ON artist_identity_credits(artist_key, role, catalog_id); + CREATE TABLE IF NOT EXISTS albums_materialized ( edition_id INTEGER PRIMARY KEY REFERENCES editions(edition_id) ON DELETE CASCADE, logical_album_id INTEGER NOT NULL REFERENCES logical_albums(logical_album_id), diff --git a/crates/qbz-local-catalog/src/tests.rs b/crates/qbz-local-catalog/src/tests.rs index 477024c92..17968688b 100644 --- a/crates/qbz-local-catalog/src/tests.rs +++ b/crates/qbz-local-catalog/src/tests.rs @@ -203,6 +203,7 @@ fn schema_is_versioned_frontend_agnostic_and_fts5_enabled() { "source_copies", "tracks", "artist_credits", + "artist_identity_credits", "albums_materialized", "artists_materialized", "artist_source_stats", @@ -372,6 +373,103 @@ fn artist_credit_counts_and_album_relationship_span_every_source() { assert_eq!(remote_row.source, "jellyfin"); } +#[test] +fn repeated_album_artist_families_collapse_without_losing_contributors() { + let mut catalog = Catalog::open_in_memory(1).unwrap(); + let family = "新世紀エヴァンゲリオン"; + let contributors = [ + ("鷺巣詩郎", "Shiro Sagisu"), + ("林原めぐみ", "Megumi Hayashibara"), + ("高橋洋子", "Yoko Takahashi"), + ]; + let mut tracks = contributors + .iter() + .enumerate() + .map(|(index, (artist, romanized))| { + let mut track = projected(20_000 + index, SourceKind::Subsonic); + track.native_album_id = Some(format!("eva-{index}")); + track.artist = (*artist).to_string(); + track.album_artist = format!("{family} • {romanized}"); + track.credits = vec![ + ArtistCredit { + display_name: (*artist).to_string(), + role: CreditRole::TrackArtist, + ordinal: 0, + }, + ArtistCredit { + display_name: track.album_artist.clone(), + role: CreditRole::AlbumArtist, + ordinal: 0, + }, + ]; + track + }) + .collect::>(); + + let mut collaboration = projected(20_010, SourceKind::Subsonic); + collaboration.native_album_id = Some("one-off-collaboration".to_string()); + collaboration.artist = "Alice".to_string(); + collaboration.album_artist = "Alice • Bob".to_string(); + collaboration.credits = vec![ + ArtistCredit { + display_name: "Alice".to_string(), + role: CreditRole::TrackArtist, + ordinal: 0, + }, + ArtistCredit { + display_name: collaboration.album_artist.clone(), + role: CreditRole::AlbumArtist, + ordinal: 0, + }, + ]; + tracks.push(collaboration); + + let source = SourceKey { + source: SourceKind::Subsonic, + source_instance: source_instance(SourceKind::Subsonic).to_string(), + }; + catalog + .apply_bootstrap_batch(&BootstrapBatch { + source, + snapshot_version: "album-artist-families-v1".to_string(), + expected_cursor: String::new(), + next_cursor: tracks.len().to_string(), + tracks, + complete: true, + }) + .unwrap(); + catalog.rebuild_materialized_views().unwrap(); + + let raw_family_variants: i64 = catalog + .connection() + .query_row( + "SELECT COUNT(DISTINCT display_name) FROM artist_credits + WHERE role='album_artist' AND display_name LIKE ?1", + [format!("{family} • %")], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(raw_family_variants, contributors.len() as i64); + catalog.rebuild_materialized_views().unwrap(); + + let artists = collect_all_artists(&catalog, &QueryDescriptor::artists(), 2); + let keys = artists + .iter() + .map(|row| row.artist_key.as_str()) + .collect::>(); + let family_key = normalize_artist_key(family); + let family_row = artists + .iter() + .find(|row| row.artist_key == family_key) + .expect("the repeated album-artist family is one root artist"); + assert_eq!((family_row.album_count, family_row.track_count), (3, 3)); + for (artist, romanized) in contributors { + assert!(keys.contains(normalize_artist_key(artist).as_str())); + assert!(!keys.contains(normalize_artist_key(&format!("{family} • {romanized}")).as_str())); + } + assert!(keys.contains(normalize_artist_key("Alice • Bob").as_str())); +} + #[test] fn artist_album_keysets_cross_pages_without_omissions_or_duplicates() { const ALBUMS: usize = 731; @@ -413,7 +511,9 @@ fn artist_album_keysets_cross_pages_without_omissions_or_duplicates() { let key = normalize_artist_key("Remote Guest"); assert_eq!( - catalog.count_artist_albums(&key, &[source.clone()]).unwrap(), + catalog + .count_artist_albums(&key, &[source.clone()]) + .unwrap(), expected_available as u64 ); let mut cursor = None;