Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions docs/rfcs/RFC-005-remote-source-and-sync-v2.md
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,23 @@ comparable rather than merely concatenated:

A server album keeps none of the local gestures — no playlist, no cover picker,
no context menu — because none of them can accept it, and it opens the remote
detail view rather than the local one. The artists tab works the same way, on
the same filter.
detail view rather than the local one. The artists and tracks tabs work the same
way, on the same filter.

The tracks tab adds one consequence worth stating plainly: **playing a row
queues the run of rows from its own source.** Decision 9 keeps the remote queue
parallel to the local one — they are two structures, and a mixed list cannot
produce a mixed queue. So clicking a local track queues the local rows and
clicking a server track queues the server ones. The chip on every row is what
makes that legible, and narrowing the source filter is how a user gets one
continuous queue.

A server track also carries none of the local user data: no rating, no like, no
playlist membership, no tag editing. Those are absent rather than inert — five
hollow stars that do nothing read as "unrated", which is a different claim from
"cannot be rated here". Only tracks the catalogue walk mirrored are listed: one
cached because a playlist referenced it is not part of the browsable catalogue
and would appear with no album and no way to reach it.

Artists need one thing albums did not: the walk mirrors them into
`remote_artist` rather than deriving them by grouping on `artist_id`. Grouping
Expand Down
471 changes: 471 additions & 0 deletions src-tauri/crates/app/src/commands/browse.rs

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src-tauri/crates/app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ pub fn run() {
commands::browse::list_albums,
commands::browse::list_library_albums,
commands::browse::list_library_artists,
commands::browse::list_library_tracks,
commands::browse::list_artists,
commands::browse::search_albums,
commands::browse::search_artists,
Expand Down
22 changes: 8 additions & 14 deletions src-tauri/crates/app/src/remote/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,20 +296,14 @@ mod tests {
.connect(":memory:")
.await
.unwrap();
for migration in [
include_str!(
"../../../../migrations/profile/20260810120000_remote_source_projection.sql"
),
include_str!("../../../../migrations/profile/20260810140000_remote_track_cache.sql"),
include_str!(
"../../../../migrations/profile/20260813090000_remote_track_full_hash.sql"
),
include_str!(
"../../../../migrations/profile/20260816120000_remote_track_artist_id.sql"
),
] {
sqlx::raw_sql(migration).execute(&pool).await.unwrap();
}
// The real migrator rather than a hand-listed subset: a
// migration that touches a table this list happened to omit
// breaks the fixture and not the code, which is a failure that
// teaches nothing. Same fixture the browse tests use.
sqlx::migrate!("../../migrations/profile")
.run(&pool)
.await
.unwrap();
pool
}

Expand Down
86 changes: 65 additions & 21 deletions src-tauri/crates/app/src/remote/mirror.rs
Original file line number Diff line number Diff line change
Expand Up @@ -933,27 +933,14 @@ mod tests {
.execute(&pool)
.await
.unwrap();
for migration in [
include_str!(
"../../../../migrations/profile/20260810120000_remote_source_projection.sql"
),
include_str!("../../../../migrations/profile/20260810140000_remote_track_cache.sql"),
include_str!(
"../../../../migrations/profile/20260813090000_remote_track_full_hash.sql"
),
include_str!(
"../../../../migrations/profile/20260816120000_remote_track_artist_id.sql"
),
include_str!(
"../../../../migrations/profile/20260824210000_remote_catalogue_mirror.sql"
),
include_str!(
"../../../../migrations/profile/20260826090000_remote_album_sort_keys.sql"
),
include_str!("../../../../migrations/profile/20260826140000_remote_artist_mirror.sql"),
] {
sqlx::raw_sql(migration).execute(&pool).await.unwrap();
}
// The real migrator, like every other remote fixture: a hand-listed
// subset breaks whenever a migration touches a table the list
// happened to omit. The upgrade test below lists them on purpose,
// because it needs the schema as it was *before* one of them.
sqlx::migrate!("../../migrations/profile")
.run(&pool)
.await
.unwrap();
pool
}

Expand Down Expand Up @@ -1402,6 +1389,63 @@ mod tests {
clear(&pool).await.unwrap();
}

/// The upgrade path for the track sort keys: an album already walked, whose
/// count has not changed, would otherwise never be walked again — so its
/// tracks would keep their empty keys and keep sorting on the wrong
/// expression forever. The migration clears the stamps to force one walk.
#[tokio::test]
async fn the_sort_key_migration_makes_every_walked_album_stale() {
let pool = SqlitePoolOptions::new()
.max_connections(1)
.connect(":memory:")
.await
.unwrap();
// Everything up to, but not including, the migration under test.
for migration in [
include_str!(
"../../../../migrations/profile/20260810120000_remote_source_projection.sql"
),
include_str!("../../../../migrations/profile/20260810140000_remote_track_cache.sql"),
include_str!(
"../../../../migrations/profile/20260813090000_remote_track_full_hash.sql"
),
include_str!(
"../../../../migrations/profile/20260816120000_remote_track_artist_id.sql"
),
include_str!(
"../../../../migrations/profile/20260824210000_remote_catalogue_mirror.sql"
),
include_str!(
"../../../../migrations/profile/20260826090000_remote_album_sort_keys.sql"
),
] {
sqlx::raw_sql(migration).execute(&pool).await.unwrap();
}

let listed = album("al-1", 5);
upsert(&pool, &listed).await;
sqlx::query("UPDATE remote_album SET mirrored_at = 1 WHERE remote_id = 'al-1'")
.execute(&pool)
.await
.unwrap();
assert!(
album_is_fresh(&pool, &listed).await.unwrap(),
"the album is walked and unchanged before the upgrade"
);

sqlx::raw_sql(include_str!(
"../../../../migrations/profile/20260826180000_remote_track_sort_keys.sql"
))
.execute(&pool)
.await
.unwrap();

assert!(
!album_is_fresh(&pool, &listed).await.unwrap(),
"the upgrade must send it back through the walk"
);
}

/// The defect the `mirrored_at` CASE exists for: an album that gains a
/// track and is interrupted before its fetch must not read as fresh.
#[tokio::test]
Expand Down
22 changes: 8 additions & 14 deletions src-tauri/crates/app/src/remote/mutation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,20 +574,14 @@ mod tests {
.connect(":memory:")
.await
.unwrap();
for migration in [
include_str!(
"../../../../migrations/profile/20260810120000_remote_source_projection.sql"
),
include_str!("../../../../migrations/profile/20260810140000_remote_track_cache.sql"),
include_str!(
"../../../../migrations/profile/20260813090000_remote_track_full_hash.sql"
),
include_str!(
"../../../../migrations/profile/20260816120000_remote_track_artist_id.sql"
),
] {
sqlx::raw_sql(migration).execute(&pool).await.unwrap();
}
// The real migrator rather than a hand-listed subset: a
// migration that touches a table this list happened to omit
// breaks the fixture and not the code, which is a failure that
// teaches nothing. Same fixture the browse tests use.
sqlx::migrate!("../../migrations/profile")
.run(&pool)
.await
.unwrap();
pool
}

Expand Down
43 changes: 26 additions & 17 deletions src-tauri/crates/app/src/remote/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

use serde_json::Value;
use sqlx::SqliteConnection;
use waveflow_core::metadata::name_match::normalize_name;

use crate::{
error::AppResult,
Expand Down Expand Up @@ -233,8 +234,9 @@ pub async fn cache_song(conn: &mut SqliteConnection, song: &SongItem) -> AppResu
sqlx::query(
"INSERT INTO remote_track
(remote_id, title, artist, artist_id, album, album_id, duration_ms, track_no, disc_no,
year, genre, suffix, bitrate, size, artwork_hash, library_id, full_hash, cached_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
year, genre, suffix, bitrate, size, artwork_hash, library_id, full_hash, cached_at,
sort_artist, sort_album)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
ON CONFLICT(remote_id) DO UPDATE SET
-- A later cache pass for a bare id binds an empty title; keep the
-- existing one rather than blanking a row we already labelled.
Expand All @@ -254,7 +256,12 @@ pub async fn cache_song(conn: &mut SqliteConnection, song: &SongItem) -> AppResu
artwork_hash = COALESCE(excluded.artwork_hash, artwork_hash),
library_id = COALESCE(excluded.library_id, library_id),
full_hash = COALESCE(excluded.full_hash, full_hash),
cached_at = excluded.cached_at",
cached_at = excluded.cached_at,
-- Keyed off the same COALESCE as the strings they are derived
-- from: a sparser later sighting must not blank a key any more
-- than it blanks the title it came from.
sort_artist = COALESCE(excluded.sort_artist, sort_artist),
sort_album = COALESCE(excluded.sort_album, sort_album)",
)
.bind(&song.id)
// An untitled row is still better than no row: the playlist keeps
Expand All @@ -276,6 +283,14 @@ pub async fn cache_song(conn: &mut SqliteConnection, song: &SongItem) -> AppResu
.bind(song.library_id.as_deref())
.bind(song.full_hash.as_deref())
.bind(chrono::Utc::now().timestamp_millis())
// The comparison keys the unified track listing sorts artist and album
// on, through the very normaliser the local half's `canonical_*` columns
// went through. SQLite cannot fold a diacritic, so without these the two
// halves interleave wrongly. The title is deliberately absent: it has no
// canonical form locally either, so both halves sort it on their display
// string and stay consistent that way.
.bind(song.artist.as_deref().map(normalize_name))
.bind(song.album.as_deref().map(normalize_name))
.execute(&mut *conn)
.await?;
Ok(())
Expand Down Expand Up @@ -709,20 +724,14 @@ mod tests {
.connect(":memory:")
.await
.unwrap();
for migration in [
include_str!(
"../../../../migrations/profile/20260810120000_remote_source_projection.sql"
),
include_str!("../../../../migrations/profile/20260810140000_remote_track_cache.sql"),
include_str!(
"../../../../migrations/profile/20260813090000_remote_track_full_hash.sql"
),
include_str!(
"../../../../migrations/profile/20260816120000_remote_track_artist_id.sql"
),
] {
sqlx::raw_sql(migration).execute(&pool).await.unwrap();
}
// The real migrator rather than a hand-listed subset: a
// migration that touches a table this list happened to omit
// breaks the fixture and not the code, which is a failure that
// teaches nothing. Same fixture the browse tests use.
sqlx::migrate!("../../migrations/profile")
.run(&pool)
.await
.unwrap();
pool
}

Expand Down
22 changes: 8 additions & 14 deletions src-tauri/crates/app/src/remote/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,20 +214,14 @@ mod tests {
.connect(":memory:")
.await
.unwrap();
for migration in [
include_str!(
"../../../../migrations/profile/20260810120000_remote_source_projection.sql"
),
include_str!("../../../../migrations/profile/20260810140000_remote_track_cache.sql"),
include_str!(
"../../../../migrations/profile/20260813090000_remote_track_full_hash.sql"
),
include_str!(
"../../../../migrations/profile/20260816120000_remote_track_artist_id.sql"
),
] {
sqlx::raw_sql(migration).execute(&pool).await.unwrap();
}
// The real migrator rather than a hand-listed subset: a
// migration that touches a table this list happened to omit
// breaks the fixture and not the code, which is a failure that
// teaches nothing. Same fixture the browse tests use.
sqlx::migrate!("../../migrations/profile")
.run(&pool)
.await
.unwrap();
pool
}

Expand Down
22 changes: 8 additions & 14 deletions src-tauri/crates/app/src/remote/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,20 +909,14 @@ mod tests {
.connect(":memory:")
.await
.unwrap();
for migration in [
include_str!(
"../../../../migrations/profile/20260810120000_remote_source_projection.sql"
),
include_str!("../../../../migrations/profile/20260810140000_remote_track_cache.sql"),
include_str!(
"../../../../migrations/profile/20260813090000_remote_track_full_hash.sql"
),
include_str!(
"../../../../migrations/profile/20260816120000_remote_track_artist_id.sql"
),
] {
sqlx::raw_sql(migration).execute(&pool).await.unwrap();
}
// The real migrator rather than a hand-listed subset: a
// migration that touches a table this list happened to omit
// breaks the fixture and not the code, which is a failure that
// teaches nothing. Same fixture the browse tests use.
sqlx::migrate!("../../migrations/profile")
.run(&pool)
.await
.unwrap();
pool
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
-- Sort keys for the mirrored tracks, for the same reason the albums have them.
--
-- The unified track listing sorts both halves against each other, and the
-- local half sorts artist and album on `artist.canonical_name` /
-- `album.canonical_title` — forms produced by
-- `waveflow_core::metadata::name_match::normalize_name`, which lowercases,
-- folds diacritics and drops punctuation. SQLite reproduces none of that, so
-- a remote half sorted on its raw display strings interleaves wrongly and
-- splits an artist in two down the middle of the list.
--
-- Only artist and album. The title has no canonical form on the local side
-- either — it sorts on `track.title COLLATE NOCASE` — so both halves sort the
-- title on their display string, which keeps that column consistent by using
-- the same expression rather than by normalising one side only.
--
-- Filled by `projection::cache_song`, the single place a remote track row is
-- written: from the snapshot, from a change event, from a search hit and from
-- the catalogue walk alike. Nullable because rows cached before this migration
-- have none; the listing falls back to the display string, and any later
-- sighting of the track fills them in.
ALTER TABLE remote_track ADD COLUMN sort_artist TEXT;
ALTER TABLE remote_track ADD COLUMN sort_album TEXT;

CREATE INDEX idx_remote_track_sort ON remote_track (sort_artist, sort_album, title);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

-- Existing rows have no keys, and nothing would ever give them any.
--
-- The columns are nullable and the listing coalesces to the display string,
-- so an un-keyed row still renders — but it sorts on the wrong expression,
-- which is the whole defect these columns exist to fix. And it would sort that
-- way *forever*: the walk skips an album whose `song_count` is unchanged, so
-- `cache_song` never runs again for its tracks and the keys stay NULL.
--
-- SQLite cannot compute them (it cannot fold a diacritic), so the stamps are
-- cleared instead: every album becomes stale, the next walk re-fetches it, and
-- `cache_song` fills the keys on the way through. One walk, and it is
-- incremental again afterwards.
UPDATE remote_album SET mirrored_at = NULL;
Loading