Jonathan
e8483ee22b
Two related monitor bugs surfaced by owned albums that never leave the press:
1. Ownership was matched on (artist, album) strings, but MusicBrainz canonicalizes
the artist on import ("Kanye West"->"Ye", "Ne-Yo"->"Ne‐Yo", "The Goo Goo Dolls"
->"Goo Goo Dolls"). The strings no longer matched, so reconcile/enqueue/backfill
never recognized the album as owned — it stayed "wanted" and re-pressed every
retry interval. Match on the release-group MBID when the library row has one
(falling back to artist+album for scanned rows without an MBID) across
_in_library_at_cutoff, fulfill_owned_releases, and reconcile.
2. Each failed attempt left a needs_attention Request behind, so a hard-to-match
album piled up phantom "needs attention" cards on the press even after a later
attempt imported it (e.g. Maroon 5 "V": 3 stale cards + 1 success). Retire prior
failed attempts when a release is fulfilled or re-attempted (_retire_failed_attempts,
monitor-owned requests only — manual requests have no monitoredReleaseId).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
170 lines
7.8 KiB
Python
170 lines
7.8 KiB
Python
from lyra_worker.monitor import MonitorConfig, fulfill_owned_releases, reconcile
|
||
from tests.conftest import insert_monitored_release
|
||
|
||
CFG = MonitorConfig(enabled=True, quality_cutoff=2)
|
||
|
||
|
||
def _make_job(conn, rel_id, artist, album, state):
|
||
with conn.cursor() as cur:
|
||
cur.execute(
|
||
'INSERT INTO "Request" (id, artist, album, status, "monitoredReleaseId", "createdAt") '
|
||
"VALUES (gen_random_uuid()::text, %s, %s, 'pending', %s, now()) RETURNING id",
|
||
(artist, album, rel_id),
|
||
)
|
||
req = cur.fetchone()[0]
|
||
cur.execute(
|
||
'INSERT INTO "Job" (id, "requestId", state, "currentStage", attempts, "createdAt", "updatedAt") '
|
||
"VALUES (gen_random_uuid()::text, %s, %s, 'import', 1, now(), now()) RETURNING id",
|
||
(req, state),
|
||
)
|
||
job = cur.fetchone()[0]
|
||
conn.commit()
|
||
return req, job
|
||
|
||
|
||
def _add_library(conn, req, artist, album, quality, rg_mbid=None):
|
||
with conn.cursor() as cur:
|
||
cur.execute(
|
||
'INSERT INTO "LibraryItem" (id, "requestId", artist, album, path, source, format, '
|
||
'"qualityClass", "rgMbid", "importedAt") '
|
||
"VALUES (gen_random_uuid()::text, %s, %s, %s, '/m', 'qobuz', 'FLAC', %s, %s, now())",
|
||
(req, artist, album, quality, rg_mbid),
|
||
)
|
||
conn.commit()
|
||
|
||
|
||
def _needs_attention_count(conn, rel_id):
|
||
with conn.cursor() as cur:
|
||
cur.execute(
|
||
'SELECT count(*) FROM "Request" r JOIN "Job" j ON j."requestId" = r.id '
|
||
"WHERE r.\"monitoredReleaseId\" = %s AND j.state = 'needs_attention'",
|
||
(rel_id,),
|
||
)
|
||
return cur.fetchone()[0]
|
||
|
||
|
||
def _release(conn, rel_id):
|
||
with conn.cursor() as cur:
|
||
cur.execute(
|
||
'SELECT state, "currentQualityClass", "firstGrabbedAt" IS NOT NULL '
|
||
'FROM "MonitoredRelease" WHERE id = %s',
|
||
(rel_id,),
|
||
)
|
||
return cur.fetchone()
|
||
|
||
|
||
def test_imported_below_cutoff_becomes_grabbed(conn):
|
||
rel = insert_monitored_release(conn, artist_name="A", album="B", rg_mbid="rg-1",
|
||
monitored=True, state="wanted")
|
||
req, job = _make_job(conn, rel, "A", "B", "imported")
|
||
_add_library(conn, req, "A", "B", 1)
|
||
reconcile(conn, job, CFG)
|
||
assert _release(conn, rel) == ("grabbed", 1, True)
|
||
|
||
|
||
def test_imported_at_cutoff_becomes_fulfilled(conn):
|
||
rel = insert_monitored_release(conn, artist_name="A", album="B", rg_mbid="rg-1",
|
||
monitored=True, state="wanted")
|
||
req, job = _make_job(conn, rel, "A", "B", "imported")
|
||
_add_library(conn, req, "A", "B", 3)
|
||
reconcile(conn, job, CFG)
|
||
assert _release(conn, rel) == ("fulfilled", 3, True)
|
||
|
||
|
||
def test_needs_attention_leaves_wanted(conn):
|
||
rel = insert_monitored_release(conn, artist_name="A", album="B", rg_mbid="rg-1",
|
||
monitored=True, state="wanted")
|
||
_req, job = _make_job(conn, rel, "A", "B", "needs_attention")
|
||
reconcile(conn, job, CFG)
|
||
assert _release(conn, rel) == ("wanted", None, False)
|
||
|
||
|
||
def test_finish_job_reconciles_even_when_monitor_disabled(conn):
|
||
# Regression: a manually-wanted release that gets pressed must transition to fulfilled
|
||
# (and drop off the Wanted list) even with the monitor off — reconcile is core
|
||
# correctness, not a monitor-sweep feature.
|
||
from lyra_worker.main import finish_job
|
||
|
||
rel = insert_monitored_release(conn, artist_name="A", album="B", rg_mbid="rg-1",
|
||
monitored=True, state="wanted")
|
||
req, job = _make_job(conn, rel, "A", "B", "imported")
|
||
_add_library(conn, req, "A", "B", 3)
|
||
finish_job(conn, job, MonitorConfig(enabled=False, quality_cutoff=2))
|
||
assert _release(conn, rel) == ("fulfilled", 3, True)
|
||
|
||
|
||
def test_fulfill_owned_releases_backfills_stuck_wanted(conn):
|
||
# A release pressed while the monitor was off: owned in the library at cutoff, but its
|
||
# MonitoredRelease is still 'wanted' (never reconciled). Startup backfill fixes it.
|
||
rel = insert_monitored_release(conn, artist_name="A", album="B", rg_mbid="rg-1",
|
||
monitored=True, state="wanted")
|
||
other = insert_monitored_release(conn, artist_name="C", album="D", rg_mbid="rg-2",
|
||
monitored=True, state="wanted") # not owned → stays wanted
|
||
_add_library(conn, None, "A", "B", 3)
|
||
cfg = MonitorConfig(quality_cutoff=2)
|
||
assert fulfill_owned_releases(conn, cfg) == 1
|
||
assert _release(conn, rel) == ("fulfilled", 3, True)
|
||
assert _release(conn, other)[0] == "wanted"
|
||
assert fulfill_owned_releases(conn, cfg) == 0 # idempotent
|
||
|
||
|
||
def test_fulfill_owned_releases_ignores_below_cutoff(conn):
|
||
rel = insert_monitored_release(conn, artist_name="A", album="B", rg_mbid="rg-1",
|
||
monitored=True, state="wanted")
|
||
_add_library(conn, None, "A", "B", 1) # owned but below cutoff → keep wanting (upgrade)
|
||
assert fulfill_owned_releases(conn, MonitorConfig(quality_cutoff=2)) == 0
|
||
assert _release(conn, rel)[0] == "wanted"
|
||
|
||
|
||
def test_reconcile_matches_by_rgmbid_when_artist_name_canonicalized(conn):
|
||
# Real 'Kanye West' -> 'Ye' bug: the MonitoredRelease keeps the followed name, but the
|
||
# imported LibraryItem carries MusicBrainz's canonical artist. Matching on rgMbid lets the
|
||
# release reconcile to fulfilled instead of looking un-owned and re-pressing forever.
|
||
rel = insert_monitored_release(conn, artist_name="Kanye West", album="Graduation",
|
||
rg_mbid="rg-grad", monitored=True, state="wanted")
|
||
req, job = _make_job(conn, rel, "Kanye West", "Graduation", "imported")
|
||
_add_library(conn, req, "Ye", "Graduation", 3, rg_mbid="rg-grad")
|
||
reconcile(conn, job, CFG)
|
||
assert _release(conn, rel) == ("fulfilled", 3, True)
|
||
|
||
|
||
def test_reconcile_fulfilled_retires_failed_sibling_attempts(conn):
|
||
# 'V' by Maroon 5 phantom: earlier failed attempts leave needs_attention Request rows that
|
||
# keep showing on the press even after a later attempt imports. A successful import retires
|
||
# those failed siblings so the press stops showing an owned album as needing attention.
|
||
rel = insert_monitored_release(conn, artist_name="Maroon 5", album="V", rg_mbid="rg-v",
|
||
monitored=True, state="wanted")
|
||
_make_job(conn, rel, "Maroon 5", "V", "needs_attention")
|
||
_make_job(conn, rel, "Maroon 5", "V", "needs_attention")
|
||
req, job = _make_job(conn, rel, "Maroon 5", "V", "imported")
|
||
_add_library(conn, req, "Maroon 5", "V", 2)
|
||
reconcile(conn, job, CFG)
|
||
assert _release(conn, rel)[0] == "fulfilled"
|
||
assert _needs_attention_count(conn, rel) == 0 # failed siblings retired
|
||
|
||
|
||
def test_fulfill_owned_releases_matches_by_rgmbid(conn):
|
||
# Ne-Yo -> Ne‐Yo (typographic hyphen) name drift: still owned by rgMbid.
|
||
rel = insert_monitored_release(conn, artist_name="Ne-Yo", album="Because of You",
|
||
rg_mbid="rg-ney", monitored=True, state="grabbed")
|
||
_add_library(conn, None, "Ne‐Yo", "Because of You", 2, rg_mbid="rg-ney")
|
||
assert fulfill_owned_releases(conn, MonitorConfig(quality_cutoff=2)) == 1
|
||
assert _release(conn, rel) == ("fulfilled", 2, True)
|
||
|
||
|
||
def test_unlinked_job_is_ignored(conn):
|
||
with conn.cursor() as cur:
|
||
cur.execute(
|
||
'INSERT INTO "Request" (id, artist, album, status, "createdAt") '
|
||
"VALUES (gen_random_uuid()::text, 'A', 'B', 'completed', now()) RETURNING id"
|
||
)
|
||
req = cur.fetchone()[0]
|
||
cur.execute(
|
||
'INSERT INTO "Job" (id, "requestId", state, "currentStage", attempts, "createdAt", "updatedAt") '
|
||
"VALUES (gen_random_uuid()::text, %s, 'imported', 'import', 1, now(), now()) RETURNING id",
|
||
(req,),
|
||
)
|
||
job = cur.fetchone()[0]
|
||
conn.commit()
|
||
reconcile(conn, job, CFG) # no monitoredReleaseId → no-op, must not raise
|