From c3b921efff3e553fa5880eab40bc6e43b5ac57eb Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Sat, 27 Jun 2026 21:05:57 -0600 Subject: [PATCH] fix(reco): Weekly rows resolve fast, cache per-week, show per-row skeletons Discover > Recommendations Weekly Exploration/Jams kept vanishing and loaded slowly. Three root causes: - Weekly track resolution ran every track through MusicBrainz's serial 1.1s/req rate limiter (~110s for 100 tracks), so the rows never painted in practice (Fresh Releases, album-based with no MusicBrainz, returned fine). Resolve weekly tracks via fuzzy Qobuz search instead (skip_mb) -- fast and reliable for these mainstream playlists. - The external-reco cache was only initialized on the offline entry path; the two online login paths (auth.rs) never bound it, so nothing cached online (0-byte db). Initialize it there too. - Weeklies were bundled in the shared 48h results blob keyed only on auth-presence. Give them their own per-week cache (keyed by the ListenBrainz playlist mbid, 9d TTL, written only when non-empty, with a stale-fallback to the last good week) and rebuild them independently of the blob, so a transient empty build cannot hide them. Skip the 7d negative cache for weekly tracks so a throttle does not lock a row empty. Per-row skeletons: the progressive paint now marks each expected row pending at spawn and clears it as its builder resolves, so a slow row shows its own skeleton instead of silently missing. Adds [reco] diagnostics. cc --- crates/qbz-external-reco/src/cache.rs | 116 +++++++++++- crates/qbz-external-reco/src/carousels.rs | 110 ++++++++++- crates/qbz-external-reco/src/validate.rs | 43 ++++- .../qbz-ui/ui/discover/ExternalRecoView.slint | 53 +++++- crates/qbz-ui/ui/discover/HomeSkeleton.slint | 93 ++++++++++ crates/qbz-ui/ui/state.slint | 14 ++ crates/qbz/src/auth.rs | 10 + crates/qbz/src/external_reco.rs | 172 +++++++++++++++--- 8 files changed, 563 insertions(+), 48 deletions(-) diff --git a/crates/qbz-external-reco/src/cache.rs b/crates/qbz-external-reco/src/cache.rs index 1d9ed072c..b44b75913 100644 --- a/crates/qbz-external-reco/src/cache.rs +++ b/crates/qbz-external-reco/src/cache.rs @@ -18,6 +18,18 @@ const MISS_TTL_SECS: i64 = 7 * 24 * 60 * 60; /// tab paints instantly from cache within the window, and rebuilds every 48h so /// the content is never "eternally the same"). const RESULTS_TTL_SECS: i64 = 48 * 60 * 60; +/// TTL for a RESOLVED weekly playlist, keyed by its ListenBrainz playlist mbid — +/// 9 days (a week + slack). ListenBrainz regenerates Weekly Exploration / Weekly +/// Jams every Monday with a NEW mbid, so a new week is a natural cache miss while +/// the current week is served instantly. This is deliberately SEPARATE from the +/// 48h results blob: the weeklies have their own ListenBrainz cadence (a date per +/// playlist) and must not be clobbered by the unrelated 48h rotation. +const WEEKLY_TTL_SECS: i64 = 9 * 24 * 60 * 60; +/// How long a successfully-resolved weekly stays usable as a STALE FALLBACK (any +/// week, newest first) when the current build comes back empty — 21 days. Better +/// to show last week's row than an empty one on a transient ListenBrainz/Qobuz +/// hiccup. +const WEEKLY_STALE_FALLBACK_SECS: i64 = 21 * 24 * 60 * 60; /// A cache lookup outcome. pub enum CacheLookup { @@ -56,7 +68,15 @@ impl RecoCache { key TEXT PRIMARY KEY, data TEXT NOT NULL, built_at INTEGER NOT NULL - );", + ); + CREATE TABLE IF NOT EXISTS reco_weekly ( + key TEXT PRIMARY KEY, + source_patch TEXT NOT NULL, + data TEXT NOT NULL, + built_at INTEGER NOT NULL + ); + CREATE INDEX IF NOT EXISTS idx_reco_weekly_patch + ON reco_weekly(source_patch, built_at);", ) .map_err(|e| format!("Failed to init reco cache schema: {}", e))?; Ok(Self { conn }) @@ -136,6 +156,60 @@ impl RecoCache { ); } + /// Get a RESOLVED weekly playlist (JSON `Vec`) by its exact + /// `"{source_patch}:{playlist_mbid}"` key, IF still within the 9d TTL. The + /// mbid changes weekly, so a fresh week misses here and triggers a rebuild; + /// within the week the resolved set is served without re-paying Qobuz / + /// MusicBrainz validation. `None` -> rebuild. + pub fn get_weekly(&self, key: &str) -> Option { + let row: Option<(String, i64)> = self + .conn + .query_row( + "SELECT data, built_at FROM reco_weekly WHERE key = ?", + params![key], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .optional() + .unwrap_or(None); + match row { + Some((data, built_at)) if Self::now() - built_at <= WEEKLY_TTL_SECS => Some(data), + _ => None, + } + } + + /// Store a resolved weekly playlist under its `"{source_patch}:{mbid}"` key. + /// Callers MUST only store NON-empty results so a transient empty build can + /// never poison the row. + pub fn put_weekly(&self, key: &str, source_patch: &str, data: &str) { + let _ = self.conn.execute( + "INSERT OR REPLACE INTO reco_weekly (key, source_patch, data, built_at) + VALUES (?, ?, ?, ?)", + params![key, source_patch, data, Self::now()], + ); + } + + /// The most recent successfully-cached weekly for a `source_patch` (any + /// week), within the stale-fallback window — used so a transient empty + /// build still shows last week's row instead of nothing. + pub fn get_latest_weekly_for_patch(&self, source_patch: &str) -> Option { + let row: Option<(String, i64)> = self + .conn + .query_row( + "SELECT data, built_at FROM reco_weekly WHERE source_patch = ? + ORDER BY built_at DESC LIMIT 1", + params![source_patch], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .optional() + .unwrap_or(None); + match row { + Some((data, built_at)) if Self::now() - built_at <= WEEKLY_STALE_FALLBACK_SECS => { + Some(data) + } + _ => None, + } + } + /// Drop expired rows (both regimes). Safe to call opportunistically. pub fn cleanup_expired(&self) -> usize { let now = Self::now(); @@ -153,7 +227,14 @@ impl RecoCache { params![now - MISS_TTL_SECS], ) .unwrap_or(0); - found + miss + let weekly = self + .conn + .execute( + "DELETE FROM reco_weekly WHERE built_at <= ?", + params![now - WEEKLY_STALE_FALLBACK_SECS], + ) + .unwrap_or(0); + found + miss + weekly } } @@ -186,4 +267,35 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + + #[test] + fn weekly_cache_per_week_and_stale_fallback() { + let dir = tmp_dir("weekly"); + let cache = RecoCache::open_at(&dir).expect("open"); + + // Empty until something is stored. + assert!(cache.get_weekly("weekly-jams:mbid-A").is_none()); + assert!(cache.get_latest_weekly_for_patch("weekly-jams").is_none()); + + // Store week A; exact-key hit + patch-latest both return it. + cache.put_weekly("weekly-jams:mbid-A", "weekly-jams", "[\"A\"]"); + assert_eq!(cache.get_weekly("weekly-jams:mbid-A").as_deref(), Some("[\"A\"]")); + assert_eq!( + cache.get_latest_weekly_for_patch("weekly-jams").as_deref(), + Some("[\"A\"]") + ); + + // A different week (new mbid) is a natural miss -> triggers a rebuild, + // but the latest-for-patch fallback still serves week A. + assert!(cache.get_weekly("weekly-jams:mbid-B").is_none()); + assert_eq!( + cache.get_latest_weekly_for_patch("weekly-jams").as_deref(), + Some("[\"A\"]") + ); + + // Patches are isolated from each other. + assert!(cache.get_latest_weekly_for_patch("weekly-exploration").is_none()); + + let _ = std::fs::remove_dir_all(dir); + } } diff --git a/crates/qbz-external-reco/src/carousels.rs b/crates/qbz-external-reco/src/carousels.rs index 561696b20..14220125e 100644 --- a/crates/qbz-external-reco/src/carousels.rs +++ b/crates/qbz-external-reco/src/carousels.rs @@ -92,10 +92,12 @@ async fn validate_track_pool( mb: &qbz_integrations::MusicBrainzClient, cache: Cache<'_>, cands: Vec, + skip_negative: bool, + skip_mb: bool, ) -> Vec { - let resolved: Vec> = stream::iter( - cands.into_iter().map(|cand| async move { validate_track(catalog, mb, cache, &cand).await }), - ) + let resolved: Vec> = stream::iter(cands.into_iter().map(|cand| async move { + validate_track(catalog, mb, cache, &cand, skip_negative, skip_mb).await + })) .buffered(VALIDATE_CONCURRENCY) .collect() .await; @@ -345,30 +347,81 @@ pub async fn build_fresh_releases(inputs: &RecoInputs<'_>) -> Vec { } // ── Weekly playlists (ListenBrainz curated: exploration / jams) ───────────── +// +// These have their OWN ListenBrainz cadence: a brand-new playlist (new mbid + +// date) every Monday. They are cached per-week, keyed by the playlist mbid, and +// are DELIBERATELY decoupled from the shared 48h results blob — bundling them in +// it is what made them vanish (a transient empty build got cached for 48h, and +// the 7d per-track negative cache compounded it across rebuilds). See cache.rs. + +/// Last resort when the current week can't be built (no playlist returned, or a +/// transient empty resolve): the most recent successfully-cached week, so the +/// row shows something instead of disappearing. +fn cached_weekly_fallback(cache: Cache<'_>, source_patch: &str) -> Vec { + cache + .and_then(|c| c.lock().ok().and_then(|g| g.get_latest_weekly_for_patch(source_patch))) + .and_then(|json| serde_json::from_str::>(&json).ok()) + .unwrap_or_default() +} pub async fn build_weekly(inputs: &RecoInputs<'_>, source_patch: &str) -> Vec { let Some(lb) = &inputs.listenbrainz else { + log::info!("[reco] weekly '{source_patch}': ListenBrainz not connected — skipping"); return Vec::new(); }; + + // Discover the current week's playlist for this patch (one cheap call). let playlists = lb .client .get_created_for_playlists(&lb.username, 50) .await .unwrap_or_default(); + let matching = playlists + .iter() + .filter(|p| p.source_patch.as_deref() == Some(source_patch)) + .count(); + log::info!( + "[reco] weekly '{source_patch}': {} created-for playlists from ListenBrainz, {matching} match the patch", + playlists.len() + ); // Newest playlist matching the source_patch (created_at desc). let chosen = playlists .into_iter() .filter(|p| p.source_patch.as_deref() == Some(source_patch)) .max_by(|a, b| a.created_at.cmp(&b.created_at)); let Some(meta) = chosen else { - return Vec::new(); + log::warn!( + "[reco] weekly '{source_patch}': ListenBrainz returned no matching playlist \ + (rate-limit / not generated yet) — serving last cached week" + ); + return cached_weekly_fallback(inputs.cache, source_patch); }; - let tracks = lb + let week = meta.created_at.as_deref().unwrap_or("?"); + + // Week-keyed cache: the mbid changes every Monday, so a new week is a natural + // miss and the current week is served instantly (no Qobuz/MusicBrainz round-trips). + let cache_key = format!("{}:{}", source_patch, meta.playlist_mbid); + if let Some(c) = inputs.cache { + if let Some(json) = c.lock().ok().and_then(|g| g.get_weekly(&cache_key)) { + if let Ok(tracks) = serde_json::from_str::>(&json) { + if !tracks.is_empty() { + log::info!( + "[reco] weekly '{source_patch}': cache hit — {} tracks (week {week})", + tracks.len() + ); + return tracks; + } + } + } + } + + // Cache miss for this week: fetch + resolve to Qobuz. + let raw = lb .client .get_playlist_tracks(&meta.playlist_mbid) .await .unwrap_or_default(); - let candidates: Vec = tracks + let candidates: Vec = raw .into_iter() .filter(|t| !t.title.is_empty() && !t.artist_name.is_empty()) .map(|t| TrackCandidate { @@ -382,9 +435,48 @@ pub async fn build_weekly(inputs: &RecoInputs<'_>, source_patch: &str) -> Vec = pool.into_iter().take(PLAYLIST_CAP).collect(); + log::info!( + "[reco] weekly '{source_patch}': resolved {} / {cand_count} tracks to Qobuz (week {week}, mbid {})", + resolved.len(), + meta.playlist_mbid + ); + + if !resolved.is_empty() { + // Persist the resolved set for this week (only when non-empty). + if let Some(c) = inputs.cache { + if let (Ok(g), Ok(json)) = (c.lock(), serde_json::to_string(&resolved)) { + g.put_weekly(&cache_key, source_patch, &json); + } + } + return resolved; + } + + // Resolved empty this build (transient) — show last cached week, not nothing. + log::warn!( + "[reco] weekly '{source_patch}': resolved 0 tracks this build — serving last cached week" + ); + cached_weekly_fallback(inputs.cache, source_patch) } // ── Deep-cut albums from artists you know (Qobuz catalog, not heard) ──────── diff --git a/crates/qbz-external-reco/src/validate.rs b/crates/qbz-external-reco/src/validate.rs index 8b4decce9..e2bc59833 100644 --- a/crates/qbz-external-reco/src/validate.rs +++ b/crates/qbz-external-reco/src/validate.rs @@ -110,17 +110,26 @@ async fn resolve_track_live( catalog: &dyn RecoCatalog, mb: &MusicBrainzClient, cand: &TrackCandidate, + skip_mb: bool, ) -> Option { if let Some(isrc) = cand.isrc.as_deref().filter(|s| !s.is_empty()) { if let Some(track) = find_by_isrc(catalog, isrc).await { return Some(build_track_reco(&track, cand.source)); } } - if let Some(mbid) = cand.recording_mbid.as_deref().filter(|s| !s.is_empty()) { - let isrcs = mb.get_recording_isrcs(mbid).await.unwrap_or_default(); - for isrc in isrcs { - if let Some(track) = find_by_isrc(catalog, &isrc).await { - return Some(build_track_reco(&track, cand.source)); + // The MusicBrainz recording->ISRC lookup is gated behind a SERIAL 1.1s/req + // rate limiter; for a 50-track weekly playlist (×2 rows) that is ~110s of + // pure waiting, so the row never paints in practice. `skip_mb` bypasses it + // and relies on the fuzzy Qobuz search below — reliable for the mainstream + // tracks these playlists contain. (This is THE reason the Weekly rows did + // not appear while Fresh Releases — album-based, no MusicBrainz — did.) + if !skip_mb { + if let Some(mbid) = cand.recording_mbid.as_deref().filter(|s| !s.is_empty()) { + let isrcs = mb.get_recording_isrcs(mbid).await.unwrap_or_default(); + for isrc in isrcs { + if let Some(track) = find_by_isrc(catalog, &isrc).await { + return Some(build_track_reco(&track, cand.source)); + } } } } @@ -140,11 +149,23 @@ async fn resolve_track_live( } } +/// Resolve a track candidate to a Qobuz track. +/// +/// `skip_negative`: when true, the per-track NEGATIVE cache is ignored on BOTH +/// read and write. This is for ListenBrainz weekly playlists, whose resolution +/// runs under heavy concurrent first-build load: a transient throttle/timeout +/// returns `None`, and persisting that as a 7-day negative would lock the track +/// (and so the whole weekly row) out long after the hiccup passed — the exact +/// mechanism that made Weekly Exploration/Jams "vanish". POSITIVE hits are still +/// cached (cheap re-resolution), and the resolved set is cached per-week by the +/// caller. pub async fn validate_track( catalog: &dyn RecoCatalog, mb: &MusicBrainzClient, cache: Cache<'_>, cand: &TrackCandidate, + skip_negative: bool, + skip_mb: bool, ) -> Option { let key = track_cache_key(cand); if let Some(c) = cache { @@ -156,17 +177,21 @@ pub async fn validate_track( return Some(reco); } } - CacheLookup::Negative => return None, - CacheLookup::Miss => {} + // A cached negative is authoritative only when we trust negatives. + CacheLookup::Negative if !skip_negative => return None, + _ => {} } } } - let reco = resolve_track_live(catalog, mb, cand).await; + let reco = resolve_track_live(catalog, mb, cand, skip_mb).await; if let Some(c) = cache { if let Ok(guard) = c.lock() { match &reco { Some(r) => guard.put(&key, "track", Some(&serde_json::to_string(r).unwrap_or_default())), - None => guard.put(&key, "track", None), + // Only persist a negative when negatives are trusted (NOT for + // weeklies — a transient miss must not stick for 7 days). + None if !skip_negative => guard.put(&key, "track", None), + None => {} } } } diff --git a/crates/qbz-ui/ui/discover/ExternalRecoView.slint b/crates/qbz-ui/ui/discover/ExternalRecoView.slint index d28eb0b3b..c5961f90e 100644 --- a/crates/qbz-ui/ui/discover/ExternalRecoView.slint +++ b/crates/qbz-ui/ui/discover/ExternalRecoView.slint @@ -3,8 +3,11 @@ // External-recommendations engine (Last.fm + ListenBrainz -> Qobuz). Artist / // album / track carousels, each with a `length > 0` self-hide gate; the // controller paints each row the moment its builder resolves (progressive). -// Data lives in ExternalRecoState (crate::external_reco). Reuses the shared -// Carousel / ArtistCarousel / SlimCarousel components. +// Slow rows (the Weekly ListenBrainz playlists) carry a per-row `pending-*` +// flag so they show their own skeleton while still loading instead of silently +// missing while the fast rows are already painted. Data lives in +// ExternalRecoState (crate::external_reco). Reuses the shared Carousel / +// ArtistCarousel / SlimCarousel components. import { Theme } from "../foundation/semantic-colors.slint"; import { Typography } from "../foundation/typography.slint"; @@ -12,7 +15,7 @@ import { ExternalRecoState } from "../state.slint"; import { Carousel } from "Carousel.slint"; import { SlimCarousel } from "SlimCarousel.slint"; import { ArtistCarousel } from "ArtistCarousel.slint"; -import { HomeSkeleton } from "HomeSkeleton.slint"; +import { HomeSkeleton, SkeletonCarouselRow, SkeletonSlimRow } from "HomeSkeleton.slint"; export component ExternalRecoView inherits VerticalLayout { callback open-album(string); @@ -31,10 +34,25 @@ export component ExternalRecoView inherits VerticalLayout { || ExternalRecoState.top-albums.albums.length > 0 || ExternalRecoState.top-artists.length > 0; + // True while ANY row is still resolving — drives whether the initial + // full-page skeleton or the per-row skeletons are showing. + property any-pending: + ExternalRecoState.pending-rec-artists-common + || ExternalRecoState.pending-rec-artists-recent + || ExternalRecoState.pending-rec-albums + || ExternalRecoState.pending-fresh-releases + || ExternalRecoState.pending-weekly-exploration + || ExternalRecoState.pending-weekly-jams + || ExternalRecoState.pending-deep-cut-albums + || ExternalRecoState.pending-top-albums + || ExternalRecoState.pending-top-artists; + spacing: 40px; alignment: start; - if ExternalRecoState.loading && !root.any-content: HomeSkeleton { } + // Initial full-page skeleton — only until the per-row pending flags are set + // (or content arrives), after which the individual row skeletons take over. + if ExternalRecoState.loading && !root.any-content && !root.any-pending: HomeSkeleton { } // More like the artists you love (COMMON taste — overall top). if ExternalRecoState.rec-artists-common.length > 0: ArtistCarousel { @@ -48,6 +66,9 @@ export component ExternalRecoView inherits VerticalLayout { root.media-action(kind, id, action); } } + if ExternalRecoState.rec-artists-common.length == 0 && ExternalRecoState.pending-rec-artists-common: SkeletonCarouselRow { + card-size: 180px; + } // Based on what you've been into lately (RECENT taste — 1-month top). if ExternalRecoState.rec-artists-recent.length > 0: ArtistCarousel { @@ -61,6 +82,9 @@ export component ExternalRecoView inherits VerticalLayout { root.media-action(kind, id, action); } } + if ExternalRecoState.rec-artists-recent.length == 0 && ExternalRecoState.pending-rec-artists-recent: SkeletonCarouselRow { + card-size: 180px; + } // Recommended Albums (Last.fm artist top-albums, not scrobbled). if ExternalRecoState.rec-albums.albums.length > 0: Carousel { @@ -73,6 +97,9 @@ export component ExternalRecoView inherits VerticalLayout { root.media-action(kind, id, action); } } + if ExternalRecoState.rec-albums.albums.length == 0 && ExternalRecoState.pending-rec-albums: SkeletonCarouselRow { + card-size: 180px; + } // Fresh Releases (ListenBrainz, from artists you follow). if ExternalRecoState.fresh-releases.albums.length > 0: Carousel { @@ -85,6 +112,9 @@ export component ExternalRecoView inherits VerticalLayout { root.media-action(kind, id, action); } } + if ExternalRecoState.fresh-releases.albums.length == 0 && ExternalRecoState.pending-fresh-releases: SkeletonCarouselRow { + card-size: 180px; + } // Weekly Exploration (ListenBrainz discovery playlist). if ExternalRecoState.weekly-exploration.length > 0: SlimCarousel { @@ -98,6 +128,7 @@ export component ExternalRecoView inherits VerticalLayout { root.play-track(id); } } + if ExternalRecoState.weekly-exploration.length == 0 && ExternalRecoState.pending-weekly-exploration: SkeletonSlimRow { } // Weekly Jams (ListenBrainz familiar playlist). if ExternalRecoState.weekly-jams.length > 0: SlimCarousel { @@ -111,6 +142,7 @@ export component ExternalRecoView inherits VerticalLayout { root.play-track(id); } } + if ExternalRecoState.weekly-jams.length == 0 && ExternalRecoState.pending-weekly-jams: SkeletonSlimRow { } // Deep cuts from artists you know (Qobuz catalog albums). if ExternalRecoState.deep-cut-albums.albums.length > 0: Carousel { @@ -123,6 +155,9 @@ export component ExternalRecoView inherits VerticalLayout { root.media-action(kind, id, action); } } + if ExternalRecoState.deep-cut-albums.albums.length == 0 && ExternalRecoState.pending-deep-cut-albums: SkeletonCarouselRow { + card-size: 180px; + } // Cold-start fallback — Qobuz editorial top albums. if ExternalRecoState.top-albums.albums.length > 0: Carousel { @@ -135,6 +170,9 @@ export component ExternalRecoView inherits VerticalLayout { root.media-action(kind, id, action); } } + if ExternalRecoState.top-albums.albums.length == 0 && ExternalRecoState.pending-top-albums: SkeletonCarouselRow { + card-size: 180px; + } // Cold-start fallback — Qobuz editorial top artists. if ExternalRecoState.top-artists.length > 0: ArtistCarousel { @@ -148,9 +186,12 @@ export component ExternalRecoView inherits VerticalLayout { root.media-action(kind, id, action); } } + if ExternalRecoState.top-artists.length == 0 && ExternalRecoState.pending-top-artists: SkeletonCarouselRow { + card-size: 180px; + } - // Empty state. - if !root.any-content && !ExternalRecoState.loading: VerticalLayout { + // Empty state — nothing loaded, nothing pending, not loading. + if !root.any-content && !root.any-pending && !ExternalRecoState.loading: VerticalLayout { spacing: 8px; Text { text: @tr("No recommendations yet"); diff --git a/crates/qbz-ui/ui/discover/HomeSkeleton.slint b/crates/qbz-ui/ui/discover/HomeSkeleton.slint index e61ecbe1d..5a16b0a96 100644 --- a/crates/qbz-ui/ui/discover/HomeSkeleton.slint +++ b/crates/qbz-ui/ui/discover/HomeSkeleton.slint @@ -68,3 +68,96 @@ export component HomeSkeleton inherits VerticalLayout { SkeletonRow { phase: root.phase; } SkeletonRow { phase: root.phase; } } + +// ── Standalone per-row skeletons (Discover > Recommendations) ─────────────── +// +// Each owns its breathing Timer so it can drop into a layout in place of a row +// whose builder has not resolved yet. Shapes mirror the real rows: a card +// carousel (artists/albums) and a slim 4-wide track grid (Weekly playlists). + +// Card-carousel placeholder: title bar + a row of card blocks (art + 2 lines). +export component SkeletonCarouselRow inherits VerticalLayout { + in property card-size: 180px; + property phase: false; + spacing: 12px; + + Timer { + interval: 900ms; + running: true; + triggered => { + root.phase = !root.phase; + } + } + + Shimmer { + phase: root.phase; + width: 180px; + height: 22px; + } + HorizontalLayout { + spacing: 24px; + for _ in [0, 1, 2, 3, 4, 5]: VerticalLayout { + spacing: 8px; + Shimmer { + phase: root.phase; + width: root.card-size; + height: root.card-size; + } + Shimmer { + phase: root.phase; + width: root.card-size * 0.7; + height: 14px; + } + Shimmer { + phase: root.phase; + width: root.card-size * 0.45; + height: 12px; + } + } + } +} + +// Slim-grid placeholder: title bar + a 4-column x 3-row grid of slim row +// blocks, matching SlimCarousel's 4x3 layout (the Weekly Exploration/Jams rows). +export component SkeletonSlimRow inherits VerticalLayout { + property phase: false; + spacing: 12px; + + Timer { + interval: 900ms; + running: true; + triggered => { + root.phase = !root.phase; + } + } + + Shimmer { + phase: root.phase; + width: 180px; + height: 22px; + } + HorizontalLayout { + spacing: 8px; + for _ in [0, 1, 2, 3]: Shimmer { + phase: root.phase; + height: 60px; + horizontal-stretch: 1; + } + } + HorizontalLayout { + spacing: 8px; + for _ in [0, 1, 2, 3]: Shimmer { + phase: root.phase; + height: 60px; + horizontal-stretch: 1; + } + } + HorizontalLayout { + spacing: 8px; + for _ in [0, 1, 2, 3]: Shimmer { + phase: root.phase; + height: 60px; + horizontal-stretch: 1; + } + } +} diff --git a/crates/qbz-ui/ui/state.slint b/crates/qbz-ui/ui/state.slint index d8a3b3482..7f070267a 100644 --- a/crates/qbz-ui/ui/state.slint +++ b/crates/qbz-ui/ui/state.slint @@ -335,6 +335,20 @@ export global ForYouState { export global ExternalRecoState { in property loading: false; in-out property loaded: false; + // Per-row "still loading" flags. The controller sets the rows it is about + // to build true at spawn and clears each as its builder resolves, so a SLOW + // row (the Weekly ListenBrainz playlists resolve a few seconds after the + // fast rows) shows its own skeleton instead of silently missing — the UI now + // signals "more is still loading" during the progressive paint. + in property pending-rec-artists-common: false; + in property pending-rec-artists-recent: false; + in property pending-rec-albums: false; + in property pending-fresh-releases: false; + in property pending-weekly-exploration: false; + in property pending-weekly-jams: false; + in property pending-deep-cut-albums: false; + in property pending-top-albums: false; + in property pending-top-artists: false; // True when no external source is connected: the editorial fallback // (top-albums + top-artists) shows instead of the personalized rows. in property editorial-fallback: false; diff --git a/crates/qbz/src/auth.rs b/crates/qbz/src/auth.rs index 12538bf66..1434c2edc 100644 --- a/crates/qbz/src/auth.rs +++ b/crates/qbz/src/auth.rs @@ -114,6 +114,12 @@ where // after init (off-thread) so the seeds reflect this session's events. crate::reco::init_for_user(&dir); crate::reco::train_async(); + // External-recommendations resolution + per-week Weekly cache (4th + // Discover tab). MUST init here on the ONLINE path too — it was only + // wired into the offline entry, so online the reco cache was never + // bound: every open re-resolved from scratch and the Weekly per-week + // cache never persisted (that is why the Weekly rows kept vanishing). + crate::external_reco::init_for_user(&dir); // Playlist Suggested Songs: open the per-user artist-vector store on // the core (the suggestions engine reads/writes it). if let Ok(store) = qbz_reco::ArtistVectorStore::open_at(&dir) { @@ -240,6 +246,10 @@ where // session's events. crate::reco::init_for_user(&dir); crate::reco::train_async(); + // External-recommendations resolution + per-week Weekly cache + // (online-path parity — see login_via_system_browser; without + // it the reco cache is never bound online). + crate::external_reco::init_for_user(&dir); // Playlist Suggested Songs: open the per-user artist-vector // store on the core (the suggestions engine reads/writes it). if let Ok(store) = qbz_reco::ArtistVectorStore::open_at(&dir) { diff --git a/crates/qbz/src/external_reco.rs b/crates/qbz/src/external_reco.rs index 1d4b89e27..d4c833575 100644 --- a/crates/qbz/src/external_reco.rs +++ b/crates/qbz/src/external_reco.rs @@ -35,8 +35,12 @@ pub fn init_for_user(base_dir: &Path) { if let Ok(mut g) = CACHE_DIR.lock() { *g = Some(base_dir.to_path_buf()); } - if let Ok(cache) = RecoCache::open_at(base_dir) { - let _ = cache.cleanup_expired(); + match RecoCache::open_at(base_dir) { + Ok(cache) => { + let _ = cache.cleanup_expired(); + log::info!("[reco] cache initialized at {}", base_dir.display()); + } + Err(e) => log::warn!("[reco] cache open failed at {}: {e}", base_dir.display()), } } @@ -176,12 +180,20 @@ fn spawn( let catalog = CoreRecoCatalog { runtime: runtime.clone(), }; - let cache = CACHE_DIR - .lock() - .ok() - .and_then(|g| g.clone()) - .and_then(|dir| RecoCache::open_at(&dir).ok()) - .map(Mutex::new); + let cache_dir = CACHE_DIR.lock().ok().and_then(|g| g.clone()); + let cache = match &cache_dir { + Some(dir) => match RecoCache::open_at(dir) { + Ok(c) => Some(Mutex::new(c)), + Err(e) => { + log::warn!("[reco] spawn: cache open failed ({e}) — running uncached"); + None + } + }, + None => { + log::warn!("[reco] spawn: cache dir not set (init_for_user not run?) — running uncached"); + None + } + }; let inputs = RecoInputs { lastfm, @@ -198,13 +210,36 @@ fn spawn( inputs.lastfm.is_some(), inputs.listenbrainz.is_some() ); + log::info!( + "[reco] spawn: lastfm={} listenbrainz={} source_key={source_key}", + inputs.lastfm.is_some(), + inputs.listenbrainz.is_some() + ); - // 1. Results cache: paint INSTANTLY if a fresh (<48h) build is cached. + // 1. Results cache: paint the NON-weekly rows INSTANTLY if a fresh + // (<48h) build is cached. The Weekly Exploration/Jams rows are NOT + // trusted from this blob — they follow ListenBrainz's own weekly cadence + // and have their own per-week cache, so we ALWAYS (re)build them via + // build_and_apply_weeklies (cheap on a weekly-cache hit). This is what + // stops a single transient empty build from hiding them for 48h. if let Some(cache_mutex) = inputs.cache { let cached = cache_mutex.lock().ok().and_then(|g| g.get_results(&source_key)); if let Some(json) = cached { if let Ok(result) = serde_json::from_str::(&json) { apply_all(&weak, &image_cache, result); + // The non-weekly rows painted instantly from the blob; the + // two Weekly rows rebuild from their own per-week cache, so + // show their skeletons until build_and_apply_weeklies fills + // them (instant on a weekly-cache hit). + if inputs.listenbrainz.is_some() { + let w = weak.clone(); + let _ = w.upgrade_in_event_loop(|w| { + let s = w.global::(); + s.set_pending_weekly_exploration(true); + s.set_pending_weekly_jams(true); + }); + } + build_and_apply_weeklies(&inputs, &weak, &image_cache).await; latch_loaded(&weak); return; } @@ -214,9 +249,20 @@ fn spawn( // 2. Cache miss / stale: tell the user we're working, then build. crate::toast::info_weak(&weak, qbz_i18n::t("Generating recommendations…")); + // Show per-row skeletons for the rows we're about to build, so the slow + // rows (Weekly) read as "still loading" rather than absent during the + // progressive paint. Each builder clears its own flag as it resolves. + let cold_start = is_cold_start(&inputs); + { + let w = weak.clone(); + let _ = w.upgrade_in_event_loop(move |w| { + set_pending(&w, cold_start); + }); + } + let collector: Arc> = Arc::new(Mutex::new(ExternalCarousels::default())); - if is_cold_start(&inputs) { + if cold_start { let (albums, artists) = build_editorial(&inputs).await; if let Ok(mut g) = collector.lock() { g.editorial_fallback = true; @@ -319,22 +365,77 @@ fn latch_loaded(weak: &slint::Weak) { let s = w.global::(); s.set_loading(false); s.set_loaded(true); + // Defensive: every builder clears its own pending flag as it resolves; + // this guarantees no skeleton can stick after the whole build settles. + clear_all_pending(&w); }); } -/// Paint every row from a cached result (empty rows self-hide). +/// Mark the rows the controller is about to build as pending, so their per-row +/// skeletons show immediately while the builders run. +fn set_pending(w: &AppWindow, cold_start: bool) { + let s = w.global::(); + if cold_start { + s.set_pending_top_albums(true); + s.set_pending_top_artists(true); + } else { + s.set_pending_rec_artists_common(true); + s.set_pending_rec_artists_recent(true); + s.set_pending_rec_albums(true); + s.set_pending_fresh_releases(true); + s.set_pending_weekly_exploration(true); + s.set_pending_weekly_jams(true); + s.set_pending_deep_cut_albums(true); + } +} + +fn clear_all_pending(w: &AppWindow) { + let s = w.global::(); + s.set_pending_rec_artists_common(false); + s.set_pending_rec_artists_recent(false); + s.set_pending_rec_albums(false); + s.set_pending_fresh_releases(false); + s.set_pending_weekly_exploration(false); + s.set_pending_weekly_jams(false); + s.set_pending_deep_cut_albums(false); + s.set_pending_top_albums(false); + s.set_pending_top_artists(false); +} + +/// Paint the NON-weekly rows from a cached 48h blob (empty rows self-hide). The +/// two Weekly rows are intentionally NOT painted here — they are (re)built from +/// their own per-week cache by `build_and_apply_weeklies`, so the blob can never +/// pin a stale/empty weekly for the 48h window. fn apply_all(weak: &slint::Weak, cache: &ImageCache, r: ExternalCarousels) { apply_artists(weak, cache, r.rec_artists_common, ArtistRow::RecArtistsCommon); apply_artists(weak, cache, r.rec_artists_recent, ArtistRow::RecArtistsRecent); apply_albums(weak, cache, r.rec_albums, AlbumRow::RecAlbums); apply_albums(weak, cache, r.fresh_releases, AlbumRow::FreshReleases); - apply_tracks(weak, cache, r.weekly_exploration, TrackRow::WeeklyExploration); - apply_tracks(weak, cache, r.weekly_jams, TrackRow::WeeklyJams); apply_albums(weak, cache, r.deep_cut_albums, AlbumRow::DeepCuts); apply_albums(weak, cache, r.top_albums, AlbumRow::TopAlbums); apply_artists(weak, cache, r.top_artists, ArtistRow::TopArtists); } +/// Build + paint the two Weekly rows from their own per-week cache (cheap on a +/// hit; one ListenBrainz `createdfor` call + a SQLite read). Used on the +/// instant-paint path so the weeklies follow ListenBrainz's weekly cadence +/// independently of the 48h results blob. The full-build path paints them via +/// its own `b_explore`/`b_jams` branches, which call the same cache-backed +/// builders. +async fn build_and_apply_weeklies( + inputs: &RecoInputs<'_>, + weak: &slint::Weak, + image_cache: &ImageCache, +) { + if inputs.listenbrainz.is_none() { + return; + } + let (explore, jams) = + tokio::join!(build_weekly_exploration(inputs), build_weekly_jams(inputs)); + apply_tracks(weak, image_cache, explore, TrackRow::WeeklyExploration); + apply_tracks(weak, image_cache, jams, TrackRow::WeeklyJams); +} + // ── Per-row apply (models built on the UI thread; slint::Image is !Send) ──── #[derive(Clone, Copy)] @@ -438,9 +539,18 @@ fn apply_artists( )); let s = w.global::(); match which { - ArtistRow::RecArtistsCommon => s.set_rec_artists_common(model), - ArtistRow::RecArtistsRecent => s.set_rec_artists_recent(model), - ArtistRow::TopArtists => s.set_top_artists(model), + ArtistRow::RecArtistsCommon => { + s.set_rec_artists_common(model); + s.set_pending_rec_artists_common(false); + } + ArtistRow::RecArtistsRecent => { + s.set_rec_artists_recent(model); + s.set_pending_rec_artists_recent(false); + } + ArtistRow::TopArtists => { + s.set_top_artists(model); + s.set_pending_top_artists(false); + } } }); crate::artwork::spawn_loads(jobs, weak.clone(), cache.clone()); @@ -471,8 +581,14 @@ fn apply_tracks( )); let s = w.global::(); match which { - TrackRow::WeeklyExploration => s.set_weekly_exploration(model), - TrackRow::WeeklyJams => s.set_weekly_jams(model), + TrackRow::WeeklyExploration => { + s.set_weekly_exploration(model); + s.set_pending_weekly_exploration(false); + } + TrackRow::WeeklyJams => { + s.set_weekly_jams(model); + s.set_pending_weekly_jams(false); + } } }); crate::artwork::spawn_loads(jobs, weak.clone(), cache.clone()); @@ -519,10 +635,22 @@ fn apply_albums( }; let s = w.global::(); match which { - AlbumRow::RecAlbums => s.set_rec_albums(section), - AlbumRow::FreshReleases => s.set_fresh_releases(section), - AlbumRow::DeepCuts => s.set_deep_cut_albums(section), - AlbumRow::TopAlbums => s.set_top_albums(section), + AlbumRow::RecAlbums => { + s.set_rec_albums(section); + s.set_pending_rec_albums(false); + } + AlbumRow::FreshReleases => { + s.set_fresh_releases(section); + s.set_pending_fresh_releases(false); + } + AlbumRow::DeepCuts => { + s.set_deep_cut_albums(section); + s.set_pending_deep_cut_albums(false); + } + AlbumRow::TopAlbums => { + s.set_top_albums(section); + s.set_pending_top_albums(false); + } } }); crate::artwork::spawn_loads(jobs, weak.clone(), cache.clone());