feat(lyrics): add waveflow-syncedlyrics crate with multi-provider lookup#207
Open
InstaZDLL wants to merge 4 commits into
Open
feat(lyrics): add waveflow-syncedlyrics crate with multi-provider lookup#207InstaZDLL wants to merge 4 commits into
InstaZDLL wants to merge 4 commits into
Conversation
Rust port of the syncedlyrics Python project providing multi-provider lyrics lookup (Musixmatch, LRCLIB, NetEase, Megalobiz, Genius). Wired into commands/lyrics.rs: Musixmatch Enhanced is tried before LRCLIB but only when it returns true word-level lyrics; LRCLIB exact stays the strict match. Adds THIRD_PARTY_NOTICES.md preserving the original MIT notice (Copyright (c) 2022 Momo, Copyright (c) 2026 InstaZDLL); WaveFlow remains GPL-3.0-only.
InstaZDLL
added
scope: backend
|
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)
📝 WalkthroughWalkthroughCette PR ajoute une crate Rust indépendante ChangesArchitecture multi-fournisseurs de paroles
Sequence DiagramsequenceDiagram
participant App as App::fetch_lyrics / prefetch
participant Synced as SyncedLyricsClient
participant Provider as Provider (Musixmatch / LRCLIB / NetEase / Megalobiz / Genius)
participant Cache as LyricsCache
App->>Cache: check existing entry
alt miss
App->>Synced: search(SearchOptions)
Synced->>Provider: call provider.search(...)
Provider-->>Synced: Candidate (synced/unsynced)
Synced-->>App: LyricsResult | None
alt result
App->>Cache: upsert result
else no result
App->>Cache: upsert empty miss
end
else hit
Cache-->>App: return cached content
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/app/src/commands/lyrics.rs`:
- Around line 289-292: Le match arm Err(err) => { tracing::debug!(?err,
"external lyrics search failed"); None } wrongly maps provider errors to “no
result” causing transient failures to be cached as empty; change the function to
propagate errors instead of returning None on Err: update the function signature
to return Result<Option<...>, E> (or propagate the provider error type), replace
the Err(...) branch with returning Err(err) (or use the ? operator where
appropriate), and ensure the caching path only stores an empty entry for
explicit Ok(None) results so transient provider errors are not cached as misses.
- Around line 249-253: The external_query function builds a search string using
artist_name but incorrectly splits multi-artist tags with split(", "), which
breaks names containing commas; update the logic in external_query (the branch
that defines primary_artist) to split on "; " instead (i.e., use split(";
").next().unwrap_or(artist)) so multi-artist queries follow the project's "; "
separator convention while preserving artists containing commas.
- Around line 268-294: Add a process-wide offline guard inside
external_lyrics_search so outbound network calls are short-circuited: before
constructing SyncedLyricsClient or calling client.search(...), check
offline::is_offline() and immediately return None when offline; this ensures
external_lyrics_search enforces the “offline process-wide” contract regardless
of callers and prevents the SyncedLyricsClient::search path from performing HTTP
requests while offline.
In `@src-tauri/crates/syncedlyrics/src/lib.rs`:
- Around line 116-123: Replace the panicking SyncedLyricsClient::new() with a
fallible constructor: add SyncedLyricsClient::try_new() -> Result<Self,
crate::Error> that builds the reqwest::Client and maps reqwest::Error into
Error::Http (or your crate::Error::Http variant); update all callsites that
construct the client (notably external_lyrics_search and the lyrics command at
src-tauri/crates/app/src/commands/lyrics.rs:275) to call try_new() and propagate
or handle the Result instead of letting it panic; keep or adapt impl Default to
avoid a global panic by using try_new().unwrap_or_else(|err| { log the error and
return a safe fallback Self { http: reqwest::Client::new() } }) so Default
remains usable without aborting the process.
In `@src-tauri/crates/syncedlyrics/src/providers/genius.rs`:
- Around line 17-23: The code currently uses a fixed index sections[1] when
extracting the Genius URL which breaks if section order changes; instead
iterate/search the sections array to find the first section that has a non-empty
"hits" array and extract the first hit's "result.url". Replace the chain
starting with response["response"]["sections"].as_array().and_then(|sections|
sections.get(1)) with logic that calls .as_array() on sections and then
.iter().find_map(|section| section["hits"].as_array().and_then(|hits|
hits.first()).and_then(|hit| hit["result"]["url"].as_str())) so you return the
first available URL reliably (referencing the response variable and the
"hits"/"result.url" accessors).
In `@src-tauri/crates/syncedlyrics/src/providers/lrclib.rs`:
- Around line 54-83: Ajoute un seuil minimal de score pour éviter de choisir des
pistes hors sujet : dans la fonction best_track, après calculer score via
utils::str_score (et/ou avant d'appeler is_better), ignore tout track dont score
est inférieur à un nouveau MIN_SCORE constant (ou config) et/ou retourne None si
best_score final est < MIN_SCORE; référencez best_track, utils::str_score et
is_better pour localiser l'endroit à modifier et définir MIN_SCORE en haut du
module ou en paramètre de la fonction.
In `@src-tauri/crates/syncedlyrics/src/providers/megalobiz.rs`:
- Around line 39-43: The loop that scans HTML using while let Some(idx) =
html[pos..].find("href=\"/lrc/maker/") currently uses the ? operator on computed
indices (href_end, tag_end, close) which causes the function to return None on a
single malformed link; change each of those uses so they don’t propagate None
but instead handle missing finds locally (e.g., use if let Some(...) or match
for href_end, tag_end and close) and skip the malformed link by advancing pos
(e.g., pos += idx + "href=\"".len() or similar) and continue the loop; update
the logic around the href_start/href_end/tag_end/close variables in that while
loop so it keeps scanning rather than returning early.
In `@src-tauri/crates/syncedlyrics/src/providers/musixmatch.rs`:
- Around line 142-149: La boucle qui utilise lrc.replace(original, ...) remplace
globalement toutes les occurrences et peut corrompre le LRC; instead iterate the
LRC by lines and replace only the specific line that equals
translation["subtitle_matched_line"] with the original plus "\n({translated})"
(or the first match per translation) rather than calling String::replace; locate
the translations loop (the for item in translations block) and replace the
global replace with a line-based mapping (split/join or iterate lines) that
compares exact equality to original and substitutes only that line.
- Around line 110-112: Le problème : l'appel add_translations(...).await? fait
échouer tout le provider si la traduction optionnelle échoue; remplacez le ? par
une gestion d'erreur non-fataile (par ex. match/if let Ok(result) =
add_translations(...).await { lrc = result } else { garder lrc existant et
logger l'erreur avec un warning}). Ciblez l'appel dans le bloc where if let
Some(lang) = lang { ... } et utilisez les symboles add_translations, lrc, http,
token, track_id, lang pour localiser le code; assurez-vous de ne pas perdre
l'original lrc en cas d'erreur et d'enregistrer l'erreur pour debug.
In `@src-tauri/crates/syncedlyrics/src/utils.rs`:
- Around line 8-20: La fonction synced_to_plaintext laisse des "word-stamps"
Enhanced LRC du type <mm:ss.xx> dans la sortie; mettez à jour la logique (soit
en étendant strip_lrc_stamp pour reconnaître et retirer aussi les word-stamps,
soit en ajoutant un nettoyage post-traitement dans synced_to_plaintext) pour
supprimer toutes les occurrences de <mm:ss.xx> dans chaque ligne avant de les
collecter; référez-vous à la fonction synced_to_plaintext et à strip_lrc_stamp
pour localiser où appliquer ce filtre afin que la sortie plaintext ne contienne
plus ces balises.
🪄 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: c564ce24-fdbe-4153-9b3e-62d46f8d5d61
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock,!src-tauri/Cargo.lock
📒 Files selected for processing (17)
README.mdTHIRD_PARTY_NOTICES.mddocs/README.mddocs/architecture/crates.mddocs/features/integrations.mdsrc-tauri/Cargo.tomlsrc-tauri/crates/app/Cargo.tomlsrc-tauri/crates/app/src/commands/lyrics.rssrc-tauri/crates/syncedlyrics/Cargo.tomlsrc-tauri/crates/syncedlyrics/src/lib.rssrc-tauri/crates/syncedlyrics/src/providers/genius.rssrc-tauri/crates/syncedlyrics/src/providers/lrclib.rssrc-tauri/crates/syncedlyrics/src/providers/megalobiz.rssrc-tauri/crates/syncedlyrics/src/providers/mod.rssrc-tauri/crates/syncedlyrics/src/providers/musixmatch.rssrc-tauri/crates/syncedlyrics/src/providers/netease.rssrc-tauri/crates/syncedlyrics/src/utils.rs
- propagate provider errors instead of caching transient failures as empty misses (lib.rs surfaces last_error; external_lyrics_search returns Result) - split multi-artist tags on "; " not ", " per project convention - enforce the offline contract inside external_lyrics_search - add SyncedLyricsClient::try_new() so client init no longer panics - genius: pick the "song" section instead of a hardcoded index - lrclib: drop results below a 65.0 score floor (matches sibling providers) - megalobiz: skip a malformed anchor instead of aborting the whole scan - utils: strip inline <mm:ss.xx> word-stamps from plaintext output
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/lib.rs`:
- Around line 127-136: The current pub fn new() -> Self pretends to be
infallible but falls back to reqwest::Client::new() which can panic; change
new() to be fallible and propagate the try_new() error instead (i.e. change
signature to pub fn new() -> Result<Self, <the error type returned by
try_new()>) and implement it as simply Self::try_new(), removing the panic-prone
unwrap_or_else fallback; alternatively, if you must keep an infallible API,
replace the fallback with a safe build path (e.g. use
reqwest::Client::builder().build() and handle its Result) and update the doc
comment to reflect the real behavior — refer to new(), try_new(), and
reqwest::Client::new()/builder().build() when making the change.
🪄 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: 61ee2a13-3347-4102-8ca6-f87372675adf
📒 Files selected for processing (6)
src-tauri/crates/app/src/commands/lyrics.rssrc-tauri/crates/syncedlyrics/src/lib.rssrc-tauri/crates/syncedlyrics/src/providers/genius.rssrc-tauri/crates/syncedlyrics/src/providers/lrclib.rssrc-tauri/crates/syncedlyrics/src/providers/megalobiz.rssrc-tauri/crates/syncedlyrics/src/utils.rs
reqwest::Client::new() can itself panic on TLS/backend init failure, so the "infallible" new() never truly was. Remove new() and the Default impl (both unused — the only callsite uses try_new()), leaving try_new() as the sole, fallible constructor.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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.
Résumé
Ajoute la nouvelle crate
waveflow-syncedlyrics— un portage Rust du projet Pythonsyncedlyrics(projet original local :/home/nayeon/workspace/syncedlyrics) qui fournit un lookup de paroles multi-providers : Musixmatch, LRCLIB, NetEase, Megalobiz, Genius.L'intégration est branchée dans
src-tauri/crates/app/src/commands/lyrics.rs:Conformité licence
WaveFlow reste sous GPL-3.0-only. Ajout d'un
THIRD_PARTY_NOTICES.mdqui préserve la notice MIT originale (Copyright (c) 2022 Momo,Copyright (c) 2026 InstaZDLL), lié depuis le README. MIT est compatible GPL-3.Vérifications
cargo fmt --manifest-path src-tauri/Cargo.toml --all --checkcargo check --manifest-path src-tauri/Cargo.toml -p waveflow-syncedlyrics --all-targetscargo test --manifest-path src-tauri/Cargo.toml -p waveflow-syncedlyricscargo check --manifest-path src-tauri/Cargo.toml -p waveflow --all-targets(passe sans workaround ALSA depuis l'installation delibasound2-dev)Summary by CodeRabbit
Nouvelles Fonctionnalités
Documentation