fix(subsonic): a song is not a release

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
 

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`. |

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;

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,
@@ -10653,6 +10664,15 @@ async fn an_album_reports_its_release_details_and_its_disc_titles() {
             .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(scan_id, 0).await.unwrap();
 
     let album_id = state
@@ -10722,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
@@ -10733,13 +10762,132 @@ 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!(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!([]));
+
     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!([]));
     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
+    // 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();
+    // 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, &[])
+        .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 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!(
+                    omits(song, key),
+                    "a song must not answer {key}: {}",
+                    song["title"]
+                );
+            }
+        }
+    }
 }
 
 #[tokio::test]