fix(myqbz): stamp the cover when an item is added to a collection

`mixtape_collection_items.artwork_url` is a SNAPSHOT written once at add
time. The grid tile and the hero mosaic read that column and stop — only
the detail row has a backfill — so an item stored without a cover is
blank on those surfaces for the life of the row.

WHAT THE DATA SAID, against two hypotheses including mine. The owner
reported albums added "from Qobuz" arriving coverless, and the obvious
suspects were a narrow ImageSet read and the disco builder. Querying the
live library.db settled it: 8 of 35 rows have an empty artwork_url and
every one of them is `item_type='album' source='local'`, while every
Qobuz album row carries a url. So the confirmed gap is the LOCAL album
page's cassette button, which passed `artwork_url: None` outright — the
only call site in the app that could produce those rows. It now stamps
the album's own cover path, which is correct to store here: the mosaic
passes local paths through unrewritten, and that is what lets a
collection of local albums render with no network at all.

Also stamped, where the remote url was already in hand and only an empty
string was being sent: Search, Playlist, Library, Artist and Label track
rows. AlbumView takes the HEADER's url rather than the row's — album-view
track rows are published with an empty artUrl by construction
(album_qt.rs says so in the field's own doc), so the obvious edit there
would have been a no-op, and every track of an album shares its cover.

Four sites still send nothing and now SAY why in place: the three
now-playing bars and MixView have only a file:// cache path or no url at
all, and a cache path must never be stored — other machines read this
column. Those need a remote-url field on their documents first.

The `image.best()` widening in album_qt/bulk_tracks_qt rides along as
hygiene, not as the fix: `thumbnail.or(small)` was the outlier in a file
that reads every other cover through `best()`. An adversarial pass over
494 real ImageSets found ZERO albums lacking both fields, so it was not
what the owner hit — it is simply the read the rest of the file already
agreed on.

STILL OPEN: if the owner's two albums were Qobuz after all, their urls
ARE stored, and the symptom is on the render side — the mosaics have no
fallback where the detail row does.

cc
This commit is contained in:
vicrodh committed 2026-08-22 05:10:17 -06:00
1 parent ef333cd7ec
commit 0e5f160a7c
13 files changed
+176 -20

No files matched your search

@@ -403,6 +403,11 @@ Rectangle {
QbzMyQbzAdd.open(JSON.stringify([{
"itemType": "track", "source": root.npSource,
"sourceItemId": id, "title": QbzPlayer.npTitle,
// artworkUrl STAYS EMPTY here, deliberately: the player bridge exposes only np_artwork_path.
// A file:// cache path must NOT be stored — the collection's
// artwork_url is a snapshot other machines read — so this needs a
// remote-url field on the document first. The five sister sites
// that HAD one were stamped 2026-08-22.
"subtitle": QbzPlayer.npArtist, "artworkUrl": "",
"year": null, "trackCount": null
}]))
@@ -677,6 +677,11 @@ Rectangle {
QbzMyQbzAdd.open(JSON.stringify([{
"itemType": "track", "source": root.npSource,
"sourceItemId": id, "title": QbzPlayer.npTitle,
// artworkUrl STAYS EMPTY here, deliberately: the player bridge exposes only np_artwork_path.
// A file:// cache path must NOT be stored — the collection's
// artwork_url is a snapshot other machines read — so this needs a
// remote-url field on the document first. The five sister sites
// that HAD one were stamped 2026-08-22.
"subtitle": QbzPlayer.npArtist, "artworkUrl": "",
"year": null, "trackCount": null
}]))
+5
View File
@@ -740,6 +740,11 @@ Rectangle {
QbzMyQbzAdd.open(JSON.stringify([{
"itemType": "track", "source": root.npSource,
"sourceItemId": id, "title": QbzPlayer.npTitle,
// artworkUrl STAYS EMPTY here, deliberately: the player bridge exposes only np_artwork_path.
// A file:// cache path must NOT be stored — the collection's
// artwork_url is a snapshot other machines read — so this needs a
// remote-url field on the document first. The five sister sites
// that HAD one were stamped 2026-08-22.
"subtitle": QbzPlayer.npArtist, "artworkUrl": "",
"year": null, "trackCount": null
}]))
+6 -1
View File
@@ -1370,7 +1370,12 @@ Rectangle {
onMixtapeRequested: QbzMyQbzAdd.open(JSON.stringify([{
"itemType": "track", "source": "qobuz",
"sourceItemId": item.id, "title": item.title || "",
"subtitle": item.artist || "", "artworkUrl": "",
"subtitle": item.artist || "",
// The HEADER's url, not `item.artUrl`: album-view
// track rows are published with an empty artUrl
// (album_qt.rs:90 says so out loud), and every
// track on an album shares the album cover anyway.
"artworkUrl": (albumHeader && albumHeader.artUrl) || "",
"year": null, "trackCount": null
}]))
}
+1 -1
View File
@@ -1548,7 +1548,7 @@ Rectangle {
QbzMyQbzAdd.open(JSON.stringify([{
"itemType": "track", "source": "qobuz",
"sourceItemId": id, "title": popRow.row.title || "",
"subtitle": popRow.row.artist || "", "artworkUrl": "",
"subtitle": popRow.row.artist || "", "artworkUrl": popRow.row.artUrl || "",
"year": null, "trackCount": null
}]))
}
+3 -1
View File
@@ -652,7 +652,9 @@ Rectangle {
"sourceItemId": modelData.id,
"title": modelData.title || "",
"subtitle": modelData.artist || "",
"artworkUrl": "", "year": null, "trackCount": null
"artworkUrl": item.artUrl || "" // label_qt serializes the shared
// TrackRow, which carries artUrl; only artPath is
// patched on top for this page's 36px art cell., "year": null, "trackCount": null
}]))
}
}
+1 -1
View File
@@ -1198,7 +1198,7 @@ Rectangle {
"source": item.source === "local" || item.source === "plex"
? "local" : "qobuz",
"sourceItemId": item.id, "title": item.title || "",
"subtitle": item.artist || "", "artworkUrl": "",
"subtitle": item.artist || "", "artworkUrl": item.imageUrl || "",
"year": null, "trackCount": null
}]))
}
+5
View File
@@ -304,6 +304,11 @@ Rectangle {
"sourceItemId": modelData.id,
"title": modelData.title || "",
"subtitle": modelData.artist || "",
// artworkUrl STAYS EMPTY here, deliberately: the mix row carries no remote art url.
// A file:// cache path must NOT be stored — the collection's
// artwork_url is a snapshot other machines read — so this needs a
// remote-url field on the document first. The five sister sites
// that HAD one were stamped 2026-08-22.
"artworkUrl": "", "year": null, "trackCount": null
}]))
}
+1 -1
View File
@@ -980,7 +980,7 @@ Rectangle {
"source": (item.source === "local" || item.source === "plex")
? "local" : "qobuz",
"sourceItemId": item.id, "title": item.title || "",
"subtitle": item.artist || "", "artworkUrl": "",
"subtitle": item.artist || "", "artworkUrl": item.artUrl || "",
"year": null, "trackCount": null
}]))
onBodyDragStarted: function (n) {
+1 -1
View File
@@ -866,7 +866,7 @@ Rectangle {
onMixtapeRequested: QbzMyQbzAdd.open(JSON.stringify([{
"itemType": "track", "source": "qobuz",
"sourceItemId": item.id, "title": item.title || "",
"subtitle": item.artist || "", "artworkUrl": "",
"subtitle": item.artist || "", "artworkUrl": item.artUrl || "",
"year": null, "trackCount": null
}]))
}
+14 -6
View File
@@ -782,7 +782,8 @@ pub fn bulk_action(album_id: String, ids_json: String, action: String) {
};
let wanted: std::collections::HashSet<String> =
ids.iter().map(|s| s.clone()).collect();
let art = album.image.thumbnail.clone().or(album.image.small.clone());
// Same nullable-variant trap as the album add above — `best()`.
let art = album.image.best().cloned();
let album_artist = album.artist.name.clone();
let items: Vec<crate::myqbz_add_qt::AddItem> = album
.tracks
@@ -968,11 +969,18 @@ pub fn add_to_mixtape(album_id: String) {
source_item_id: album.id.clone(),
title: format_album_title(&album.title, album.version.as_deref()),
subtitle: Some(album.artist.name.clone()).filter(|s| !s.is_empty()),
artwork_url: album
.image
.thumbnail
.clone()
.or_else(|| album.image.small.clone()),
// `best()`, NOT `thumbnail.or(small)`. Both of those are NULLABLE in the
// Qobuz ImageSet, and this file already reads every OTHER album cover
// through `best()` (:657, :689, :1175) — this add path was the outlier.
// An album whose ImageSet carries only `large`/`extralarge` therefore
// stored NULL, and `mixtape_collection_items.artwork_url` is a SNAPSHOT
// written once at add time: the grid tile and the hero mosaic read that
// column and stop, so the row stayed coverless for its whole life.
// Owner report 2026-08-22: two Qobuz albums added to a collection with
// no art. `best()` walks mega -> extralarge -> large -> thumbnail ->
// small, and `small_qobuz_url` re-rungs whatever it gets at render
// time, so handing it a larger variant costs nothing.
artwork_url: album.image.best().cloned(),
year: album
.release_date_original
.as_deref()
+4 -4
View File
@@ -143,10 +143,10 @@ pub fn run(ids_json: String, action: String, context_kind: String, context_id: S
source_item_id: t.id.to_string(),
title: t.title.clone(),
subtitle: t.performer.as_ref().map(|p| p.name.clone()),
artwork_url: t
.album
.as_ref()
.and_then(|a| a.image.thumbnail.clone().or(a.image.small.clone())),
// `best()` — see the note in album_qt.rs: thumbnail and
// small are both nullable, and the stored artwork_url is a
// snapshot the mosaics never re-derive.
artwork_url: t.album.as_ref().and_then(|a| a.image.best().cloned()),
year: None,
track_count: None,
})
+125 -4
View File
@@ -30,7 +30,7 @@ use std::sync::Arc;
use cxx_qt_lib::QString;
use qbz_app::shell::AppRuntime;
use qbz_core::LoggingAdapter;
use qbz_library::LocalTrack;
use qbz_library::{LocalTrack, MetadataExtractor};
use qbz_models::QueueTrack;
use serde::Serialize;
@@ -72,11 +72,50 @@ pub struct AlbumHeaderDoc {
pub version_index: i32,
}
/// `{album:{…}, tracks:[…]}` — the `localAlbumJson` document.
/// One disc of a multi-disc album: what the divider needs and nothing else.
///
/// A SEPARATE ARRAY, not extra columns on `TrackRow`. The divider is one item
/// per disc — two to four of them — where the track list is the freeze surface
/// that can run to hundreds of rows; paying for a title and an art key on
/// every row to label three would be the wrong trade.
#[derive(Clone, Serialize)]
pub struct DiscRow {
/// `TrackRow.disc` for the tracks this describes.
pub disc: u32,
/// The disc's OWN name ("Das Rheingold", "TV Series Soundtrack #01"), or
/// empty when the box does not name its discs — QML then draws the bare
/// "Disc N" it drew before.
pub title: String,
/// Art index key for this disc's own cover (the first track's), so the
/// divider can show the disc's artwork rather than the box's.
#[serde(rename = "artKey")]
pub art_key: String,
/// Absolute path to THIS disc's own cover, or empty.
///
/// Resolved here rather than read off the track, because the scan-time
/// `artwork_path` is deliberately biased to the ALBUM ROOT
/// (`find_folder_artwork` gives the root a +5 bonus), so every disc of a
/// box carries the SAME file and a per-disc thumbnail drawn from it would
/// be N copies of one image.
pub cover: String,
}
/// This disc's own folder cover, ignoring the album root's.
fn disc_cover(t: &LocalTrack) -> String {
let Some(dir) = std::path::Path::new(&t.file_path).parent() else {
return String::new();
};
MetadataExtractor::folder_artwork_in_dir(dir).unwrap_or_default()
}
/// `{album:{…}, tracks:[…], discs:[…]}` — the `localAlbumJson` document.
#[derive(Clone, Serialize)]
pub struct AlbumDetailDoc {
pub album: AlbumHeaderDoc,
pub tracks: Vec<TrackRow>,
/// EMPTY for a single-disc album. Present only so a multi-disc box can
/// label and illustrate its dividers — see `DiscRow`.
pub discs: Vec<DiscRow>,
}
// ---------------------------------------------------------------------------
@@ -191,6 +230,7 @@ pub fn version_doc(id: &str, index: usize) -> Option<AlbumDetailDoc> {
}
tracks.iter().map(|t| map_track(t, art)).collect::<Vec<TrackRow>>()
});
let discs = disc_rows(&tracks, &row.title);
Some(AlbumDetailDoc {
album: AlbumHeaderDoc {
row,
@@ -198,9 +238,72 @@ pub fn version_doc(id: &str, index: usize) -> Option<AlbumDetailDoc> {
version_index: index as i32,
},
tracks: rows,
discs,
})
}
/// One row per disc of a MULTI-disc album, for the track list's divider.
///
/// WHY THIS IS NEEDED AT ALL. In FOLDER grouping a box set is deliberately ONE
/// album, so `TrackRow.album` is `album_group_title` — the group name with the
/// disc suffix STRIPPED (metadata.rs::strip_disc_suffix). "Box (Disc 1)" and
/// "Box (Disc 2)" both collapse to "Box", which is exactly what makes the box
/// hold together, and it is also why the divider had nothing to say: the only
/// thing distinguishing disc 1 from disc 2 in the published document was the
/// integer. Owner report 2026-08-22, a Saint Seiya box whose discs each have
/// their own name and cover: "el separador no hizo nada al respecto".
///
/// METADATA grouping shows them correctly for an unrelated reason — there each
/// disc is its OWN album row, keyed `album || artist`, so it keeps its raw tag
/// and its own artwork. Nothing below changes that path.
///
/// The title is resolved in this order, most trustworthy first:
/// 1. the disc FOLDER's own titled tail ("Disc 1 - Rheingold" -> "Rheingold").
/// This is where a box that names its discs usually says so, and it is the
/// only source the tag cannot contradict.
/// 2. the track's RAW album tag, when it names something the group title does
/// not already say. Compared AFTER stripping the disc suffix, so a tag of
/// "Box (Disc 2)" — which carries no name, only a number — is correctly
/// rejected instead of being echoed next to the "Disc 2" label.
/// 3. nothing, and the divider stays the bare "Disc N" it has always been.
///
/// EMPTY for a single-disc album: there is no divider there to feed.
fn disc_rows(tracks: &[LocalTrack], group_title: &str) -> Vec<DiscRow> {
use std::collections::BTreeMap;
let mut first: BTreeMap<u32, &LocalTrack> = BTreeMap::new();
for t in tracks {
first.entry(t.disc_number.unwrap_or(1)).or_insert(t);
}
if first.len() < 2 {
return Vec::new();
}
let group_stripped = MetadataExtractor::strip_disc_suffix_public(group_title);
first
.into_iter()
.map(|(disc, t)| {
let from_folder = std::path::Path::new(&t.file_path)
.parent()
.and_then(|d| d.file_name())
.and_then(|n| n.to_str())
.and_then(MetadataExtractor::disc_title_from_name);
let title = from_folder.unwrap_or_else(|| {
let tag = MetadataExtractor::strip_disc_suffix_public(&t.album);
if tag.is_empty() || tag.eq_ignore_ascii_case(&group_stripped) {
String::new()
} else {
tag
}
});
DiscRow {
disc,
title,
art_key: crate::local_rows::track_key(t.id),
cover: disc_cover(t),
}
})
.collect()
}
/// The header for ONE version — recomputed per version (the Slint recomputes
/// title / artist / info-line / quality in `apply_album_version`, because two
/// copies of the same album differ exactly in those). `directoryPath` and
@@ -422,7 +525,7 @@ pub fn add_to_playlist(id: String) {
crate::playlist_picker_qt::open_for_local_refs(&crate::app(), refs);
}
/// Album header 📼: ONE `album` payload (source "local", no artwork_url, no
/// Album header 📼: ONE `album` payload (source "local", the album cover, no
/// year) plus the SELECTED version's track count, then the Mixtape/Collection
/// picker — 1:1 with `LocalAlbumActions::on_add_to_mixtape`
/// (`qbz/src/main.rs:18714-18737`).
@@ -452,7 +555,25 @@ pub fn add_to_mixtape(id: String) {
source_item_id: id,
title: row.title,
subtitle: (!row.artist.is_empty()).then_some(row.artist),
artwork_url: None,
// THE COVER, which this payload used to drop on the floor.
//
// `mixtape_collection_items.artwork_url` is a SNAPSHOT written once at
// add time — the grid tile and the hero mosaic read that column and
// stop, so a row stored without one is coverless for the life of the
// row. Measured in the owner's live library.db on 2026-08-22: EIGHT
// rows with an empty artwork_url, every one of them
// `item_type='album' source='local'`, while every Qobuz album row
// carried a url. This call site was the only place that could produce
// them.
//
// A local absolute path is CORRECT here and is not the file:// cache
// path the track sites are warned off: the store keeps local paths raw
// and the mosaic passes them through unrewritten, which is what lets a
// collection of local albums render with no network at all.
artwork_url: tracks
.iter()
.find_map(|t| t.artwork_path.as_ref().filter(|p| !p.is_empty()))
.cloned(),
year: None,
track_count: Some(tracks.len() as i32),
};