feat(models): Track.streamable becomes Option<bool>, and absence stops meaning dead
`#[serde(default)]` on a bare bool means an absent key deserializes to FALSE, so a catalog Track from any endpoint that omits `streamable` came back marked unavailable. The models' own tests document this as a hazard rather than a bug (the "streamable split": PurchaseTrack defaults true, catalog Track defaults false, and the comment warns that a wrong default "inverts that whole screen's behaviour"). The inferred OpenAPI disagrees with the code and documents `default: true`. That was survivable only because nothing read the field. It stops being survivable the moment a frontend greys rows out and keeps them out of the queue: one terse endpoint would blank an entire view, silently. `None` now means the endpoint made no claim, and only `Some(false)` marks a track pulled. `is_streamable()` is the ONE place absence is interpreted, and it resolves to available: refusing to play a track because a response was terse is strictly worse than playing one that turns out to be dead, which the reactive skip path already handles. Album.streamable was already Option<bool> with a doc comment naming the same causes, so this makes Track consistent rather than novel. Every consumer in the workspace moves to the helper: both matchers, the ISRC validator, the daemon driver, the purchases service (which maps catalog Track into PurchaseTrack and therefore feeds the very screen the split protects), the radio pool, the mixtape resolver and the QConnect renderer. The replaced tests pin both directions of the new contract. Also here, because it lives in the same file as the matcher's streamable skip: `rank_candidates`, an additive public entry point over the existing weighted matcher (ISRC short-circuit, title/artist/album weights, duration bonus, stop-word normalisation, quality tie-break). `match_tracks` is untouched. cc
This commit is contained in:
13 files changed
+445
-24
No files matched your search
@@ -30,6 +30,14 @@ pub struct PersistedQueueTrack {
|
|||||||
pub is_local: bool,
|
pub is_local: bool,
|
||||||
pub album_id: Option<String>,
|
pub album_id: Option<String>,
|
||||||
pub artist_id: Option<u64>,
|
pub artist_id: Option<u64>,
|
||||||
|
/// Resolved availability — absence was already interpreted upstream by
|
||||||
|
/// `qbz_models::Track::is_streamable`, so this stays a plain `bool` while
|
||||||
|
/// the catalog model is an `Option<bool>`, and `default_streamable` (TRUE)
|
||||||
|
/// covers a JSON snapshot written before the column existed. Values written
|
||||||
|
/// before the model became tri-state are untrustworthy and are cleared once
|
||||||
|
/// by the `user_version = 1` migration in `open_at`; from that point on
|
||||||
|
/// `false` here means only "Qobuz said no", which is a state a DOWNLOADED
|
||||||
|
/// track legitimately persists in.
|
||||||
#[serde(default = "default_streamable")]
|
#[serde(default = "default_streamable")]
|
||||||
pub streamable: bool,
|
pub streamable: bool,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
@@ -252,6 +260,51 @@ impl SessionStore {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── One-shot: distrust every `streamable` written before the model
|
||||||
|
// became tri-state ────────────────────────────────────────────────
|
||||||
|
//
|
||||||
|
// `queue_tracks.streamable` is persisted from `QueueTrack.streamable`,
|
||||||
|
// which was fed from `qbz_models::Track.streamable` — a plain `bool`
|
||||||
|
// whose `#[serde(default)]` turned an ABSENT key into `false`. So a row
|
||||||
|
// queued from any endpoint that omits the key was stored as `0` meaning
|
||||||
|
// "the payload was terse", not "Qobuz pulled this track".
|
||||||
|
//
|
||||||
|
// The unavailability work gives `0` a new, destructive meaning: an
|
||||||
|
// unavailable track never enters the queue. Left alone, those legacy
|
||||||
|
// zeroes would silently delete tracks from every restored session, for
|
||||||
|
// good — on a player whose entire point is not losing the user's music.
|
||||||
|
//
|
||||||
|
// The original value is unrecoverable (nothing on disk records WHY a
|
||||||
|
// row is 0), so the only honest repair is to clear the poison once and
|
||||||
|
// let the live API re-establish the truth on the next listing fetch,
|
||||||
|
// with the reactive auto-skip as the backstop for anything genuinely
|
||||||
|
// dead. Erring toward "available" here is the same asymmetry
|
||||||
|
// `Track::is_streamable` documents: a wasted round trip is cheap, a
|
||||||
|
// vanished track is not.
|
||||||
|
//
|
||||||
|
// It must be ONE-SHOT rather than a coercion on read, because after the
|
||||||
|
// model change a `0` is meaningful again: a track Qobuz pulled but the
|
||||||
|
// user already DOWNLOADED stays in the queue and plays from disk, and
|
||||||
|
// it stays there with `streamable = 0`. Coercing on read would
|
||||||
|
// resurrect that row as available forever and re-hide the very state
|
||||||
|
// the render is meant to distinguish.
|
||||||
|
//
|
||||||
|
// `PRAGMA user_version` is the guard — it is 0 on every database that
|
||||||
|
// predates this build and on a freshly created one, where the UPDATE
|
||||||
|
// simply matches no rows. The column-probe idiom used by the migrations
|
||||||
|
// above cannot express this: nothing is being added, only rewritten.
|
||||||
|
let schema_version: i64 = conn
|
||||||
|
.query_row("PRAGMA user_version", [], |row| row.get(0))
|
||||||
|
.unwrap_or(0);
|
||||||
|
if schema_version < 1 {
|
||||||
|
let _ = conn.execute_batch(
|
||||||
|
"
|
||||||
|
UPDATE queue_tracks SET streamable = 1 WHERE streamable = 0;
|
||||||
|
PRAGMA user_version = 1;
|
||||||
|
",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
Ok(Self { conn })
|
Ok(Self { conn })
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -597,6 +650,72 @@ mod tests {
|
|||||||
let _ = std::fs::remove_dir_all(dir);
|
let _ = std::fs::remove_dir_all(dir);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The `user_version = 1` repair clears legacy `streamable = 0` rows ONCE,
|
||||||
|
/// and never fires again.
|
||||||
|
///
|
||||||
|
/// Both halves matter and they pull in opposite directions. A `0` written
|
||||||
|
/// by an older build is untrustworthy — it may only mean the endpoint was
|
||||||
|
/// terse — and under the new queue filter it would silently delete the
|
||||||
|
/// track from every restored session. A `0` written AFTER the repair is
|
||||||
|
/// real: it is a track Qobuz pulled that the user has downloaded, which
|
||||||
|
/// stays in the queue and plays from disk. A coercion on read would satisfy
|
||||||
|
/// the first half and destroy the second, which is why this is a migration.
|
||||||
|
#[test]
|
||||||
|
fn legacy_streamable_zeroes_are_cleared_once_then_honoured() {
|
||||||
|
let dir = unique_test_dir("session-streamable-migration");
|
||||||
|
|
||||||
|
let session_with_sample = || {
|
||||||
|
let mut session = PersistedSessionSnapshot::default();
|
||||||
|
session.playback.queue_tracks = vec![sample_track()];
|
||||||
|
session.playback.current_index = Some(0);
|
||||||
|
session
|
||||||
|
};
|
||||||
|
|
||||||
|
// A database as an older build left it: a row at `streamable = 0` and
|
||||||
|
// no schema stamp. Rewinding the pragma is what makes it "older" —
|
||||||
|
// `sample_track()` already carries `streamable: false`.
|
||||||
|
{
|
||||||
|
let store = SessionStore::new_at(&dir).expect("open store");
|
||||||
|
store
|
||||||
|
.save_session(&session_with_sample())
|
||||||
|
.expect("save legacy session");
|
||||||
|
store
|
||||||
|
.conn
|
||||||
|
.execute_batch("PRAGMA user_version = 0;")
|
||||||
|
.expect("rewind schema stamp");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Reopening runs the repair: the untrustworthy 0 is cleared.
|
||||||
|
{
|
||||||
|
let store = SessionStore::new_at(&dir).expect("reopen store");
|
||||||
|
let loaded = store.load_session().expect("load repaired session");
|
||||||
|
assert!(
|
||||||
|
loaded.playback.queue_tracks[0].streamable,
|
||||||
|
"a pre-migration 0 is untrustworthy and must be cleared, not \
|
||||||
|
read as 'Qobuz pulled this track'"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Now write a 0 that IS trustworthy — the downloaded-but-pulled
|
||||||
|
// state the render distinguishes.
|
||||||
|
store
|
||||||
|
.save_session(&session_with_sample())
|
||||||
|
.expect("save post-migration session");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Reopening again must leave it alone: the repair is one-shot.
|
||||||
|
{
|
||||||
|
let store = SessionStore::new_at(&dir).expect("reopen store again");
|
||||||
|
let loaded = store.load_session().expect("load session");
|
||||||
|
assert!(
|
||||||
|
!loaded.playback.queue_tracks[0].streamable,
|
||||||
|
"the repair must not fire twice — a post-migration 0 is real \
|
||||||
|
and resurrecting it would hide a downloaded-but-pulled track"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
let _ = std::fs::remove_dir_all(dir);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn quick_saves_update_only_targeted_playback_fields() {
|
fn quick_saves_update_only_targeted_playback_fields() {
|
||||||
let dir = unique_test_dir("session-quick-save");
|
let dir = unique_test_dir("session-quick-save");
|
||||||
|
|||||||
@@ -1291,6 +1291,27 @@ impl<A: FrontendAdapter + Send + Sync + 'static> QbzCore<A> {
|
|||||||
.map_err(CoreError::Api)
|
.map_err(CoreError::Api)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Move tracks within a playlist (the replacement flow's position step).
|
||||||
|
///
|
||||||
|
/// Takes MEMBERSHIP ids, like [`Self::remove_tracks_from_playlist`].
|
||||||
|
/// `insert_before` is the 0-based slot to land in front of. Callers must
|
||||||
|
/// treat a failure as non-fatal: position is a nicety, and the flows that
|
||||||
|
/// use this have already made a write that must not be rolled back.
|
||||||
|
pub async fn update_playlist_tracks_position(
|
||||||
|
&self,
|
||||||
|
playlist_id: u64,
|
||||||
|
playlist_track_ids: &[u64],
|
||||||
|
insert_before: u32,
|
||||||
|
) -> Result<(), CoreError> {
|
||||||
|
let client = self.client.read().await;
|
||||||
|
let client = client.as_ref().ok_or(CoreError::NotInitialized)?;
|
||||||
|
|
||||||
|
client
|
||||||
|
.update_playlist_tracks_position(playlist_id, playlist_track_ids, insert_before)
|
||||||
|
.await
|
||||||
|
.map_err(CoreError::Api)
|
||||||
|
}
|
||||||
|
|
||||||
/// Create a new playlist
|
/// Create a new playlist
|
||||||
pub async fn create_playlist(
|
pub async fn create_playlist(
|
||||||
&self,
|
&self,
|
||||||
@@ -3450,7 +3471,9 @@ mod tests {
|
|||||||
hires_streamable: false,
|
hires_streamable: false,
|
||||||
maximum_sampling_rate: None,
|
maximum_sampling_rate: None,
|
||||||
maximum_bit_depth: None,
|
maximum_bit_depth: None,
|
||||||
streamable: false,
|
// `None` = the fixture makes no availability claim, which is the
|
||||||
|
// honest value: nothing in `track_with`'s callers reads it.
|
||||||
|
streamable: None,
|
||||||
parental_warning: false,
|
parental_warning: false,
|
||||||
playlist_track_id: None,
|
playlist_track_id: None,
|
||||||
performers: None,
|
performers: None,
|
||||||
|
|||||||
@@ -28,6 +28,10 @@ use qbz_models::{Album, Artist, Track};
|
|||||||
|
|
||||||
pub use cache::RecoCache;
|
pub use cache::RecoCache;
|
||||||
pub use carousels::{compose_artist_rails, ArtistRailComposition, ARTIST_DISPLAY_CAP};
|
pub use carousels::{compose_artist_rails, ArtistRailComposition, ARTIST_DISPLAY_CAP};
|
||||||
|
// `validate` stays a private module — only this one helper is public, because
|
||||||
|
// the unavailable-track replacement flow (qbz-qt) needs the ISRC -> streamable
|
||||||
|
// -track lookup and nothing else in it.
|
||||||
|
pub use validate::find_by_isrc;
|
||||||
pub use types::{
|
pub use types::{
|
||||||
AlbumReco, ArtistReco, ExternalCarousels, ExtHistory, LocalHistory, RecoSource, TrackReco,
|
AlbumReco, ArtistReco, ExternalCarousels, ExtHistory, LocalHistory, RecoSource, TrackReco,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -5,7 +5,7 @@
|
|||||||
//! duration/isrc) against a `qbz_models::Track`. Kept self-contained here so the
|
//! duration/isrc) against a `qbz_models::Track`. Kept self-contained here so the
|
||||||
//! engine has no dependency on the importer crate; the algorithm is identical
|
//! engine has no dependency on the importer crate; the algorithm is identical
|
||||||
//! (ISRC short-circuit -> title*0.6 + artist*0.3 + album*0.1 + duration bonus,
|
//! (ISRC short-circuit -> title*0.6 + artist*0.3 + album*0.1 + duration bonus,
|
||||||
//! `!streamable` skipped, hi-res tiebreak).
|
//! `!is_streamable()` skipped, hi-res tiebreak).
|
||||||
|
|
||||||
use qbz_models::Track;
|
use qbz_models::Track;
|
||||||
|
|
||||||
@@ -33,7 +33,10 @@ pub fn select_best_match<'a>(input: &MatchInput, candidates: &'a [Track]) -> (Op
|
|||||||
let mut best_quality = 0.0f32;
|
let mut best_quality = 0.0f32;
|
||||||
|
|
||||||
for candidate in candidates {
|
for candidate in candidates {
|
||||||
if !candidate.streamable {
|
// Only an explicit `streamable: false` disqualifies; an endpoint that
|
||||||
|
// omits the key must not silently empty the candidate set. Kept
|
||||||
|
// identical to the importer's scorer, which this file mirrors.
|
||||||
|
if !candidate.is_streamable() {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
let score = score_candidate(input, candidate);
|
let score = score_candidate(input, candidate);
|
||||||
|
|||||||
@@ -95,10 +95,27 @@ fn build_track_reco(track: &Track, source: RecoSource) -> TrackReco {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn find_by_isrc(catalog: &dyn RecoCatalog, isrc: &str) -> Option<Track> {
|
/// ISRC -> the streamable Qobuz track carrying it, or `None`.
|
||||||
|
///
|
||||||
|
/// Public because it is the FIRST thing the "find available version" flow tries
|
||||||
|
/// on a track Qobuz pulled from the catalogue: a dead row keeps its `isrc` (the
|
||||||
|
/// 2026-08-17 capture of album `0886443985094` confirms it), and licensing churn
|
||||||
|
/// most often re-publishes the SAME recording under a new track id with the same
|
||||||
|
/// ISRC. A hit there is an exact relink, and it skips the fuzzy text search and
|
||||||
|
/// the human judgement entirely.
|
||||||
|
///
|
||||||
|
/// Qobuz has no ISRC endpoint — this is a free-text search FOR the ISRC string
|
||||||
|
/// plus an exact, case-folded verification of the returned `isrc`, which is why
|
||||||
|
/// it can answer `None` for an ISRC that does exist. Treat a miss as "no exact
|
||||||
|
/// relink", never as "the recording is gone".
|
||||||
|
pub async fn find_by_isrc(catalog: &dyn RecoCatalog, isrc: &str) -> Option<Track> {
|
||||||
let results = catalog.search_tracks(isrc, 5).await;
|
let results = catalog.search_tracks(isrc, 5).await;
|
||||||
results.into_iter().find(|t| {
|
results.into_iter().find(|t| {
|
||||||
t.streamable
|
// `is_streamable()`, not the raw field: a search result that simply did
|
||||||
|
// not report availability is still a valid relink target, and this
|
||||||
|
// lookup is the cheapest, most certain path there is (same ISRC = same
|
||||||
|
// recording under a new id). Only an explicit false is rejected.
|
||||||
|
t.is_streamable()
|
||||||
&& t.isrc
|
&& t.isrc
|
||||||
.as_deref()
|
.as_deref()
|
||||||
.map(|c| c.eq_ignore_ascii_case(isrc))
|
.map(|c| c.eq_ignore_ascii_case(isrc))
|
||||||
|
|||||||
@@ -275,7 +275,10 @@ pub async fn resolve_qobuz_album(
|
|||||||
is_local: false,
|
is_local: false,
|
||||||
album_id: Some(album_id_str.clone()),
|
album_id: Some(album_id_str.clone()),
|
||||||
artist_id,
|
artist_id,
|
||||||
streamable: track.streamable,
|
// Resolved once here: `CoreQueueTrack.streamable` is a plain
|
||||||
|
// `bool`, and absence on the catalog `Track` means "the
|
||||||
|
// endpoint was terse", not "pulled" (`Track::is_streamable`).
|
||||||
|
streamable: track.is_streamable(),
|
||||||
source: Some("qobuz".to_string()),
|
source: Some("qobuz".to_string()),
|
||||||
parental_warning: track.parental_warning,
|
parental_warning: track.parental_warning,
|
||||||
// Stamped centrally by resolve_collection_tracks; left None here.
|
// Stamped centrally by resolve_collection_tracks; left None here.
|
||||||
@@ -462,7 +465,8 @@ pub fn track_to_queue_track_from_api(track: &ApiTrack) -> CoreQueueTrack {
|
|||||||
is_local: false,
|
is_local: false,
|
||||||
album_id,
|
album_id,
|
||||||
artist_id,
|
artist_id,
|
||||||
streamable: track.streamable,
|
// Same resolution as the album path above, single-track flavour.
|
||||||
|
streamable: track.is_streamable(),
|
||||||
source: Some("qobuz".to_string()),
|
source: Some("qobuz".to_string()),
|
||||||
parental_warning: track.parental_warning,
|
parental_warning: track.parental_warning,
|
||||||
source_item_id_hint: None,
|
source_item_id_hint: None,
|
||||||
|
|||||||
@@ -490,8 +490,32 @@ pub struct Track {
|
|||||||
pub hires_streamable: bool,
|
pub hires_streamable: bool,
|
||||||
pub maximum_sampling_rate: Option<f64>,
|
pub maximum_sampling_rate: Option<f64>,
|
||||||
pub maximum_bit_depth: Option<u32>,
|
pub maximum_bit_depth: Option<u32>,
|
||||||
|
/// Whether Qobuz will stream this track today.
|
||||||
|
///
|
||||||
|
/// `None` means the endpoint did not report availability — it is **NOT** a
|
||||||
|
/// claim of unavailability. Only `Some(false)` marks a track as pulled from
|
||||||
|
/// the catalogue. The distinction is load-bearing: this field used to be a
|
||||||
|
/// plain `bool` with a bare `#[serde(default)]`, and that collapses an
|
||||||
|
/// ABSENT key into `false`, so every track returned by an endpoint that
|
||||||
|
/// omits it came back marked unavailable. That was harmless while nothing
|
||||||
|
/// rendered the flag; it stops being harmless the moment the flag greys a
|
||||||
|
/// row out and keeps it out of the queue, where a single terse endpoint
|
||||||
|
/// would silently take out an entire view.
|
||||||
|
///
|
||||||
|
/// `/album/get` does send the key (captured 2026-08-17, album
|
||||||
|
/// `0886443985094`, where track 1 is `false` while the ALBUM around it is
|
||||||
|
/// `true` — the album-level flag can never stand in for this one).
|
||||||
|
/// `/playlist/get?extra=tracks` has never been captured, and playlists are
|
||||||
|
/// where dead tracks show up most; that unverified endpoint is exactly what
|
||||||
|
/// this `Option` guards against.
|
||||||
|
///
|
||||||
|
/// Never read the field directly — go through [`Track::is_streamable`],
|
||||||
|
/// which is the one place absence is interpreted.
|
||||||
|
///
|
||||||
|
/// Mirrors [`Album::streamable`], which has been `Option<bool>` all along
|
||||||
|
/// for the same reasons; the two shapes are now consistent.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub streamable: bool,
|
pub streamable: Option<bool>,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub parental_warning: bool,
|
pub parental_warning: bool,
|
||||||
/// Playlist-specific: ID within the playlist (for removal)
|
/// Playlist-specific: ID within the playlist (for removal)
|
||||||
@@ -504,6 +528,33 @@ pub struct Track {
|
|||||||
pub copyright: Option<String>,
|
pub copyright: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl Track {
|
||||||
|
/// The ONE place an absent `streamable` is interpreted: **absent means
|
||||||
|
/// available**.
|
||||||
|
///
|
||||||
|
/// The asymmetry is deliberate, and it is the whole reason the field is an
|
||||||
|
/// `Option` rather than a `bool` with a convenient default.
|
||||||
|
///
|
||||||
|
/// Refusing to play a track because an endpoint was terse is strictly worse
|
||||||
|
/// than offering one that turns out to be dead. The second case is already
|
||||||
|
/// caught downstream by the reactive path — the play fails, the error is
|
||||||
|
/// recognised as terminal (`is_terminal_unavailable`), and the bounded
|
||||||
|
/// auto-skip moves on — so it costs one failed round trip and one toast,
|
||||||
|
/// and the user hears the next song. The first case has no recovery at all:
|
||||||
|
/// the user is shown a greyed, unplayable catalogue, no request is ever
|
||||||
|
/// made, nothing fails loudly, and there is no signal anywhere that would
|
||||||
|
/// tell them (or us) that the data was merely missing rather than the music
|
||||||
|
/// gone. On a music player, silent refusal is the unrecoverable failure and
|
||||||
|
/// a wasted request is the cheap one.
|
||||||
|
///
|
||||||
|
/// Call this instead of touching `streamable` directly. A hand-written
|
||||||
|
/// `unwrap_or` at a call site is how the two halves of this rule drift
|
||||||
|
/// apart, and the drift is invisible until a whole view goes grey.
|
||||||
|
pub fn is_streamable(&self) -> bool {
|
||||||
|
self.streamable.unwrap_or(true)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Album summary (embedded in track responses)
|
/// Album summary (embedded in track responses)
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||||
pub struct AlbumSummary {
|
pub struct AlbumSummary {
|
||||||
@@ -1845,6 +1896,15 @@ mod purchase_deserializer_tests {
|
|||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
// ── §2.6: the `streamable` split ─────────────────────────────────────────
|
// ── §2.6: the `streamable` split ─────────────────────────────────────────
|
||||||
|
//
|
||||||
|
// Two structs, same field name, three semantics. `PurchaseTrack.streamable`
|
||||||
|
// is a `bool` defaulting TRUE (`serde_true`) — the purchases endpoints are
|
||||||
|
// terse and that screen gates click-to-play on the flag, so a default of
|
||||||
|
// false would make every purchased album unclickable. Catalog
|
||||||
|
// `Track.streamable` is now `Option<bool>`: it does not guess in EITHER
|
||||||
|
// direction, because the greyout and the queue filter key on it and a wrong
|
||||||
|
// guess is visible on every screen in the app. Absence is resolved in
|
||||||
|
// exactly one function, `Track::is_streamable`, and nowhere else.
|
||||||
|
|
||||||
/// A purchases-list track defaults `streamable` to **true**.
|
/// A purchases-list track defaults `streamable` to **true**.
|
||||||
#[test]
|
#[test]
|
||||||
@@ -1856,18 +1916,44 @@ mod purchase_deserializer_tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A CATALOG track — the one `/album/get` returns, which is what builds the
|
/// A CATALOG track that does not mention `streamable` deserializes to
|
||||||
/// album-detail screen — defaults `streamable` to **false**.
|
/// `None`, and `None` reads as **available**.
|
||||||
///
|
///
|
||||||
/// These two together are the trap: the detail screen gates click-to-play on
|
/// This is the regression that would otherwise grey out the whole
|
||||||
/// `streamable`, so a blanket "streamable defaults true" inverts that whole
|
/// catalogue. While the field was a `bool` with a bare `#[serde(default)]`,
|
||||||
/// screen's behaviour, and nobody here can click a purchased album to notice.
|
/// an endpoint that omits the key produced `false` — "pulled from Qobuz" —
|
||||||
|
/// for every track it returned. `/album/get` does send the key (captured
|
||||||
|
/// 2026-08-17), but `/playlist/get?extra=tracks` never has been, and that is
|
||||||
|
/// the surface where dead tracks actually show up.
|
||||||
#[test]
|
#[test]
|
||||||
fn catalog_track_streamable_defaults_false() {
|
fn catalog_track_streamable_absent_is_none_and_reads_available() {
|
||||||
let t: Track = serde_json::from_str(r#"{"id":1,"title":"T"}"#).unwrap();
|
let t: Track = serde_json::from_str(r#"{"id":1,"title":"T"}"#).unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
t.streamable, None,
|
||||||
|
"an absent key must stay absent — never collapse into Some(false)"
|
||||||
|
);
|
||||||
assert!(
|
assert!(
|
||||||
!t.streamable,
|
t.is_streamable(),
|
||||||
"catalog Track.streamable defaults FALSE — the detail screen depends on it"
|
"absence is NOT a claim of unavailability: absent reads as available"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// An explicit `"streamable": false` is honoured, and it is the ONLY shape
|
||||||
|
/// that marks a track as pulled.
|
||||||
|
///
|
||||||
|
/// This is the real wire signal, captured from
|
||||||
|
/// `/album/get?album_id=0886443985094` on 2026-08-17: track 1 carries
|
||||||
|
/// `false` (and keeps its `isrc`, which is what makes the replacement
|
||||||
|
/// search's ISRC short-circuit reachable) while the album around it is
|
||||||
|
/// `true`.
|
||||||
|
#[test]
|
||||||
|
fn catalog_track_streamable_false_is_honoured() {
|
||||||
|
let t: Track =
|
||||||
|
serde_json::from_str(r#"{"id":1,"title":"T","streamable":false}"#).unwrap();
|
||||||
|
assert_eq!(t.streamable, Some(false));
|
||||||
|
assert!(
|
||||||
|
!t.is_streamable(),
|
||||||
|
"Some(false) is the ONLY shape that marks a track unavailable"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -357,7 +357,15 @@ pub fn build_purchase_album(
|
|||||||
hires: track.hires,
|
hires: track.hires,
|
||||||
maximum_sampling_rate: track.maximum_sampling_rate,
|
maximum_sampling_rate: track.maximum_sampling_rate,
|
||||||
maximum_bit_depth: track.maximum_bit_depth,
|
maximum_bit_depth: track.maximum_bit_depth,
|
||||||
streamable: track.streamable,
|
// `is_streamable()`, and the choice is load-bearing here:
|
||||||
|
// `PurchaseTrack.streamable` defaults TRUE on purpose (the
|
||||||
|
// §2.6 split in `qbz-models`) so a terse purchases payload
|
||||||
|
// never makes a purchased row unclickable — and this bridge
|
||||||
|
// was handing it the catalog `bool`, whose absence meant
|
||||||
|
// `false`, re-introducing exactly that inversion through
|
||||||
|
// the back door. Nobody would have caught it by clicking:
|
||||||
|
// Qobuz Purchases is not sold in the owner's region.
|
||||||
|
streamable: track.is_streamable(),
|
||||||
// Server-derived; set below from the registry.
|
// Server-derived; set below from the registry.
|
||||||
downloaded: false,
|
downloaded: false,
|
||||||
downloaded_format_ids: Vec::new(),
|
downloaded_format_ids: Vec::new(),
|
||||||
|
|||||||
@@ -33,6 +33,10 @@ mod http;
|
|||||||
|
|
||||||
pub use errors::PlaylistImportError;
|
pub use errors::PlaylistImportError;
|
||||||
pub use importer::{import_public_playlist, preview_public_playlist};
|
pub use importer::{import_public_playlist, preview_public_playlist};
|
||||||
|
// The Qobuz matcher's public surface. `match_tracks` stays the importer's own
|
||||||
|
// entry point; `rank_candidates` is the ranked-list form the "find available
|
||||||
|
// version" replacement flow needs (match_qobuz.rs).
|
||||||
|
pub use match_qobuz::{rank_candidates, MIN_MATCH_SCORE};
|
||||||
pub use models::{
|
pub use models::{
|
||||||
ImportPlaylist, ImportProgress, ImportProvider, ImportSummary, ImportTrack, TrackMatch,
|
ImportPlaylist, ImportProgress, ImportProvider, ImportSummary, ImportTrack, TrackMatch,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -132,7 +132,12 @@ fn select_best_match<'a>(track: &ImportTrack, candidates: &'a [Track]) -> (Optio
|
|||||||
let mut best_quality = 0.0f32;
|
let mut best_quality = 0.0f32;
|
||||||
|
|
||||||
for candidate in candidates {
|
for candidate in candidates {
|
||||||
if !candidate.streamable {
|
// Absence is not unavailability: only an explicit `streamable: false`
|
||||||
|
// disqualifies a candidate. Before `Track.streamable` became
|
||||||
|
// `Option<bool>`, a `/track/search` payload that omitted the key
|
||||||
|
// disqualified EVERY candidate, so the import matched nothing at all —
|
||||||
|
// and did it silently, since "no match" is a normal import outcome.
|
||||||
|
if !candidate.is_streamable() {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -152,6 +157,69 @@ fn select_best_match<'a>(track: &ImportTrack, candidates: &'a [Track]) -> (Optio
|
|||||||
(best, best_score)
|
(best, best_score)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The confidence floor [`match_tracks`] gates on, exposed so a UI that shows
|
||||||
|
/// the RANKED list can label a weak candidate instead of silently offering it
|
||||||
|
/// as if it were a match. Nothing here applies it — the importer still owns
|
||||||
|
/// that decision, and the replacement flow deliberately shows sub-threshold
|
||||||
|
/// rows because a human confirms the swap.
|
||||||
|
pub const MIN_MATCH_SCORE: f32 = MIN_SCORE;
|
||||||
|
|
||||||
|
/// Every candidate for `track`, best first — the ordering [`select_best_match`]
|
||||||
|
/// picks its winner out of, handed over whole instead of collapsed to one row.
|
||||||
|
///
|
||||||
|
/// This exists for the "find an available version" flow of a track Qobuz pulled
|
||||||
|
/// from the catalogue. There a HUMAN confirms the swap, so the surface needs the
|
||||||
|
/// order and every candidate's score, not just the winner; the reference (Tauri)
|
||||||
|
/// showed Qobuz's raw relevance order with no scoring at all, which is what this
|
||||||
|
/// replaces.
|
||||||
|
///
|
||||||
|
/// Purely ADDITIVE by construction: [`match_tracks`] and [`select_best_match`]
|
||||||
|
/// are untouched, so the importer's behaviour cannot move because of it.
|
||||||
|
///
|
||||||
|
/// The same three rules as `select_best_match`, in the same order, deliberately:
|
||||||
|
///
|
||||||
|
/// 1. Non-streamable candidates are DROPPED, not ranked low. Offering a
|
||||||
|
/// replacement that is itself unplayable is the one outcome the whole
|
||||||
|
/// feature exists to prevent.
|
||||||
|
/// 2. The score is [`score_candidate`] verbatim — the ISRC short-circuit to
|
||||||
|
/// 1.0 first, then title 0.6 / artist 0.3 / album 0.1 plus the duration
|
||||||
|
/// bonus.
|
||||||
|
/// 3. Equal scores are broken by [`quality_score`] (bit depth first, sample
|
||||||
|
/// rate second). That is the owner's *"eliminan una calidad y agregan
|
||||||
|
/// otra"* case: the same recording re-published one tier up or down.
|
||||||
|
///
|
||||||
|
/// # Why the score is BUCKETED rather than compared with a tolerance
|
||||||
|
///
|
||||||
|
/// `select_best_match` treats scores within `0.01` as equal. Written directly
|
||||||
|
/// into a comparator that window is NOT transitive (a≈b, b≈c, a<c), and since
|
||||||
|
/// Rust 1.81 `slice::sort_by` may PANIC on a comparator that does not implement
|
||||||
|
/// a total order. Rounding to hundredths first is the same window expressed as
|
||||||
|
/// a key, so the ordering is total and the head of this list is still the track
|
||||||
|
/// `select_best_match` would have returned.
|
||||||
|
///
|
||||||
|
/// The sort is STABLE, so candidates tied on both score and quality keep
|
||||||
|
/// Qobuz's own relevance order — the reference's ordering, kept as the last
|
||||||
|
/// tiebreak instead of being scrambled.
|
||||||
|
pub fn rank_candidates(track: &ImportTrack, candidates: &[Track]) -> Vec<(Track, f32)> {
|
||||||
|
let mut ranked: Vec<(Track, f32)> = candidates
|
||||||
|
.iter()
|
||||||
|
.filter(|candidate| candidate.is_streamable())
|
||||||
|
.map(|candidate| (candidate.clone(), score_candidate(track, candidate)))
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
// The 0.01 equality window of `select_best_match`, as a transitive key.
|
||||||
|
fn bucket(score: f32) -> i32 {
|
||||||
|
(score * 100.0).round() as i32
|
||||||
|
}
|
||||||
|
|
||||||
|
ranked.sort_by(|(a_track, a_score), (b_track, b_score)| {
|
||||||
|
bucket(*b_score)
|
||||||
|
.cmp(&bucket(*a_score))
|
||||||
|
.then_with(|| quality_score(b_track).total_cmp(&quality_score(a_track)))
|
||||||
|
});
|
||||||
|
ranked
|
||||||
|
}
|
||||||
|
|
||||||
fn score_candidate(track: &ImportTrack, candidate: &Track) -> f32 {
|
fn score_candidate(track: &ImportTrack, candidate: &Track) -> f32 {
|
||||||
if let (Some(isrc), Some(candidate_isrc)) = (&track.isrc, &candidate.isrc) {
|
if let (Some(isrc), Some(candidate_isrc)) = (&track.isrc, &candidate.isrc) {
|
||||||
if isrc.eq_ignore_ascii_case(candidate_isrc) {
|
if isrc.eq_ignore_ascii_case(candidate_isrc) {
|
||||||
@@ -341,7 +409,7 @@ mod tests {
|
|||||||
hires_streamable: false,
|
hires_streamable: false,
|
||||||
maximum_sampling_rate: None,
|
maximum_sampling_rate: None,
|
||||||
maximum_bit_depth: None,
|
maximum_bit_depth: None,
|
||||||
streamable: true,
|
streamable: Some(true),
|
||||||
parental_warning: false,
|
parental_warning: false,
|
||||||
playlist_track_id: None,
|
playlist_track_id: None,
|
||||||
performers: None,
|
performers: None,
|
||||||
@@ -483,7 +551,9 @@ mod tests {
|
|||||||
fn select_best_match_skips_non_streamable() {
|
fn select_best_match_skips_non_streamable() {
|
||||||
let source = import_track("hey jude", "the beatles");
|
let source = import_track("hey jude", "the beatles");
|
||||||
let mut perfect = qobuz_track(1, "hey jude", "the beatles");
|
let mut perfect = qobuz_track(1, "hey jude", "the beatles");
|
||||||
perfect.streamable = false;
|
// Explicit `Some(false)` — the skip must fire on the server SAYING no,
|
||||||
|
// never on a payload that simply stayed quiet (see `is_streamable`).
|
||||||
|
perfect.streamable = Some(false);
|
||||||
let weaker = qobuz_track(2, "hey jude na na", "the beatles");
|
let weaker = qobuz_track(2, "hey jude na na", "the beatles");
|
||||||
|
|
||||||
let candidates = [perfect, weaker];
|
let candidates = [perfect, weaker];
|
||||||
@@ -535,6 +605,74 @@ mod tests {
|
|||||||
assert_eq!(score, 0.0);
|
assert_eq!(score, 0.0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── rank_candidates ──
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rank_candidates_drops_non_streamable() {
|
||||||
|
// The dropped one is the PERFECT match, so a rank that merely scored it
|
||||||
|
// low would still surface it first — only an actual drop passes.
|
||||||
|
let source = import_track("hey jude", "the beatles");
|
||||||
|
let mut dead = qobuz_track(1, "hey jude", "the beatles");
|
||||||
|
dead.streamable = Some(false);
|
||||||
|
let live = qobuz_track(2, "hey jude na na", "the beatles");
|
||||||
|
|
||||||
|
let ranked = rank_candidates(&source, &[dead, live]);
|
||||||
|
assert_eq!(ranked.len(), 1);
|
||||||
|
assert_eq!(ranked[0].0.id, 2);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rank_candidates_head_is_select_best_match() {
|
||||||
|
// The contract that lets the modal preselect row 0: the head of the
|
||||||
|
// ranked list IS the winner the importer would have taken.
|
||||||
|
let source = import_track("hey jude", "the beatles");
|
||||||
|
let weak = qobuz_track(1, "let it be", "the beatles");
|
||||||
|
let strong = qobuz_track(2, "hey jude", "the beatles");
|
||||||
|
|
||||||
|
let candidates = [weak, strong];
|
||||||
|
let (best, _) = select_best_match(&source, &candidates);
|
||||||
|
let ranked = rank_candidates(&source, &candidates);
|
||||||
|
assert_eq!(ranked[0].0.id, best.map(|t| t.id).unwrap());
|
||||||
|
assert!(ranked[0].1 > ranked[1].1);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rank_candidates_isrc_hit_leads_with_score_one() {
|
||||||
|
// The "same recording, new album id" case: a candidate whose TEXT is
|
||||||
|
// wrong but whose ISRC matches must still lead at 1.0.
|
||||||
|
let mut source = import_track("Completely Different", "Nobody");
|
||||||
|
source.isrc = Some("uskO11600123".to_string());
|
||||||
|
let mut relink = qobuz_track(1, "Other Title", "Other Artist");
|
||||||
|
relink.isrc = Some("USKO11600123".to_string());
|
||||||
|
let decoy = qobuz_track(2, "Completely Different", "Nobody");
|
||||||
|
|
||||||
|
let ranked = rank_candidates(&source, &[decoy, relink]);
|
||||||
|
assert_eq!(ranked[0].0.id, 1);
|
||||||
|
assert_eq!(ranked[0].1, 1.0);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rank_candidates_equal_scores_prefer_the_better_tier() {
|
||||||
|
// The owner's "eliminan una calidad y agregan otra" case, and the reason
|
||||||
|
// the score is bucketed: these two score identically.
|
||||||
|
let source = import_track("hey jude", "the beatles");
|
||||||
|
let mut cd = qobuz_track(1, "hey jude", "the beatles");
|
||||||
|
cd.maximum_bit_depth = Some(16);
|
||||||
|
cd.maximum_sampling_rate = Some(44.1);
|
||||||
|
let mut hires = qobuz_track(2, "hey jude", "the beatles");
|
||||||
|
hires.maximum_bit_depth = Some(24);
|
||||||
|
hires.maximum_sampling_rate = Some(192.0);
|
||||||
|
|
||||||
|
let ranked = rank_candidates(&source, &[cd, hires]);
|
||||||
|
assert_eq!(ranked[0].0.id, 2);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rank_candidates_empty_is_empty_not_a_panic() {
|
||||||
|
let source = import_track("hey jude", "the beatles");
|
||||||
|
assert!(rank_candidates(&source, &[]).is_empty());
|
||||||
|
}
|
||||||
|
|
||||||
// ── quality_score ──
|
// ── quality_score ──
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -70,8 +70,13 @@ impl<'a> RadioPoolBuilder<'a> {
|
|||||||
fallback
|
fallback
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Pool gate. `is_streamable()` rather than the raw field: this predicate
|
||||||
|
/// runs over whatever endpoint filled the pool, and an endpoint that simply
|
||||||
|
/// does not report availability would otherwise reject every candidate and
|
||||||
|
/// leave radio with an empty pool — a total failure of the feature, with no
|
||||||
|
/// error anywhere to explain it.
|
||||||
fn is_music_track(track: &Track) -> bool {
|
fn is_music_track(track: &Track) -> bool {
|
||||||
track.streamable && track.duration > 0 && track.album.is_some()
|
track.is_streamable() && track.duration > 0 && track.album.is_some()
|
||||||
}
|
}
|
||||||
|
|
||||||
fn derive_rng_seed(default_seed: u64, salt: u64) -> u64 {
|
fn derive_rng_seed(default_seed: u64, salt: u64) -> u64 {
|
||||||
|
|||||||
@@ -502,7 +502,9 @@ pub(crate) fn track_to_queue_track(track: &Track) -> QueueTrack {
|
|||||||
is_local: false,
|
is_local: false,
|
||||||
album_id,
|
album_id,
|
||||||
artist_id,
|
artist_id,
|
||||||
streamable: track.streamable,
|
// `QueueTrack.streamable` is a resolved `bool`; absence is interpreted
|
||||||
|
// once, here, by `Track::is_streamable` (absent = available).
|
||||||
|
streamable: track.is_streamable(),
|
||||||
source: Some("qobuz".to_string()),
|
source: Some("qobuz".to_string()),
|
||||||
parental_warning: track.parental_warning,
|
parental_warning: track.parental_warning,
|
||||||
source_item_id_hint: None,
|
source_item_id_hint: None,
|
||||||
@@ -707,7 +709,11 @@ mod tests {
|
|||||||
hires: true,
|
hires: true,
|
||||||
maximum_bit_depth: Some(24),
|
maximum_bit_depth: Some(24),
|
||||||
maximum_sampling_rate: Some(96.0),
|
maximum_sampling_rate: Some(96.0),
|
||||||
streamable: true,
|
// The catalog shape reports availability explicitly here — that is
|
||||||
|
// what `/album/get` sends. `Some(true)`, not a bare `true`, since
|
||||||
|
// the model distinguishes "the server said yes" from "the server
|
||||||
|
// said nothing".
|
||||||
|
streamable: Some(true),
|
||||||
parental_warning: false,
|
parental_warning: false,
|
||||||
..Default::default()
|
..Default::default()
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -93,7 +93,11 @@ pub fn model_track_to_core_queue_track(track: &Track) -> QueueTrack {
|
|||||||
is_local: false,
|
is_local: false,
|
||||||
album_id: album_id.clone(),
|
album_id: album_id.clone(),
|
||||||
artist_id,
|
artist_id,
|
||||||
streamable: track.streamable,
|
// A queue pushed by a QConnect peer carries real Qobuz tracks, so the
|
||||||
|
// flag is worth keeping — but resolved through `is_streamable()`, since
|
||||||
|
// a peer's payload is one more endpoint we have never captured and a
|
||||||
|
// terse one must not arrive here marked dead.
|
||||||
|
streamable: track.is_streamable(),
|
||||||
source: Some(QCONNECT_REMOTE_QUEUE_SOURCE.to_string()),
|
source: Some(QCONNECT_REMOTE_QUEUE_SOURCE.to_string()),
|
||||||
parental_warning: track.parental_warning,
|
parental_warning: track.parental_warning,
|
||||||
source_item_id_hint: album_id,
|
source_item_id_hint: album_id,
|
||||||
|
|||||||
Reference in new issue
Block a user