From 91ccbc2de3162e384818246d6f6c140c150ce57c Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 21 Sep 2026 07:30:40 +0200 Subject: [PATCH 1/6] fix(lyrics): one failing provider no longer vetoes the cached miss The query chain turned any single provider error into an error for the whole search, and an error is never cached. With Megalobiz unreachable and Genius answering 403 to every install, no track outside LRCLIB could ever be concluded: each panel open replayed the whole chain, about ten seconds, and the veto was logged at debug only. The search now reports who answered and who failed. A miss where some providers were not heard is a partial miss, cached with a retry_after date a week ahead; a complete miss stays cached for good. A provider that fails at the transport level sits out automatic lookups for ten minutes, and its first failure of the session is logged at WARN. Online providers can be switched off per profile (Genius off by default: it cannot answer a normal install and never has timestamps), and tracks in an excluded genre skip the online search, Instrumental and Lo-fi by default, matched by whole words so variants are caught. Nothing is cached for an excluded track, and Refetch ignores the list. Closes #720 Closes #721 Closes #722 --- docs/architecture/storage.md | 2 +- docs/features/integrations.md | 19 +- src-tauri/crates/app/src/commands/lyrics.rs | 512 +++++++++++++----- .../app/src/commands/lyrics_providers.rs | 352 ++++++++++++ src-tauri/crates/app/src/commands/mod.rs | 1 + src-tauri/crates/syncedlyrics/src/lib.rs | 104 +++- .../app/20260921120000_lyrics_retry_after.sql | 20 + src/components/views/SettingsView.tsx | 4 + .../settings/LyricsExcludedGenresCard.tsx | 107 ++++ .../views/settings/LyricsProvidersCard.tsx | 94 ++++ .../views/settings/settingsCatalog.ts | 4 + src/hooks/useLyricsLookupSettings.ts | 149 +++++ src/i18n/locales/ar.json | 15 + src/i18n/locales/de.json | 15 + src/i18n/locales/en.json | 15 + src/i18n/locales/es.json | 15 + src/i18n/locales/fr.json | 15 + src/i18n/locales/hi.json | 15 + src/i18n/locales/id.json | 15 + src/i18n/locales/it.json | 15 + src/i18n/locales/ja.json | 15 + src/i18n/locales/ko.json | 15 + src/i18n/locales/nl.json | 15 + src/i18n/locales/pt-BR.json | 15 + src/i18n/locales/pt.json | 15 + src/i18n/locales/ru.json | 15 + src/i18n/locales/tr.json | 15 + src/i18n/locales/zh-CN.json | 15 + src/i18n/locales/zh-TW.json | 15 + 29 files changed, 1449 insertions(+), 174 deletions(-) create mode 100644 src-tauri/crates/app/src/commands/lyrics_providers.rs create mode 100644 src-tauri/migrations/app/20260921120000_lyrics_retry_after.sql create mode 100644 src/components/views/settings/LyricsExcludedGenresCard.tsx create mode 100644 src/components/views/settings/LyricsProvidersCard.tsx create mode 100644 src/hooks/useLyricsLookupSettings.ts diff --git a/docs/architecture/storage.md b/docs/architecture/storage.md index ced4eb5bf..b394fa4c2 100644 --- a/docs/architecture/storage.md +++ b/docs/architecture/storage.md @@ -55,7 +55,7 @@ The startup order matters and is easy to get backwards: `app.db` lives at the *d - `profile` — profile list (one row per profile). - `app_setting` — typed key/value: `app.last_profile_id`, `lastfm_api_key`, `lastfm_session_key`, `app.theme`, `integrations.discord_rpc`, … - `deezer_artist` / `deezer_album` — shared metadata cache (Deezer enrichment + Last.fm bios), 30-day TTL via `expires_at`. -- `lyrics` — shared LRCLIB cache (no TTL). +- `lyrics` — shared lyrics cache, kept for good except a partial miss, which carries a `retry_after` date ([integrations](../features/integrations.md#which-providers-are-asked-for-which-tracks)). Migrations: [`src-tauri/migrations/app/`](../../src-tauri/migrations/app). diff --git a/docs/features/integrations.md b/docs/features/integrations.md index dfa14e27c..aca144721 100644 --- a/docs/features/integrations.md +++ b/docs/features/integrations.md @@ -166,10 +166,20 @@ Triggered from [`emit_track_changed`](../../src-tauri/crates/app/src/commands/pl 5. **Musixmatch Enhanced** — asks for word-level karaoke first. It only wins early when the result is actually Enhanced LRC; regular line-level LRC from Musixmatch falls through so LRCLIB's stricter metadata match can still win. 6. **LRCLIB** — synced lyrics first, falls back to plain text. Result cached as a new row. -7. **Query-based fallback providers** — LRCLIB (again), then NetEase, Megalobiz, then Genius. This broader scan only runs after tier 6 returns 404 or an empty payload, and prefers synced content over plain text. Musixmatch is deliberately absent: tier 5 owns it, and listing it here would re-issue an identical request. +7. **Query-based fallback providers** — LRCLIB (again), then NetEase, Megalobiz, then Genius, minus the ones switched off (below). This broader scan only runs after tier 6 returns 404 or an empty payload, and prefers synced content over plain text. Musixmatch is deliberately absent: tier 5 owns it, and listing it here would re-issue an identical request. LRCLIB leads the chain even though tier 6 just asked it, because **the two ask differently**: tier 6 uses `/api/get`, which matches on artist + track + album + duration and 404s when any of them disagrees with the file's tags — a remaster, a `Deluxe` album name, a rip a few seconds off. This tier goes through the provider's `/api/search`, which is fuzzy. Without it, that 404 fell straight through to providers that answer for almost any query, so a track LRCLIB _does_ carry came back from Genius while picking LRCLIB by hand in the panel found it instantly (issue #463). +### Which providers are asked, for which tracks + +All three rules live in [`lyrics_providers.rs`](../../src-tauri/crates/app/src/commands/lyrics_providers.rs), and none of them touches the local tiers — the tag, the sidecar, the description cost nothing and belong to the file. + +**Provider switches (#722).** `profile_setting['lyrics.disabled_providers']`, a JSON array of provider ids, Settings → Images and lyrics → Lyrics. A switched-off provider is never asked by an automatic lookup — panel open, prefetch, radio, remote tracks — though it can still be picked by name in the panel's provider picker, which is a request about that one provider. NetEase, Megalobiz and Genius can be switched; LRCLIB cannot (it is also tier 6, and a chain without it is #463 by choice), and Musixmatch keeps its own opt-in. **Genius is off by default**: its search endpoint answers a normal install with a 403, since its cookie comes from an environment variable nothing sets, and when it does answer it has no timestamps and keeps its `[Chorus]` markers. Switching a provider off leaves the lyrics it already supplied in the cache; Refetch replaces them. + +**Excluded genres (#721).** `profile_setting['lyrics.excluded_genres']`, default `Instrumental` and `Lo-fi`. A track with any matching genre skips every network tier, plugins included, after the local tiers have run. Each entry covers its variants by whole words: a run of consecutive words in the genre, glued together, must spell the entry with its separators removed, or that plus an `s` — so `Lo-fi` catches `Lofi`, `Lo Fi` and `lo-fi hip hop`, `Instrumental` catches `Instrumentals` and `Instrumental Hip-Hop`, and a user's `rap` does not catch `Trap`. **Nothing is cached for a skipped track**: it is not known to be lyric-less, only not worth asking about, so taking the genre off the list brings the lookup back — an empty row would outlive the setting. Refetch ignores the list; the prefetch drops those tracks before counting them. + +**Provider cooldown (#720).** A provider that fails at the transport level — connect timeout, refused request, 4xx/5xx — sits out automatic lookups for ten minutes; the first answer from it ends that early. Otherwise a host that is down costs its 5 s connect timeout on every track: seconds per panel open, hours over a library prefetch. The first failure of each provider in a session is logged at `WARN`, naming it, and the rest at `debug`. + ### Prefer-LRCLIB toggle (online-first order) `profile_setting['lyrics.prefer_lrclib']`, default off, Settings → Images and lyrics → Lyrics (issue #378). When on, the on-demand [`fetch_lyrics`](../../src-tauri/crates/app/src/commands/lyrics.rs) flips the waterfall so the online providers (tiers 5–7) run **before** the local ones (embedded, sidecar, description), which become the fallback used only when the network has nothing — so a track LRCLIB doesn't carry still shows its own embedded lyrics. The cache tier stays first either way. @@ -178,7 +188,7 @@ The bulk `run_prefetch` gap-filler stays local-first regardless of the toggle: i ### Web Radio now-playing lyrics -[`fetch_radio_lyrics`](../../src-tauri/crates/app/src/commands/lyrics.rs) keys a dedicated `radio_lyrics` table in `app.db` by `blake3(artist + title)` parsed from the ICY stream title. A radio session has no library row and no `file_hash`, so it can't use the normal `lyrics` cache. It queries LRCLIB plus the query fallback chain and caches misses as empty rows, same as the library path. +[`fetch_radio_lyrics`](../../src-tauri/crates/app/src/commands/lyrics.rs) keys a dedicated `radio_lyrics` table in `app.db` by `blake3(artist + title)` parsed from the ICY stream title. A radio session has no library row and no `file_hash`, so it can't use the normal `lyrics` cache. It queries the query fallback chain, LRCLIB first, and caches a complete miss as an empty row, same as the library path. A partial miss is not cached — `radio_lyrics` has no expiry to put on it — and costs little on the next spin, since the providers that failed are cooling down. The panel re-fetches per song — the sentinel track id stays constant for the whole session, so the effect keys on title + artist instead — and renders **statically**: synced LRC is timestamp-stripped, because the live stream position can't be aligned to a song that may have been joined mid-play. Library-row mutation actions (edit / import / refetch / clear) are hidden for radio. @@ -207,11 +217,12 @@ It hangs off `upsert_lyrics`, the funnel every primary write already passes thro - Hit (embedded, sidecar, LRCLIB, or query provider) → row written. - Instrumental flag from LRCLIB → empty row written (suppresses retries). - LRCLIB 404 / empty payload and every query provider misses → empty row written. Without this, lo-fi / ambient libraries would re-hit the network on every panel open since many of their tracks are genuinely missing from public lyric providers. The lyrics panel renders "no lyrics found" against an empty cached row, and the "Refetch" button (`clearLyrics + fetchLyrics`) is the manual escape hatch for the user to retry once they think providers might have added the track. -- Network error → **not cached** and bubbled up to the UI as `Err` so the panel can show "retry" instead of a misleading "no lyrics" state. +- **Partial miss** — some query providers answered "nothing", others failed or sat out their cooldown → empty row written with `retry_after` a week ahead (#720). It is served as a miss until then; after it, the cache read treats it as absent and the prefetch selects it again. Every other write resets `retry_after` to `NULL`, so lyrics found later do not inherit the expiry. Before this, one failing provider turned the whole search into an error: the waterfall is cache-first, a failure must never be cached, and with a provider down for good (Megalobiz unreachable, Genius refusing) no track outside LRCLIB could ever be concluded — every open replayed the chain, silently, because the veto was logged at `debug`. +- Nobody in the chain answered → **not cached**. A network error on LRCLIB's tier 6 is bubbled up to the UI as `Err` so the panel can show "retry" instead of a misleading "no lyrics" state. **Network defaults.** 15 s overall timeout + 5 s connect-timeout in both `LrclibClient` and `SyncedLyricsClient` so a slow provider still gets a chance to respond while a truly unreachable host fails fast. Genius and NetEase can receive cookies through `SYNCEDLYRICS_GENIUS_COOKIE` and `SYNCEDLYRICS_NETEASE_COOKIE` for deployments that need them; secrets stay in the environment, never in the database. -**Library-wide prefetch.** `prefetch_library_lyrics` walks every available track without a cached row (deduped by `file_hash`), runs the same waterfall, and persists each hit. Network calls are throttled at 500 ms (~2 req/s) to be a polite guest; embedded and sidecar hits skip the throttle. Progress streams over `lyrics:prefetch-progress`. A single global run is enforced via an `AtomicBool`; `cancel_lyrics_prefetch` flips a second `AtomicBool` the worker checks per iteration. Resumable — a partial cancel just leaves uncached rows for the next run. +**Library-wide prefetch.** `prefetch_library_lyrics` walks every available track without a cached row, or with a partial miss past its date (deduped by `file_hash`), minus the excluded genres, runs the same waterfall, and persists each hit. Network calls are throttled at 500 ms (~2 req/s) to be a polite guest; embedded and sidecar hits skip the throttle. Progress streams over `lyrics:prefetch-progress`. A single global run is enforced via an `AtomicBool`; `cancel_lyrics_prefetch` flips a second `AtomicBool` the worker checks per iteration. Resumable — a partial cancel just leaves uncached rows for the next run. The lyrics panel renders synced lines with auto-scroll and a 200 ms transition; un-synced lyrics fall back to a static block. diff --git a/src-tauri/crates/app/src/commands/lyrics.rs b/src-tauri/crates/app/src/commands/lyrics.rs index bb05a7728..8996b0078 100644 --- a/src-tauri/crates/app/src/commands/lyrics.rs +++ b/src-tauri/crates/app/src/commands/lyrics.rs @@ -25,10 +25,22 @@ //! tracks nobody had opened yet. //! //! Because of that cache-first rule, **only a confirmed negative may be -//! cached** (#391). A provider outage is `Unavailable`, not a miss: it -//! persists nothing, so the next panel open retries. Caching it would -//! freeze a one-off network blip into a permanent "no lyrics" that only a -//! manual refetch could clear. +//! cached for good** (#391). A lookup where nobody answered is +//! `Unavailable`, not a miss: it persists nothing, so the next panel open +//! retries. Caching it would freeze a one-off network blip into a +//! permanent "no lyrics" that only a manual refetch could clear. +//! +//! Between the two sits the **partial miss** (#720): some providers +//! answered "nothing", others failed or were left out. It is cached with +//! a `retry_after` stamp — served as a miss until then, looked up again +//! after. It used to be treated as `Unavailable`, so a single provider +//! that was down for good (a host refusing connections, an endpoint that +//! wants a cookie) kept every lyrics-less track from ever being cached, +//! and each open replayed the whole chain. +//! +//! Which providers are asked, and for which tracks, is +//! [`super::lyrics_providers`]: the user's per-provider switches (#722), +//! the excluded genres (#721), and the cooldown of a failing provider. //! //! **Prefer-LRCLIB toggle** (`profile_setting['lyrics.prefer_lrclib']`, //! issue #378): when on, [`fetch_lyrics`] flips tiers 2–4 (the local @@ -67,6 +79,8 @@ use crate::{ state::AppState, }; +use super::lyrics_providers; + /// Guards against two concurrent prefetch runs and exposes a /// cancellation flag the user can flip from the UI. Module-local — the /// prefetch is a single global operation, so a bare `AtomicBool` pair @@ -367,38 +381,6 @@ fn external_query(title: &str, artist_name: Option<&str>) -> String { } } -/// Provider order for the query-based fallback chain that runs after -/// LRCLIB's exact-metadata match has missed. -/// -/// **Musixmatch is intentionally absent.** Both [`fetch_lyrics`] and -/// [`run_prefetch`] already invoke Musixmatch on its own dedicated tier -/// (the enhanced word-level lookup, gated on `MUSIXMATCH_ENABLED`). If -/// we listed it here too, every cache miss that fell through the -/// dedicated tier would issue a second Musixmatch request — same -/// endpoint, same query, same token — for a result we already knew -/// wasn't word-level. The fallback chain stays Musixmatch-free; the -/// dedicated tier owns it. -fn external_fallback_providers() -> Vec { - // LRCLIB leads even though tier 5 already asked it, because the two - // ask *differently*: tier 5 hits `/api/get`, which matches on artist - // + track + album + duration and 404s when any of them disagrees - // with the file's tags (a remaster, a "Deluxe" album name, a rip a - // few seconds off). This chain goes through the provider's - // `/api/search`, which is fuzzy. - // - // Without it, that 404 dropped straight to providers that answer for - // almost anything — so a track LRCLIB *does* carry came back from - // Genius, and picking LRCLIB by hand in the panel (fuzzy search) - // found it immediately. That contradiction is what issue #463 - // reported. - vec![ - Provider::Lrclib, - Provider::NetEase, - Provider::Megalobiz, - Provider::Genius, - ] -} - /// Process-wide shared `SyncedLyricsClient`. Standing one up per call /// re-initialises rustls + builds a fresh reqwest connection pool every /// time `external_lyrics_search` runs — wasted work, especially during @@ -434,10 +416,15 @@ fn shared_external_client() -> AppResult<&'static SyncedLyricsClient> { /// caches a permanent "no lyrics". enum SearchOutcome { Found(ExternalLyricsResult), - /// Providers were queried and none had lyrics. A real negative. - Miss, - /// Nothing was queried at all — offline mode, or the provider list was - /// empty once the Musixmatch opt-in filter ran. Never a negative. + /// At least one provider answered and none had lyrics. A real + /// negative — complete when every provider asked answered, `partial` + /// when some failed or were left out (#720). + Miss { + partial: bool, + }, + /// Nobody answered — nothing was queried at all (offline mode, an + /// empty provider list once the Musixmatch opt-in filter ran, every + /// provider cooling down). Never a negative. Unavailable, } @@ -475,9 +462,7 @@ async fn external_lyrics_search( }; let query = external_query(&meta.title, meta.artist_name.as_deref()); let client = shared_external_client()?; - // A transient provider error surfaces as Err here (never as a miss) so - // callers don't cache an empty row on a network blip. - let found = client + let mut report = client .search(SearchOptions { query, mode, @@ -487,12 +472,69 @@ async fn external_lyrics_search( genius_cookie: std::env::var("SYNCEDLYRICS_GENIUS_COOKIE").ok(), netease_cookie: std::env::var("SYNCEDLYRICS_NETEASE_COOKIE").ok(), }) - .await - .map_err(|err| AppError::Other(format!("external lyrics search failed: {err}")))?; - // We got here only by actually querying, so `None` is a real negative. - Ok(match found { - Some(result) => SearchOutcome::Found(result), - None => SearchOutcome::Miss, + .await; + for provider in &report.answered { + lyrics_providers::record_answer(*provider); + } + for failure in &report.failures { + lyrics_providers::record_failure(failure.provider, &failure.error); + } + if let Some(result) = report.result.take() { + return Ok(SearchOutcome::Found(result)); + } + if report.has_verdict() { + return Ok(SearchOutcome::Miss { + partial: !report.failures.is_empty(), + }); + } + // Nobody answered. An error rather than `Unavailable` when something + // was asked and failed, so a user who picked one provider by name is + // told it failed instead of being shown nothing. + match report.failures.pop() { + Some(failure) => Err(AppError::Other(format!( + "external lyrics search failed: {}: {}", + failure.provider.as_str(), + failure.error + ))), + None => Ok(SearchOutcome::Unavailable), + } +} + +/// Ask the query-based chain, as this profile has configured it. +/// +/// Every automatic lookup that walks the chain comes through here — +/// the panel, the bulk prefetch, radio, remote tracks — so the user's +/// switches (#722) and the provider cooldown (#720) apply everywhere +/// without each caller spelling them. A provider named in the panel's +/// picker does not: that is a request about that one provider. +/// +/// A provider left out for its cooldown makes a miss partial, exactly +/// as if it had been asked and failed: it was not heard. A provider the +/// user switched off does not — they decided not to ask it. +async fn search_fallback_chain( + pool: &sqlx::SqlitePool, + meta: &TrackMeta, + enhanced: bool, +) -> AppResult { + let (cooling, live): (Vec, Vec) = lyrics_providers::enabled_chain(pool) + .await? + .into_iter() + .partition(|p| lyrics_providers::is_cooling_down(*p)); + let outcome = external_lyrics_search( + meta, + live, + SearchMode::PreferSynced, + enhanced, + // The chain excludes Musixmatch, so `external_lyrics_search` + // would drop a language anyway. + None, + ) + .await?; + Ok(match outcome { + SearchOutcome::Miss { partial } => SearchOutcome::Miss { + partial: partial || !cooling.is_empty(), + }, + other => other, }) } @@ -558,6 +600,10 @@ const PLUGIN_PROVIDER_PREFIX: &str = "plugin:"; /// `language` is what the SOURCE said, not a user preference: only the /// v2 plugin world reports one today, and every other tier passes `None` /// exactly as this row has always been written. +/// +/// `retry_after` is written on every call, `NULL` included, so a row that +/// replaces a partial miss never inherits its expiry — lyrics found after +/// one must not be thrown away a week later. #[allow(clippy::too_many_arguments)] async fn write_primary_lyrics( conn: &mut sqlx::SqliteConnection, @@ -568,17 +614,20 @@ async fn write_primary_lyrics( provider: Option<&str>, language: Option<&str>, fetched_at: i64, + retry_after: Option, ) -> AppResult<()> { sqlx::query( - "INSERT INTO app.lyrics (file_hash, content, format, source, provider, language, fetched_at) - VALUES (?, ?, ?, ?, ?, ?, ?) + "INSERT INTO app.lyrics + (file_hash, content, format, source, provider, language, fetched_at, retry_after) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(file_hash) DO UPDATE SET content = excluded.content, format = excluded.format, source = excluded.source, provider = excluded.provider, language = excluded.language, - fetched_at = excluded.fetched_at", + fetched_at = excluded.fetched_at, + retry_after = excluded.retry_after", ) .bind(file_hash) .bind(content) @@ -587,6 +636,7 @@ async fn write_primary_lyrics( .bind(provider) .bind(language) .bind(fetched_at) + .bind(retry_after) .execute(&mut *conn) .await?; Ok(()) @@ -728,6 +778,7 @@ async fn cache_lyrics_bundle( Some(&provider_id), bundle.primary.language.as_deref(), now, + None, ) .await?; @@ -817,44 +868,40 @@ async fn read_associated( enum FallbackOutcome { /// A provider had lyrics — already cached. Found(LyricsPayload), - /// Every provider answered and none had lyrics. A real negative, safe - /// to remember so the panel stops re-hitting the network on each open. - Miss, - /// The chain could not be consulted at all. NOT a negative — nothing - /// may be cached, so the next attempt retries. + /// Providers answered and none had lyrics. A real negative, safe to + /// remember so the panel stops re-hitting the network on each open — + /// for good when complete, until [`PARTIAL_MISS_RETRY`] when `partial`. + Miss { partial: bool }, + /// Nobody in the chain answered. NOT a negative — nothing may be + /// cached, so the next attempt retries. Unavailable, } -/// Walk the post-LRCLIB fallback chain (NetEase / Megalobiz / Genius) and -/// persist the first hit. +/// How long a partial miss (#720) is served before the track is looked up +/// again. Long enough that a provider down for good costs one lookup per +/// track per week instead of one per panel open; short enough that a +/// provider which was only briefly down gets its chance. +const PARTIAL_MISS_RETRY: Duration = Duration::from_secs(7 * 24 * 60 * 60); + +/// Walk the post-LRCLIB fallback chain and persist the first hit. /// /// A provider `Err` stays non-fatal for the waterfall — LRCLIB has already /// given its verdict and this chain is best-effort enrichment, so we don't -/// fail the whole lookup. But it now surfaces as [`FallbackOutcome::Unavailable`] -/// rather than masquerading as a miss, so the caller knows not to persist -/// anything. +/// fail the whole lookup. When nobody in the chain answered it surfaces as +/// [`FallbackOutcome::Unavailable`] rather than masquerading as a miss, so +/// the caller knows not to persist anything. async fn try_external_fallback( pool: &sqlx::SqlitePool, track_id: i64, meta: &TrackMeta, ) -> AppResult { - match external_lyrics_search( - meta, - external_fallback_providers(), - SearchMode::PreferSynced, - true, - // Fallback chain excludes Musixmatch — `external_lyrics_search` - // would drop the lang anyway. Keep it `None` to flag intent. - None, - ) - .await - { + match search_fallback_chain(pool, meta, true).await { Ok(SearchOutcome::Found(result)) => Ok(FallbackOutcome::Found( cache_external_lyrics(pool, track_id, &meta.file_hash, result).await?, )), // Propagated straight through: only the search itself knows whether // a request was issued, so nothing here re-derives it. - Ok(SearchOutcome::Miss) => Ok(FallbackOutcome::Miss), + Ok(SearchOutcome::Miss { partial }) => Ok(FallbackOutcome::Miss { partial }), Ok(SearchOutcome::Unavailable) => Ok(FallbackOutcome::Unavailable), Err(err) => { tracing::debug!(?err, "external fallback chain failed; not caching a miss"); @@ -863,6 +910,12 @@ async fn try_external_fallback( } } +/// When a miss written now should be looked up again: never for a +/// complete one, after [`PARTIAL_MISS_RETRY`] for a partial one. +fn miss_retry_after(partial: bool) -> Option { + partial.then(|| now_ms() + PARTIAL_MISS_RETRY.as_millis() as i64) +} + /// Cache an empty "miss" row so the panel doesn't re-hit the network on /// every open, and return it as a payload. The user can force a /// re-search via "Refetch" in the lyrics panel (clears the row, re-runs @@ -872,15 +925,17 @@ async fn cache_lyrics_miss( pool: &sqlx::SqlitePool, track_id: i64, meta: &TrackMeta, + partial: bool, ) -> AppResult { let empty = String::new(); - upsert_lyrics( + upsert_lyrics_until( pool, &meta.file_hash, &empty, &LyricsFormat::Plain, &LyricsSource::Api, None, + miss_retry_after(partial), ) .await?; Ok(LyricsPayload { @@ -927,7 +982,9 @@ async fn resolve_after_lrclib_miss( } match outcome { - FallbackOutcome::Miss => cache_lyrics_miss(pool, track_id, meta).await.map(Some), + FallbackOutcome::Miss { partial } => cache_lyrics_miss(pool, track_id, meta, partial) + .await + .map(Some), // Transient: persist nothing so the next attempt can retry. FallbackOutcome::Unavailable => Ok(None), // Handled above. @@ -1424,6 +1481,20 @@ async fn upsert_lyrics( format: &LyricsFormat, source: &LyricsSource, provider: Option<&str>, +) -> AppResult<()> { + upsert_lyrics_until(pool, file_hash, content, format, source, provider, None).await +} + +/// [`upsert_lyrics`] for a row that is only good until `retry_after` +/// (epoch ms) — a partial miss (#720). `None` is a row kept for good. +async fn upsert_lyrics_until( + pool: &sqlx::SqlitePool, + file_hash: &str, + content: &str, + format: &LyricsFormat, + source: &LyricsSource, + provider: Option<&str>, + retry_after: Option, ) -> AppResult<()> { let mut tx = pool.begin().await?; write_primary_lyrics( @@ -1435,6 +1506,7 @@ async fn upsert_lyrics( provider, None, now_ms(), + retry_after, ) .await?; // Replacing the primary document ends the bundle it belonged to. @@ -1589,8 +1661,8 @@ async fn export_fetched_sidecar( /// numeric `track_id` so we look up the file hash first, then key into the /// shared `app.lyrics` cache. async fn read_cached(pool: &sqlx::SqlitePool, track_id: i64) -> AppResult> { - let row: Option<(String, String, String, Option)> = sqlx::query_as( - "SELECT l.content, l.format, l.source, l.provider + let row: Option<(String, String, String, Option, Option)> = sqlx::query_as( + "SELECT l.content, l.format, l.source, l.provider, l.retry_after FROM track t JOIN app.lyrics l ON l.file_hash = t.file_hash WHERE t.id = ?", @@ -1599,6 +1671,17 @@ async fn read_cached(pool: &sqlx::SqlitePool, track_id: i64) -> AppResult