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
This commit is contained in:
4 files changed
+300
-14
No files matched your search
@@ -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();
|
||||
|
||||
@@ -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<String, u64>,
|
||||
suffixes: HashSet<String>,
|
||||
}
|
||||
|
||||
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<usize> {
|
||||
let mut families = HashMap::<String, AlbumArtistFamilyEvidence>::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::<Vec<_>>();
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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::<Vec<_>>();
|
||||
|
||||
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::<HashSet<_>>();
|
||||
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;
|
||||
|
||||
Reference in new issue
Block a user