Code-review polish on GetAlbumCoverageRollup's comment block:
- Records the invariant that with_art + pending + settled = total
(and that pending_no_mbid is a subset of pending, not a fourth
bucket).
- Notes that the IN ('sidecar','embedded','mbcaa') list must stay
in sync with the cover_art_source CHECK constraint in migration
0016 — without this note a future source addition could silently
undercount with_art.
FILTER-aggregate count over albums.cover_art_source and albums.mbid
returning total, with_art, pending, settled, pending_no_mbid in a
single round-trip. Sub-millisecond on realistic libraries; no index
needed for v1. pending_no_mbid is a subset of pending — the UI
tooltip uses it to show how many "pending" rows are blocked on
missing MBID and won't be moved by another scan.
Also bumps the Makefile sqlc pin from 1.27.0 to 1.31.1 to match the
version that produced all existing committed dbq/*.go files. The
prior pin was stale; running make generate against 1.27.0 silently
rolled the codegen back (cosmetic version headers on most files
plus a real codegen shape change in DeleteArtistIfEmpty).
Forward-fix from running the prior two commits in production. Backfill
on a real library exposed two further issues:
(1) gofmt -s flagged the column-aligned stubMeta methods in
mbids_test.go. Switched to the gofmt-canonical single-space form.
(2) Many albums failed to heal with SQLSTATE 23505 (unique constraint
albums_mbid_unique). Cause: the operator's library has duplicate
album rows in the DB — same MusicBrainz release, split into
multiple rows by the pre-MBID scanner because of subtle title or
artist disagreements between files. When backfill now correctly
extracts the MBID, two rows want to claim the same one and the
partial unique index rejects the second.
The conflict is correct DB behavior — we shouldn't have two rows
with the same MBID — but it's not a write failure to alarm the
operator about. Detect 23505 specifically:
- downgrade the log line from Warn to Info
- track these in a new BackfillMBIDsResult.Duplicates counter
(separate from Skipped, which retains its "no MBID in tag"
meaning)
- leave the duplicate row's mbid NULL; merging duplicates is a
separate (future) operator workflow
Same handling threaded through the scanner heal path so a regular
rescan doesn't generate the noise either.
JSON tally + admin UI gain a "Duplicates" line so the operator
can see how many duplicate rows their library carries.
Follow-up scope (separate task): a duplicate-album merge UX that
reparents the duplicate's tracks to the canonical row and deletes
the orphaned album row.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to 947f944. With the dhowden/tag/mbz extractor now correctly
matching ID3v2 TXXX frames, mbid backfill started rejecting every row
with "invalid byte sequence for encoding UTF8: 0x00 (SQLSTATE 22021)".
Cause: dhowden's readTextWithDescrFrame (id3v2frames.go:454) — the path
that decodes TXXX text — does NOT strip the trailing/embedded NUL bytes
that ID3v2.4 uses as frame terminator and multi-value separator. Unlike
readTFrame, which does (line 313). So a TXXX:MusicBrainz Album Id of
"abc-123\x00" comes back to us verbatim, and Postgres refuses to store
NULs in text columns.
cleanMBID splits on NUL and returns the first non-empty segment, which
also handles the multi-value case (collaboration artist IDs are
NUL-separated; Minstrel uses the primary for MBCAA, so first wins).
Tests cover the trailing-NUL, multi-value, and all-NUL paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The hand-rolled extractor in internal/library/mbids.go looked up keys that
dhowden/tag never produces, for every supported audio format:
Vorbis (FLAC/OGG): dhowden lowercases keys at parse time (vorbis.go:59),
we looked up MUSICBRAINZ_ALBUMID. Always missed.
ID3v2 (MP3): dhowden stores TXXX frames under bare TXXX/TXXX_N keys
with *tag.Comm values; the Picard tag name is on the value's
Description field, not part of the key. We looked up
TXXX:MusicBrainz Album Id. Always missed.
MP4 (M4A): dhowden stores freeform iTunes atoms under their bare
sub-name. We looked up ----:com.apple.iTunes:MusicBrainz Album Id.
Always missed.
Net effect: every album in the m7-380 boot backfill reported
processed=N, healed=0, skipped=N. The m7-379 enricher gate then
short-circuited NULL on every album, so MBCAA was never queried.
Symptom: "all albums skipped" during cover enrichment.
The pre-existing unit tests passed because they hand-built Raw() maps
with the intended-but-incorrect keys, never round-tripping through
dhowden's parser.
Replace the hand-rolled extractor with dhowden's own mbz sub-package,
which knows the per-format Raw() conventions and uses lowercase
canonical keys (mbz.Album, mbz.Artist). Update both call sites to pass
the full tag.Metadata instead of just Raw(). Rewrite the test with a
stubMeta implementing tag.Metadata, asserting against the actual
per-format Raw() shape, plus a regression guard that pins "uppercase
Vorbis keys must not match" so the v0 bug can't sneak back.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds getScanStatus/triggerScan helpers, createScanStatusQuery factory
(3s poll), qk.scanStatus(), and a Library Scan section on the admin
overview page with per-stage tallies and inline Run scan button.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds GET /api/admin/scan/status and POST /api/admin/scan/run under the
existing RequireAdmin middleware block; plumbs *library.Scanner and
library.RunScanConfig through api.Mount, server.New, and main.go so the
manual trigger reuses the same RunScan orchestrator as startup scans.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds internal/library/scanrun.go with RunScan, which creates a scan_runs
row and sequences file-walk → MBID backfill → cover enrich, persisting
per-stage tallies as jsonb. Extends coverart.EnrichBatch to return
(processed, succeeded, failed, err) so the orchestrator can classify
outcomes. Replaces main.go's two separate boot goroutines with a single
RunScan call; passes nil scanner in the no-startup-scan branch.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a boot-time goroutine that walks albums with NULL mbid, re-reads
tags from one track per album via dhowden/tag, and persists album + artist
MBIDs. Healed albums also get their cover_art_source='none' cleared so the
enricher's next batch retries the MBCAA fetch. Caps at 5000 albums per
boot to avoid stalling startup on huge libraries.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- gofmt -s on system.go, system_cron.go, api.go
- rename unused r → _ in 3 fetcher_test.go HTTP handlers
- TrackRow +queue test uses /add .* to queue/i (track-aware aria-label from #377)
- /library/artists page test mocks likes + tanstack-query (ArtistCard now embeds LikeButton)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds cover_art_source to AlbumRef, two admin API helpers (refetchAlbumCover,
refetchMissingCovers), a per-album retry button gated on is_admin in the album
detail page, and a bulk-refetch section in the admin overview.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bvandeusen