lyrics: don't match LRCLib entries that have no lyrics

[davidbhoward]

Description

LRCLib stores track metadata independently of the lyrics themselves, so an entry can come back with both plainLyrics and syncedLyrics null while instrumental is still False.

LRCLyrics.is_valid accepted such an entry as a match on duration alone, and get_text then returned self.plain, i.e. None, despite being annotated -> str. The None propagated into Lyrics, and the first access of its text raised:

AttributeError: 'NoneType' object has no attribute 'splitlines'
  File "beets/util/lyrics.py", line 108, in _split_lines
    for line in self.text.splitlines()

beet lyrics surfaces this per track, but during an import the exception escapes the pipeline stage and aborts the entire run, so one such track strands every file queued behind it. Instrumental-heavy material (lo-fi, game soundtracks, ambient) hits it often.

Reproducing

Real response from the public API, https://lrclib.net/api/search?track_name=Anther&artist_name=Blue%20Wednesday:

{
  "id": 37048543,
  "trackName": "Anther",
  "artistName": "Blue Wednesday",
  "duration": 189.0,
  "instrumental": false,
  "plainLyrics": null,
  "syncedLyrics": null
}

Against master, with no configuration involved:

candidate.is_valid  = True     <- accepted as a match
get_text() returned = None     <- annotated `-> str`
AttributeError: 'NoneType' object has no attribute 'splitlines'

Changes

• An entry with no lyrics text at all is no longer a valid match, so the search continues to other candidates and backends and ultimately reports that no lyrics were found. This is deliberately kept distinct from an instrumental track, where "no lyrics" is itself the answer and the existing instrumental handling is unchanged. Marking these as instrumental would assert something the API response does not tell us: the lyrics may simply not have been contributed yet.
• A null plainLyrics now falls back to synced lyrics rather than discarding lyrics that are present.
• LRCLibAPI.Item.plainLyrics and LRCLyrics.plain are annotated as nullable, matching what the API actually returns.

Tests

Two cases added to TestLRCLibLyrics, both of which fail on master and pass with this change:

• none: no lyrics text despite instrumental being False
• test_null_plain_lyrics_falls_back_to_synced

Full test/plugins/test_lyrics.py suite passes (127 passed, 16 skipped), along with ruff check, ruff format --check, and mypy.

[davidbhoward]

Full disclosure on where this came from: I have not personally reviewed the code in this
PR. Claude found the issue while backfilling lyrics across my music library and wrote the
fix and the tests. What I hit was imports aborting partway through on instrumental lo-fi
tracks, which left the rest of the batch sitting unimported without any obvious error.

The repro in the description is a real response from the LRCLib API, so at minimum the
underlying behavior is worth knowing about even if you want to solve it differently.

Posting it in case it helps. Take it or leave it, no hard feelings either way.

[davidbhoward]

The two failures in the ubuntu 3.10 job look unrelated to this change. I reverted
beetsplug/lyrics.py and _typing.py to master locally and ran the same two tests, and
they fail identically without my change.

Both are live network tests in TestLyricsSources. The lyricsmania one is the Google
backend, which this PR does not touch at all. The lrclib one has matching lyrics text
and only differs on the record URL, 23863037 versus the expected 19648857, so it looks
like LRCLib is serving a different record ID for the same song now.

These only run when lyrics source changes, so I think this PR just happened to be what
made them run.

[semohr]

Is this still an issue on the master branch?

I think #6864 should have fixed this already.

[davidbhoward]

Is this still an issue on the master branch?

I think #6864 should have fixed this already.

Thanks for pointing at #6864. I checked and I don't think it covers this one: #6864 is
already in master (77b9dff) and the crash still reproduces on it.

The difference is where the None comes from. #6864 guards Lyrics.from_item(), which handles
item.lyrics being None for a track with no stored lyrics. This crash never goes through
from_item. LRCLib.fetch() builds the Lyrics object directly:

lyrics = item.get_text(self.config["synced"].get(bool))   # returns None
return Lyrics(lyrics, self.__class__.name, ...)

get_text() falls through to return self.plain, and plainLyrics is null in the API response
even though instrumental is false, so None reaches the constructor rather than from_item.

The repro in the PR description is a real LRCLib response and it still fails on current
master. Happy to be wrong if I've misread something.

[semohr]

Thanks for the clarification! Makes sense to me.

[snejus]

@davidbhoward I fixed lrclib integration test as it failed. Please address my comment and we will merge it :)

[davidbhoward]

OK will do. I'm on vacation but will do it when I get back next week. Thank you.

[davidbhoward]

Both review points are addressed in f68aefa.

I also rebased onto current master rather than merging, so the earlier commit SHAs changed.
Your lrclib fixture fix is still there as b19f0c0 with your authorship, I cherry-picked it onto
the rebase so it did not get lost. Both TestLyricsSources live tests pass locally now.

I wasn't sure if this project prefers rebase or merging master into branch. Let me know if you need to take any action here. Sorry about that.

[davidbhoward]

CI is red on ubuntu 3.10 again but it looks like more provider drift rather than anything
from this change.

The failure is now TestLyricsSources[lrcmux-lrcmux]:

FAILED test/plugins/test_lyrics.py::TestLyricsSources::test_backend_source[lrcmux-lrcmux]
AssertionError: assert '[00:00.00]\n...ake ends meet' == '[00:08.71] L...ake ends meet'

The only difference is a leading empty timestamp line that ytmusic has started returning. Every
other line matches exactly:

+ [00:00.00]
  [00:08.71] Lady Madonna, children at your feet
  [00:13.08] Wonder how you manage to make ends meet
  [00:17.43] Who finds the money when you pay the rent?
  ...

Reasons I do not think it is this PR:

I reverted beetsplug/lyrics.py and beetsplug/_typing.py to origin/master locally and the test
fails identically, so it reproduces without any of my changes.

This PR does not touch that fixture. The only change to lyrics_pages.py on this branch is your
lrclib URL fix.

The change itself cannot add a line. _synced_as_plain only removes timestamps and
_format_synced only trims whitespace on existing lines.

For what it is worth this is the third live source to drift on this PR now, first lyricsmania,
then the lrclib record ID you fixed, now ytmusic. Since these only run when lyrics source files
change, the drift seems to pile up and then land on whoever next touches the plugin.

Happy to add the leading [00:00.00] to the expected text in this PR if you want it fixed here,
or leave it for a separate one. Let me know which you prefer.