Two improvements to the system playlist builder:
1. Per-artist (<=3) and per-album (<=2) caps applied to the
pickTopN truncation step, using the same numeric caps Discover
already enforces. Both For-You and Songs-like-X benefit. Same
skewed candidate pool no longer collapses to "10 tracks from
the same artist" — the playlist always carries at least 9
distinct artists in 25 slots.
2. New pickHeadAndTail function for For-You: 20 top-similarity
tracks + 5 sampled from the tail (positions 2*headN onward of
the score-sorted, cap-applied pool). Tail sampling uses
tieBreakHash for daily determinism — same user same day still
sees the same playlist, but the daily refresh feels less
stuck-in-a-rut. Tail tracks are still similarity-related
(they passed the similarity candidate filter) so the user
should enjoy them, just from artists they wouldn't have surfaced
via strict top-N ranking.
Songs-like-X keeps the simple pickTopN call — the seed-artist
context already provides the "you'll like this" framing without
needing a tail injection.
Refactors pickTopN internals: now sorts candidates first via
scoreAndSortCandidates, applies the cap on []Candidate via
capCandidatesByAlbumAndArtist, and truncates. Removes the now-
dead stableSortByScoreThenHash helper (only used in old pickTopN).
The cap helper mirrors capByAlbumAndArtist in discover.go but
operates on recommendation.Candidate so it sees Track.AlbumID /
Track.ArtistID directly.
Tests cover the cap helper truth table, head+tail split with
small/large pools, buffer-zone exclusion, daily determinism, and
cross-day tail variance.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The collage builder pulled exactly 4 tracks (in playlist position
order) and rendered each track's album cover into a 2x2 cell. A
playlist where the first 4 tracks share an album rendered as four
identical cells of the same cover; a "Songs like X" mix where two
similar artists dominated produced very similar collages.
Now pulls 50 rows from the same query and drops adjacent /
duplicate covers in Go before passing to the renderer. NULL and
empty cover_art_paths are NOT deduped (they each render the
fallback glyph in their own cell — the right behavior for a
partially-covered playlist).
Preserves playlist position order: the first occurrence of each
unique cover wins. Five unit tests cover the behavior: drops
duplicates, keeps NULLs, handles empty-string paths, preserves
order, mixed null/dup case.
Tangential to the For-You CTA slice but lands here as a small
quality-of-life fix that benefits every playlist's collage.
Operator can refresh their Discover playlist without waiting for
the next daily cron tick. Two surfaces, both gated on
system_variant === 'discover':
- Detail page header: "Refresh" button next to the cover art / meta.
- Home Playlists row tile kebab: "Refresh Discover" menu item.
Both call POST /api/playlists/system/discover/refresh, show a
confirmation toast, and invalidate the playlist + playlists queries
so the new content lands without a manual page reload.
Extends Playlist.system_variant union with 'discover'. Adds
refreshDiscover() typed client. Tests cover client behaviour
(success + empty-library) plus conditional rendering on both
surfaces.
POST /api/playlists/system/discover/refresh re-runs the system
playlist build synchronously for the calling user, then returns
their newly-built Discover playlist's UUID and track count.
Used by the frontend's "Refresh" affordances (next task) on the
Discover detail page header and the home Playlists row tile kebab.
Operator's escape hatch when they want a different randomness
without waiting for tomorrow's cron tick (e.g., after pasting a
Last.fm key, after the daily cover-art enrichment expanded their
playable library, or just for the heck of it).
Authenticated user only; the authed sub-router's RequireUser
middleware handles 401. Each user refreshes only their own
Discover — no admin route, no cross-user impersonation.
Adds GetSystemPlaylistByVariantForUser sqlc query for the response
lookup. Returns playlist_id=null and track_count=0 if the build
succeeded but the user's library yielded no eligible tracks
(degenerate empty-library).
Adds the third system playlist variant: 'discover'. Surfaces 100
tracks the operator has not played and not liked, biased toward
artists they rarely play (< 10 plays) and complemented by tracks
liked by other users plus a random unheard sample. Cold start
collapses to all-random; single-user servers redistribute the
cross-user-likes allocation equally across the other two buckets.
The build runs as a fourth phase inside BuildSystemPlaylists
alongside For You and the seed-artist mixes. Same daily-deterministic
md5(track_id || dateStr) ordering as the other variants. Per-album
(<=2) and per-artist (<=3) caps prevent collapse onto a single
prolific artist. Buckets interleave round-robin so the playlist
order doesn't front-load one bucket's flavour.
Post-commit collage generation (already wired) gives Discover the
same 4-cell cover treatment as the other system playlists — no
new collage code needed.
Tests cover the slot-redistribution table (all-available,
cold-start, single-user, partial-dormant, fully-empty), the
per-album/per-artist caps, the round-robin interleave, and a
DB-backed cold-start integration test that asserts a Discover
playlist lands with non-zero tracks.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds 'discover' as an accepted system_variant on the playlists
table — alongside 'for_you' and 'songs_like_artist' — and three
sqlc bucket queries that back the new playlist's daily candidate
selection.
The three buckets surface tracks the user hasn't played and
hasn't liked:
- Dormant artists: artists this user has played < 10 times total.
Surfaces the long tail of their library.
- Cross-user likes: tracks any OTHER user has liked but this one
hasn't engaged with. Empty on single-user servers; the Go-side
allocator redistributes the deficit across the other two
buckets.
- Random unheard: pure random sample as the safety net and the
cold-start fallback.
All three share exclusion filters: not-played by this user, not
liked by this user, not in lidarr_quarantine for this user. All
order by md5(track_id || dateStr) for daily determinism — same
pattern as the existing tieBreakHash logic in system.go.
LIMIT values are generous (80/60/200) so the per-album (<=2) /
per-artist (<=3) caps and slot redistribution in T2 have headroom.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
System-generated playlists ("For You", "Songs like X") rendered with
empty placeholder boxes in the UI even when their contributing tracks
had album art. The build path inserted playlist rows directly without
ever invoking the existing playlists.GenerateCollage machinery —
that was only wired into user-edited playlist mutations.
After this change, BuildSystemPlaylists generates a 4-cell collage
for every system playlist it creates, post-commit. Same cover
mechanism user playlists use, same on-disk layout
(<DataDir>/playlist_covers/<id>.jpg). Cells whose source album lacks
art fall back to the FabledSword glyph, so a partially-covered
library still produces a sensible visual.
Threading: BuildSystemPlaylists, StartSystemPlaylistCron, and the
api lazy-build path all gain a dataDir string parameter; main.go
passes cfg.Storage.DataDir.
Drops the now-redundant PickTopAlbumCoverForArtistByUser lookup —
the collage is more visually informative than a single album cover
copy, and the seed-artist's top album is one of the contributing
tracks in the mix anyway, so it'll appear as one of the four cells.
Tests pass t.TempDir() for the dataDir parameter.
Adds an embedded-art layer to the album cover chain between the
sidecar lookup and the external provider chain. Reads ID3v2 APIC,
FLAC METADATA_BLOCK_PICTURE, and MP4 covr atoms via the dhowden/tag
library (already a dependency for MBID extraction). Whatever Picard
or Lidarr embedded into the file at tag time becomes available
without any network call.
Diagnostic context: file managers (GNOME Files, Dolphin, etc.)
generate audio-file thumbnails by reading exactly this picture
block. An album that "has no art" in Minstrel but renders a
thumbnail in the file manager is the smoking-gun signal that
embedded art is present and we were ignoring it. Adds 'embedded'
to the chain so those albums get covered without any external
provider hit.
Chain after this change:
sidecar → embedded → MBCAA → TheAudioDB → Deezer → Last.fm → none
Embedded beats every external provider for accuracy because it's
the canonical art the operator/Picard chose at tag time, not a
guess from a remote catalog.
Refactors the post-fetch write logic into Enricher.writeAlbumArt
so the embedded layer and the provider chain share the same
sidecar-then-managed-cache fallback semantics. Behavior identical
to before for the provider path.
POST /api/admin/cover-sources/research bumps cover_art_sources_meta.
current_version unconditionally; the next enrichment pass re-eligibles
every 'none' row. Existing positively-sourced rows are not affected —
the version-mismatch eligibility rule only kicks in for 'none'.
Admin Cover Art panel gains a "Re-search missing art" button that
calls the endpoint and shows a confirmation toast. Operator's escape
hatch when:
- They've added a Last.fm key and want to retry failed rows
immediately rather than waiting for the next scheduled scan.
- An upstream provider's catalog has updated (e.g. Deezer added a
back-catalog import).
- They want to verify a fix end-to-end before the next scheduled
scan fires.
SettingsService.BumpVersion (the unconditional version of T5's
BumpVersionIfProvidersChanged) is the single-call DB writer; both
endpoints route through it.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On boot, the SettingsService computes a SHA-256 hash of the sorted
registered-provider IDs and compares to the hash stored on
cover_art_sources_meta. Mismatch → atomic bump of current_version +
update of stored hash. The next enrichment pass picks up the new
version, all 'none' rows become eligible, and they get retried
against the new chain (now including Deezer / Last.fm).
One-shot: subsequent boots without a provider-set change don't bump
again. The check is best-effort — failures are logged at Warn but
don't refuse startup; a temporarily unreachable DB at boot just
means 'none' rows stay parked until a manual re-search (next task).
Tests cover first-boot (empty stored hash → bump), repeat-boot
(no change → no bump), and the registered-providers-hash
determinism.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds Last.fm as a hybrid MBID-or-name provider in the artist + album
chains. Default-disabled; activates when the operator pastes a free
API key into the admin Cover Art panel. Rate-limited to 4 req/s under
their 5/s per-key ceiling.
Prefers MBID lookup when present (artist.getInfo accepts &mbid=),
falls back to artist-name search. For albums, requires both
ArtistName and AlbumTitle; MBID is sent as an additional hint.
Includes placeholder-image detection: Last.fm returns a generic
"default star" image hash for many artists rather than 404'ing.
Provider parses the response URL's basename hash and treats known
placeholders as ErrNotFound so we never persist a placeholder thumb
as real art. Fail-open: when in doubt, return ErrNotFound rather than
risk persisting wrong art.
When API key is empty, every call returns ErrNotFound regardless of
the enabled flag — defensive so an operator who toggles enabled
without setting a key doesn't see confusing transient errors.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds Deezer as a name-based provider in the artist + album chains.
Public read API, no API key required, default-on. Rate-limited to
5 req/s (well under their 50/5s per-IP ceiling).
Includes exact-name match guard against false positives: if the top
search result's artist/album name doesn't case-insensitive-match
what we sent, returns ErrNotFound rather than persisting the wrong
art. This is critical for one-word artist names (e.g. "Time")
where Deezer's relevance ranking can't distinguish.
Provides the long-promised path for the 53 MBID-less artist rows in
the dev DB (featured/collab artists from track-tag splits) which
TheAudioDB cannot reach.
For artists, Deezer returns a single image which we use as thumb;
fanart stays nil. The enricher's persistence layer already handles
thumb-only correctly.
Changes the AlbumCoverProvider and ArtistArtProvider interfaces from
MBID-only string parameters to ref-struct signatures so name-based
providers (Deezer, Last.fm in upcoming tasks) can act on rows
without MBIDs.
Today, 53 of 213 artists in the dev DB have NULL MBIDs — featured
artists, remixers, and compilation contributors created from
track-tag splits where the primary-artist MBID is in the file but
collaborator IDs aren't. These rows are unreachable by the current
MBID-only chain. Refactoring the interface unblocks future
name-based providers from acting on them.
TheAudioDB and MBCAA keep MBID-only behavior internally: they return
ErrNotFound when ref.MBID is empty rather than attempting a
name-based fallback. The MBID guard inside EnrichAlbum/EnrichArtist
is removed; providers receive the ref unconditionally and decide
whether they can act on it.
GetAlbumWithFirstTrackPath now JOINs artists to return artist_name
alongside the existing fields, supporting AlbumRef construction.
No behavioral change today (only TheAudioDB + MBCAA registered, both
keep MBID-only behavior). The change unblocks T3 (Deezer) and T4
(Last.fm) which can now register and act on every artist/album
regardless of MBID presence.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Relaxes the artists/albums art_source CHECK constraints to allow the
new 'deezer' and 'lastfm' source values alongside the existing
accepted set. Adds cover_art_sources_meta.last_registered_providers_hash
for the boot-time auto-bump detection: when the registered-provider
set changes between deploys, we bump current_version once so 'none'
rows become eligible for retry against the new chain.
Two sqlc queries added: GetCoverArtProvidersHash reads the stored
hash; BumpVersionAndSetProvidersHash atomically increments the
version and stores a new hash, returning the new version. Both used
by the boot logic in a later task.
The admin tab strip (Overview / Integrations / Requests / Quarantine)
scrolled out of view as the operator paged through long sections like
Library Scan, Cover Art, etc. Pin it to the top of the <main>
scroll container with sticky positioning, an opaque background to
hide content scrolling behind it, and z-10 to layer above section
content.
The existing processed/succeeded/failed tally written to scan_runs
collapses too much to debug "0 successes" symptoms — failed includes
"settled to none (correct)" alongside "left NULL (transient/IO error,
will retry)" alongside "skipped (no MBID)". An operator looking at
{"failed": 144, "succeeded": 0} can't tell whether the enricher is
broken, the upstream is broken, the data is missing MBIDs, or the
artists genuinely aren't in TheAudioDB.
Adds a single end-of-batch Info log per stage with the breakdown:
- eligible: rows returned by the SQL filter
- processed: rows the loop actually touched
- succeeded: art attached (provider success)
- settled_none: tried, no provider had art (legit miss)
- no_mbid: skipped inside the enricher because MBID was NULL
(artist stage only)
- left_null: tried, transient/IO failure, will retry next pass
- errored: internal error during the entry (logged separately)
The JSON written to scan_runs keeps its existing shape so the admin
UI's stage badges aren't disturbed.
When the album sidecar location isn't writable (e.g. read-only music
mount in a container), the album-cover write previously failed and
left cover_art_source NULL forever — every scan retried the same
provider call and re-failed at the same os.WriteFile. With a managed
data_dir available, we fall back to <DataDir>/album-art/<album_id>/
cover.jpg and record that path on the album row. The cover-serving
HTTP path already serves whatever cover_art_path stores, so the
client sees the art uniformly regardless of where it landed.
DataDir is opt-in via the Enricher field (cmd/minstrel/main.go sets
it from cfg.Storage.DataDir on the production path; tests leave it
empty and behave as before).
The Dockerfile created the minstrel user but never set a WORKDIR or
pre-created a writable data dir. With the default DataDir of "./data"
and CWD of "/", every MkdirAll attempt failed with "permission denied"
under the non-root user, and the boot code only logged a Warn before
continuing. Result: every artist-art enrichment failed at
"mkdir artist-art: mkdir data: permission denied"; the operator saw
"0 successes" with no signal as to why.
Three changes:
- Dockerfile: WORKDIR /app, pre-create /app/data with minstrel
ownership, and ENV MINSTREL_STORAGE_DATA_DIR=/app/data so the
binary doesn't fall back to the broken "./data" default even if
the operator forgets to set it.
- docker-compose.yml: explicit MINSTREL_STORAGE_DATA_DIR + a named
volume mounted at /app/data so cached art persists across
container recreates.
- cmd/minstrel/main.go: MkdirAll failure is now fatal. Silent breakage
here cascades into every cache (playlist covers, artist art,
album-cover fallbacks); refusing to start surfaces the misconfig
immediately. Also wires Enricher.DataDir from cfg in preparation
for the album-cover sidecar fallback (next commit).
revive flagged the if/else where the else branch was a return — invert
the condition and return the non-stale skip path early so the reap
flow drops one indent level.
The PATCH-daily test parsed the wire RFC3339 timestamp and asserted
.UTC().Hour() == 3, but the handler computes NextFire against
time.Now() in the host zone before serializing as UTC — so non-UTC
runners would see a non-3 UTC hour. Switch the assertion to
.Local().Hour() to match the handler's computation regardless of
the runner's TZ.
Adds a "Scan schedule" section to the admin overview page between
the existing Library Scan and Cover Art sections. Mode picker
(Off / Every N hours / Daily / Weekly) with conditional interval /
time / day inputs. Save button disabled until local state diverges
from the server state. "Next scan: in N hours (timestamp)"
inline indicator when mode != off.
formatRelative is a static helper (recomputes on render); operator
sees the configuration confirmation at a glance — not a countdown
timer.
admin.test.ts mock extended with createScanScheduleQuery returning
mode='off' default. New test asserts the section renders. Existing
assertions still pass.
Adds ScanSchedule type, getScanSchedule / updateScanSchedule
against the new admin endpoints, createScanScheduleQuery (60s
staleTime; refetches on focus + after PATCH invalidation — no
polling).
New qk.scanSchedule() key follows the existing tuple pattern.
ScanScheduleUpdate uses optional fields so mode=off updates can
omit per-mode fields entirely (server normalizes anyway).
Vitest covers the GET path, PATCH with body, and the mode=off
minimal-body case.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two new endpoints under existing RequireAdmin middleware:
- GET /api/admin/scan/schedule — returns config + computed
next_scheduled_at (ISO 8601; null when mode='off')
- PATCH /api/admin/scan/schedule — validates per-mode required
fields, normalizes mode='off' to NULL the per-mode fields,
writes via UpdateScanSchedule, calls scheduler.Refresh(), returns
the updated record.
server.New + api.Mount gain a *library.Scheduler parameter; handlers
struct + Server struct gain the scheduler field. Test stubs
(server_test.go, library_test.go's Mount call) updated to pass nil.
Tests gated on MINSTREL_TEST_DATABASE_URL: GET default is mode=off
with null next; PATCH daily produces a 03:00 next-fire; PATCH
weekly without weekly_day → 400; PATCH off normalizes lingering
per-mode fields to NULL; non-admin → 403.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds ScheduleConfig + NextFire (per-mode logic for off/interval/
daily/weekly) and the Scheduler struct + loop goroutine. The loop
reads scan_schedule on boot and on Refresh signals, computes
next-fire, sleeps via time.Timer, and calls TryStartScan when the
timer fires. Skip-on-conflict logging happens inside tryFire when
TryStartScan reports started=false with a non-stale in-flight row.
Boot wiring: cmd/minstrel/main.go constructs the scheduler after
coverEnricher and calls Start with the server's long-lived ctx.
The scheduler is not yet plumbed into the HTTP handlers (the next
task does that — handlers gain a scheduler field, admin schedule
endpoints land).
Tests cover the NextFire truth table (off / interval / daily today
+ tomorrow / weekly today-before-time + today-after-time + upcoming
day) plus invalid configs and parseHHMM edge cases. Refresh is
verified non-blocking via the buffered-channel + default pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extracts the in-flight check + scan-start logic from handleTriggerScan
into a shared library.TryStartScan helper used by both manual and
(future) scheduled paths. Folds in the lazy 1-hour stuck-scan
reaper: in-flight rows older than StuckScanThreshold get
FinishScanRun-ed with error_message="reaped (stale)" and the new
scan proceeds.
handleTriggerScan becomes a thin wrapper preserving the same
external HTTP behavior (202/409/500). Operators no longer need to
manually clear stuck rows after a server crash — the next manual
trigger reaps it.
Tests gated on MINSTREL_TEST_DATABASE_URL: no-in-flight starts;
fresh-in-flight skips with row pointer; stale-in-flight reaps
(verified via finished_at + error_message); DB error propagates.
Adds the singleton scan_schedule table holding the operator's
recurring-scan config. mode is the discriminator (off/interval/
daily/weekly); CHECK constraints enforce per-mode field validity
(interval_hours required for interval; time_of_day for daily and
weekly; weekly_day 1..7 for weekly). Seed row is mode='off'.
Two new queries: GetScanSchedule (read singleton; used by scheduler
boot + admin GET) and UpdateScanSchedule (admin PATCH; application
normalizes per-mode fields to NULL when mode='off').
B-T2 changed Scanner.Scan to take a progressCb parameter, but the
ScanTrigger interface in internal/server/server.go was missed. The
interface gains the same progressCb arg; the HTTP handler at
handleAdminScan passes nil (fire-and-forget triggers don't observe
partial tallies). The stubScanner in server_test.go gets the new
arg too.
Each of the 4 stage cards in the admin Library Scan section gains a
StageBadge in its header. The badge derives state via stageState()
from the existing ScanStatus payload — no new query, no schema
change. Active stage shows spinner + "Processing" in moss green;
done shows check + "Done" in moss green; pending and skipped show
muted text.
Card body now shows either the (possibly partial) numbers OR a
"Waiting for previous stage…" placeholder when the badge is
pending. The previous bottom-of-card "Pending…" / "Skipped."
fallback text disappears (the badge replaces it).
StageBadge.test.ts covers the four state renderings.
admin.test.ts adds one new case asserting the Processing badge
shows when scan.library is populated and in_flight: true. Existing
admin.test.ts assertions still pass (the default mock data has no
stage tallies + in_flight: false → all stages skipped, no Loader/
Check icons rendered).
Pure-function module that derives per-stage StageState (one of
pending / in_progress / done / skipped) from the existing ScanStatus
payload. No new query, no new endpoint — reuses what
createScanStatusQuery already polls.
Truth table covered by tests: scan undefined → skipped; in-flight
with no tallies → all pending; in-flight with library tally only →
library in_progress, rest pending; in-flight with library +
mbid_backfill → library done, mbid in_progress; finished with all
tallies → all done; finished with missing library tally → library
skipped, others done; in-flight with last stage tally → only last
stage in_progress (all earlier done).
Each of the 4 stage workers (library walk, MBID backfill, cover
enrich, artist art enrich) gains a progressCb parameter that
receives a snapshot of the current running state per item processed.
The orchestrator stores the snapshot in a local var that the
publisher's marshal closure reads at flush time.
scanrun.go's 4 stage blocks are rewritten to construct one publisher
per stage, pass a Tick-firing closure to the worker as progressCb,
and call Flush() after the worker returns (routing any persist error
to captureErr — preserves today's diagnostic behavior).
The previous post-stage UpdateScanRun* calls are removed; the
publisher's Flush handles the final write. Cadence: every 500 items
OR every 1s, whichever fires first.
Outside-orchestrator callers (scanner_test.go, enricher_test.go)
pass nil for progressCb — no-op behavior preserved for tests that
don't need progress observability.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Persists a stage tally to scan_runs at a bounded cadence — every
flushEvery items OR every flushPeriod elapsed, whichever fires
first. Caller supplies marshal + persist closures so each stage owns
its own tally type. Tick swallows mid-stage persist errors silently
(observability bug, not scan failure); Flush propagates errors so
the orchestrator's captureErr can surface them.
Tests cover: count-based flush, time-based flush, Flush always
persists, Tick swallows errors and keeps firing, Flush propagates
errors, counters reset after each flush.
Sweep round 4: 5 more httptest handlers in provider_theaudiodb_test.go
where r is unused. Lint flagged 3 (lines 89/103/118); also fixing
2 more (TestConnection_SuccessOnEmptyAlbum, FailureOn401) that lint
would catch on the next run. The 4 handlers that DO use r.URL.Path
or r.Host are left as-is.
revive's unused-parameter rule flagged 7 test handler/method args:
- provider_test.go: fakeProvider.Configure(s) and
fakeAlbumProvider.FetchAlbumCover(ctx, mbid) — interface
conformance dummies that don't use their args.
- provider_mbcaa_test.go: 4 httptest handlers that ignore one or
both of (w, r).
- provider_theaudiodb_test.go: the disabled-provider negative
test's httptest handler.
All 7 renamed to _ per revive's convention.
T11 added *coverart.SettingsService to Mount() between the
coverArtBackfillCap (int) and the *library.Scanner pointer. The
TestRoutesRegisteredInMount call site in library_test.go missed
the new arg. Insert h.coverSettings at the right position.
Three issues from the previous push:
1. internal/api/admin_covers_test.go used t.Context() (Go 1.24+) but
go.mod declares go 1.23.0. Replace with context.Background() in
the testHandlersWithCovers helper; add "context" import.
2. internal/server/server_test.go's New() call missed the
*coverart.SettingsService argument added in T11. The new param
sits between the cover-art-backfill cap (int) and the *library.Scanner
pointer in the function signature; the test was using 12 args
instead of 13. Insert nil at the right position in all six call sites.
3. integrations.test.ts had 11 failing tests after T13 added the
cover-art-providers section. Both panels render "Save changes",
"Test connection", and "API key" controls, so the existing
Lidarr tests' role/text queries matched multiple elements. Add
helper functions (lidarrSection, coverProvidersSection,
providerCard) and scope every relevant query with within() so
each panel's tests interact only with their own controls.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three failures from the prior push:
1. internal/api/library.go and 3 other callers of q.GetArtistByID
broke because T2 modified the query to use an explicit column
list, making sqlc generate GetArtistByIDRow instead of returning
dbq.Artist. dbq.Artist already has every column added by
migration 0018 (artist_thumb_path / artist_fanart_path /
artist_art_source / artist_art_sources_version), so the explicit
column list was unnecessary work. Revert the SELECT to SELECT *,
regenerate sqlc, GetArtistByIDRow disappears, all callers compile
again.
2. internal/tracks/service_test.go missed the dataDir parameter
added to NewService in T9. Add "" as the 4th arg to every test
call site (tests don't exercise the artist-art cleanup path; the
empty dataDir is fine).
3. integrations.test.ts:356's mock.calls.filter callback had a
too-narrow type annotation that svelte-check rejected. Relax to
any[] to match what mock.calls actually is.
Adds a "Cover art providers" section to the existing /admin/integrations
page with one card per registered provider (currently MBCAA + TheAudioDB).
Each card has:
- Provider name + capability badges (album cover · artist thumb · artist fanart)
- Status dot (green if enabled; muted if disabled)
- Enabled toggle + API key text field (only when requires_api_key)
- Save button (explicit save matches existing Lidarr card pattern)
- Test connection button (when testable) with inline ok/fail indicator
- Footer help line for TheAudioDB linking to the free key application
Save invalidates qk.coverProviders() always and qk.coverage() when
the PATCH response indicates version_bumped: true (so the coverage
gauge re-queries to show settled→with_art transitions on the next
scan).
API key field uses type=password with a placeholder that clarifies
the test-key fallback behaviour — never echoes back the stored key
(api_key_set bool from the GET response controls the "✓ Set"
indicator).
Local edit state is keyed by provider_id and survives query refetches
so in-flight edits aren't clobbered.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds CoverProvider type + capability union, getCoverProviders /
updateCoverProvider / testCoverProvider against the three new admin
endpoints, plus createCoverProvidersQuery (60s staleTime; refetches
on focus + after mutations rather than polling — settings rarely
change in operator time).
New api.patch helper added to client.ts (the existing helpers were
get/post/put/del; PATCH was missing).
New qk.coverProviders() key follows the existing tuple pattern.
api_key_set: bool replaces echoing the credential. UpdateCoverProviderPatch
uses optional fields so the operator can change just enabled, just
key, or both atomically.
Vitest covers GET path, PATCH path with body, version_bumped
response, POST /test path with empty body, ok=false error response.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three endpoints for the admin Settings UI:
- GET /api/admin/cover-sources — lists registered providers with
current settings + capability badges + sources_version
- PATCH /api/admin/cover-sources/{provider_id} — updates enabled
and/or api_key; returns version_bumped: true only when the
enabled set changed
- POST /api/admin/cover-sources/{provider_id}/test — invokes
TestableProvider.TestConnection; returns ok:bool + duration_ms +
error string. Returns 200 in both success and failure cases (the
operation completed; the test result is data, not an error).
api_key is never echoed in GET — replaced with api_key_set: bool
to follow the standard credential-display convention. PATCH
api_key: "" clears; omitting the field leaves it unchanged. PATCH
on an unknown provider_id returns 404.
All under existing RequireAdmin middleware. Routes registered
alongside the library/coverage endpoint in the /api/admin/ tree.
handlers struct gains a coverSettings *coverart.SettingsService
field; server.New + cmd/minstrel/main.go plumb the existing
coverSettings instance through.
coverart.ResetRegistryForTests() exported wrapper added so api
package tests can reset the registry without importing internals.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an artist-art-enrich stage after cover-enrich in RunScan. The
stage drains EnrichArtistBatch with a configurable cap and the
operator's data_dir, persists processed/succeeded/failed tallies to
the scan_runs.artist_art_enrich jsonb column added in migration
0018.
The admin /admin Library Scan card grid expands from 3 to 4 columns
with the new "Artist art" card mirroring the cover-enrichment one
(Processed / Succeeded / Failed). The TS ScanStatus type gains an
optional artist_art_enrich field. The Go scanStatusResp gains the
matching ArtistArtEnrich json.RawMessage field.
cmd/minstrel/main.go (and admin scan-trigger handler) wire the new
RunScanConfig fields: ArtistEnrichCap reuses cfg.Library.CoverArtBackfillCap
for now (can be split if separate budgets are useful), DataDir is
cfg.Storage.DataDir.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Parallels EnrichAlbum but with no sidecar layer (artists have no
per-artist directory in a music library). Iterates
EnabledArtistProviders, writes thumb.jpg + fanart.jpg to
<data_dir>/artist-art/<artist_id>/, stamps artist_art_source +
artist_art_sources_version atomically.
Atomicity: provider returns whichever images it has — partial
success (thumb-only or fanart-only) is persisted with whichever
paths landed, source still stamped. ErrNotFound from every enabled
provider settles the row to 'none' with the current version stamp.
Any ErrTransient leaves the row NULL for next-pass retry.
CleanupArtistArt removes <data_dir>/artist-art/<artist_id>/ on
artist delete; idempotent on missing dir. Hooked into the artist-
delete cascade in tracks/service.go after the transaction commits;
non-fatal on failure (logs but doesn't fail the delete).
EnrichArtistBatch drains rows from ListArtistsMissingArt; the
orchestrator's 4th stage (added in T10) calls this. Tests cover
the eligibility table, atomicity (thumb-only / fanart-only),
all-404-settles, ErrTransient-leaves-NULL, filesystem layout, and
CleanupArtistArt idempotency.
tracks.Service gains a dataDir field; tracks.NewService takes it
as a new parameter. All three call sites updated (server/server.go,
api/auth_test.go, api/admin_tracks_test.go). The dataDir flows from
cfg.Storage.DataDir → server.New → s.DataDir → tracks.NewService
without any change to cmd/minstrel/main.go.
Rewires EnrichAlbum to iterate EnabledAlbumProviders from the
SettingsService instead of a single hardcoded MBCAA fetcher. Sidecar
layer is unchanged (always tried first). Per-row eligibility check
honors cover_art_sources_version: terminal 'found' values skip;
'none' flips to NULL via ClearAlbumCoverNone and proceeds (the DB-
level ListAlbumsMissingCover query guards the version check for batch
callers; RetryAlbum clears via ClearAlbumCover first); NULL is eligible.
The provider chain uses an inline allWere404 accumulator to settle
the row to 'none' only when every provider returned ErrNotFound.
Any provider returning ErrTransient leaves the row NULL for next-
pass retry — even if other providers returned ErrNotFound — since
the transient call's MBID might succeed on a future attempt.
Boot wiring (cmd/minstrel/main.go) replaces the
NewFetcher+NewEnricher(fetcher, mbcaaOn) shape with
NewMBCAAProviderFromConfig (swaps in production User-Agent on the
registered provider) + NewSettingsService + NewEnricher(settings).
The old cfg.Library.CoverArtFromMBCAA flag is no-op'd; DB-backed
settings replace it. Field stays in the config struct for backward
compat with deployed config files.
Consolidates fetcher.go's HTTP logic into provider_mbcaa.go (no more
wrapper indirection): mbcaaProvider now holds cfg/mu/lastCall/client
directly. Deletes fetcher.go and fetcher_test.go. ErrNotFound and
ErrTransient move into provider.go alongside the other sentinels.
Updates admin_covers_test.go and provider_mbcaa_test.go to the new
constructor shapes. Adds multi-provider-chain, transient-leaves-null,
and stale-none-flips-and-retries test cases in enricher_test.go.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reconciles the compile-time provider registry with the
cover_art_provider_settings table at boot, and on every admin
update. Calls Configure() on each provider with the row data so
runtime config (enabled, api_key) propagates immediately.
Owns the cover_art_sources_meta.current_version stamp.
UpdateProvider returns versionBumped=true only when the change
altered the *enabled set* of providers — key rotations don't bump
version (the recheck trigger is for "we have a new source to ask,"
not config rotations).
Capability filtering: EnabledAlbumProviders / EnabledArtistProviders
return only registered providers that (a) are enabled per DB and
(b) implement the requested capability interface. Snapshots —
callers don't mutate.
ListProviderInfo builds the wire-shaped slice the admin GET handler
returns. APIKeySet bool replaces echoing the key (credential never
leaves the server).
TestProvider dispatches to TestableProvider.TestConnection on the
named provider, ErrNotTestable if the provider doesn't implement
the capability, ErrProviderNotFound if not registered.
UpdateProviderSettingsParams uses the sqlc-generated field names:
Enabled bool (not *bool — the COALESCE is handled by passing the
cached current value when patch.Enabled is nil), Column3 bool
(apiKeyChanged flag), ApiKey *string (the api_key value — sqlc
named this from the column, not Column4 as the task spec assumed).
Tests gated on MINSTREL_TEST_DATABASE_URL: boot-inserts-defaults,
flip-enabled-bumps-version, key-only-doesn't-bump, clear-key,
TestProvider routing for testable / non-testable / missing,
ListProviderInfo capability badges. Reuses newPool / discardLogger
and fakeProvider / fakeAlbumProvider from enricher_test.go and
provider_test.go (same package).
New album-cover and artist-art source implementing
AlbumCoverProvider, ArtistArtProvider, and TestableProvider. Hits
the documented /album-mb.php and /artist-mb.php JSON endpoints by
MBID, then GETs the returned image URLs (CDN, key-less). 2 req/sec
rate limit per the upstream's documented free/test-key tier;
mutex+lastCall serialises ALL HTTP calls regardless of method so
JSON metadata fetches and image GETs share the same budget.
429 / 5xx triggers exponential backoff with jitter (250ms × 2^attempt
± 20%) up to 3 retries before surfacing ErrTransient. 401 / 403
surface as ErrTransient (auth issues are config errors, not "this
album has no art" — the row stays NULL across passes so the operator
sees pending count not decreasing as the diagnostic signal).
Artist art is atomic at the JSON metadata step (one round-trip
yields both URLs); the two image GETs are independent — partial
success returns whatever bytes landed and ErrNotFound only if
neither URL is present in the response.
Configure() falls back to the documented public test key (2) when
the operator's APIKey is empty, per the no-coercive-settings
principle. Test connection hits a stable popular release MBID
(Pink Floyd, "The Dark Side of the Moon").
theAudioDBBaseURL is a var (not const) so tests can override it.
Adapts the existing Fetcher-based MBCAA client to the new Provider
abstraction. mbcaaProvider embeds *Fetcher and implements
Provider + AlbumCoverProvider + TestableProvider. init() registers a
default-config instance; NewMBCAAProviderFromConfig swaps in
production config (User-Agent, MinPeriod) at boot.
Wrapper approach (rather than reimplementing the HTTP logic) keeps
the package compiling during the T4–T8 window: enricher.go and
main.go still reference the legacy Fetcher type / NewFetcher
constructor / Fetch method, all of which remain unchanged in
fetcher.go. A-T8 will consolidate the HTTP logic into this file and
delete fetcher.go after the enricher rewrite removes the last
legacy call sites.
TestConnection hits a hardcoded popular release MBID (Beatles
"Abbey Road" UK release) and treats both 200 and 404 as "connection
works" — MBCAA needs no auth, so only 5xx / network failures
surface. Tests cover ID/DisplayName/Capability metadata, success
path, 404→ErrNotFound, disabled-returns-ErrNotFound, Configure
toggle, TestConnection variants, and NewMBCAAProviderFromConfig
swap.
Defines the abstraction every cover-art source will implement:
- Provider — base interface (ID, DisplayName, RequiresAPIKey,
DefaultEnabled, Configure).
- AlbumCoverProvider — opt-in capability for fetching album covers
by MBID.
- ArtistArtProvider — opt-in capability for fetching artist thumb +
fanart atomically by MBID.
- TestableProvider — opt-in capability for the admin Test-Connection
button.
Sentinel ErrNotTestable / ErrProviderNotFound. ErrNotFound and
ErrTransient stay in fetcher.go for now; they move into provider.go
in the next commit when fetcher.go is deleted as part of the MBCAA
refactor.
Compile-time registry: providers register from init() in their own
file. Duplicate IDs panic at startup (compiled-in registration is a
build-time bug, not a runtime concern).
Tests cover: Register adds, duplicate panics, ProviderByID success
+ ErrProviderNotFound, capability-interface filtering via Go type
assertion.
- Extends GetAlbumCoverageRollup's with_art FILTER to include
'theaudiodb' so F's coverage gauge surfaces TheAudioDB-found rows
as with_art rather than silently undercounting.
- Modifies ListAlbumsMissingCover to take the current sources_version
as a parameter; eligibility now includes stale-'none' rows.
- Adds SetAlbumCoverWithVersion (atomic write of path + source +
version stamp) and the parallel SetArtistArtWithVersion +
ClearArtistArtNone + ListArtistsMissingArt for the artist-art
enricher.
- New coverart_settings.sql with ListProviderSettings,
UpsertProviderSettings (boot reconciliation, preserves operator
values on conflict), UpdateProviderSettings (admin PATCH; tri-
state api_key handling), GetCurrentSourcesVersion,
BumpSourcesVersion (RETURNING the new value).
Adds the schema for the pluggable cover-art provider abstraction:
- Extends albums.cover_art_source CHECK to accept 'theaudiodb' (the
constraint from migration 0016 only allowed embedded/sidecar/mbcaa/
none, which would block writes from the new provider).
- Adds artist_thumb_path / artist_fanart_path / artist_art_source
on artists, with a parallel CHECK constraint and source index.
- Adds *_sources_version stamps on both albums and artists for the
per-row recheck eligibility logic. Default 0 so every existing row
becomes stale relative to the seeded current_version=1, retrying
through the new chain on first scan after migrate.
- New cover_art_provider_settings table (per-provider enabled / api_key
/ display_order) and cover_art_sources_meta singleton holding
current_version. Seeds both v1 providers (mbcaa + theaudiodb) as
enabled.
- New artist_art_enrich jsonb column on scan_runs for the 4th
scan-orchestrator stage tally.
The seed leaves theaudiodb.api_key NULL; the application supplies the
upstream's documented test key (2) as the default when the column is
NULL, per the no-coercive-settings principle.
Adds a library-wide cover-art coverage gauge inline to the right of
the "Refetch missing covers" button. Three buckets: with_art ·
pending · settled. Native HTML title= tooltip on "pending" surfaces
the pending_no_mbid sub-count when > 0, telling the operator how
many "pending" rows are blocked on missing MBID and won't be moved
by another scan.
Run-scan and Refetch-missing handlers extend their TanStack
invalidation to also clear qk.coverage(), so the gauge ticks
immediately after the operator clicks instead of waiting up to 3s
for the next refetch interval.
flex-wrap on the row so the gauge drops below the button on narrow
viewports instead of overflowing.
The existing admin page vitest mock for $lib/api/admin gains a
createCoverageQuery stub returning data: undefined so the new
import doesn't break the existing page tests (which don't touch
the gauge).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bvandeusen