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
This commit is contained in:
8 files changed
+563
-48
No files matched your search
@@ -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<TrackReco>`) 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<String> {
|
||||
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<String> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
@@ -92,10 +92,12 @@ async fn validate_track_pool(
|
||||
mb: &qbz_integrations::MusicBrainzClient,
|
||||
cache: Cache<'_>,
|
||||
cands: Vec<TrackCandidate>,
|
||||
skip_negative: bool,
|
||||
skip_mb: bool,
|
||||
) -> Vec<TrackReco> {
|
||||
let resolved: Vec<Option<TrackReco>> = stream::iter(
|
||||
cands.into_iter().map(|cand| async move { validate_track(catalog, mb, cache, &cand).await }),
|
||||
)
|
||||
let resolved: Vec<Option<TrackReco>> = 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<AlbumReco> {
|
||||
}
|
||||
|
||||
// ── 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<TrackReco> {
|
||||
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::<Vec<TrackReco>>(&json).ok())
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
pub async fn build_weekly(inputs: &RecoInputs<'_>, source_patch: &str) -> Vec<TrackReco> {
|
||||
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::<Vec<TrackReco>>(&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<TrackCandidate> = tracks
|
||||
let candidates: Vec<TrackCandidate> = 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<Tr
|
||||
score: 0.0,
|
||||
})
|
||||
.collect();
|
||||
let pool =
|
||||
validate_track_pool(inputs.catalog, inputs.musicbrainz, inputs.cache, candidates).await;
|
||||
pool.into_iter().take(PLAYLIST_CAP).collect()
|
||||
let cand_count = candidates.len();
|
||||
log::info!(
|
||||
"[reco] weekly '{source_patch}': fetched {cand_count} tracks (week {week}, mbid {}); resolving to Qobuz…",
|
||||
meta.playlist_mbid
|
||||
);
|
||||
|
||||
// skip_negative=true: a transient throttle on these tracks must NOT stick as
|
||||
// a 7-day negative (that locked the rows empty across rebuilds).
|
||||
// skip_mb=true: bypass the serial 1.1s/req MusicBrainz ISRC lookup (~110s
|
||||
// for 100 tracks) and resolve via fuzzy Qobuz search — fast and reliable
|
||||
// for these mainstream playlists. This is the fix for "the row never paints".
|
||||
let pool = validate_track_pool(
|
||||
inputs.catalog,
|
||||
inputs.musicbrainz,
|
||||
inputs.cache,
|
||||
candidates,
|
||||
true,
|
||||
true,
|
||||
)
|
||||
.await;
|
||||
let resolved: Vec<TrackReco> = 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) ────────
|
||||
|
||||
@@ -110,17 +110,26 @@ async fn resolve_track_live(
|
||||
catalog: &dyn RecoCatalog,
|
||||
mb: &MusicBrainzClient,
|
||||
cand: &TrackCandidate,
|
||||
skip_mb: bool,
|
||||
) -> Option<TrackReco> {
|
||||
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<TrackReco> {
|
||||
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 => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user