fix(lyrics): strip Genius header + track real provider + user-driven refetch (closes #284)#287
Merged
Merged
Conversation
Genius recently moved the contributor badge + `<song title> Lyrics` heading inside the same `data-lyrics-container` div that holds the actual verses. Once our HTML decoder flattens the tags we end up with a glued-together prefix like `"17 ContributorsFall on Me Lyrics"` sitting at the head of every result, plus — for songs that have been catalogued on Genius without any transcribed lyrics yet — pages whose ONLY visible content is that bare header. Image 1 in issue #284 shows the bare-header case ("Solamente una vez": panel renders `"2 ContributorsSolamente Una Vez Lyrics"` and nothing else); image 2 shows the prefix-glue case ("Fall on Me": header runs into the chorus). Add `strip_genius_header` to pattern-match the `^\d+ Contributors.*? Lyrics` shape and drop it before returning the candidate. Pure prefix surgery: anything that doesn't match the shape (no leading digits, no " Contributors" token, no trailing " Lyrics" closer) is returned untouched, so a future Genius layout change won't silently lose the first line. After stripping, an empty body is treated as a miss — the host then falls through to the next provider in the chain (NetEase / Megalobiz) instead of caching the bare header as the user-visible lyrics. Seven new unit tests cover the happy path, the stub-page case, the multi-byte title path (Déjà Vu), and four no-op shapes (leading whitespace, no digit prefix, missing " Contributors", missing " Lyrics", title starting with a number à la "99 Red Balloons").
Backend half of the fix for issue #284. Two problems being addressed: 1. The lyrics panel hard-codes "LRCLIB" as the source badge no matter which provider actually responded. After the LRCLIB miss, the waterfall falls through to NetEase / Megalobiz / Genius — and when Genius hands back junk (see the sibling Genius header-strip commit in this branch), the user has no way to know what they're looking at or how to retry from a cleaner source. 2. The auto-waterfall has no manual override. If Genius caches a bad result, the only escape today is "Clear" + "Refetch" which just re-runs the same waterfall and lands on the same Genius hit. ## Migration `20260620120000_lyrics_provider.sql` adds a nullable `provider` column to `app.lyrics`. NULL means "unknown" — covers pre-1.5.1 cached rows plus every non-API tier (embedded / sidecar / manual) where the broad `source` is already meaningful attribution. No CHECK constraint on the allowed set: provider strings come from `Provider::as_str()` at the single write site, and a CHECK that hard-codes the list would force another migration every time the SDK gains a provider. ## Provider plumbing - `LyricsPayload` gains `provider: Option<String>`, serialised via `skip_serializing_if = "Option::is_none"` so the JSON wire shape stays compact on tiers where the field is meaningless. - `Provider::as_str()` + `Provider::from_str()` added to `waveflow_syncedlyrics`, matching the existing `serde(rename_all = "snake_case")` ids (`"lrclib"` / `"genius"` / `"net_ease"` / `"megalobiz"` / `"musixmatch"`). Single canonical id format across DB rows, payload field, and the frontend ↔ backend boundary. - `upsert_lyrics` takes a 6th `provider: Option<&str>` parameter. Every call site updated: embedded / sidecar / manual write `None`; LRCLIB direct + LRCLIB instrumental cache + LRCLIB pre-cache in `fetch_lyrics` and the prefetch path all write `Some("lrclib")`; `cache_external_lyrics` derives the provider id from `LyricsResult::provider.as_str()` so NetEase / Megalobiz / Genius / Musixmatch all attribute correctly. - `read_cached` SELECTs the new column and surfaces it on the payload. - Empty miss rows (after a full waterfall failure) write `provider = None` — attribution to a specific provider would be misleading when every provider returned nothing. ## refetch_lyrics command New `#[tauri::command] pub async fn refetch_lyrics(track_id, provider: Option<String>)`. Drops the cached row first so the waterfall (or single-provider query below) is forced to re-query, then two modes: - `provider = None` → re-runs the full waterfall by calling `fetch_lyrics` directly. Identical to the legacy "Clear + Refetch" flow. - `provider = Some(id)` → bypasses local tiers (embedded / sidecar) and queries ONLY the named provider via `external_lyrics_search`. Honours the Musixmatch translation lang and the offline-mode short-circuit on the same chokepoints as the waterfall. A miss caches an empty row attributed to the requested provider so the UI badge reflects what the user just tried — picking a different provider and trying again is then the natural next step. Registered alongside `fetch_lyrics` in `lib.rs::generate_handler![]`. 84 desktop unit tests + workspace `cargo check --all-targets` still pass with the migration applied to a fresh `app.db`. Frontend wiring (provider badge + dropdown picker) lands in the next commit.
Frontend half of the fix for issue #284. Pairs with the migration + backend plumbing in the prior commit. ## Provider badge `sourceLabel` now reads `payload.provider` and renders the canonical brand name when the row is API-sourced (`"LRCLIB"` / `"Genius"` / `"NetEase"` / `"Megalobiz"` / `"Musixmatch"`). Falls back to the generic localised "Online source" label when the provider is `null` — covers pre-1.5.1 cached rows and the empty-miss rows the backend deliberately leaves unattributed. The badge text was previously hardcoded as "LRCLIB" regardless of which provider actually responded, which made the auto-waterfall's Genius fallback look like a LRCLIB bug. With this change the user can read what actually ran. ## Provider picker When the row is API-sourced, the source label becomes a chip-button with a chevron. Clicking opens a dropdown of all five providers; the currently-active one carries a check mark. Picking any provider calls the new `refetch_lyrics(track_id, provider)` Tauri command, which drops the cached row + queries ONLY that provider — the escape hatch users need when the auto-waterfall caches a low-quality hit they can't recover from with the existing "Clear + Refetch". Picker is anchored to the source label via a `<span class="relative">` wrapper so the menu floats above the footer instead of pushing it. Closed by an outside click + by `handleRefetch` itself after the new fetch lands. `role="menu"` + `menuitemradio` items for WAI-ARIA. Embedded / sidecar / manual rows render the label as static text — the picker would have nothing meaningful to do for a tag-embedded lyric, and the chevron would just be noise. ## Backend command swap `handleRefetch` switches from the legacy `clearLyrics + fetchLyrics` two-call gesture to the new single-call `refetchLyrics`, which does the cache drop + the re-query atomically on the backend. The parameter-less variant (clicked from the existing refresh button) re-runs the full waterfall identical to the legacy flow; the provider-bearing variant (from the picker) targets one source. ## i18n Two new keys + one rewrite across all 17 locales: - `lyrics.source.api` rewritten from the verbatim "LRCLIB" to a localised "Online source" — the generic fallback when the actual provider is unknown. - `lyrics.source.pickerHint` added — tooltip on the chip-button ("Switch source"). - `lyrics.provider.{lrclib,genius,net_ease,megalobiz,musixmatch}` added — provider display names. Brand tokens stay verbatim per the CLAUDE.md "Daily Mix / On Repeat" precedent (Spotify keeps product names untranslated, we do the same for the providers themselves). `bun run typecheck` clean.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCe PR ajoute l'attribution du fournisseur de paroles (LRCLIB, Genius, NetEase, Megalobiz, Musixmatch) depuis une nouvelle colonne SQL ChangesTraçage du fournisseur de paroles
Sequence Diagram(s)sequenceDiagram
participant Utilisateur
participant LyricsPanel as LyricsPanel (UI)
participant refetchLyrics_TS as refetchLyrics (TS)
participant refetch_lyrics_Rust as refetch_lyrics (Rust)
participant upsert_lyrics
participant DB as lyrics (DB)
Utilisateur->>LyricsPanel: Sélectionne un provider dans le dropdown
LyricsPanel->>refetchLyrics_TS: refetchLyrics(trackId, provider)
refetchLyrics_TS->>refetch_lyrics_Rust: invoke("refetch_lyrics", {trackId, provider})
refetch_lyrics_Rust->>upsert_lyrics: provider = Some(provider_str) ou None (miss)
upsert_lyrics->>DB: INSERT/UPDATE provider
DB-->>refetch_lyrics_Rust: OK
refetch_lyrics_Rust-->>LyricsPanel: LyricsPayload { provider }
LyricsPanel->>LyricsPanel: Affiche badge provider + coche active
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
InstaZDLL
added
scope: frontend
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/crates/syncedlyrics/src/providers/genius.rs`:
- Around line 113-118: The use of find(" Lyrics") at line 113 in the lyrics
extraction logic is fragile because it locates the first occurrence of the
string " Lyrics", but if the song title itself contains "Lyrics" (such as "No
Lyrics Needed"), the code will cut at the wrong position and leave header
artifacts. Replace the find() method with rfind() to search for the last
occurrence of " Lyrics" instead, which will correctly identify the actual lyrics
section header separator rather than any occurrence within the title text.
In `@src/components/layout/LyricsPanel.tsx`:
- Line 71: The pickerRef variable is declared with the wrong TypeScript type. It
is currently typed as HTMLDivElement but is being attached to a span element in
the JSX. Change the type annotation of pickerRef from useRef<HTMLDivElement |
null>(null) to useRef<HTMLSpanElement | null>(null) to match the actual HTML
element it references.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e8f15fb1-258e-4f91-9660-ced73fddcde2
📒 Files selected for processing (24)
src-tauri/crates/app/src/commands/lyrics.rssrc-tauri/crates/app/src/lib.rssrc-tauri/crates/syncedlyrics/src/providers/genius.rssrc-tauri/crates/syncedlyrics/src/providers/mod.rssrc-tauri/migrations/app/20260620120000_lyrics_provider.sqlsrc/components/layout/LyricsPanel.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/id.jsonsrc/i18n/locales/it.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/nl.jsonsrc/i18n/locales/pt-BR.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/ru.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/zh-CN.jsonsrc/i18n/locales/zh-TW.jsonsrc/lib/tauri/lyrics.ts
Two findings flagged on PR #287. ## Genius header strip anchors on newline (not first/last match) CodeRabbit pointed out that `find(" Lyrics")` cuts at the WRONG position when the song title itself contains "Lyrics" (e.g. "No Lyrics Needed" by The Avalanches): the first match lands inside the title rather than at the closer, and the strip leaves " Needed Lyrics\n…" stuck to the body. The suggested fix (swap `find` for `rfind`) trades one bug for another: a body line like "Some Lyrics" would then match LAST and the strip would eat past the closer into the verses. Neither extreme is correct — the header closer is the FIRST " Lyrics" immediately followed by a newline or end-of-string, since Genius always writes either `<title> Lyrics\n[Verse…]` (when transcribed) or `<title> Lyrics` at end-of-string (stub catalogue entry). Iterate via `match_indices(" Lyrics")` and take the first hit whose tail starts with `\n` / `\r` / is empty. Two new tests pin both edge cases: - `strip_header_picks_closer_when_title_contains_lyrics_word` - `strip_header_does_not_swallow_body_containing_lyrics_word` ## pickerRef typed as HTMLDivElement attached to a span `LyricsPanel`'s provider picker hangs off a `<span class="relative inline-flex">` wrapper around the source label, so `useRef`'s parameter must be `HTMLSpanElement`. The mismatch compiled under React's loose ref typing today, but any future code that reads `pickerRef.current.classList` or similar would see a stale `DOMTokenList | undefined` shape against the actual span.
Owner
Author
|
@coderabbitai Both findings addressed in Finding 1 (genius.rs strip) — flagged the issue accurately, but
Finding 2 (pickerRef type) — straight fix, |
|
Tip For best results, initiate chat on the files or code changes.
🐇 ✓ |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/layout/LyricsPanel.tsx (1)
172-194:⚠️ Potential issue | 🟠 Major | ⚡ Quick winÉvite l’écrasement par une réponse
refetchobsolète.Si la piste change pendant
await refetchLyrics(...), la réponse de l’ancienne piste peut écraserpayloadde la nouvelle. Ajoute un garde (id de requête ou ref dutrackIdcourant) avantsetPayload(next).Correctif minimal proposé
+ const latestTrackIdRef = useRef<typeof trackId>(trackId); + useEffect(() => { + latestTrackIdRef.current = trackId; + }, [trackId]); + const handleRefetch = async (provider?: LyricsProvider) => { if (trackId == null) return; + const requestTrackId = trackId; try { setIsFetching(true); setPickerOpen(false); const next = await refetchLyrics(trackId, provider); + if (latestTrackIdRef.current !== requestTrackId) return; setPayload(next); setError(null);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/LyricsPanel.tsx` around lines 172 - 194, The handleRefetch function has a race condition where if the trackId changes while awaiting refetchLyrics, the old response could overwrite the payload of the new track. Capture the trackId at the start of handleRefetch before the await call, then add a guard condition before setPayload(next) to verify that the trackId hasn't changed since the refetch was initiated. Only proceed with setPayload if the captured trackId matches the current trackId, ensuring stale responses from previous tracks do not overwrite the current payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/crates/syncedlyrics/src/providers/genius.rs`:
- Around line 106-108: The strip_prefix call in the genius provider only handles
the plural form " Contributors" but Genius can also return the singular form "
Contributor". Modify the strip_prefix logic to handle both singular and plural
cases. You can achieve this by checking if after_digits starts with either "
Contributor" or " Contributors", handling both possibilities before continuing
with the text processing.
---
Outside diff comments:
In `@src/components/layout/LyricsPanel.tsx`:
- Around line 172-194: The handleRefetch function has a race condition where if
the trackId changes while awaiting refetchLyrics, the old response could
overwrite the payload of the new track. Capture the trackId at the start of
handleRefetch before the await call, then add a guard condition before
setPayload(next) to verify that the trackId hasn't changed since the refetch was
initiated. Only proceed with setPayload if the captured trackId matches the
current trackId, ensuring stale responses from previous tracks do not overwrite
the current payload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cd168db3-0c18-4ea6-931a-82f798971805
📒 Files selected for processing (2)
src-tauri/crates/syncedlyrics/src/providers/genius.rssrc/components/layout/LyricsPanel.tsx
Two new CodeRabbit findings on PR #287. ## Singular "Contributor" badge Genius pluralises the contributor badge conditionally — a track with a single transcriber renders "1 Contributor" (no trailing 's'). The previous `strip_prefix(" Contributors")` missed this shape and a 1-transcriber track shipped the bare header to the user. Strip the singular `" Contributor"` first and consume an optional `'s'` to cover both forms in a single pass instead of two near-identical branches. New test `strip_header_handles_singular_contributor_badge` pins the singular case. ## handleRefetch stale-response race A refetch on track A that takes longer than the user's switch to track B would land its result into B's payload — the same trap the initial-fetch useEffect already avoided with its per-render `cancelled` flag. User-triggered handlers can't rely on effect cleanup for the same job, so add a `trackIdRef` synced via a sibling effect and have `handleRefetch` capture the requested trackId at call time and guard every setter (`setPayload`, `setError`, `setIsFetching`) against it. The `setIsFetching(false)` guard is what kept this from being a simple one-line capture — clearing the spinner after the user switched would race the new track's useEffect which already flipped it to `true` for its own request.
Owner
Author
|
@coderabbitai Both round-2 findings addressed in Finding 1 (singular "Contributor" badge) — valid. Strip Finding 2 (handleRefetch race) — valid. Captured |
|
Tip For best results, initiate chat on the files or code changes.
🐇 ✓ |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/layout/LyricsPanel.tsx (2)
485-513: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueLe menu dropdown ne gère pas la touche Escape pour fermer.
Le picker utilise
role="menu"ce qui implique une navigation clavier selon les pratiques ARIA. Actuellement, seul le clic extérieur ferme le menu. Ajouter Escape améliorerait l'UX clavier.♿ Suggestion d'amélioration
Ajouter un handler
onKeyDownsur le conteneur du menu ou étendre l'effet click-outside :useEffect(() => { if (!pickerOpen) return; const handler = (e: MouseEvent) => { if ( pickerRef.current && e.target instanceof Node && pickerRef.current.contains(e.target) ) { return; } setPickerOpen(false); }; + const keyHandler = (e: KeyboardEvent) => { + if (e.key === "Escape") { + setPickerOpen(false); + } + }; document.addEventListener("mousedown", handler); + document.addEventListener("keydown", keyHandler); return () => { document.removeEventListener("mousedown", handler); + document.removeEventListener("keydown", keyHandler); }; }, [pickerOpen]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/LyricsPanel.tsx` around lines 485 - 513, The menu dropdown with role="menu" lacks keyboard support for closing via the Escape key, which is expected behavior for ARIA menus. Add an onKeyDown event handler to the div element with role="menu" that checks if the pressed key is Escape, and if so, calls a function to close the picker (set pickerOpen to false). This handler should be added alongside the existing className and aria-label attributes to provide proper keyboard navigation support.
165-183:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
handleImporta la même vulnérabilité de race condition quehandleRefetchmais n'est pas protégé.Le PR corrige explicitement la race condition dans
handleRefetch(un refetch sur track A qui atterrit dans le payload de track B après un switch).handleImportsuit exactement le même pattern async : l'utilisateur peut ouvrir le file picker sur track A, changer de track pendant que le picker est ouvert, puis les lyrics importées écrasent le payload de track B.🐛 Correction proposée
const handleImport = async () => { if (trackId == null) return; + const requestedTrackId = trackId; try { const path = await pickFile( ["lrc", "elrc", "ttml", "xml", "txt"], t("lyrics.importTitle"), ); if (!path) return; - const next = await importLrcFile(trackId, path); + const next = await importLrcFile(requestedTrackId, path); + if (requestedTrackId !== trackIdRef.current) return; setPayload(next); - // Drop any error left from a prior failed fetch — otherwise the - // error-vs-notFound conditional below would mask the freshly - // imported lyrics behind the stale error state. setError(null); } catch (err) { console.error("[LyricsPanel] import failed", err); + if (requestedTrackId !== trackIdRef.current) return; setError(String(err)); } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/LyricsPanel.tsx` around lines 165 - 183, The handleImport function has a race condition where the trackId can change after the async file picker operation starts but before setPayload is called, causing imported lyrics from one track to overwrite another track's payload. Capture the current trackId value at the beginning of the handleImport function before calling pickFile, and add a guard check after the importLrcFile call completes to verify the trackId has not changed (compare the captured value with the current trackId). Only call setPayload if the trackIds match, preventing stale imports from overwriting the payload of a different track after the user has switched tracks during the picker operation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/layout/LyricsPanel.tsx`:
- Around line 485-513: The menu dropdown with role="menu" lacks keyboard support
for closing via the Escape key, which is expected behavior for ARIA menus. Add
an onKeyDown event handler to the div element with role="menu" that checks if
the pressed key is Escape, and if so, calls a function to close the picker (set
pickerOpen to false). This handler should be added alongside the existing
className and aria-label attributes to provide proper keyboard navigation
support.
- Around line 165-183: The handleImport function has a race condition where the
trackId can change after the async file picker operation starts but before
setPayload is called, causing imported lyrics from one track to overwrite
another track's payload. Capture the current trackId value at the beginning of
the handleImport function before calling pickFile, and add a guard check after
the importLrcFile call completes to verify the trackId has not changed (compare
the captured value with the current trackId). Only call setPayload if the
trackIds match, preventing stale imports from overwriting the payload of a
different track after the user has switched tracks during the picker operation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cdb89c78-9556-4d08-9e78-afb78d0dcb5d
📒 Files selected for processing (2)
src-tauri/crates/syncedlyrics/src/providers/genius.rssrc/components/layout/LyricsPanel.tsx
Two CodeRabbit round-3 findings on PR #287. ## Escape closes the provider picker The picker dropdown carried `role="menu"` but only closed on outside-click — the WAI-ARIA APG dismissal pattern also requires Escape regardless of where focus sits. Extended the existing mousedown-listener useEffect to also bind a document-level keydown handler for Escape (catches focus-on-trigger AND focus-on-item cases in one place), plus mirrored the handler on the menu div itself with `e.stopPropagation()` so the event doesn't bubble into any ancestor panel that might also listen for Escape. ## handleImport stale-response race Same shape as the handleRefetch race fixed in the previous commit: the user can switch tracks during the file picker (which can sit on screen for a while) and again during the `importLrcFile` disk + DB work. Without a guard the stale payload would clobber the new track's UI. The fix mirrors handleRefetch: capture `requestedTrackId` at the call site, compare against `trackIdRef.current` after each await, skip the UI updates when stale. Crucially the `importLrcFile` call itself is NOT cancelled when the user switches away — the write goes to the captured track's DB row, which IS what the user asked for; only the UI updates skip when no longer current.
Owner
Author
|
@coderabbitai Both round-3 findings addressed in Finding 1 (Escape key on menu) — valid per WAI-ARIA APG. Extended the existing mousedown useEffect to ALSO bind a document-level keydown for Escape (catches focus-on-trigger AND focus-on-item without depending on event bubbling through the menu div), plus mirrored the handler on the menu div itself with Finding 2 (handleImport race) — valid, same shape as handleRefetch from round 2. Captured |
|
Tip For best results, initiate chat on the files or code changes.
🐇 ✓ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft pour test manuel. Trois commits cohérents qui répondent à toutes les frictions de l'issue #284.
Closes #284
Le bug d'origine
Quand LRCLIB miss, la cascade tombait sur NetEase / Megalobiz / Genius automatiquement. Trois problèmes empilés :
Les trois fixes
1. `fix(lyrics): strip Genius header artifact + treat stub pages as miss` (`5e889c4`)
Ajout de `strip_genius_header` dans crates/syncedlyrics/src/providers/genius.rs : pattern `^\d+ Contributors.*? Lyrics` strippé en début de body. Si après strip le contenu est vide, le candidate retourne None — la cascade passe au provider suivant au lieu de cacher le header brut.
7 nouveaux tests unitaires : happy path, stub page (Solamente Una Vez), multibyte title (Déjà Vu), 4 cas no-op (whitespace prefix, no digits, no "Contributors" token, titre commençant par un nombre genre "99 Red Balloons").
2. `fix(lyrics): track real provider per row + add user-driven refetch` (`5e8f7c4`)
Migration `20260620120000_lyrics_provider.sql` : nouvelle colonne `lyrics.provider TEXT NULL`. NULL = inconnu (rows pré-1.5.1 + tiers non-API).
Provider plumbing :
Nouvelle commande `refetch_lyrics(track_id, provider: Option)` :
Registrée dans `lib.rs::generate_handler![]`.
3. `fix(lyrics): expose real provider in source badge + add picker dropdown` (`71b05ff`)
Badge : `sourceLabel` lit `payload.provider` et rend le brand name réel ("LRCLIB" / "Genius" / "NetEase" / "Megalobiz" / "Musixmatch"). Fallback générique "Online source" localisé quand provider = NULL.
Picker : quand row est API-sourced, le label devient un chip-button avec chevron. Click → dropdown des 5 providers avec checkmark sur l'actif. Click sur un provider → `refetchLyrics(track_id, provider)`. `role="menu"` + `menuitemradio`, fermé par click outside.
Embedded/sidecar/manual rows : label statique sans chevron (picker n'a aucun sens pour un tag embedded).
Backend command swap : `handleRefetch` passe du legacy `clearLyrics + fetchLyrics` au nouveau single-call `refetchLyrics`.
i18n sur les 17 locales :
Tests
Summary by CodeRabbit
Nouvelles fonctionnalités et améliorations