From 5f59283bf0ed02d324b88673396e1ea601477023 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Mon, 20 Jul 2026 22:27:44 +0200 Subject: [PATCH] fix(worker): resolve MB to the original edition + loosen duration gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of albums re-downloading forever despite completing (Linkin Park "Hybrid Theory"): the MB resolver read the tracklist from an arbitrary releases[0], often a later reissue/deluxe. HT resolved to a 2023 13-track reissue (~52 min); a complete 2000 12-track download (~38 min) then failed _download_problem's duration gate (2267s < 2877s×0.85) and fell through peer after peer, never importing. - _pick_release: choose the ORIGINAL standard edition (Official, earliest date) instead of releases[0], so completeness/duration are judged against the edition sources actually deliver. - _DURATION_MIN_RATIO 0.85 -> 0.65: a source legitimately delivering a shorter edition than MB's release shouldn't be rejected; only genuinely truncated/preview downloads (a fraction of expected runtime) should be. Together with the est-time peer ranking, this stops the download→reject→re-download loop. 334 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- worker/lyra_worker/_musicbrainz.py | 22 ++++++++++++++++++++-- worker/lyra_worker/pipeline.py | 6 ++++-- worker/tests/test_musicbrainz.py | 26 +++++++++++++++++++++++++- 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/worker/lyra_worker/_musicbrainz.py b/worker/lyra_worker/_musicbrainz.py index 063e853..03a1b3d 100644 --- a/worker/lyra_worker/_musicbrainz.py +++ b/worker/lyra_worker/_musicbrainz.py @@ -75,6 +75,23 @@ def _best_release_group(artist: str, album: str, groups: list) -> dict | None: return best +def _pick_release(releases: list) -> dict | None: + """Choose which release of a release-group to read the tracklist from. MB returns them in an + arbitrary order and ``releases[0]`` is often a later reissue/deluxe/anniversary edition with + more tracks and a longer runtime than the standard album a source actually delivers — which + then makes a *complete* download fail the pipeline's track-count/duration completeness checks + and re-download forever (Linkin Park "Hybrid Theory": MB's arbitrary pick was a 2023 13-track + reissue at ~52 min vs the 2000 original's 12 tracks / ~38 min). Prefer the ORIGINAL standard + edition: an Official release, then the earliest date, then a dated one. Pure — unit-tested.""" + if not releases: + return None + def _key(r: dict) -> tuple: + official = 0 if (r.get("status") == "Official") else 1 # official editions first + date = r.get("date") or "" + return (official, 0 if date else 1, date) # then dated, then earliest date + return min(releases, key=_key) + + class MusicBrainzResolver: """Real MbResolver using the MusicBrainz webservice via musicbrainzngs. @@ -119,9 +136,10 @@ class MusicBrainzResolver: if rgid: rgfull = _with_retry(lambda: musicbrainzngs.get_release_group_by_id(rgid, includes=["releases"])) releases = rgfull.get("release-group", {}).get("release-list", []) - if releases: + chosen = _pick_release(releases) + if chosen: rel = _with_retry( - lambda: musicbrainzngs.get_release_by_id(releases[0]["id"], includes=["recordings"]) + lambda: musicbrainzngs.get_release_by_id(chosen["id"], includes=["recordings"]) ).get("release", {}) tracks = [] for medium in rel.get("medium-list", []): diff --git a/worker/lyra_worker/pipeline.py b/worker/lyra_worker/pipeline.py index 3e42c72..e8c6ea1 100644 --- a/worker/lyra_worker/pipeline.py +++ b/worker/lyra_worker/pipeline.py @@ -22,8 +22,10 @@ _AUDIO_EXT = {".flac", ".mp3", ".m4a", ".opus", ".ogg", ".aac", ".wav"} # Reject a download whose total playtime falls below this fraction of MusicBrainz's expected # total — catches truncated files / preview clips substituted for real tracks that keep the # track COUNT right. Generous, so edition/encoding differences don't false-positive; bonus -# tracks (over-long) are always fine. -_DURATION_MIN_RATIO = 0.85 +# tracks (over-long) are always fine. Kept well below 1.0 because a source legitimately delivers a +# different edition than MB's resolved release (shorter radio edits, no bonus/live tracks), and only +# a genuinely truncated/preview-filled download (playtime a fraction of expected) should be caught. +_DURATION_MIN_RATIO = 0.65 # Cap how many ranked candidates a single job will actually download+verify before giving up. # Without this, the incomplete-download fall-through can grind through dozens of sources (a popular # album returns ~200 Soulseek candidates), each a slow attempt that also leaves an abandoned slskd diff --git a/worker/tests/test_musicbrainz.py b/worker/tests/test_musicbrainz.py index 0dbdfbd..48d8f25 100644 --- a/worker/tests/test_musicbrainz.py +++ b/worker/tests/test_musicbrainz.py @@ -1,7 +1,7 @@ import musicbrainzngs import pytest -from lyra_worker._musicbrainz import _best_release_group, _with_retry +from lyra_worker._musicbrainz import _best_release_group, _pick_release, _with_retry # Offline unit tests for the pure release-group selection logic. The live resolve() # path (network) is covered by test_musicbrainz_live.py. @@ -68,6 +68,30 @@ def test_prefers_credited_artist_over_same_titled_other_artist_album(): # --- _with_retry: transient MusicBrainz failures (SSL EOF, 503 rate-limit) --- +def test_pick_release_prefers_original_official_over_later_reissue(): + # Real "Hybrid Theory" bug: MB's arbitrary releases[0] was a 2023 13-track reissue; the 2000 + # original is what sources deliver, so completeness checks must reference it. + releases = [ + {"id": "reissue", "status": "Official", "date": "2023-06-01"}, + {"id": "original", "status": "Official", "date": "2000-10-24"}, + {"id": "deluxe", "status": "Official", "date": "2020-10-09"}, + ] + assert _pick_release(releases)["id"] == "original" + + +def test_pick_release_skips_non_official_and_undated(): + releases = [ + {"id": "promo", "status": "Promotion", "date": "1999-01-01"}, # earliest but not official + {"id": "undated", "status": "Official", "date": ""}, + {"id": "official", "status": "Official", "date": "2001-03-03"}, + ] + assert _pick_release(releases)["id"] == "official" + + +def test_pick_release_empty_is_none(): + assert _pick_release([]) is None + + def test_retry_returns_value_without_backoff_when_first_call_succeeds(): sleeps: list[float] = [] calls = {"n": 0}