Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
37 KiB
Cleanup Batch (Tier A + B) Implementation Plan
For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (
- [ ]) syntax for tracking.
Goal: Close the deferred backlog's fix-items — PATCH allowlist hardening + a11y/copy polish (Tier A), and the two design tradeoffs the user chose to fix properly: per-seed discovery scoring and a persisted scan worklist (Tier B).
Architecture: Tier A is small web edits to two Next.js route handlers and one client component. Tier B adds two additive tables via one Prisma migration: DiscoverySeedContribution (normalizes discovery scores so a suggestion's score = sum of every seed's latest contribution, independent of chunk/sweep boundaries) and ScanWorkItem (a frozen worklist so scan_chunk pops by indexed cursor instead of re-walking the tree each chunk). All worker DB access is raw psycopg SQL.
Tech Stack: Next.js (App Router, TypeScript, vitest, node test env), Prisma (migration source of truth), Python worker (psycopg, pytest against lyra_test).
Global Constraints
- Tests run against
lyra_testonly; a guard refuses non-*_testDBs. Never point at livelyra. - Web vitest env is node, not jsdom — no DOM-render tests. UI-only changes (aria-label, copy) are verified by
tsc+ build + Playwright, not unit tests. - Preserve existing
aria-labels / button text — web route tests and Playwright key off them. - Worker SQL uses quoted PascalCase table/column identifiers (
"DiscoverySuggestion","seedMbid"). - Discovery
_upsert_artist/_upsert_albumMUST keep theirWHERE "DiscoverySuggestion".status = 'pending'guard (dismissed/wanted rows are never revived). - Frequent commits: one per task minimum, following the TDD step order.
- Migration reaches live
lyravia the web entrypoint'smigrate deployon rebuild; nodown -v, ever.
File Structure
Tier A (web):
web/src/app/api/monitor/config/route.ts— PATCH allowlist →hasOwnProperty.web/src/app/api/discover/config/route.ts— PATCH allowlist →hasOwnProperty.web/src/app/api/monitor/config/route.test.ts— prototype-chain rejection test.web/src/app/api/discover/config/route.test.ts— prototype-chain rejection test.web/src/app/api/config/route.test.ts— addconstructorcase.web/src/app/settings/settings-form.tsx— Monitor togglearia-label+ eyebrow copy.
Tier B (schema + worker):
web/prisma/schema.prisma— add both models.web/prisma/migrations/<ts>_add_contributions_and_scan_worklist/migration.sql— CREATE TABLE ×2.worker/tests/conftest.py— clean the two new tables between tests.worker/lyra_worker/discovery.py— contributions write + recompute.worker/tests/test_discovery.py— new cross-chunk / partial-sweep / drop-off tests.worker/lyra_worker/scan.py—build_worklist/ newscan_chunk/clear_worklist/ reimplementedscan_library.worker/lyra_worker/main.py—maybe_run_scandrives the worklist byscan.id.worker/tests/test_scan_chunk.py— rewritten to the worklist API.
Task 1: Tier A1 + A3 — PATCH allowlist hardening + tests
Files:
- Modify:
web/src/app/api/monitor/config/route.ts:26-28 - Modify:
web/src/app/api/discover/config/route.ts:28-30 - Test:
web/src/app/api/monitor/config/route.test.ts - Test:
web/src/app/api/discover/config/route.test.ts - Test:
web/src/app/api/config/route.test.ts(add one case)
Interfaces:
- Consumes: nothing new.
- Produces: nothing consumed by later tasks.
Background: Both PATCH routes filter with ([k]) => k in DEFAULTS, which walks the prototype chain, so a body key of __proto__ or constructor passes the filter and writes a junk Config row (harmless — no delete — but inconsistent with the DELETE handler at web/src/app/api/config/route.ts:51, which already uses Object.prototype.hasOwnProperty.call(...)).
- Step 1: Write the failing test — monitor PATCH rejects prototype-chain keys
Append to web/src/app/api/monitor/config/route.test.ts (mirror the file's existing PATCH test style — read the top of the file for its patchReq helper; if none exists, add one like below):
function patchReq(body: unknown) {
return new Request("http://localhost/api/monitor/config", {
method: "PATCH",
headers: { "content-type": "application/json" },
body: JSON.stringify(body),
});
}
it("ignores prototype-chain keys (__proto__, constructor) and writes no Config row", async () => {
const before = await prisma.config.count();
const res = await PATCH(patchReq({ __proto__: "x", constructor: "y" }));
expect((await res.json()).updated).toBe(0);
expect(await prisma.config.count()).toBe(before);
});
Ensure PATCH and prisma are imported at the top of the test file (import { GET, PATCH } from "./route"; and import { prisma } from "@/lib/db";).
- Step 2: Run it to verify it fails
Run: cd web && npx vitest run src/app/api/monitor/config/route.test.ts
Expected: FAIL — updated is 2 (or a Config row was written), because __proto__/constructor pass k in DEFAULTS.
- Step 3: Harden both PATCH routes
In web/src/app/api/monitor/config/route.ts change the filter:
const entries = Object.entries((body ?? {}) as Record<string, unknown>).filter(
([k]) => Object.prototype.hasOwnProperty.call(DEFAULTS, k),
);
Apply the identical change in web/src/app/api/discover/config/route.ts.
- Step 4: Add the mirror test for discover PATCH
Append the same test (with a patchReq pointing at /api/discover/config and importing this route's PATCH) to web/src/app/api/discover/config/route.test.ts.
- Step 5: Add the
constructorcase to the DELETE allowlist test
In web/src/app/api/config/route.test.ts, the test at line 85 ("rejects prototype-chain field names") only exercises __proto__. Add constructor alongside it:
for (const bad of ["__proto__", "constructor"]) {
const res = await DELETE(delReq(bad));
expect(res.status).toBe(400);
}
// Must not have collapsed to deleteMany({ where: {} }) and wiped the table.
expect(await prisma.config.count()).toBe(before);
expect(await prisma.config.findUnique({ where: { key: "qobuz.password" } })).not.toBeNull();
(Replace the existing single __proto__ DELETE block; keep the surrounding before/PUT setup.)
- Step 6: Run all three affected test files to verify they pass
Run: cd web && npx vitest run src/app/api/monitor/config/route.test.ts src/app/api/discover/config/route.test.ts src/app/api/config/route.test.ts
Expected: PASS.
- Step 7: Commit
git add web/src/app/api/monitor/config/route.ts web/src/app/api/discover/config/route.ts \
web/src/app/api/monitor/config/route.test.ts web/src/app/api/discover/config/route.test.ts \
web/src/app/api/config/route.test.ts
git commit -m "fix(web): harden config PATCH allowlists against prototype-chain keys"
Task 2: Tier A2 — Monitor toggle aria-label + Settings eyebrow copy
Files:
- Modify:
web/src/app/settings/settings-form.tsx:159(eyebrow) and:273-277(toggle)
Interfaces:
- Consumes: nothing.
- Produces: nothing.
No unit test (node vitest env can't render this component; verified by build + Playwright later).
- Step 1: Add an aria-label to the Monitor enable checkbox
At web/src/app/settings/settings-form.tsx:273, add aria-label to the <input type="checkbox"> inside the Monitor <label className="toggle">:
<input
type="checkbox"
aria-label="automatic monitoring"
checked={monitorCfg["monitor.enabled"] === "true"}
onChange={(e) => setMon("monitor.enabled", e.target.checked ? "true" : "false")}
/>
- Step 2: Fix the Settings eyebrow copy
At web/src/app/settings/settings-form.tsx:159, the eyebrow lists only three of the five tabs. Update it:
<PageHead title="Settings" eyebrow="Credentials · library · monitor · discovery" />
- Step 3: Typecheck + build to confirm no breakage
Run: cd web && npx tsc --noEmit && npm run build
Expected: clean (no type errors, build succeeds).
- Step 4: Commit
git add web/src/app/settings/settings-form.tsx
git commit -m "fix(web): label Monitor toggle for a11y + correct Settings eyebrow"
Task 3: Migration — add DiscoverySeedContribution + ScanWorkItem tables
Files:
- Modify:
web/prisma/schema.prisma - Create:
web/prisma/migrations/<timestamp>_add_contributions_and_scan_worklist/migration.sql - Modify:
worker/tests/conftest.py(clean the new tables)
Interfaces:
-
Produces (raw SQL tables consumed by Tasks 4 & 5):
DiscoverySeedContribution(id, "candidateMbid", "candidateName", "seedMbid", score float8, sources text[], "updatedAt"),UNIQUE("candidateMbid","seedMbid").ScanWorkItem(id, "scanId", artist, album, path, done bool),UNIQUE("scanId", path).
-
Step 1: Add both models to
web/prisma/schema.prisma
Append after the DiscoverySuggestion model:
model DiscoverySeedContribution {
id String @id @default(cuid())
candidateMbid String
candidateName String
seedMbid String
score Float
sources String[]
updatedAt DateTime @updatedAt
@@unique([candidateMbid, seedMbid])
@@index([candidateMbid])
@@index([seedMbid])
}
model ScanWorkItem {
id String @id @default(cuid())
scanId String
artist String
album String
path String
done Boolean @default(false)
@@unique([scanId, path])
@@index([scanId, done, artist, album])
}
- Step 2: Hand-author the migration SQL
Create web/prisma/migrations/20260713120000_add_contributions_and_scan_worklist/migration.sql (use a timestamp lexicographically after 20260711215631):
-- CreateTable
CREATE TABLE "DiscoverySeedContribution" (
"id" TEXT NOT NULL,
"candidateMbid" TEXT NOT NULL,
"candidateName" TEXT NOT NULL,
"seedMbid" TEXT NOT NULL,
"score" DOUBLE PRECISION NOT NULL,
"sources" TEXT[],
"updatedAt" TIMESTAMP(3) NOT NULL,
CONSTRAINT "DiscoverySeedContribution_pkey" PRIMARY KEY ("id")
);
-- CreateTable
CREATE TABLE "ScanWorkItem" (
"id" TEXT NOT NULL,
"scanId" TEXT NOT NULL,
"artist" TEXT NOT NULL,
"album" TEXT NOT NULL,
"path" TEXT NOT NULL,
"done" BOOLEAN NOT NULL DEFAULT false,
CONSTRAINT "ScanWorkItem_pkey" PRIMARY KEY ("id")
);
-- CreateIndex
CREATE UNIQUE INDEX "DiscoverySeedContribution_candidateMbid_seedMbid_key" ON "DiscoverySeedContribution"("candidateMbid", "seedMbid");
CREATE INDEX "DiscoverySeedContribution_candidateMbid_idx" ON "DiscoverySeedContribution"("candidateMbid");
CREATE INDEX "DiscoverySeedContribution_seedMbid_idx" ON "DiscoverySeedContribution"("seedMbid");
-- CreateIndex
CREATE UNIQUE INDEX "ScanWorkItem_scanId_path_key" ON "ScanWorkItem"("scanId", "path");
CREATE INDEX "ScanWorkItem_scanId_done_artist_album_idx" ON "ScanWorkItem"("scanId", "done", "artist", "album");
- Step 3: Regenerate the Prisma client and apply to lyra_test
Run:
cd web && npx prisma generate
DATABASE_URL=postgresql://lyra:lyra@localhost:5432/lyra_test npx prisma migrate deploy
Expected: prisma generate succeeds; migrate deploy reports the new migration applied (8 migrations). Confirm npx prisma migrate status (against lyra_test) shows "Database schema is up to date".
- Step 4: Clean the new tables in the worker test fixture
In worker/tests/conftest.py, the conn fixture DELETEs a set of tables both before (setup) and after (teardown). Add the two new tables to both DELETE blocks, before the Config delete in each:
cur.execute('DELETE FROM "DiscoverySeedContribution"')
cur.execute('DELETE FROM "ScanWorkItem"')
- Step 5: Verify the tables exist and the suite still boots
Run: cd worker && python -m pytest tests/test_discovery_schema.py -q
Expected: PASS (existing schema tests unaffected; the new tables are reachable). If it errors on a missing table, re-check Step 3 applied to lyra_test.
- Step 6: Commit
git add web/prisma/schema.prisma web/prisma/migrations worker/tests/conftest.py
git commit -m "feat: add DiscoverySeedContribution + ScanWorkItem tables"
Task 4: Tier B1 — per-seed discovery contributions + recompute
Files:
- Modify:
worker/lyra_worker/discovery.py(run_discoveryand helpers) - Test:
worker/tests/test_discovery.py(add new tests)
Interfaces:
- Consumes:
DiscoverySeedContributiontable (Task 3);_upsert_artist,_derive_albums,_healthy_sources,_seed_artists,_followed_mbids(existing, unchanged signatures). - Produces:
run_discovery(conn, sources, browser, cfg) -> DiscoveryResult(same signature); score is now sum-of-all-seed-contributions.
Design: Replace the in-memory chunk-local aggregation with contribution rows. Per swept seed, replace its rows; then recompute every affected candidate's suggestion from the summed contributions.
- Step 1: Write the failing test — score sums across separate chunks
Add to worker/tests/test_discovery.py:
def _artist_score(conn, mbid):
with conn.cursor() as cur:
cur.execute('SELECT score, "seedCount" FROM "DiscoverySuggestion" WHERE "artistMbid" = %s', (mbid,))
return cur.fetchone()
def test_score_sums_across_separate_chunks(conn):
# Two seeds, both eligible, forced into separate one-seed chunks by chunk_size=1.
insert_watched_artist(conn, mbid="s1", name="Seed One",
last_discovered_sql="now() - interval '200 hours'")
insert_watched_artist(conn, mbid="s2", name="Seed Two",
last_discovered_sql="now() - interval '190 hours'")
src = FakeSimilaritySource(similar={
"s1": [SimilarArtist("c1", "Shared", 0.6)],
"s2": [SimilarArtist("c1", "Shared", 0.5)],
})
cfg = DiscoveryConfig(chunk_size=1)
run_discovery(conn, [src], FakeMbBrowser(), cfg) # processes s1 (older)
run_discovery(conn, [src], FakeMbBrowser(), cfg) # processes s2
# replace-not-accumulate would leave 0.5/seedCount1; contributions give the sum.
assert _artist_score(conn, "c1") == (1.1, 2)
- Step 2: Run it to verify it fails
Run: cd worker && python -m pytest tests/test_discovery.py::test_score_sums_across_separate_chunks -q
Expected: FAIL — score is 0.5, seedCount 1 (last chunk overwrote via score = EXCLUDED.score).
- Step 3: Rewrite the aggregation in
discovery.py
Replace the body of run_discovery (lines ~146-194) and add two helpers. Keep _upsert_artist, _upsert_album, _derive_albums, _existing_rg_mbids, _healthy_sources, _seed_artists, _followed_mbids as-is.
def _record_contributions(conn: psycopg.Connection, seed_mbid: str, live, followed: set[str],
cfg: DiscoveryConfig) -> set[str]:
"""Replace seed_mbid's contribution rows with its current similar-artist scores.
Returns the set of candidate mbids whose aggregate may have changed (old contributors
of this seed ∪ newly inserted), so callers recompute drop-offs too."""
with conn.cursor() as cur:
cur.execute('SELECT "candidateMbid" FROM "DiscoverySeedContribution" WHERE "seedMbid" = %s',
(seed_mbid,))
affected = {r[0] for r in cur.fetchall()}
cur.execute('DELETE FROM "DiscoverySeedContribution" WHERE "seedMbid" = %s', (seed_mbid,))
per_cand: dict[str, dict] = {}
for src in live:
try:
similar = src.similar_artists(seed_mbid)
except Exception as e: # a bad source/seed must not abort the sweep
print(f"worker: discovery source {src.name} failed for {seed_mbid}: {e}", flush=True)
continue
for sa in sorted(similar, key=lambda a: a.score, reverse=True)[: cfg.similar_per_seed]:
if sa.mbid in followed:
continue
c = per_cand.setdefault(sa.mbid, {"name": sa.name, "score": 0.0, "sources": set()})
c["score"] += sa.score
c["sources"].add(src.name)
if sa.name and not c["name"]:
c["name"] = sa.name
with conn.cursor() as cur:
for mbid, c in per_cand.items():
cur.execute(
'INSERT INTO "DiscoverySeedContribution" (id, "candidateMbid", "candidateName", '
'"seedMbid", score, sources, "updatedAt") '
'VALUES (gen_random_uuid()::text, %s, %s, %s, %s, %s, now())',
(mbid, c["name"], seed_mbid, c["score"], sorted(c["sources"])),
)
affected.add(mbid)
return affected
def _recompute_suggestions(conn: psycopg.Connection, affected: set[str],
cfg: DiscoveryConfig) -> list[dict]:
"""Recompute each affected candidate's DiscoverySuggestion from the sum of its
contributions. Returns the candidates that produced a live pending suggestion."""
if not affected:
return []
with conn.cursor() as cur:
cur.execute(
'SELECT "candidateMbid", "candidateName", score, "seedMbid", sources '
'FROM "DiscoverySeedContribution" WHERE "candidateMbid" = ANY(%s)',
(list(affected),))
rows = cur.fetchall()
agg: dict[str, dict] = {}
for mbid, name, score, seed, sources in rows:
a = agg.setdefault(mbid, {"mbid": mbid, "name": name, "score": 0.0,
"seeds": set(), "sources": set()})
a["score"] += score
a["seeds"].add(seed)
a["sources"].update(sources or [])
if name and not a["name"]:
a["name"] = name
surfaced: list[dict] = []
for a in sorted(agg.values(), key=lambda a: a["score"], reverse=True):
if a["score"] < cfg.min_score:
continue
cand = {"mbid": a["mbid"], "name": a["name"], "score": a["score"],
"seed_count": len(a["seeds"]), "sources": a["sources"]}
if _upsert_artist(conn, cand): # rowcount 1 => a live pending suggestion
surfaced.append(cand)
return surfaced
def run_discovery(conn: psycopg.Connection, sources: list[SimilaritySource],
browser: MbBrowser, cfg: DiscoveryConfig) -> DiscoveryResult:
"""Record each seed's similar-artist contributions, then recompute affected
suggestions from the summed contributions — so a candidate's score is the sum of
every seed's latest contribution, independent of chunk and sweep boundaries."""
seeds = _seed_artists(conn, cfg)
followed = _followed_mbids(conn)
live = _healthy_sources(sources)
affected: set[str] = set()
for seed_mbid, _seed_name in seeds:
affected |= _record_contributions(conn, seed_mbid, live, followed, cfg)
surfaced = _recompute_suggestions(conn, affected, cfg)
albums = _derive_albums(conn, browser, surfaced, cfg)
with conn.cursor() as cur:
for seed_mbid, _seed_name in seeds:
cur.execute('UPDATE "WatchedArtist" SET "lastDiscoveredAt" = now() WHERE mbid = %s',
(seed_mbid,))
conn.commit()
return DiscoveryResult(artists=len(surfaced), albums=albums, seeds=len(seeds))
- Step 4: Run the new test + the full discovery suite to verify green
Run: cd worker && python -m pytest tests/test_discovery.py tests/test_discovery_albums.py tests/test_discovery_trigger.py -q
Expected: PASS — including the pre-existing test_aggregates_scores_across_seeds (single call, 2 seeds → c1 1.1/seedCount 2), which the contributions model reproduces.
- Step 5: Write the partial-sweep + drop-off tests
Add to worker/tests/test_discovery.py:
def test_partial_sweep_preserves_other_seeds_contribution(conn):
# Full sweep: c1 gets s1(0.6)+s2(0.5)=1.1. Then only s1 is due again with a new
# score; s2's contribution must survive untouched.
insert_watched_artist(conn, mbid="s1", name="Seed One",
last_discovered_sql="now() - interval '200 hours'")
insert_watched_artist(conn, mbid="s2", name="Seed Two",
last_discovered_sql="now() - interval '200 hours'")
src = FakeSimilaritySource(similar={
"s1": [SimilarArtist("c1", "Shared", 0.6)],
"s2": [SimilarArtist("c1", "Shared", 0.5)],
})
run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()) # both -> 1.1/2
assert _artist_score(conn, "c1") == (1.1, 2)
# s1 due again (older than interval), s2 fresh; s1 now scores c1 at 0.7.
with conn.cursor() as cur:
cur.execute('UPDATE "WatchedArtist" SET "lastDiscoveredAt" = now() - interval \'200 hours\' WHERE mbid = %s', ("s1",))
cur.execute('UPDATE "WatchedArtist" SET "lastDiscoveredAt" = now() WHERE mbid = %s', ("s2",))
conn.commit()
src2 = FakeSimilaritySource(similar={"s1": [SimilarArtist("c1", "Shared", 0.7)]})
run_discovery(conn, [src2], FakeMbBrowser(), DiscoveryConfig())
assert _artist_score(conn, "c1") == (1.2, 2) # 0.7 (new s1) + 0.5 (kept s2)
def test_seed_dropoff_lowers_seedcount(conn):
insert_watched_artist(conn, mbid="s1", name="Seed One",
last_discovered_sql="now() - interval '200 hours'")
insert_watched_artist(conn, mbid="s2", name="Seed Two",
last_discovered_sql="now() - interval '200 hours'")
src = FakeSimilaritySource(similar={
"s1": [SimilarArtist("c1", "Shared", 0.6)],
"s2": [SimilarArtist("c1", "Shared", 0.5)],
})
run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig())
assert _artist_score(conn, "c1") == (1.1, 2)
# s1 re-swept but no longer finds c1 -> its contribution is removed, c1 recomputed.
with conn.cursor() as cur:
cur.execute('UPDATE "WatchedArtist" SET "lastDiscoveredAt" = now() - interval \'200 hours\' WHERE mbid = %s', ("s1",))
cur.execute('UPDATE "WatchedArtist" SET "lastDiscoveredAt" = now() WHERE mbid = %s', ("s2",))
conn.commit()
src2 = FakeSimilaritySource(similar={"s1": [SimilarArtist("c9", "Other", 0.9)]})
run_discovery(conn, [src2], FakeMbBrowser(), DiscoveryConfig())
assert _artist_score(conn, "c1") == (0.5, 1) # only s2 remains
- Step 6: Run to verify the new tests pass
Run: cd worker && python -m pytest tests/test_discovery.py -q
Expected: PASS.
- Step 7: Commit
git add worker/lyra_worker/discovery.py worker/tests/test_discovery.py
git commit -m "feat(worker): per-seed discovery contributions so score sums across chunks/sweeps"
Task 5: Tier B2 — persisted scan worklist
Files:
- Modify:
worker/lyra_worker/scan.py - Modify:
worker/lyra_worker/main.py(maybe_run_scan, imports) - Test:
worker/tests/test_scan_chunk.py(rewrite to the worklist API)
Interfaces:
- Consumes:
ScanWorkItemtable (Task 3); existing_iter_album_entries,_process_album,ScanResult. - Produces:
build_worklist(conn, scan_id: str, dest_root: str = "/music") -> intscan_chunk(conn, resolver, probe, browser, scan_id: str, limit: int) -> tuple[int, int, bool]— returns(imported, skipped, done).clear_worklist(conn, scan_id: str) -> Nonescan_library(conn, resolver, probe, browser, dest_root="/music") -> ScanResult— unchanged signature, reimplemented on the worklist.
Note: scan_chunk's signature CHANGES (drops dest_root/cursor, adds scan_id, drops the returned cursor). test_scan.py uses only scan_library and is unaffected; test_scan_trigger.py checks scan.progress/scan.result/scan.inProgress (not scan.cursor) and is unaffected; only test_scan_chunk.py must be rewritten.
- Step 1: Write the failing tests — worklist build/pop/resume
Replace the entire contents of worker/tests/test_scan_chunk.py with:
from lyra_worker.adapters.fakes import FakeMbBrowser
from lyra_worker.browser import ReleaseGroupInfo
from lyra_worker.scan import build_worklist, scan_chunk, clear_worklist
from lyra_worker.types import MBTarget
from tests.test_scan import FakeProbe, FakeResolver, _album, _counts
def _tree(tmp_path):
"""Three albums across two artists: Artist A/Alpha, Artist A/Beta, Artist B/Gamma."""
_album(tmp_path, "Artist A", "Alpha (2001)")
_album(tmp_path, "Artist A", "Beta (2002)")
_album(tmp_path, "Artist B", "Gamma (2003)")
resolver = FakeResolver({
("Artist A", "Alpha"): MBTarget(artist="Artist A", album="Alpha", year=2001, rg_mbid="rg-a1", artist_mbid="ma"),
("Artist A", "Beta"): MBTarget(artist="Artist A", album="Beta", year=2002, rg_mbid="rg-a2", artist_mbid="ma"),
("Artist B", "Gamma"): MBTarget(artist="Artist B", album="Gamma", year=2003, rg_mbid="rg-b1", artist_mbid="mb"),
})
browser = FakeMbBrowser(releases={
"ma": [ReleaseGroupInfo("rg-a1", "Alpha", "Album", (), "2001"),
ReleaseGroupInfo("rg-a2", "Beta", "Album", (), "2002")],
"mb": [ReleaseGroupInfo("rg-b1", "Gamma", "Album", (), "2003")],
})
return resolver, browser
def test_build_worklist_inserts_one_row_per_album(conn, tmp_path):
resolver, _ = _tree(tmp_path)
n = build_worklist(conn, "scan1", str(tmp_path))
assert n == 3
with conn.cursor() as cur:
cur.execute('SELECT count(*) FROM "ScanWorkItem" WHERE "scanId" = %s AND NOT done', ("scan1",))
assert cur.fetchone()[0] == 3
def test_build_worklist_is_idempotent(conn, tmp_path):
resolver, _ = _tree(tmp_path)
assert build_worklist(conn, "scan1", str(tmp_path)) == 3
assert build_worklist(conn, "scan1", str(tmp_path)) == 0 # ON CONFLICT DO NOTHING
with conn.cursor() as cur:
cur.execute('SELECT count(*) FROM "ScanWorkItem" WHERE "scanId" = %s', ("scan1",))
assert cur.fetchone()[0] == 3
def test_scan_chunk_respects_limit_and_marks_done(conn, tmp_path):
resolver, browser = _tree(tmp_path)
build_worklist(conn, "scan1", str(tmp_path))
imported, skipped, done = scan_chunk(conn, resolver, FakeProbe(quality_class=2), browser, "scan1", limit=2)
assert (imported, skipped, done) == (2, 0, False) # 2 of 3 processed, more remain
with conn.cursor() as cur:
cur.execute('SELECT count(*) FROM "ScanWorkItem" WHERE "scanId" = %s AND done', ("scan1",))
assert cur.fetchone()[0] == 2
cur.execute('SELECT count(*) FROM "LibraryItem"')
assert cur.fetchone()[0] == 2
def test_scan_chunk_resumes_and_finishes(conn, tmp_path):
resolver, browser = _tree(tmp_path)
build_worklist(conn, "scan1", str(tmp_path))
scan_chunk(conn, resolver, FakeProbe(quality_class=2), browser, "scan1", limit=2)
imported, skipped, done = scan_chunk(conn, resolver, FakeProbe(quality_class=2), browser, "scan1", limit=25)
assert (imported, skipped, done) == (1, 0, True) # last album, then drained
with conn.cursor() as cur:
cur.execute('SELECT album FROM "LibraryItem" ORDER BY album')
assert cur.fetchall() == [("Alpha",), ("Beta",), ("Gamma",)]
def test_scan_chunk_one_per_chunk_totals_correctly(conn, tmp_path):
resolver, browser = _tree(tmp_path)
build_worklist(conn, "scan1", str(tmp_path))
total_i = total_s = guard = 0
done = False
while not done and guard < 10:
i, s, done = scan_chunk(conn, resolver, FakeProbe(quality_class=2), browser, "scan1", limit=1)
total_i += i
total_s += s
guard += 1
assert (total_i, total_s) == (3, 0)
assert guard == 3
assert _counts(conn) == {"LibraryItem": 3, "MonitoredRelease": 3, "WatchedArtist": 2}
def test_clear_worklist_removes_this_scans_rows(conn, tmp_path):
resolver, _ = _tree(tmp_path)
build_worklist(conn, "scan1", str(tmp_path))
clear_worklist(conn, "scan1")
with conn.cursor() as cur:
cur.execute('SELECT count(*) FROM "ScanWorkItem" WHERE "scanId" = %s', ("scan1",))
assert cur.fetchone()[0] == 0
- Step 2: Run to verify failure
Run: cd worker && python -m pytest tests/test_scan_chunk.py -q
Expected: FAIL — ImportError (build_worklist/clear_worklist don't exist; scan_chunk has the old signature).
- Step 3: Rewrite
scan.py
Add import uuid at the top. Replace scan_chunk and scan_library (lines ~142-176), keeping everything above (_iter_album_entries, _process_album, _cursor_key/_cursor_tuple may be removed since nothing uses them now — remove _cursor_key and _cursor_tuple):
def build_worklist(conn: psycopg.Connection, scan_id: str, dest_root: str = "/music") -> int:
"""Walk the tree once and persist this scan's album worklist. Idempotent
(ON CONFLICT DO NOTHING), so a re-run after a crash adds only missing rows.
Returns the number of rows inserted."""
inserted = 0
with conn.cursor() as cur:
for artist_name, album_folder, album_path in _iter_album_entries(dest_root):
cur.execute(
'INSERT INTO "ScanWorkItem" (id, "scanId", artist, album, path, done) '
"VALUES (gen_random_uuid()::text, %s, %s, %s, %s, false) "
'ON CONFLICT ("scanId", path) DO NOTHING',
(scan_id, artist_name, album_folder, album_path),
)
inserted += cur.rowcount
conn.commit()
return inserted
def scan_chunk(conn: psycopg.Connection, resolver, probe: AudioProbe, browser,
scan_id: str, limit: int) -> tuple[int, int, bool]:
"""Process up to `limit` not-yet-done worklist items for `scan_id`, in stable
(artist, album) order, marking each done. Returns (imported, skipped, done) where
`done` is True when no undone rows remain. Idempotent: all record writes are
ON CONFLICT, and an item is marked done only after it is processed."""
with conn.cursor() as cur:
cur.execute(
'SELECT id, artist, album, path FROM "ScanWorkItem" '
'WHERE "scanId" = %s AND NOT done ORDER BY artist, album LIMIT %s',
(scan_id, limit),
)
items = cur.fetchall()
imported = skipped = 0
browsed: set[str] = set() # artists whose discography we've populated this chunk
for item_id, artist_name, album_folder, album_path in items:
outcome = _process_album(conn, resolver, probe, browser, artist_name, album_folder, album_path, browsed)
if outcome == "imported":
imported += 1
elif outcome == "skipped":
skipped += 1
with conn.cursor() as cur:
cur.execute('UPDATE "ScanWorkItem" SET done = true WHERE id = %s', (item_id,))
conn.commit()
with conn.cursor() as cur:
cur.execute('SELECT count(*) FROM "ScanWorkItem" WHERE "scanId" = %s AND NOT done', (scan_id,))
remaining = cur.fetchone()[0]
return imported, skipped, remaining == 0
def clear_worklist(conn: psycopg.Connection, scan_id: str) -> None:
with conn.cursor() as cur:
cur.execute('DELETE FROM "ScanWorkItem" WHERE "scanId" = %s', (scan_id,))
conn.commit()
def scan_library(conn: psycopg.Connection, resolver, probe: AudioProbe, browser,
dest_root: str = "/music") -> ScanResult:
"""Walk `{dest_root}/{Artist}/{Album}/` in one unbounded pass (build the worklist,
drain it, clean up), recording matched albums as have + monitored + followed plus
each followed artist's full discography. Test/one-shot convenience; the worker loop
uses the chunked build_worklist + scan_chunk so a large library doesn't block
job-claim."""
scan_id = "oneshot-" + uuid.uuid4().hex
build_worklist(conn, scan_id, dest_root)
imported = skipped = 0
done = False
while not done:
imp, skp, done = scan_chunk(conn, resolver, probe, browser, scan_id, limit=1000)
imported += imp
skipped += skp
clear_worklist(conn, scan_id)
return ScanResult(imported, skipped)
- Step 4: Run scan.py's direct tests to verify green
Run: cd worker && python -m pytest tests/test_scan_chunk.py tests/test_scan.py -q
Expected: PASS (test_scan.py uses scan_library, unchanged behavior; test_scan_chunk.py exercises the new API).
- Step 5: Rewire
maybe_run_scaninmain.py
Update the import at worker/lyra_worker/main.py:17:
from lyra_worker.scan import build_worklist, clear_worklist, scan_chunk
Add import uuid near the top imports. Replace maybe_run_scan (lines ~58-96) with:
def maybe_run_scan(conn, resolver, probe, browser, config, dest_root: str = DEST_ROOT) -> None:
"""Advance a chunked library scan by at most one chunk per call, so the worker loop
keeps claiming jobs and ticking the monitor between chunks. `scan.requested` starts a
fresh scan (walk once -> ScanWorkItem worklist); `scan.inProgress`/`scan.id`/`scan.progress`
persist state until the worklist drains, at which point `scan.result` is written."""
requested = str(config.get("scan.requested", "")).strip().lower() in _TRUE
in_progress = str(config.get("scan.inProgress", "")).strip().lower() in _TRUE
if not requested and not in_progress:
return
if requested and not in_progress: # start a fresh scan
scan_id = uuid.uuid4().hex
_set_config(conn, "scan.inProgress", "true")
_set_config(conn, "scan.requested", "false")
_set_config(conn, "scan.id", scan_id)
_set_config(conn, "scan.progress", "0/0")
try:
build_worklist(conn, scan_id, dest_root)
except Exception as e: # a scan error must never kill the worker
print(f"worker: library scan worklist build failed: {e}", flush=True)
conn.rollback()
_set_config(conn, "scan.inProgress", "false")
_set_config(conn, "scan.requested", "false")
return
imported_so_far, skipped_so_far = 0, 0
else: # resume the in-progress scan
scan_id = config.get("scan.id", "") or ""
imported_so_far, skipped_so_far = _parse_progress(config.get("scan.progress", ""))
if not scan_id: # corrupt/absent state -> abandon; a new request restarts it
_set_config(conn, "scan.inProgress", "false")
return
try:
imp, skp, done = scan_chunk(conn, resolver, probe, browser, scan_id, _scan_chunk_size(config))
except Exception as e: # a scan error must never kill the worker
print(f"worker: library scan chunk failed: {e}", flush=True)
conn.rollback()
_set_config(conn, "scan.inProgress", "false")
_set_config(conn, "scan.requested", "false")
return
imported_total = imported_so_far + imp
skipped_total = skipped_so_far + skp
if done:
_set_config(conn, "scan.result", f"imported {imported_total}, skipped {skipped_total}")
_set_config(conn, "scan.progress", "")
_set_config(conn, "scan.inProgress", "false")
clear_worklist(conn, scan_id)
_set_config(conn, "scan.id", "")
print(f"worker: library scan done — {imported_total} imported, {skipped_total} skipped", flush=True)
else:
_set_config(conn, "scan.progress", f"{imported_total}/{skipped_total}")
- Step 6: Run the scan trigger tests + the full worker suite
Run: cd worker && python -m pytest tests/test_scan_trigger.py -q && python -m pytest -q
Expected: PASS. test_scan_trigger.py is unchanged (it asserts on scan.progress/scan.result/scan.inProgress, all preserved). Full suite green (previous baseline 173 passed / 7 skipped, now higher with the added tests).
- Step 7: Commit
git add worker/lyra_worker/scan.py worker/lyra_worker/main.py worker/tests/test_scan_chunk.py
git commit -m "feat(worker): persisted scan worklist (O(N) scan, robust to mid-scan changes)"
Final verification (before merge)
- Worker suite:
cd worker && python -m pytest -q→ all green. - Web suite:
cd web && npx vitest run→ all green. - Typecheck + build:
cd web && npx tsc --noEmit && npm run build→ clean. - Migration status (lyra_test):
cd web && DATABASE_URL=postgresql://lyra:lyra@localhost:5432/lyra_test npx prisma migrate status→ up to date, 8 migrations. - Opus whole-branch review (via subagent-driven-development's final review), then ff-merge
cleanup/tier-a-b→main, push,docker compose up -d --build web worker, verify live (web 200, worker healthy,migrate deployreports the new migration applied).
Self-review notes
- Spec coverage: Tier A1 (Task 1), A2 (Task 2), A3 (Task 1 Step 5); Tier B1 (Tasks 3+4); Tier B2 (Tasks 3+5); migration (Task 3); testing + delivery (Final verification). All spec sections mapped.
- Type consistency:
scan_chunknew signature(conn, resolver, probe, browser, scan_id, limit) -> (imported, skipped, done)used identically inscan.py,main.py, andtest_scan_chunk.py.build_worklist/clear_worklistsignatures match across producer and callers. Discoveryrun_discoverysignature unchanged; helpers_record_contributions/_recompute_suggestionsare internal. - Backward-compat: existing
test_aggregates_scores_across_seeds,test_scan.py,test_scan_trigger.pypass unchanged by design (verified against their current assertions).