From 537e011a1ff5936d36c5eeb9c53e406fd7eae3de Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 24 Aug 2026 21:28:04 +0200 Subject: [PATCH 1/4] fix(subsonic): a song is not a release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #140 widened the required-array list so an album keeps its arrays under the `child` element name a directory renames it to. The list is keyed on that name, and the guard that tells the shapes apart answers `true` for every entry that is a song — so every song on `main` has been carrying `recordLabels: []`, `releaseTypes: []` and `discTitles: []`. Under the presence rule that says the server reads a record label off a recording. The guard now names what a song is refused, the way it already names what an artist and an album are allowed. The test that should have caught it did not, and the reason is worth recording: the array rule only decides anything when an element has exactly one child, and the fixture gave every album two of each. A third album carrying one record label, one release type and one disc title is added, and the directory assertions now compare each album's `child` rendering against its own `album` rendering across all three shapes — several values, one, and none — rather than restating expected values. Both halves were confirmed to fail: without the `child` array rule one record label renders as a bare object, and without this commit's restriction a song answers `recordLabels`. Also updates the two lists the operator asked for. The handoff's mission list is closed except §4, which the other three have made due. The compatibility matrix gains the caveat that its rows are dated 2026-08-23 against a model that has moved: five album fields were added, so a row saying `pass` says it about a response that no longer carries exactly the same keys. Signed-off-by: InstaZDLL --- docs/handoff-2026-08-23.md | 74 ++++++++++++++++------- docs/subsonic-compatibility.md | 12 ++++ src/subsonic/protocol.rs | 8 ++- tests/v2_foundations.rs | 106 +++++++++++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 23 deletions(-) diff --git a/docs/handoff-2026-08-23.md b/docs/handoff-2026-08-23.md index 28f6bee..cccdef9 100644 --- a/docs/handoff-2026-08-23.md +++ b/docs/handoff-2026-08-23.md @@ -8,14 +8,15 @@ one continues from it. ## Where the project stands -`main` at `d36e929`. Tag `v2.0.0-beta.0` on `e19a117`, unmoved. No open pull -requests. 42 unit and 48 integration tests pass; `cargo fmt --all --check` and +`main` at `1d15c16`. Tag `v2.0.0-beta.0` on `e19a117`, unmoved. No open pull +requests. 42 unit and 53 integration tests pass; `cargo fmt --all --check` and `cargo clippy --all-targets --all-features -- -D warnings` are clean, and a third CI job now checks the crate against its declared minimum Rust version. -> **Updated 2026-08-24.** The mission list below has moved: §1 and the first -> half of §2 shipped, and a trap was added that cost `main` six hours of red. -> Everything above this line is the record of 2026-08-23 and stays as written. +> **Updated 2026-08-24.** The mission list below is closed except §4, which the +> other three have now made due: §1, §2 and §3 all shipped, as PRs #137, #139 +> and #140. A trap was added that cost `main` six hours of red. Everything above +> this line is the record of 2026-08-23 and stays as written. The release is real and verified from the registry rather than from a green tick: `ghcr.io/instazdll/waveflow-server` carries `2.0.0-beta.0` and `beta` — @@ -48,7 +49,8 @@ lesson, not the score. ## The mission from here -Nothing is blocking. In the order I would take them: +Written on 2026-08-23 in the order I would take them. Three of the four are +closed; what is left is §4, and the other three are what made it due. ### 1. Break up `src/` — done, PR #137 @@ -90,22 +92,50 @@ person has to check. that a stale hint "costs a missed relocation, never a wrong one" was written into `CLAUDE.md` and the migration before it was true; both were corrected. -- **Artist favourite remapping**, if judged worth the thirty lines. The - documentation currently states plainly that it does not exist, which is the - honest position until it does. - -### 3. Album output fields - -`originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]`, -`discTitles[]` are still absent, and honestly declared so under the presence -rule. `ItemKey::Label` and `ItemKey::Publisher` exist in lofty, so -`recordLabels[]` is the cheap one. - -### 4. Before a stable tag - -Replay the clients again if the wire moves. The rule that has paid twice: **never -conclude from what the client shows when the server can be asked.** It -disqualified three "server defects" reported in good faith in one evening. +- **~~Artist favourite remapping.~~** Done, PR #140. It was worth the thirty + lines. `reconcile_catalog_identity` remaps `user_star` and `user_rating` + when `pid.artist` changed, **before** requesting the rescan — the only moment + the old identifier and the name that produced it are both still on the + `artist` row. Albums are not remapped and cannot be: their spec reads + `albumversion` and `releasedate`, which live on the files. + + The review caught that moving the rows one at a time depends on the order they + came back in — one artist's new identifier can be another's old one, and the + first artist's favourite was then carried onto the second's row, or lost. Every + pair is now computed first and moved in two phases through a namespace no + identifier can occupy. No spec this engine can parse actually produces that + chain, because only `albumartistid` carries a value for an artist; the test + builds the rows by hand and says so, because the code cannot see that argument + and a wider `PidSource` would make it reachable. + +### 3. Album output fields — done, PR #140 + +All five. `originalReleaseDate`, `releaseDate`, `releaseTypes[]` and +`recordLabels[]` describe the release, so they sit on the album and fill the way +`year` does; `discTitles[]` holds one title per disc, so the tag lands on the +track and the album derives the list from its available tracks, grouped per disc +with `MIN`. `LABEL` first, `PUBLISHER` second. The three arrays are emitted empty +rather than absent — under both names an album is rendered by, `album` and, in a +directory, `child` — and the dates are omitted when unknown, as the reference +omits them. They are stored as the file spelled them and taken apart only at the +wire: `1998-11` is a year and a month, and a head that is not exactly four ASCII +digits is no date at all. + +The same pull request closed the four findings deferred from #137: the N+1 reads +in `playlists_on` and `now_playing`, `search3` paging in memory rather than in +SQL, unbounded identifier lists on `star`/`unstar`/`scrobble`, and unbounded +playlists. + +### 4. Before a stable tag — now due + +**The wire moved.** Five album fields are new Subsonic output, so the condition +this section names is met and replaying the four clients is no longer optional. +[`subsonic-compatibility.md`](subsonic-compatibility.md) still dates them +2026-08-23, against a model that has changed since. + +The rule that has paid twice: **never conclude from what the client shows when +the server can be asked.** It disqualified three "server defects" reported in +good faith in one evening. ## Traps this session paid for diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index 1b553fc..e2646e5 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -2,6 +2,18 @@ Automated protocol coverage is enforced by `tests/v2_foundations.rs` for XML, JSON, `.view`, GET, form POST, `u/p`, `u/t/s`, `apiKey`, catalogue isolation, all documented mutations, media and artwork. +> **The rows below are dated 2026-08-23 and the wire has moved since.** On +> 2026-08-24 an album gained five output fields — `originalReleaseDate`, +> `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]` — under +> both names an album is rendered by, `album` and, in a directory, `child`. No +> field was removed and none changed shape, so nothing a client read on the 23rd +> reads differently today; but a row that says `pass` says it about a response +> that no longer carries exactly the same keys. **The campaign wants replaying +> before a stable tag**, which is what §4 of +> [`handoff-2026-08-23.md`](handoff-2026-08-23.md) asks for. Until it is, read +> every date in this table as naming the model the run exercised rather than the +> one `main` serves. + | Client | Version | Login | Browse/search | Native/transcode | Playlists/user data | Status | |---|---:|---:|---:|---:|---:|---| | Symfonium | 15.0.1 | pass | pass | limit | limit | Re-run 2026-08-23 against the aligned model, on an Android 17 emulator through the logging proxy. **This is a different major version from the 14.1.0 row it replaces**, so its differences from the previous run are not attributable to the server alone. Passed: a failed login decoded from its HTTP 200, 5 albums and 18 tracks, sleeve order, one canonical `Jazz` answering 4 tracks, both artists of a two-artist album holding it, the separator traps (`AC/DC` one artist, `Bach/Gounod` two composers, a padded slash two artists), and a title query returning no artists. Two limits, both established from the proxy log rather than from the client's display, and neither a server defect. **Seeking a lossless track fails**: the client re-requests the resource with no `Range` header at all and restarts from zero — in the whole session it sent exactly one byte-offset range, on an MP3, answered 206. It sent none in the 2026-08-19 run either, so no seek by this client was ever established. **Playlist creation never reaches the server**: no `createPlaylist` or `updatePlaylist` was sent. Both are contrasted directly: Feishin seeks the same seektable-less FLAC files and creates playlists against the same server. The client renders `artist`, `albumArtist` and `composer` and ignores the other ten roles, which the server emits in full at the `v=1.13.0` this client asks with. Bookmarks were exercised incidentally (5 `createBookmark`, one live). It never calls `getArtists`/`getIndexes`, syncing entirely through the match-all `search3`. | diff --git a/src/subsonic/protocol.rs b/src/subsonic/protocol.rs index 70cd24b..94f37f8 100644 --- a/src/subsonic/protocol.rs +++ b/src/subsonic/protocol.rs @@ -90,7 +90,13 @@ pub(super) fn node_json(node: &Node, parent: &str) -> Value { *name, "artists" | "genres" | "recordLabels" | "releaseTypes" | "discTitles" ), - EntryKind::Song => true, + // A song is not a release. `recordLabels`, `releaseTypes` and + // `discTitles` are in the list above because an album rendered + // inside a directory shares the `child` element name with a song, + // and this is where the two are told apart: injecting them here + // would have a recording answer `recordLabels: []`, which under the + // presence rule claims the server reads a label off a track. + EntryKind::Song => !matches!(*name, "recordLabels" | "releaseTypes" | "discTitles"), }; if !injected { continue; diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index 41bc09f..d4d4736 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -10654,6 +10654,13 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() { .unwrap(); } state.db.finish_scan_job(scan_id, 0).await.unwrap(); + // What a real scan does after applying its rows, and what makes an artist + // reachable as a folder. + state + .db + .consolidate_catalog_derivations(library_id) + .await + .unwrap(); let album_id = state .services @@ -10733,6 +10740,16 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() { .find(|album| album.title == "Bare release") .unwrap() .id; + // A song is not a release. The three arrays share their element name with + // an album inside a directory, and a recording answering `recordLabels: []` + // would claim the server reads a label off a track. + let song = &album["song"][0]; + assert!(song["recordLabels"].is_null()); + assert!(song["releaseTypes"].is_null()); + assert!(song["discTitles"].is_null()); + // Its own arrays are still there, empty rather than absent. + assert_eq!(song["moods"], serde_json::json!([])); + let bare = subsonic_json(&router, "getAlbum", api_key, &format!("&id={bare_id}")).await; let bare = &bare["subsonic-response"]["album"]; assert_eq!(bare["recordLabels"], serde_json::json!([])); @@ -10740,6 +10757,95 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() { assert_eq!(bare["discTitles"], serde_json::json!([])); assert!(bare["originalReleaseDate"].is_null()); assert!(bare["releaseDate"].is_null()); + + // A third album carrying exactly one of each. This is the shape the array + // rule is for: with one child and no rule, a record label renders as a bare + // object instead of a list of one. + let solo_scan = state + .db + .create_scan_job(library_id, Some(owner), "manual") + .await + .unwrap(); + state.db.start_scan_job(solo_scan, 1, false).await.unwrap(); + let mut solo_input = catalog_input(9, "Solo Hand"); + solo_input.album = Some("Single imprint".into()); + solo_input.is_compilation = false; + solo_input.disc_number = Some(1); + solo_input.disc_subtitle = Some("Only disc".into()); + solo_input.release_types = Some("EP".into()); + solo_input.record_labels = Some("Solo Records".into()); + state + .db + .apply_catalog_track(library_id, solo_scan, &solo_input, None, false) + .await + .unwrap(); + state.db.finish_scan_job(solo_scan, 0).await.unwrap(); + state + .db + .consolidate_catalog_derivations(library_id) + .await + .unwrap(); + let solo_id = state + .services + .catalog_snapshot(owner, &[]) + .await + .unwrap() + .albums + .into_iter() + .find(|album| album.title == "Single imprint") + .unwrap() + .id; + let solo = subsonic_json(&router, "getAlbum", api_key, &format!("&id={solo_id}")).await; + let solo = &solo["subsonic-response"]["album"]; + assert_eq!( + solo["recordLabels"], + serde_json::json!([{"name": "Solo Records"}]), + "one record label is a list of one, not a bare object" + ); + assert_eq!(solo["releaseTypes"], serde_json::json!(["EP"])); + assert_eq!( + solo["discTitles"], + serde_json::json!([{"disc": 1, "title": "Only disc"}]) + ); + + // `getMusicDirectory` renames an album to `child`, and both the array rule + // and the injection guard are keyed on that name. Under it an album has to + // answer exactly what it answers as `album` — for several values, for one, + // and for none — or a client browsing folders reads a single record label + // as a bare object and an empty list as "not supported". + for (id, rendered) in [(album_id, album), (solo_id, solo), (bare_id, bare)] { + let artist_id: String = + sqlx::query_scalar("SELECT album_artist_id FROM album WHERE id = ?") + .bind(id.to_string()) + .fetch_one(state.db.pool()) + .await + .unwrap(); + let directory = subsonic_json( + &router, + "getMusicDirectory", + api_key, + &format!("&id={artist_id}"), + ) + .await; + let children = directory["subsonic-response"]["directory"]["child"] + .as_array() + .unwrap() + .clone(); + let child = children + .iter() + .find(|child| child["id"] == id.to_string()) + .unwrap_or_else(|| panic!("the folder of {artist_id} has to list {id}")); + for key in ["recordLabels", "releaseTypes", "discTitles"] { + assert_eq!( + child[key], rendered[key], + "{key} must read the same under `child` as under `album`" + ); + } + // And a song under the same element name still carries none of them. + for song in children.iter().filter(|child| child["isDir"] == false) { + assert!(song["recordLabels"].is_null()); + } + } } #[tokio::test] From 88913b3fb8a84716393276cbcc29d7efa758b5e0 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 24 Aug 2026 21:36:29 +0200 Subject: [PATCH 2/4] test(subsonic): make the song assertion assert something The loop said "a song carries none of them" and checked one of the three. It also checked them on a folder that holds no song: an artist directory lists albums, every one of them `isDir`, so the filter for songs never matched and the loop never ran. Extending it to three keys would still have asserted nothing. It now browses the album's own folder, where the children genuinely are songs wearing the `child` element name, refuses an empty listing, and names all three keys. Confirmed by reintroducing the defect with the earlier per-song assertion neutralised, so this loop had to answer alone: `a song must not answer recordLabels: "Compilation track 0"`. Signed-off-by: InstaZDLL --- tests/v2_foundations.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index d4d4736..158c807 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -10841,9 +10841,25 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() { "{key} must read the same under `child` as under `album`" ); } - // And a song under the same element name still carries none of them. - for song in children.iter().filter(|child| child["isDir"] == false) { - assert!(song["recordLabels"].is_null()); + // And a song wearing that same element name carries none of the three. + // Browsed from the album's own folder rather than the artist's: an + // artist folder lists albums, so filtering it for songs would assert + // nothing at all. + let songs = + subsonic_json(&router, "getMusicDirectory", api_key, &format!("&id={id}")).await; + let songs = songs["subsonic-response"]["directory"]["child"] + .as_array() + .unwrap(); + assert!(!songs.is_empty(), "every album here has at least one track"); + for song in songs { + assert_eq!(song["isDir"], false); + for key in ["recordLabels", "releaseTypes", "discTitles"] { + assert!( + song[key].is_null(), + "a song must not answer {key}: {}", + song["title"] + ); + } } } } From 0e45486b418873d4611ae4ab631573ee0dec22df Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 24 Aug 2026 21:43:23 +0200 Subject: [PATCH 3/4] test(subsonic): assert a key is absent, not merely null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `value[key].is_null()` answers true for a key that is absent and for one explicitly set to `null`, and under the OpenSubsonic presence rule those say different things: absent means the server does not support the field, where a null would be a value it chose to send. The assertions meant the first and were written as the second. An `omits` helper names which one, and all six sites in this test move onto it — the three on a song, the two dates on an album with no release tags, and the one in the directory loop. Leaving two spellings of one intent in a single test is its own trap. The server emits no explicit null today, so nothing observable changes. That is precisely why the assertion has to say which it means: nothing would have caught the day it started to. Still confirmed to fail against the defect it guards: `assertion failed: omits(song, "recordLabels")`. Signed-off-by: InstaZDLL --- tests/v2_foundations.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index 158c807..0a1e590 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -3527,6 +3527,17 @@ async fn body_text(response: axum::response::Response) -> String { .unwrap() } +/// Whether a JSON object omits a key outright. +/// +/// `value[key].is_null()` cannot tell an absent key from one explicitly set to +/// `null`, and under the OpenSubsonic presence rule the two say different +/// things: absent means the server does not support the field at all, where a +/// null would be a value it chose to send. The server emits no explicit nulls +/// today, which is exactly why the assertion has to name which one it means. +fn omits(value: &serde_json::Value, key: &str) -> bool { + value.as_object().expect("a JSON object").get(key).is_none() +} + async fn subsonic_json( router: &axum::Router, method: &str, @@ -10744,9 +10755,9 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() { // an album inside a directory, and a recording answering `recordLabels: []` // would claim the server reads a label off a track. let song = &album["song"][0]; - assert!(song["recordLabels"].is_null()); - assert!(song["releaseTypes"].is_null()); - assert!(song["discTitles"].is_null()); + assert!(omits(song, "recordLabels")); + assert!(omits(song, "releaseTypes")); + assert!(omits(song, "discTitles")); // Its own arrays are still there, empty rather than absent. assert_eq!(song["moods"], serde_json::json!([])); @@ -10755,8 +10766,8 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() { assert_eq!(bare["recordLabels"], serde_json::json!([])); assert_eq!(bare["releaseTypes"], serde_json::json!([])); assert_eq!(bare["discTitles"], serde_json::json!([])); - assert!(bare["originalReleaseDate"].is_null()); - assert!(bare["releaseDate"].is_null()); + assert!(omits(bare, "originalReleaseDate")); + assert!(omits(bare, "releaseDate")); // A third album carrying exactly one of each. This is the shape the array // rule is for: with one child and no rule, a record label renders as a bare @@ -10855,7 +10866,7 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() { assert_eq!(song["isDir"], false); for key in ["recordLabels", "releaseTypes", "discTitles"] { assert!( - song[key].is_null(), + omits(song, key), "a song must not answer {key}: {}", song["title"] ); From ae260a49caa9bfed9e2478e84b6ee2a24332575c Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 24 Aug 2026 21:52:22 +0200 Subject: [PATCH 4/4] test(subsonic): drive the scans in the order the scanner drives them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bare album's scan never consolidated at all, and the two that did called it after `finish_scan_job` where the scanner calls it before — `consolidate_sort_names`, then `consolidate_catalog_derivations`, then the job is closed. A fixture that runs the pipeline in an order nothing runs proves something about that order rather than about the server. All three now match. `consolidate_sort_names` is deliberately left out, as it is in thirteen of the sixteen places this file consolidates: no assertion here reads a sort name. Nothing observable changed — the test passed before and passes now, and still fails against the defect it guards with `assertion failed: omits(song, "recordLabels")`. Signed-off-by: InstaZDLL --- tests/v2_foundations.rs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index 0a1e590..13420bd 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -10664,14 +10664,16 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() { .await .unwrap(); } - state.db.finish_scan_job(scan_id, 0).await.unwrap(); - // What a real scan does after applying its rows, and what makes an artist - // reachable as a folder. + // What a real scan does after applying its rows, before closing the job: + // it builds the artist index and the role statistics a folder listing + // reads. Driving the pipeline in another order would prove something about + // an order nothing runs. state .db .consolidate_catalog_derivations(library_id) .await .unwrap(); + state.db.finish_scan_job(scan_id, 0).await.unwrap(); let album_id = state .services @@ -10740,6 +10742,15 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() { .apply_catalog_track(library_id, bare_scan, &bare, None, false) .await .unwrap(); + // What a real scan does after applying its rows, before closing the job: + // it builds the artist index and the role statistics a folder listing + // reads. Driving the pipeline in another order would prove something about + // an order nothing runs. + state + .db + .consolidate_catalog_derivations(library_id) + .await + .unwrap(); state.db.finish_scan_job(bare_scan, 0).await.unwrap(); let bare_id = state .services @@ -10790,12 +10801,16 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() { .apply_catalog_track(library_id, solo_scan, &solo_input, None, false) .await .unwrap(); - state.db.finish_scan_job(solo_scan, 0).await.unwrap(); + // What a real scan does after applying its rows, before closing the job: + // it builds the artist index and the role statistics a folder listing + // reads. Driving the pipeline in another order would prove something about + // an order nothing runs. state .db .consolidate_catalog_derivations(library_id) .await .unwrap(); + state.db.finish_scan_job(solo_scan, 0).await.unwrap(); let solo_id = state .services .catalog_snapshot(owner, &[])