fix(artists): preserve album credits across scopes

This commit is contained in:
cucadmuh
2026-07-17 06:12:34 +03:00
parent 7791fe5dd7
commit 88945796a6
6 changed files with 314 additions and 18 deletions
+1 -1
View File
@@ -14,7 +14,7 @@ tauri = { version = "2" }
specta = { version = "=2.0.0-rc.25", features = ["derive"] }
serde = { version = "1", features = ["derive"] }
serde_json = "1"
rusqlite = { version = "0.40", features = ["bundled"] }
rusqlite = { version = "0.40", features = ["bundled", "functions"] }
reqwest = { version = "0.13", default-features = false, features = ["json", "rustls", "gzip", "brotli"] }
tokio = { version = "1", features = ["rt", "rt-multi-thread", "sync", "macros", "time"] }
@@ -0,0 +1,3 @@
-- Unicode-lowercased display name for indexed album-credit matching.
ALTER TABLE artist ADD COLUMN name_fold TEXT;
CREATE INDEX IF NOT EXISTS idx_artist_name_fold ON artist(server_id, name_fold);
@@ -578,6 +578,38 @@ fn build_multi_scope_artist(
skip_totals: bool,
applied: &mut BTreeSet<String>,
) -> Result<(Vec<LibraryArtistDto>, u32), String> {
if album_artist_credit_mode(req) && !scalar_requires_track_derived_entities(scalar) {
applied.insert("library_scope".to_string());
applied.insert("artist_credit_mode".to_string());
let mut filter = WhereBuilder::new();
if let Some(bucket) = req.artist_letter_bucket.as_deref() {
push_artist_letter_bucket(&mut filter, bucket, applied);
}
if let Some(query) = text {
filter.push_param(
"ar.name_fold LIKE ? ESCAPE '\\'",
SqlValue::Text(like_contains_folded(query)),
);
applied.insert("text".to_string());
}
for clause in scalar {
if let Some(fragment) = resolve_clause(clause, EntityKind::Artist)? {
applied.insert(clause.field.clone());
filter.push(fragment);
}
}
let order = deduped_artist_order_sql(&req.sort);
return scope_merge::list_index_artists_multi_scope_album_filtered(
store,
scopes,
&filter.where_sql(),
filter.params(),
&order,
limit,
offset,
skip_totals,
);
}
let (extra_where, extra_params) = multi_scope_track_filter_sql(
store,
req,
@@ -2387,9 +2419,19 @@ mod tests {
store
.with_conn("misc", |c| {
c.execute(
"INSERT INTO artist (server_id, id, name, album_count, synced_at, raw_json) \
VALUES (?1, ?2, ?3, ?4, 1, '{}')",
rusqlite::params![server, id, name, album_count],
"INSERT INTO artist (server_id, id, name, name_sort, name_fold, album_count, synced_at, raw_json) \
VALUES (?1, ?2, ?3, ?4, ?5, ?6, 1, '{}')",
rusqlite::params![
server,
id,
name,
crate::artist_sort::sort_key_for_display_name(
name,
crate::artist_sort::DEFAULT_IGNORED_ARTICLES,
),
name.trim().to_lowercase(),
album_count,
],
)
})
.unwrap();
@@ -3818,6 +3860,61 @@ mod tests {
);
}
#[test]
fn layer1_album_artist_search_matches_cyrillic_credit_case_variants() {
let store = LibraryStore::open_in_memory();
insert_artist_with_album_count(&store, "s1", "ar_kino", "Кино", Some(1));
insert_artist_with_album_count(&store, "s1", "ar_kinoproby", "КИНО-пробы", Some(1));
let mut kino = scoped_track(
"s1", "t-kino", "Song", "Кино", "Album", "alb-kino", "lib-a", None, None, None,
);
kino.artist_id = Some("ar_kino".into());
let mut kinoproby = scoped_track(
"s1", "t-kinoproby", "Song", "Кино-пробы", "Album", "alb-kinoproby", "lib-b", None, None, None,
);
kinoproby.artist_id = Some("ar_kinoproby".into());
TrackRepository::new(&store).upsert_batch(&[kino, kinoproby]).unwrap();
let mut r = req("s1", &[EntityKind::Artist]);
r.library_scopes = Some(vec![scope_pair("s1", "lib-a"), scope_pair("s1", "lib-b")]);
r.artist_credit_mode = Some(ArtistCreditMode::Album);
r.query = Some("Кино".into());
let resp = run_advanced_search(&store, &r).unwrap();
assert_eq!(
resp.artists.iter().map(|artist| artist.id.as_str()).collect::<Vec<_>>(),
vec!["ar_kino", "ar_kinoproby"],
);
}
#[test]
fn multi_server_album_artist_search_uses_album_credit_not_track_performer() {
let store = LibraryStore::open_in_memory();
insert_artist_with_album_count(&store, "s1", "ar-kino", "КИНО-пробы", Some(1));
insert_artist_with_album_count(&store, "s2", "ar-other", "Other", Some(1));
let mut sampler_track = scoped_track(
"s1", "t-sampler", "Song", "Guest Performer", "Tribute", "alb-tribute", "lib-a", None, None, None,
);
sampler_track.artist_id = Some("ar-guest".into());
sampler_track.album_artist = Some("Кино-пробы".into());
let mut other_track = scoped_track(
"s2", "t-other", "Song", "Other", "Other Album", "alb-other", "lib-b", None, None, None,
);
other_track.artist_id = Some("ar-other".into());
TrackRepository::new(&store)
.upsert_batch(&[sampler_track, other_track])
.unwrap();
let mut r = req("s1", &[EntityKind::Artist]);
r.library_scopes = Some(vec![scope_pair("s1", "lib-a"), scope_pair("s2", "lib-b")]);
r.artist_credit_mode = Some(ArtistCreditMode::Album);
r.query = Some("Кино".into());
let resp = run_advanced_search(&store, &r).unwrap();
assert_eq!(
resp.artists.iter().map(|artist| artist.id.as_str()).collect::<Vec<_>>(),
vec!["ar-kino"],
);
}
#[test]
fn multi_scope_track_browse_without_cluster_keys_returns_scoped_tracks() {
let store = LibraryStore::open_in_memory();
@@ -148,14 +148,15 @@ fn upsert_artist_row(
synced_at: i64,
) -> rusqlite::Result<()> {
tx.execute(
"INSERT INTO artist (server_id, id, name, name_sort, album_count, synced_at) \
VALUES (?1, ?2, ?3, ?4, ?5, ?6) \
"INSERT INTO artist (server_id, id, name, name_sort, name_fold, album_count, synced_at) \
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7) \
ON CONFLICT(server_id, id) DO UPDATE SET \
name = excluded.name, \
name_sort = excluded.name_sort, \
album_count = COALESCE(excluded.album_count, artist.album_count), \
synced_at = excluded.synced_at",
params![server_id, id, name, name_sort, album_count, synced_at],
name = excluded.name, \
name_sort = excluded.name_sort, \
name_fold = excluded.name_fold, \
album_count = COALESCE(excluded.album_count, artist.album_count), \
synced_at = excluded.synced_at",
params![server_id, id, name, name_sort, name.trim().to_lowercase(), album_count, synced_at],
)?;
Ok(())
}
@@ -706,9 +706,8 @@ pub(crate) fn list_index_artists_layer1_filtered(
format!(
"{cte}, \
album_scoped AS ( \
SELECT t.album_id, \
lower(trim(COALESCE(NULLIF(MAX(trim(t.album_artist)), ''), MIN(t.artist)))) \
AS credit_name \
SELECT t.album_id, \
COALESCE(NULLIF(MAX(trim(t.album_artist)), ''), MIN(t.artist)) AS credit_name \
{scoped_from} \
WHERE t.deleted = 0 AND t.album_id IS NOT NULL AND t.album_id != '' \
GROUP BY t.album_id \
@@ -716,8 +715,8 @@ pub(crate) fn list_index_artists_layer1_filtered(
scoped_ids AS ( \
SELECT DISTINCT ar.id \
FROM album_scoped ac \
INNER JOIN artist ar ON ar.server_id = ? AND ar.album_count IS NOT NULL \
AND lower(trim(coalesce(ar.name, ''))) = ac.credit_name \
INNER JOIN artist ar ON ar.server_id = ? AND ar.album_count IS NOT NULL \
AND ar.name_fold = psysonic_lower_name(ac.credit_name) \
)"
)
} else {
@@ -780,6 +779,78 @@ pub(crate) fn list_index_artists_layer1_filtered(
Ok((artists, total))
}
/// Multi-server Album artists browse. Album credits can differ from every track
/// performer, so derive them from `album_artist` and resolve the matching indexed
/// artist row by its persisted Unicode fold before priority-deduplicating names.
#[allow(clippy::too_many_arguments)]
pub(crate) fn list_index_artists_multi_scope_album_filtered(
store: &LibraryStore,
scopes: &[LibraryScopePair],
extra_where: &str,
extra_params: &[SqlValue],
order_sql: &str,
limit: u32,
offset: u32,
skip_totals: bool,
) -> Result<(Vec<LibraryArtistDto>, u32), String> {
let scopes = non_empty_scopes(scopes)?;
let (cte, scope_binds) = scope_cte_sql(scopes);
let artist_where = if extra_where.trim().is_empty() {
"ar.album_count IS NOT NULL".to_string()
} else {
format!("ar.album_count IS NOT NULL AND {extra_where}")
};
let credits_cte = format!(
"{cte}, \
album_credits AS ( \
SELECT t.server_id, t.album_id, s.pr, \
COALESCE(NULLIF(MAX(trim(t.album_artist)), ''), MIN(t.artist)) AS credit_name \
FROM scope s \
CROSS JOIN track t ON t.server_id = s.server_id AND t.library_id = s.library_id \
WHERE t.deleted = 0 AND t.album_id IS NOT NULL AND t.album_id != '' \
GROUP BY t.server_id, t.album_id, s.pr \
), \
matched AS ( \
SELECT ar.server_id, ar.id AS artist_id, ar.name AS artist, ar.name_fold, \
ac.album_id, ac.pr, ar.synced_at \
FROM album_credits ac \
INNER JOIN artist ar ON ar.server_id = ac.server_id \
AND ar.name_fold = psysonic_lower_name(ac.credit_name) \
WHERE {artist_where} \
), \
deduped AS ( \
SELECT server_id, artist_id, artist, synced_at, \
COUNT(DISTINCT server_id || ':' || album_id) AS album_count, \
MIN(printf('%08d|%s|%s', pr, server_id, artist_id)) AS _pick \
FROM matched GROUP BY name_fold \
)"
);
let count_sql = format!("{credits_cte} SELECT COUNT(*) FROM deduped");
let select_sql = format!(
"{credits_cte} SELECT server_id, artist_id, artist, album_count, synced_at \
FROM deduped {order_sql} LIMIT ? OFFSET ?"
);
let mut binds = merge_binds(scope_binds, extra_params);
let total = if skip_totals {
0
} else {
store.with_read_conn(|conn| {
let count: i64 = conn.query_row(&count_sql, params_from_iter(binds.iter()), |row| row.get(0))?;
Ok(count.max(0) as u32)
})?
};
binds.push(SqlValue::Integer(i64::from(limit)));
binds.push(SqlValue::Integer(i64::from(offset)));
let artists = store.with_read_conn(|conn| {
let mut stmt = conn.prepare(&select_sql)?;
let rows = stmt
.query_map(params_from_iter(binds.iter()), map_artist_list_row)?
.collect::<rusqlite::Result<Vec<_>>>()?;
Ok(rows.into_iter().map(artist_row_to_dto).collect())
})?;
Ok((artists, total))
}
/// Layer-1 scoped track browse — sargable join, no cross-library dedup window.
#[allow(clippy::too_many_arguments)]
pub(crate) fn list_tracks_layer1_filtered(
@@ -2372,6 +2443,26 @@ mod tests {
assert_eq!(artists[0].name, "Shared");
}
#[test]
fn album_credit_lookup_uses_name_fold_index() {
let store = LibraryStore::open_in_memory();
let plan: Vec<String> = store
.with_read_conn(|conn| {
let mut stmt = conn.prepare(
"EXPLAIN QUERY PLAN \
SELECT ar.id FROM artist ar \
WHERE ar.server_id = 's1' AND ar.name_fold = psysonic_lower_name('Кино')",
)?;
let rows = stmt.query_map([], |row| row.get(3))?;
rows.collect()
})
.unwrap();
assert!(
plan.iter().any(|detail| detail.contains("idx_artist_name_fold")),
"expected name-fold index lookup, got: {plan:?}"
);
}
/// Manual perf probe:
/// `cargo test --workspace scope_merge::tests::perf_probe_album_browse -- --ignored --nocapture`
#[test]
+106 -2
View File
@@ -4,7 +4,7 @@ use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
use std::sync::Mutex;
use std::time::Duration;
use rusqlite::{params, Connection, OpenFlags, OptionalExtension};
use rusqlite::{functions::FunctionFlags, params, Connection, OpenFlags, OptionalExtension};
use tauri::Manager;
/// Current head of the embedded migrations. Bump each time a new
@@ -12,10 +12,11 @@ use tauri::Manager;
///
/// Migration checklist (wiring, data backfill, open/swap path):
/// psysonic-workdocs `ai/agent-rules/08-library-db-migrations.md`.
pub const LIBRARY_DB_SCHEMA_VERSION: i64 = 21;
pub const LIBRARY_DB_SCHEMA_VERSION: i64 = 22;
/// One-time data repair after migration 014 (`artist.name_sort`).
pub(crate) const ARTIST_NAME_SORT_RECONCILE_ID: &str = "artist_name_sort_reconcile_v1";
pub(crate) const ARTIST_NAME_FOLD_RECONCILE_ID: &str = "artist_name_fold_reconcile_v1";
/// One-time backfill after migration 015 (`track.replay_gain_peak`).
pub(crate) const REPLAY_GAIN_PEAK_RECONCILE_ID: &str = "replay_gain_peak_reconcile_v1";
@@ -69,6 +70,8 @@ pub(crate) const MIGRATION_020_SCOPE_BROWSE_PROJECTION: &str =
/// Version 21: title keyset index for candidate-first scoped track browse.
pub(crate) const MIGRATION_021_SCOPE_BROWSE_TRACKS: &str =
include_str!("../migrations/021_scope_browse_tracks.sql");
pub(crate) const MIGRATION_022_ARTIST_NAME_FOLD: &str =
include_str!("../migrations/022_artist_name_fold.sql");
/// Embedded migrations. Ordered ascending by `version`; the runner sorts
/// defensively before applying so the source order can stay readable.
@@ -84,6 +87,7 @@ const MIGRATIONS: &[(i64, &str)] = &[
(19, MIGRATION_019_MAINSTAGE_FEED_INDEXES),
(20, MIGRATION_020_SCOPE_BROWSE_PROJECTION),
(21, MIGRATION_021_SCOPE_BROWSE_TRACKS),
(22, MIGRATION_022_ARTIST_NAME_FOLD),
];
/// Idempotent repair — also runs after the migration runner on every open so
@@ -630,6 +634,7 @@ fn remove_db_with_sidecars(path: &Path) -> Result<(), String> {
}
fn configure_write_connection(conn: &Connection) -> rusqlite::Result<()> {
register_sql_functions(conn)?;
conn.busy_timeout(Duration::from_secs(30))?;
conn.pragma_update(None, "journal_mode", "WAL")?;
conn.pragma_update(None, "synchronous", "NORMAL")?;
@@ -639,6 +644,7 @@ fn configure_write_connection(conn: &Connection) -> rusqlite::Result<()> {
}
fn configure_read_connection(conn: &Connection) -> rusqlite::Result<()> {
register_sql_functions(conn)?;
conn.busy_timeout(Duration::from_secs(5))?;
conn.pragma_update(None, "foreign_keys", "ON")?;
// Search / browse hot path on large libraries (read-only handle).
@@ -646,6 +652,20 @@ fn configure_read_connection(conn: &Connection) -> rusqlite::Result<()> {
Ok(())
}
/// Unicode lowercase is applied only to the grouped album credit. The persisted
/// `artist.name_fold` remains the indexed join side, avoiding a full artist scan.
fn register_sql_functions(conn: &Connection) -> rusqlite::Result<()> {
conn.create_scalar_function(
"psysonic_lower_name",
1,
FunctionFlags::SQLITE_UTF8 | FunctionFlags::SQLITE_DETERMINISTIC,
|ctx| {
let name: String = ctx.get(0)?;
Ok(name.trim().to_lowercase())
},
)
}
fn checkpoint_wal_conn(conn: &Connection, op: &str) -> rusqlite::Result<()> {
let (busy, log, checkpointed): (i32, i32, i32) =
conn.query_row("PRAGMA wal_checkpoint(TRUNCATE)", [], |row| {
@@ -685,6 +705,7 @@ fn open_database_connections(db_path: &Path) -> rusqlite::Result<(Connection, Co
fn prepare_write_connection_for_open(conn: &Connection) -> rusqlite::Result<()> {
run_migrations(conn)?;
maybe_reconcile_artist_name_sort(conn)?;
maybe_reconcile_artist_name_fold(conn)?;
maybe_reconcile_replay_gain_peak(conn)?;
maybe_reconcile_library_id_backfill(conn)?;
maybe_reconcile_orphan_browse_rows(conn)?;
@@ -707,6 +728,17 @@ fn artist_name_sort_column_exists(conn: &Connection) -> rusqlite::Result<bool> {
Ok(column_exists > 0)
}
fn artist_name_fold_column_exists(conn: &Connection) -> rusqlite::Result<bool> {
let column_exists: i64 = conn
.query_row(
"SELECT COUNT(*) FROM pragma_table_info('artist') WHERE name = 'name_fold'",
[],
|row| row.get(0),
)
.unwrap_or(0);
Ok(column_exists > 0)
}
fn sync_state_ignored_articles_column_exists(conn: &Connection) -> rusqlite::Result<bool> {
let column_exists: i64 = conn
.query_row(
@@ -823,6 +855,49 @@ fn repair_artist_name_sort_keys(conn: &Connection) -> rusqlite::Result<()> {
Ok(())
}
fn artist_name_fold_reconcile_completed(conn: &Connection) -> rusqlite::Result<bool> {
let completed: Option<Option<i64>> = conn
.query_row(
"SELECT completed_at FROM library_data_migration WHERE id = ?1",
params![ARTIST_NAME_FOLD_RECONCILE_ID],
|row| row.get(0),
)
.optional()?;
Ok(completed.flatten().is_some())
}
fn maybe_reconcile_artist_name_fold(conn: &Connection) -> rusqlite::Result<()> {
if !artist_name_fold_column_exists(conn)? || artist_name_fold_reconcile_completed(conn)? {
return Ok(());
}
let tx = conn.unchecked_transaction()?;
{
let mut stmt = tx.prepare("SELECT server_id, id, name, name_fold FROM artist")?;
let mut rows = stmt.query([])?;
while let Some(row) = rows.next()? {
let server_id: String = row.get(0)?;
let id: String = row.get(1)?;
let name: String = row.get(2)?;
let current: Option<String> = row.get(3)?;
let expected = name.trim().to_lowercase();
if current.as_deref() == Some(&expected) {
continue;
}
tx.execute(
"UPDATE artist SET name_fold = ?1 WHERE server_id = ?2 AND id = ?3",
params![expected, server_id, id],
)?;
}
}
tx.execute(
"INSERT INTO library_data_migration (id, cursor_rowid, started_at, completed_at) \
VALUES (?1, 0, strftime('%s','now'), strftime('%s','now')) \
ON CONFLICT(id) DO UPDATE SET completed_at = excluded.completed_at",
params![ARTIST_NAME_FOLD_RECONCILE_ID],
)?;
tx.commit()
}
fn replay_gain_peak_column_exists(conn: &Connection) -> rusqlite::Result<bool> {
let column_exists: i64 = conn
.query_row(
@@ -1148,6 +1223,35 @@ mod tests {
);
}
#[test]
fn migration_022_backfills_unicode_artist_name_fold() {
let store = LibraryStore::open_in_memory();
store
.with_conn("test", |conn| {
conn.execute(
"INSERT INTO artist (server_id, id, name, name_fold, synced_at) \
VALUES ('s1', 'ar-kino', 'КИНО-пробы', NULL, 1)",
[],
)?;
conn.execute(
"DELETE FROM library_data_migration WHERE id = ?1",
params![ARTIST_NAME_FOLD_RECONCILE_ID],
)?;
maybe_reconcile_artist_name_fold(conn)
})
.unwrap();
let name_fold: String = store
.with_read_conn(|conn| {
conn.query_row(
"SELECT name_fold FROM artist WHERE server_id = 's1' AND id = 'ar-kino'",
[],
|row| row.get(0),
)
})
.unwrap();
assert_eq!(name_fold, "кино-пробы");
}
#[test]
fn migration_012_repairs_db_that_recorded_legacy_versions_without_genre_tables() {
let uri = in_memory_uri();