From b0eec4182309a14f27a8873c98eef4993c5c1aaa Mon Sep 17 00:00:00 2001 From: Jonathan Date: Wed, 15 Jul 2026 13:22:01 +0200 Subject: [PATCH] fix(worker): judge download completeness by the source's edition, not MB's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The completeness + duration guards used MusicBrainz's target.track_count / total_duration_s, but MB picks an arbitrary releases[0] that is often a deluxe/expanded edition with more tracks (and runtime) than the standard album a source legitimately delivers. So complete standard-edition downloads were falsely rejected — The Script 'No Sound Without Silence' (11 vs MB 12) as 'incomplete download', Skillet 'Unleashed' (12 vs MB's 20-track deluxe) as 'too short'. Now completeness is judged against the chosen source's own track count (winner.track_count): a truncated download (fewer files than the source promised) still fails, and the duration expectation is scaled to the delivered edition's size. A separate 'implausibly small vs MB' guard (source has <= half MB's tracks) still rejects a lone-track 'full album' video. Genuine partials (e.g. John Mayer, a Qobuz track failing mid-download) still correctly fail. Co-Authored-By: Claude Opus 4.8 (1M context) --- worker/lyra_worker/pipeline.py | 34 ++++-- .../test_smaller_edition_completeness.py | 101 ++++++++++++++++++ 2 files changed, 126 insertions(+), 9 deletions(-) create mode 100644 worker/tests/test_smaller_edition_completeness.py diff --git a/worker/lyra_worker/pipeline.py b/worker/lyra_worker/pipeline.py index 5180246..91fe58f 100644 --- a/worker/lyra_worker/pipeline.py +++ b/worker/lyra_worker/pipeline.py @@ -375,23 +375,39 @@ def run_pipeline( # 5. verify completeness on the staging copy, then promote + tag _set_state(conn, job_id, "tagging", "tag") - expected = target.track_count if target.track_count is not None else winner.track_count - # Also count the NON-EMPTY audio actually on disk: a silently-skipped track leaves a - # 0-byte placeholder that the adapter still counts, so trust the files when any are - # staged. (`staged and ...` keeps fake adapters that stage nothing on the count path.) + # Judge completeness against what the CHOSEN SOURCE offered, NOT MusicBrainz's canonical + # release: MB picks an arbitrary releases[0] that is often a deluxe/expanded edition with + # more tracks than the standard album a source legitimately delivers, so gating purely on + # target.track_count falsely rejects complete standard-edition downloads (e.g. The Script + # "No Sound Without Silence" 11 vs MB 12, Skillet "Unleashed" 12 vs MB's 20-track deluxe). + expected = winner.track_count + # (a) Truncated: fewer files than the source promised (a track failed mid-download). + # Count the NON-EMPTY audio actually on disk too: a silently-skipped track leaves a 0-byte + # placeholder the adapter still counts, so trust the files when any are staged. (`staged + # and ...` keeps fake adapters that stage nothing on the count path.) staged = _count_staged_audio(staging) - if result.track_count < expected or (staged and staged < expected): + truncated = result.track_count < expected or bool(staged and staged < expected) + # (b) Implausibly small vs MB's album — a lone-track "full album" video or a wrong match, + # as distinct from a legitimately smaller edition (standard vs deluxe). A real edition + # keeps more than half of MB's tracks; a 1-of-12 match does not. + too_small = target.track_count is not None and expected * 2 <= target.track_count + if truncated or too_small: _fail(conn, job_id, "incomplete download") return # Duration check: even with the right track count, a truncated file or a short preview - # substituted for a real track shows up as a total playtime well under MB's expected - # total. Only applies when both are known (measured is None for the offline fakes). + # substituted for a real track shows up as a total playtime well under the expected total. + # Scale MB's total to the DELIVERED edition's size (source vs MB track count) so a smaller + # standard edition isn't measured against a larger deluxe edition's runtime. Only applies + # when the total is known (measured is None for the offline fakes). if target.total_duration_s: + expected_total = target.total_duration_s + if target.track_count: + expected_total *= winner.track_count / target.track_count measured = measure_duration(staging) - if measured is not None and measured < target.total_duration_s * _DURATION_MIN_RATIO: + if measured is not None and measured < expected_total * _DURATION_MIN_RATIO: _fail(conn, job_id, - f"download too short ({measured / 60:.0f} of ~{target.total_duration_s / 60:.0f} min)") + f"download too short ({measured / 60:.0f} of ~{expected_total / 60:.0f} min)") return final = album_dir(dest_root, target) diff --git a/worker/tests/test_smaller_edition_completeness.py b/worker/tests/test_smaller_edition_completeness.py new file mode 100644 index 0000000..91315a6 --- /dev/null +++ b/worker/tests/test_smaller_edition_completeness.py @@ -0,0 +1,101 @@ +"""MusicBrainz picks an arbitrary releases[0] that is often a deluxe/expanded edition with more +tracks (and a longer runtime) than the standard album a source legitimately delivers. Completeness +and the duration guard must be judged against what the CHOSEN SOURCE offered, not MB's canonical +edition — otherwise a complete standard-edition download is falsely rejected as 'incomplete' +(The Script "No Sound Without Silence": 11 delivered vs MB 12) or 'too short' (Skillet "Unleashed": +12 delivered ~40 min vs MB's 20-track deluxe ~73 min). A genuinely short download must still fail. +""" +import os +from dataclasses import replace + +from lyra_worker.adapters.fakes import FakeQobuz +from lyra_worker.claim import claim_next +from lyra_worker.pipeline import run_pipeline +from lyra_worker.types import DownloadResult, MBTarget +from tests.conftest import insert_request + + +class _StandardEditionQobuz(FakeQobuz): + """Qobuz-like source offering a fixed-size *standard* edition (source_tracks), independent of + MB's possibly-larger deluxe count; stages that many real (non-empty) files and reports them.""" + def __init__(self, source_tracks): + super().__init__() + self._n = source_tracks + + def search(self, target): + return super().search(replace(target, track_count=self._n)) + + def download(self, candidate, dest, on_progress): + os.makedirs(dest, exist_ok=True) + for i in range(candidate.track_count): + with open(os.path.join(dest, f"{i + 1:02d} Track.flac"), "wb") as fh: + fh.write(b"audio") + on_progress(1.0) + return DownloadResult(ok=True, path=dest, track_count=candidate.track_count) + + +class _PartialQobuz(FakeQobuz): + """Promises `promised` tracks but one fails mid-download: stages and reports `promised - 1` + (the real John Mayer "Roll it on Home" IncompleteRead case).""" + def __init__(self, promised): + super().__init__() + self._n = promised + + def search(self, target): + return super().search(replace(target, track_count=self._n)) + + def download(self, candidate, dest, on_progress): + os.makedirs(dest, exist_ok=True) + delivered = candidate.track_count - 1 + for i in range(delivered): + with open(os.path.join(dest, f"{i + 1:02d} Track.flac"), "wb") as fh: + fh.write(b"audio") + return DownloadResult(ok=True, path=dest, track_count=delivered) + + +class _DeluxeResolver: + """Resolver whose canonical release is a larger (deluxe) edition than the source delivers.""" + def __init__(self, mb_tracks, mb_total_s): + self._t, self._s = mb_tracks, mb_total_s + + def resolve(self, artist, album): + return MBTarget(artist=artist, album=album, track_count=self._t, total_duration_s=self._s) + + +def _state_error(conn, job_id): + with conn.cursor() as cur: + cur.execute('SELECT state, error FROM "Job" WHERE id = %s', (job_id,)) + return cur.fetchone() + + +def test_standard_edition_not_rejected_as_incomplete(conn): + # The Script: source delivers the 11-track standard edition; MB's canonical release is 12. + job_id = insert_request(conn, artist="The Script", album="No Sound Without Silence") + claim_next(conn) + run_pipeline(conn, job_id, [_StandardEditionQobuz(source_tracks=11)], + resolver=_DeluxeResolver(mb_tracks=12, mb_total_s=2943), + measure_duration=lambda _s: 2700.0, dest_root="/tmp/lib-se") + assert _state_error(conn, job_id) == ("imported", None) + + +def test_smaller_edition_not_rejected_as_too_short(conn): + # Skillet: source delivers the 12-track standard (~40 min); MB's release is a 20-track deluxe + # (~73 min). The unscaled duration guard falsely rejects; scaling to the delivered edition + # (12/20) must let it pass. + job_id = insert_request(conn, artist="Skillet", album="Unleashed") + claim_next(conn) + run_pipeline(conn, job_id, [_StandardEditionQobuz(source_tracks=12)], + resolver=_DeluxeResolver(mb_tracks=20, mb_total_s=4380), + measure_duration=lambda _s: 2400.0, dest_root="/tmp/lib-se") + assert _state_error(conn, job_id) == ("imported", None) + + +def test_genuinely_incomplete_still_rejected(conn): + # John Mayer: the source promised 12 tracks but one failed mid-download → only 11 land. + # This is a real partial and must still be rejected (the fix must not swallow it). + job_id = insert_request(conn, artist="John Mayer", album="The Search for Everything") + claim_next(conn) + run_pipeline(conn, job_id, [_PartialQobuz(promised=12)], + resolver=_DeluxeResolver(mb_tracks=12, mb_total_s=2580), + measure_duration=lambda _s: 2400.0, dest_root="/tmp/lib-se") + assert _state_error(conn, job_id) == ("needs_attention", "incomplete download")