From e8a917c93d472fd49c18bc3a1355be4305f11831 Mon Sep 17 00:00:00 2001 From: Marc Plano-Lesay Date: Wed, 5 Aug 2026 13:08:20 +1000 Subject: [PATCH 1/3] fix(lyrics): Reject LRCLib records with no lyrics Some records on LRCLib have no lyrics, and aren't marked as instrumental, but are still returned (with a null text). #6864 fixed part of the issue (#6888), but didn't handle all of it. --- beetsplug/_typing.py | 2 +- beetsplug/lyrics.py | 11 +++++++---- docs/changelog.rst | 6 ++++++ test/plugins/test_lyrics.py | 5 +++++ 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/beetsplug/_typing.py b/beetsplug/_typing.py index 6c69aeed41..39e0379aa5 100644 --- a/beetsplug/_typing.py +++ b/beetsplug/_typing.py @@ -18,7 +18,7 @@ class Item(TypedDict): albumName: str duration: float | None instrumental: bool - plainLyrics: str + plainLyrics: str | None syncedLyrics: str | None diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index c9e5c529d9..9127d22521 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -260,7 +260,7 @@ class LRCLyrics: id: int duration: float instrumental: bool - plain: str + plain: str | None synced: str | None def __le__(self, other: LRCLyrics) -> bool: @@ -308,13 +308,16 @@ def duration_dist(self) -> float: @cached_property def is_valid(self) -> bool: """Return whether the lyrics item is valid. + Lyrics duration must be within the tolerance defined by - :attr:`DURATION_DIFF_TOLERANCE`. + :attr:`DURATION_DIFF_TOLERANCE`, and the item must carry some usable + text: LRCLib serves records that have neither plain nor synced lyrics + without flagging them as instrumental, and those match nothing. """ return ( self.duration_dist <= self.target_duration * self.DURATION_DIFF_TOLERANCE - ) + ) and bool(self.instrumental or self.plain or self.synced) @cached_property def dist(self) -> tuple[bool, float]: @@ -337,7 +340,7 @@ def get_text(self, want_synced: bool) -> str: if want_synced and self.synced: return "\n".join(map(str.strip, self.synced.splitlines())) - return self.plain + return self.plain or "" class LRCLib(Backend): diff --git a/docs/changelog.rst b/docs/changelog.rst index 4d98aa214f..932b458970 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -38,6 +38,12 @@ Bug fixes - :doc:`plugins/lyrics`: ``beet lyrics`` no longer crashes with an ``AttributeError`` on tracks that have no stored lyrics when ``force`` is enabled; a missing lyrics body is now treated as empty text. :bug:`6860` +- :doc:`plugins/lyrics`: LRCLib serves records that carry neither plain nor + synced lyrics without flagging them as instrumental. Such a record was + accepted as a match and produced lyrics whose body was ``None``, crashing the + importer with ``AttributeError: 'NoneType' object has no attribute + 'splitlines'``. These records are now rejected as matches, so the search falls + through to the remaining candidates. :bug:`6888` - Flexible attributes whose names contain uppercase characters (for example ``beet import --set Tag_With_Uppercase=true``) can now be found by queries. Field names are lowercased when a query is parsed, so such attributes could diff --git a/test/plugins/test_lyrics.py b/test/plugins/test_lyrics.py index e6384c4a34..bbca4134aa 100644 --- a/test/plugins/test_lyrics.py +++ b/test/plugins/test_lyrics.py @@ -629,6 +629,11 @@ def test_synced_config_option( pytest.param( [lyrics_match(instrumental=True)], "", id="instrumental track" ), + pytest.param( + [lyrics_match(plainLyrics=None, syncedLyrics=None)], + None, + id="none: no lyrics and not flagged instrumental", + ), pytest.param( [lyrics_match(syncedLyrics=None)], "plain", From 5d670648868a2121cf31a8859d11dcac46a50e5d Mon Sep 17 00:00:00 2001 From: Marc Plano-Lesay Date: Wed, 5 Aug 2026 13:34:26 +1000 Subject: [PATCH 2/3] fix: Out-of-date URL in lrclib test The ID returned by lrclib for this record has changed, breaking the test. --- test/plugins/lyrics_pages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugins/lyrics_pages.py b/test/plugins/lyrics_pages.py index 09760ba568..df230709f2 100644 --- a/test/plugins/lyrics_pages.py +++ b/test/plugins/lyrics_pages.py @@ -278,7 +278,7 @@ def backend(self) -> str: url_title="Lady Madonna - The Beatles - LETRAS.MUS.BR", ), LyricsPage.make( - "https://lrclib.net/api/get/19648857", + "https://lrclib.net/api/get/23863037", """ [00:08.35] Lady Madonna, children at your feet [00:12.85] Wonder how you manage to make ends meet From 8c9d45323e1fe104cb4257e5e029562bdc6a8cd4 Mon Sep 17 00:00:00 2001 From: Marc Plano-Lesay Date: Wed, 5 Aug 2026 13:55:30 +1000 Subject: [PATCH 3/3] feat(lyrics): Fall-back to synced lyrics with timestamps trimmed When no plain lyrics are provided by synced lyrics are, fall back to synced without timestamps. --- beetsplug/lyrics.py | 18 ++++++++++++++++-- docs/changelog.rst | 4 +++- test/plugins/test_lyrics.py | 14 ++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 9127d22521..61cf58011f 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -332,13 +332,27 @@ def dist(self) -> tuple[bool, float]: """ return not self.synced, self.duration_dist + @staticmethod + def remove_timestamps(synced: str) -> str: + """Return synced lyrics with their LRC timestamps removed.""" + return "\n".join( + m[2] + for line in synced.splitlines() + if (m := Lyrics.LINE_PARTS_PAT.match(line)) + ) + def get_text(self, want_synced: bool) -> str: """Return the preferred text form for this candidate.""" if self.instrumental: return INSTRUMENTAL_LYRICS - if want_synced and self.synced: - return "\n".join(map(str.strip, self.synced.splitlines())) + if self.synced: + if want_synced: + return "\n".join(map(str.strip, self.synced.splitlines())) + + # A record may carry synced lyrics without the plain variant. + if not self.plain: + return self.remove_timestamps(self.synced) return self.plain or "" diff --git a/docs/changelog.rst b/docs/changelog.rst index 932b458970..cb4b0d8d2d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -43,7 +43,9 @@ Bug fixes accepted as a match and produced lyrics whose body was ``None``, crashing the importer with ``AttributeError: 'NoneType' object has no attribute 'splitlines'``. These records are now rejected as matches, so the search falls - through to the remaining candidates. :bug:`6888` + through to the remaining candidates. Relatedly, a record that carries only + synced lyrics now yields the plain variant with timestamps removed, rather + than nothing, when ``synced`` is disabled. :bug:`6888` - Flexible attributes whose names contain uppercase characters (for example ``beet import --set Tag_With_Uppercase=true``) can now be found by queries. Field names are lowercased when a query is parsed, so such attributes could diff --git a/test/plugins/test_lyrics.py b/test/plugins/test_lyrics.py index bbca4134aa..fb0939acab 100644 --- a/test/plugins/test_lyrics.py +++ b/test/plugins/test_lyrics.py @@ -618,6 +618,20 @@ def test_synced_config_option( assert lyrics.text == expected_lyrics assert lyrics.backend == backend_name + @pytest.mark.parametrize( + "response_data", [[lyrics_match(plainLyrics=None)]] + ) + @pytest.mark.parametrize("plugin_config", [{"synced": False}]) + def test_plain_lyrics_derived_from_synced(self, fetch_lyrics): + """Derive plain lyrics from the synced text when the record omits them. + + Returning nothing here would hide lyrics the match actually carries. + """ + lyrics = fetch_lyrics() + + assert lyrics + assert lyrics.text == "synced" + @pytest.mark.parametrize( "response_data, expected_lyrics", [