From 6ca39859fa79c02d7fd96660d9f6db93ee0022fd Mon Sep 17 00:00:00 2001 From: Jonathan Date: Tue, 14 Jul 2026 20:56:31 +0200 Subject: [PATCH] feat(library): surface albums the scan couldn't match The library scan silently skipped album folders with no MusicBrainz match (or an unreadable file). Persist them so they're visible and fixable. - New UnmatchedAlbum model (migration add_unmatched_album), keyed by path. - scan_chunk records a skipped album as unmatched (reason "no MusicBrainz match" or "unreadable: "), clears the row when an album later resolves, and prunes stale rows (a prior scan's, or a removed folder) when a scan completes. - GET /api/library/unmatched lists them; DELETE ?id= dismisses one. - /library shows a "Couldn't match" section (path + reason + Dismiss). Worker + web tests added. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../migration.sql | 15 ++++++ web/prisma/schema.prisma | 15 ++++++ .../app/api/library/unmatched/route.test.ts | 33 +++++++++++++ web/src/app/api/library/unmatched/route.ts | 27 +++++++++++ web/src/app/library/library-client.tsx | 47 +++++++++++++++++++ web/src/test/setup.ts | 1 + worker/lyra_worker/scan.py | 39 ++++++++++++++- worker/tests/conftest.py | 2 + worker/tests/test_scan.py | 35 ++++++++++++++ 9 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 web/prisma/migrations/20260714185225_add_unmatched_album/migration.sql create mode 100644 web/src/app/api/library/unmatched/route.test.ts create mode 100644 web/src/app/api/library/unmatched/route.ts diff --git a/web/prisma/migrations/20260714185225_add_unmatched_album/migration.sql b/web/prisma/migrations/20260714185225_add_unmatched_album/migration.sql new file mode 100644 index 0000000..e452a73 --- /dev/null +++ b/web/prisma/migrations/20260714185225_add_unmatched_album/migration.sql @@ -0,0 +1,15 @@ +-- CreateTable +CREATE TABLE "UnmatchedAlbum" ( + "id" TEXT NOT NULL, + "artist" TEXT NOT NULL, + "album" TEXT NOT NULL, + "path" TEXT NOT NULL, + "reason" TEXT NOT NULL, + "scanId" TEXT NOT NULL, + "scannedAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "UnmatchedAlbum_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE UNIQUE INDEX "UnmatchedAlbum_path_key" ON "UnmatchedAlbum"("path"); diff --git a/web/prisma/schema.prisma b/web/prisma/schema.prisma index 5c1c5e9..c9f132c 100644 --- a/web/prisma/schema.prisma +++ b/web/prisma/schema.prisma @@ -191,6 +191,21 @@ model ScanWorkItem { @@index([scanId, done, artist, album]) } +// Album folders on disk that the library scan could not resolve to a MusicBrainz release +// (no match, or an unreadable/corrupt file). Surfaced on /library so the user can fix the +// metadata or add them manually instead of them being silently skipped. Keyed by path; a +// row is removed when the album later matches, and stale rows (a prior scan's, or a folder +// since removed) are pruned when a scan completes. +model UnmatchedAlbum { + id String @id @default(cuid()) + artist String + album String + path String @unique + reason String + scanId String + scannedAt DateTime @default(now()) +} + // Generic read-through cache for external API responses shared by web + worker (keyed on // the LOGICAL operation, not the request URL, so both processes share entries). TTL is // applied at read time from fetchedAt in code (no expiresAt column → tunable, no migration). diff --git a/web/src/app/api/library/unmatched/route.test.ts b/web/src/app/api/library/unmatched/route.test.ts new file mode 100644 index 0000000..5832147 --- /dev/null +++ b/web/src/app/api/library/unmatched/route.test.ts @@ -0,0 +1,33 @@ +import { describe, it, expect } from "vitest"; +import { prisma } from "@/lib/db"; +import { GET, DELETE } from "./route"; + +describe("library unmatched API", () => { + it("lists unmatched albums ordered by artist/album", async () => { + await prisma.unmatchedAlbum.createMany({ + data: [ + { artist: "Zed", album: "Late", path: "/music/Zed/Late", reason: "no MusicBrainz match", scanId: "s1" }, + { artist: "Ann", album: "Early", path: "/music/Ann/Early", reason: "unreadable: boom", scanId: "s1" }, + ], + }); + const res = await GET(); + expect(res.status).toBe(200); + const { unmatched } = await res.json(); + expect(unmatched.map((u: { artist: string }) => u.artist)).toEqual(["Ann", "Zed"]); + expect(unmatched[0]).toMatchObject({ album: "Early", reason: "unreadable: boom" }); + }); + + it("dismisses one entry by id", async () => { + const row = await prisma.unmatchedAlbum.create({ + data: { artist: "X", album: "Y", path: "/music/X/Y", reason: "no MusicBrainz match", scanId: "s1" }, + }); + const res = await DELETE(new Request(`http://t/api/library/unmatched?id=${row.id}`, { method: "DELETE" })); + expect(res.status).toBe(200); + expect(await prisma.unmatchedAlbum.count()).toBe(0); + }); + + it("404s dismissing an unknown id", async () => { + const res = await DELETE(new Request("http://t/api/library/unmatched?id=nope", { method: "DELETE" })); + expect(res.status).toBe(404); + }); +}); diff --git a/web/src/app/api/library/unmatched/route.ts b/web/src/app/api/library/unmatched/route.ts new file mode 100644 index 0000000..b70217e --- /dev/null +++ b/web/src/app/api/library/unmatched/route.ts @@ -0,0 +1,27 @@ +import { prisma } from "@/lib/db"; + +// GET /api/library/unmatched — album folders the scan couldn't resolve to a MusicBrainz +// release (no match, or an unreadable file). Surfaced on /library so they aren't silently lost. +export async function GET() { + const rows = await prisma.unmatchedAlbum.findMany({ orderBy: [{ artist: "asc" }, { album: "asc" }] }); + return Response.json({ + unmatched: rows.map((r) => ({ + id: r.id, + artist: r.artist, + album: r.album, + path: r.path, + reason: r.reason, + scannedAt: r.scannedAt, + })), + }); +} + +// DELETE /api/library/unmatched?id= — dismiss one entry (the user has handled it, or doesn't +// care). Does not touch files on disk; a later re-scan re-surfaces it if still unresolved. +export async function DELETE(request: Request) { + const id = new URL(request.url).searchParams.get("id"); + if (!id) return Response.json({ error: "id is required" }, { status: 400 }); + const deleted = await prisma.unmatchedAlbum.deleteMany({ where: { id } }); + if (deleted.count === 0) return Response.json({ error: "not found" }, { status: 404 }); + return Response.json({ ok: true }); +} diff --git a/web/src/app/library/library-client.tsx b/web/src/app/library/library-client.tsx index c064073..191f9e7 100644 --- a/web/src/app/library/library-client.tsx +++ b/web/src/app/library/library-client.tsx @@ -31,8 +31,11 @@ const SORTS: { key: SortKey; label: string; cmp: (a: Album, b: Album) => number { key: "artist", label: "Artist A–Z", cmp: (a, b) => a.artist.localeCompare(b.artist) || (a.year ?? "").localeCompare(b.year ?? "") }, ]; +type Unmatched = { id: string; artist: string; album: string; path: string; reason: string }; + export function LibraryClient() { const [albums, setAlbums] = useState([]); + const [unmatched, setUnmatched] = useState([]); const [q, setQ] = useState(""); const [sort, setSort] = useState("added"); const [open, setOpen] = useState(null); @@ -60,11 +63,23 @@ export function LibraryClient() { fetch("/api/library") .then((r) => r.json()) .then((d) => setAlbums(d.albums ?? [])); + fetch("/api/library/unmatched") + .then((r) => r.json()) + .then((d) => setUnmatched(d.unmatched ?? [])); } useEffect(() => { refresh(); }, []); + async function dismissUnmatched(u: Unmatched) { + const res = await fetch(`/api/library/unmatched?id=${u.id}`, { method: "DELETE" }).catch(() => null); + if (res?.ok) { + setUnmatched((list) => list.filter((x) => x.id !== u.id)); + } else { + toast("Couldn't dismiss"); + } + } + const shown = useMemo(() => { const needle = q.trim().toLowerCase(); const filtered = needle @@ -119,6 +134,38 @@ export function LibraryClient() { ) : null} + {unmatched.length > 0 ? ( + <> + +

+ Album folders on disk the scan couldn’t resolve to a MusicBrainz release. Fix the + folder name (Artist / Album (Year)) or re-add them manually, then re-scan. Dismiss to + hide an entry. +

+
    + {unmatched.map((u) => ( +
  • +
    +
    + {u.artist ? `${u.artist} — ` : ""} + {u.album} +
    +
    + {u.path} ·{" "} + {u.reason} +
    +
    +
    + +
    +
    • + ))} +
+ + ) : null} + {open ? ( { await prisma.discoverySuggestion.deleteMany(); await prisma.config.deleteMany(); await prisma.apiCache.deleteMany(); // read-through cache must not leak across tests + await prisma.unmatchedAlbum.deleteMany(); }); diff --git a/worker/lyra_worker/scan.py b/worker/lyra_worker/scan.py index 6a7c983..ae1a7d5 100644 --- a/worker/lyra_worker/scan.py +++ b/worker/lyra_worker/scan.py @@ -134,6 +134,32 @@ def _process_album(conn, resolver, probe, browser, artist_name, album_folder, al return "imported" if _record_owned(conn, target, pr.quality_class, pr.fmt, album_path, watched_id) else "recorded" +def _record_unmatched(conn, scan_id: str, artist: str, album: str, path: str, reason: str) -> None: + """Persist an album folder the scan couldn't resolve, so /library can surface it. + Upsert by path; the current scan's id tags the row so stale ones can be pruned.""" + with conn.cursor() as cur: + cur.execute( + 'INSERT INTO "UnmatchedAlbum" (id, artist, album, path, reason, "scanId", "scannedAt") ' + "VALUES (gen_random_uuid()::text, %s, %s, %s, %s, %s, now()) " + 'ON CONFLICT (path) DO UPDATE SET artist = EXCLUDED.artist, album = EXCLUDED.album, ' + 'reason = EXCLUDED.reason, "scanId" = EXCLUDED."scanId", "scannedAt" = now()', + (artist or "", album or "", path, reason, scan_id), + ) + + +def _clear_unmatched(conn, path: str) -> None: + """Drop an album's unmatched row once it resolves (matched, or fixed + re-scanned).""" + with conn.cursor() as cur: + cur.execute('DELETE FROM "UnmatchedAlbum" WHERE path = %s', (path,)) + + +def _prune_unmatched(conn, scan_id: str) -> None: + """Once a scan completes, drop unmatched rows it did not touch — a prior scan's rows, + or folders removed/fixed since — so the list reflects only the latest scan.""" + with conn.cursor() as cur: + cur.execute('DELETE FROM "UnmatchedAlbum" WHERE "scanId" <> %s', (scan_id,)) + + 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. @@ -169,6 +195,7 @@ def scan_chunk(conn: psycopg.Connection, resolver, probe: AudioProbe, browser, 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: + reason = "no MusicBrainz match" try: outcome = _process_album(conn, resolver, probe, browser, artist_name, album_folder, album_path, browsed) except Exception as e: @@ -179,11 +206,15 @@ def scan_chunk(conn: psycopg.Connection, resolver, probe: AudioProbe, browser, except Exception: pass print(f"scan: skipping {artist_name}/{album_folder}: {e}", flush=True) + reason = f"unreadable: {e}" outcome = "skipped" if outcome == "imported": imported += 1 - elif outcome == "skipped": + if outcome == "skipped": skipped += 1 + _record_unmatched(conn, scan_id, artist_name, album_folder, album_path, reason) + elif outcome in ("imported", "recorded"): + _clear_unmatched(conn, album_path) # it resolved — no longer unmatched with conn.cursor() as cur: cur.execute('UPDATE "ScanWorkItem" SET done = true WHERE id = %s', (item_id,)) conn.commit() @@ -191,7 +222,11 @@ def scan_chunk(conn: psycopg.Connection, resolver, probe: AudioProbe, browser, 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 + done = remaining == 0 + if done: + _prune_unmatched(conn, scan_id) + conn.commit() + return imported, skipped, done def clear_worklist(conn: psycopg.Connection, scan_id: str) -> None: diff --git a/worker/tests/conftest.py b/worker/tests/conftest.py index 75daeba..5ef2559 100644 --- a/worker/tests/conftest.py +++ b/worker/tests/conftest.py @@ -30,6 +30,7 @@ def conn(): cur.execute('DELETE FROM "DiscoverySuggestion"') cur.execute('DELETE FROM "DiscoverySeedContribution"') cur.execute('DELETE FROM "ScanWorkItem"') + cur.execute('DELETE FROM "UnmatchedAlbum"') cur.execute('DELETE FROM "ApiCache"') cur.execute('DELETE FROM "Config"') connection.commit() @@ -48,6 +49,7 @@ def conn(): cur.execute('DELETE FROM "DiscoverySuggestion"') cur.execute('DELETE FROM "DiscoverySeedContribution"') cur.execute('DELETE FROM "ScanWorkItem"') + cur.execute('DELETE FROM "UnmatchedAlbum"') cur.execute('DELETE FROM "ApiCache"') cur.execute('DELETE FROM "Config"') connection.commit() diff --git a/worker/tests/test_scan.py b/worker/tests/test_scan.py index 3bfaf40..02e7a5c 100644 --- a/worker/tests/test_scan.py +++ b/worker/tests/test_scan.py @@ -82,11 +82,46 @@ def test_scan_skips_a_corrupt_album_and_captures_tracknames(conn, tmp_path): assert cur.fetchone()[0] == ["01 One.flac", "02 Two.flac"] # on-disk tracks captured at scan +def _unmatched(conn): + with conn.cursor() as cur: + cur.execute('SELECT artist, album, reason FROM "UnmatchedAlbum" ORDER BY album') + return cur.fetchall() + + def test_scan_skips_unmatched_albums(conn, tmp_path): _album(tmp_path, "Obscure", "Nope (1999)") result = scan_library(conn, FakeResolver({}), FakeProbe(), FakeMbBrowser(), dest_root=str(tmp_path)) assert (result.imported, result.skipped) == (0, 1) assert _counts(conn) == {"LibraryItem": 0, "MonitoredRelease": 0, "WatchedArtist": 0} + # the unmatched album is surfaced (folder name kept for display) with a reason + assert _unmatched(conn) == [("Obscure", "Nope (1999)", "no MusicBrainz match")] + + +def test_scan_records_corrupt_album_as_unmatched_with_error_reason(conn, tmp_path): + _album(tmp_path, "Bad Artist", "Bad Album (2020)") + + class RaisingProbe: + def probe(self, path): + raise RuntimeError("file said 4 bytes, read 0 bytes") + + scan_library(conn, FakeResolver({}), RaisingProbe(), FakeMbBrowser(), dest_root=str(tmp_path)) + rows = _unmatched(conn) + assert len(rows) == 1 and rows[0][:2] == ("Bad Artist", "Bad Album (2020)") + assert rows[0][2].startswith("unreadable:") + + +def test_scan_clears_unmatched_when_album_resolves(conn, tmp_path): + _album(tmp_path, "Obscure", "Nope (1999)") + # first scan: no resolver match → recorded as unmatched + scan_library(conn, FakeResolver({}), FakeProbe(), FakeMbBrowser(), dest_root=str(tmp_path)) + assert len(_unmatched(conn)) == 1 + # second scan: now it resolves → the unmatched row is dropped + resolver = FakeResolver({ + ("Obscure", "Nope"): MBTarget( + artist="Obscure", album="Nope", year=1999, rg_mbid="rgN", artist_mbid="aN"), + }) + scan_library(conn, resolver, FakeProbe(quality_class=2), FakeMbBrowser(), dest_root=str(tmp_path)) + assert _unmatched(conn) == [] def test_scan_falls_back_to_tags_when_folder_does_not_resolve(conn, tmp_path):