Final task of PR2 DRY pass. Extracts the verbatim
`offset + items.length >= total` math repeated across 8
createInfiniteQuery sites into a single Page<T>-shape helper.
The task spec described a bare-array helper signature
(lastPage: T[], pageSize-based stop), but no call site in this
repo uses that shape — every paged endpoint returns a Page<T>
envelope. The helper is named pageGetNextPageParam and matches
the actual canonical shape so the 8 copies could collapse.
Migrated:
- likes.ts: 3 sites (TrackRef, AlbumRef, ArtistRef)
- albums.ts: 1 site (AlbumRef)
- queries.ts: 4 sites (artists + 3 search facets)
history.ts left alone — uses has_more/total, different shape.
W-T3 (617477b) consolidated toast rendering into a single <ToastHost />
mounted in +layout.svelte, but page-level tests render components without
the layout, so screen.getByTestId('toast') and role=status queries broke.
Mount ToastHost in beforeEach (auto-cleaned by svelteTesting()) and reset
the store with clearToast() in afterEach so toasts can't leak between
tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add audit.WriteOrLog: a one-line wrapper around Write that logs at
Warn and swallows the error, matching the package contract that
audit failures must not break user-facing operations.
Migrate the 13 call sites across 7 files in internal/api/ from the
3-line "if err != nil { logger.Warn(...) }" shape to a single call.
audit.Write stays exported for tests + any future caller that
needs strict semantics.
Adds three tests: success (no log), failure-via-closed-pool (Warn
record with action+err keys), and nil-logger (no panic). Tests
skip when MINSTREL_TEST_DATABASE_URL is unset, matching the
existing harness convention.
The PR1-T2 admin handler migration replaced bespoke 500-class messages
("lookup failed", "refetch failed", "trigger failed", "bump failed",
"remove failed", "update failed", "schedule row missing", "re-read
failed", "read failed") with apierror.Internal(err), which forces the
wire Message to "internal server error". That violates the PR1
contract that errEnvelope{Code, Message} must remain byte-identical
for existing clients.
Add apierror.InternalMsg(message, cause) — a 500-class constructor
that preserves the call site's user-facing message while keeping the
cause attached for logging and errors.Is. Re-migrate the 12 admin
sites whose pre-1cc7eb6 source had a non-empty bespoke Message so
they restore the original wire string. Sites whose original Message
was empty stay on Internal(err) (humanization to "internal server
error" is a tolerable improvement, not a regression).
admin_smtp.go's send_failed site (line 129) was already preserved
via a raw &apierror.Error literal in 1cc7eb6 and needs no fix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewrite writeErr(w, err) to wrap *apierror.Error via apierror.From,
preserving the existing {"error": {"code", "message"}} wire envelope.
Add writeErrWithLog helper for 500-class errors that need an operator
log line. Migrate all 13 admin_*.go handler files (~76 call sites) to
the new signature; T3 will sweep the remaining api package.
The old 4-arg writeErr is removed, so non-admin call sites in
internal/api will not compile until T3 lands. This is by design — T2
and T3 are paired.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the auto_approve_requests user flag into the request creation
flow. When the flag is set (admin enabled it via /admin/users from
U2-T3), POST /api/requests transitions the just-created pending row
through Service.Approve inline, which dispatches to Lidarr the same
way a manual admin approve does.
Failures inside Approve (ErrLidarrDisabled, ErrDefaultsIncomplete,
network/Lidarr errors) leave the row pending — same fallback as
manual approve hitting the same path. The user gets a 201 either
way; admin can still resolve the row in /admin/requests if it
stayed pending. The handler logs the auto-approve failure with
user_id + request_id for observability.
The "actor" id passed to Approve is the user's own ID — they're
acting under the privilege the admin granted them via the toggle.
The trail of "admin set the flag" already lives in audit_log
(ActionAutoApproveToggle from U2-T2). Adding a per-auto-approval
audit entry is a future enhancement; for v1 the toggle audit plus
the request row's status transition is enough.
Test verifies the Lidarr-disabled fallback contract: a user with
auto_approve=true and Lidarr unavailable still gets 201 Created
with status=pending (no crash, no 500). The "auto-approve actually
succeeds" test path requires a Lidarr stub; deferred until a
broader Lidarr test fixture lands.
Cleans up the 8 svelte-check warnings I deferred in prior CI runs.
Two distinct categories — handling them differently:
REAL REACTIVITY BUGS — fixed:
- AlbumMenu.svelte / ArtistMenu.svelte: cachedTracks initialized
from the `tracks` prop only at component creation. When the menu
is reused across rows (operator opens menu on one album, then
another), the cache from the first open serves stale tracks.
Fix: initialize cachedTracks to null; ensureTracks() reads the
current `tracks` prop on each call before falling through to
the network fetch.
INTENTIONAL PATTERNS LINT FLAGGED — suppressed with explanatory
comments:
- AlbumMenu / ArtistMenu / PlaylistCard menu containers: the
onclick={(e) => e.stopPropagation()} is a guard against the
window-level "any click closes the menu" listener — not
user-facing interaction. A paired keyboard handler is
intentionally absent (Escape-to-close lives on svelte:window
onkeydown). Suppressed a11y_click_events_have_key_events with
explanatory comments.
- CompactTrackCard overlay div: catches clicks on its inner
LikeButton + queue button so they don't bubble to the wrapping
play-card button. Inner buttons carry their own keyboard
handling. Suppressed both a11y_click_events_have_key_events and
a11y_no_static_element_interactions.
- theme.svelte.ts module-scope `_resolved = $state(resolve(_theme))`:
module-init reads _theme to seed _resolved; subsequent updates
go through setTheme() and the matchMedia listener. $derived
doesn't apply at module scope. Suppressed state_referenced_locally
with an explanatory comment.
Promised in the prior fix-forward: "I'll fix bugs when I see them
or open a follow-up task with a specific name, not park them in an
umbrella." Following through.
Two real algorithm bugs in F-T1's For-You composition + Discover
allocator. Both surfaced as failing unit tests under go test -race.
1. redistributeSlots was re-redistributing a bucket's full deficit
on every pass instead of just the residual. The loop computed
`deficit = b.want - final[i]` each iteration, but final[i] for
a deficit bucket never increases (its supply is exhausted), so
pass N saw the same deficit as pass N-1 and kept shoveling it
to peers. For [want:40 avail:100, want:30 avail:0, want:30 avail:100],
four passes pushed cross-user's deficit into dormant+random four
times each, hitting the 100-slot clamp at the end and producing
[50, 0, 50] instead of the spec'd [55, 0, 45].
Fix: track per-source `redistributed[i]` and subtract it from the
deficit each pass. Multi-pass behavior still works for the case
where a peer's supply runs out mid-distribution.
2. tieBreakHash used FNV-1a 64-bit with trackID + dateStr appended.
For dateStrs differing only in the last character ("2026-05-07"
vs "2026-05-08"), the FNV state diverged only in low bits at the
final byte; multiplication by FNV_prime propagates upward but the
relative ordering of 60 small candidate UUIDs (which differ only
in their last byte) ended up identical across the two dates. The
For-You head/tail test asserted that the tail's first 5 should
change across days; it didn't.
Fix: switch to SHA-256 truncated to 8 bytes. SHA-256 has full
avalanche, so any single-bit input change roughly half-flips the
output bits and meaningfully reorders.
The hash isn't security-load-bearing; we just need strong avalanche
for tiny dateStr deltas. Determinism (same inputs → same output) is
preserved.
Three Go lint hits + two web test failures, all from the U3 push.
- internal/mailer/mailer.go: SentEmail.HtmlBody → HTMLBody (Go's
initialism convention; revive flagged); FakeSender.Send unused
ctx parameter renamed to _.
- internal/audit/audit_test.go: removed dead validUUID helper. It
was added speculatively in U1-T1 and never called by any test.
pgtype import stays — other tests use it.
- web/src/routes/settings/settings.test.ts: existing ListenBrainz
tests queried `getByRole('button', { name: /save/i })` which
worked when the page had only one Save button. The new Profile
card adds a "Save profile" button that also matches the regex,
triggering "found multiple elements". Anchored to /^save$/i for
exact-match. The newer Save profile tests still use
/save profile/i which is unique.
This is the same shape of test-fixture-lag I owe an answer for: I
landed new content that broke an existing test, and the existing
test had a too-loose selector. The right fix is to tighten the old
selector now (this commit) and to flag this pattern — selectors
that regex-match by partial words — as a candidate for the DRY
pass / test-utils consolidation.
Five fixes in one commit, all from the U1+U2+U3 push:
1. internal/api/library_test.go's Mount call wasn't updated when
U3-T4 added the mailer.Sender param. 17 args, sig wants 18.
Adds a trailing nil for the mailer. Same shape of test-fixture
lag the project has hit before — flagged as a recurring pattern
for the DRY-pass slice.
2. web/src/routes/settings/settings.test.ts mocked $lib/api/me with
`getAPIToken: vi.fn()` (no return value). The new /settings API
Token card's $effect calls `getAPIToken().then(...)` which threw
"Cannot read properties of undefined (reading 'then')" on every
ListenBrainz test. Default the mock to mockResolvedValue so any
test that doesn't override gets a valid resolution.
3. web/src/routes/admin/users/users.test.ts queried row buttons by
/^Delete$/ but T3 added per-user aria-labels ("Delete alice"),
so the row buttons' accessible name is no longer "Delete". The
modal's confirm button has no aria-label so its name IS "Delete"
exactly. Tests now click by per-user aria-label first, then by
/^Delete$/ for the modal confirm.
4. Same file: /Set password/i regex matched both the dialog's
submit button AND the row's "Reset password for ..." buttons
(because regex `Set` matches "ReSet" case-insensitively). Switched
to /^Set password$/ exact-match.
5. web/src/routes/reset-password/[token]/reset-password.test.ts had
/New password/i which matched both the "New password" and
"Confirm new password" labels. Switched to exact-match string
selectors.
6. web/src/routes/forgot-password/+page.svelte's onSubmit handler
had no catch — a rejected forgotPassword() bubbled as an
unhandled rejection in vitest. Added a silent catch since the
page intentionally shows the same success message regardless of
outcome (no-enumeration posture).
Three CI fixes from the U1+U2+U3 push:
- internal/api/admin_users.go imported the standalone
github.com/jackc/pgconn — that module isn't in go.sum and
go vet caught it. Switch to github.com/jackc/pgx/v5/pgconn
(the path used elsewhere in the package, e.g. auth_register.go
and me_profile.go).
- /reset-password/[token] dereferenced page.params.token without
the undefined narrowing svelte-kit's typegen requires. Coalesce
to '' on read and reject empty token at submit time with a
clear error.
- /admin/integrations SMTP card had a label without an associated
control as the TLS row's leading column header. Switched to a
span — the actual checkbox lives in the wrapping label below it.
Completes the U3 frontend that T5a's crashed dispatch left half-done.
- /reset-password/[token] — public; new password + confirm. Calls
POST /api/auth/reset-password with the URL's token. invalid_token,
password_too_short, and mismatched-passwords surface as inline
errors. Success redirects to /login?reset=ok.
- /admin/integrations gains an SMTP card (alongside Lidarr) with
enabled toggle + all fields + Save and Send-test-email buttons.
Password input is a separate state from the loaded form, so empty
submission preserves the stored server-side value (matches the
backend's empty-password-preserves contract). Test button error
codes (no_email_on_file / not_configured / send_failed) surface
as actionable toasts.
Tests cover both new pages plus extensions to settings + integrations
test files for the cards landed in T5a.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Lands four of the U3 frontend surfaces. Splits T5 because the
dispatch covering everything in one shot crashed the runtime
mid-execution; this is the salvageable half.
- /settings gains three cards alongside Appearance: Profile
(display name + email), Password (current + new + confirm with
client-side mismatch check), API Token (display + copy +
regenerate-with-double-click-confirm). Server error codes map
to clear toasts.
- /forgot-password — public; takes an email and always shows the
success message regardless of whether the email is on file
(mirrors the server's no-enumeration posture).
- Login page gets a "Forgot password?" link below the existing
register link.
- API client functions for the four /me endpoints (changePassword,
updateProfile, getAPIToken, regenerateAPIToken), the SMTP admin
trio (getSMTPConfig, updateSMTPConfig, testSMTPConfig), and the
forgot/reset auth pair (forgotPassword, resetPassword).
Tests for these surfaces + /reset-password page + admin SMTP card
land in T5b (next follow-up).
POST /api/auth/forgot-password and POST /api/auth/reset-password.
Forgot-password ALWAYS returns 200 with empty JSON to prevent
enumeration of registered emails. Side effect: when email matches
a user with email-on-file, generates a 32-byte hex token (24h
TTL), inserts into password_resets, and sends the reset email via
the mailer. Mailer failures are logged (not surfaced) and the
audit log carries metadata.email_match for operator visibility.
Reset-password atomically claims the token via UsePasswordReset
(:execrows; concurrent calls can't both succeed). On rows=1,
hashes the new password and writes via ChangeUserPassword.
Returns 204 on success, 400 invalid_token on stale/used/missing
tokens, 400 password_too_short for short passwords. Audits
ActionPasswordResetByEmail.
Wires the mailer.Sender into the handlers struct via Mount;
production sender (NewSMTPSender) constructed in server.Router();
tests inject FakeSender via testHandlers default. The reset
URL embedded in the email is derived from r.Host (no PublicURL
config setting in v1; self-hosted operators see their own
hostname).
Tests cover happy-path send + token-row insertion, unknown email
returns 200 with no send, mailer failure still returns 200, reset
happy path verifies bcrypt match + used_at set, already-used
token 400, expired token 400, short password 400, bogus token 400.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
New internal/mailer/ package:
- Sender interface with two impls: SMTPSender (production, reads
smtp_config at send time so admin edits apply without restart;
uses stdlib net/smtp + STARTTLS) and FakeSender (test/concurrent-
safe call recorder).
- Embedded text + HTML templates for the password reset email,
rendered via stdlib text/template + html/template. The HTML
uses the FabledSword forest-teal accent color.
- ErrNotConfigured surfaces when smtp_config.enabled is false or
required fields are empty; callers like the future forgot-password
handler will treat this as "log and pretend success" to avoid
user-enumeration leaks.
Three admin endpoints under RequireAdmin:
- GET /api/admin/smtp-config — returns the singleton; password
field is masked ("***" or "").
- PUT /api/admin/smtp-config — updates settings. Validates
host + from_address are non-empty when enabled=true. Empty
password in the request preserves the stored value (so the
operator doesn't have to re-enter it on every save).
- POST /api/admin/smtp-config/test — sends a real test email to
the calling admin's email. 400 if admin has no email; 500 with
the error message on send failure (so the operator can debug
config without grep-then-trace through logs).
Tests cover the password-mask, password-preservation-on-empty,
enabled-requires-host-and-from validation, and the no-email-on-file
rejection. Mailer unit tests cover the fake recorder and template
rendering. The "real SMTP send" path needs a live server and isn't
covered in CI; the FakeSender covers that role for downstream tests.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Four authenticated endpoints for the user's own account:
- PUT /api/me/password — change own password. Caller must
supply current_password (verified via bcrypt). Distinct from
admin-driven reset (which doesn't require knowing the old).
Audits ActionPasswordChangeSelf.
- PUT /api/me/profile — set display_name + email. Both fields
are nullable; empty string clears, omitted leaves unchanged.
Email is lowercased before store + format-validated. Unique
violation → 409 email_taken.
- GET /api/me/api-token — returns current API token (for
copy-paste into Subsonic clients).
- POST /api/me/api-token — regenerates token. Old one stops
working immediately. Audits ActionTokenRegenerate.
All four use the existing RequireUser middleware on the authed
sub-router; audit writes are best-effort (logged on failure).
Tests cover happy paths, wrong-password 401, password-too-short
400, email-invalid 400, email-taken 409, clear-by-empty-string,
omit-leaves-unchanged, token GET + regen.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds users.email (optional, lowercase-unique via partial index),
smtp_config singleton, and password_resets tokens. Plus the
queries U3-T2 / T3 / T4 use:
- ChangeUserPassword: self-service password change. HTTP layer
verifies the current password before this fires.
- UpdateUserProfile: set display_name + email together.
- RegenerateApiToken: invalidate old API token.
- GetUserByEmail: case-insensitive lookup for forgot-password.
- GetSMTPConfig + UpdateSMTPConfig: admin SMTP settings CRUD.
- CreatePasswordReset / GetPasswordReset / UsePasswordReset
(:execrows for atomic claim) / DeleteExpiredPasswordResets
(cron-style cleanup, not yet wired).
Email column is nullable; users without email have admin-reset
as their only password-recovery path. The lower() unique index
allows many NULLs and prevents case-insensitive duplicates.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extends the U1 /admin/users page with the four U2 admin endpoints.
- "New user" button opens a modal: username, optional display
name, password + confirm, optional admin checkbox. Validates
password match client-side; surface server-side errors
(username_taken, username_invalid, password_too_short).
- Per-row "Delete" opens a confirm modal explaining the cascade
(plays, likes, sessions). Last-admin guard surfaces as a clear
toast if the server refuses.
- Per-row "Reset password" opens a small modal: new password +
confirm. Toast confirms success.
- Per-row "Enable / Disable auto-approve" toggles the
per-user flag (the #355 sub-feature surface). Inline button
state reflects the current value.
AdminUser type extended with auto_approve_requests; the badge
appears next to the admin badge when enabled.
Tests cover create-user submit, delete confirm flow, last-admin
toast on delete, reset-password submit, auto-approve toggle.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Four new admin endpoints under existing RequireAdmin middleware:
- POST /api/admin/users — admin-creates-user. Body
{username, password, display_name?, is_admin?}. Same username
+ password validation as the public /register handler. 409
on duplicate. Audits ActionCreateUserAdmin.
- DELETE /api/admin/users/{id} — hard delete. Last-admin guard
refuses delete when target is the only admin (409). Schema's
ON DELETE CASCADE on user-FK tables handles plays/likes/
sessions cleanup. Audits ActionDeleteUser with target's
username + was_admin flag.
- POST /api/admin/users/{id}/reset-password — body
{password}. 8-char minimum. Admin sets a new password
without knowing the old one. Audits ActionPasswordResetAdmin.
- PUT /api/admin/users/{id}/auto-approve — body
{auto_approve: bool}. Toggles the per-user flag added in T1
(the #355 sub-feature surface). Audits ActionAutoApproveToggle.
adminUserView shape extended with auto_approve_requests so the
list and toggle responses carry the flag. ListUsers SQL query
updated to include auto_approve_requests; generated Go updated
to match. Tests cover happy paths, last-admin guard on delete,
password validation, duplicate username, and the auto-approve
round-trip.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds users.auto_approve_requests boolean default false (the #355
sub-feature surface; the request-flow handler that honors this
flag is U2.5 follow-up work).
Four new sqlc queries for the U2 admin endpoints:
- CreateUserAdmin: admin-driven user creation, accepts all five
fields explicitly.
- DeleteUser: hard delete; schema's ON DELETE CASCADE foreign
keys handle plays/likes/sessions cleanup. Last-admin guard
lives in the HTTP layer (next task).
- ResetUserPassword: admin sets a new hashed password without
knowing the old one.
- UpdateUserAutoApprove: toggles the new boolean.
Self-service equivalents (knows-current-password change, etc.)
are U3 work and use distinct queries.
New admin route /admin/users with two sections:
- Accounts — table of all users (username, optional display name,
admin badge, created_at). Per-row "Make admin" / "Remove admin"
button calls PUT /api/admin/users/{id}/admin. Last-admin guard
surfaces as a toast ("Can't remove the last admin — promote
someone else first").
- Invites — list of active invites with Copy + Revoke per row.
"Generate invite" button creates a 24h token. Operator shares
the token with the invitee, who enters it on /register.
AdminTabs gains a "Users" tab (5 tabs total). New typed client
functions in admin.ts (listUsers, updateUserAdmin, listInvites,
createInvite, deleteInvite) plus query factory functions
createAdminUsersQuery / createAdminInvitesQuery and query keys
qk.adminUsers / qk.adminInvites.
Tests cover: list rendering, admin badge, promote/demote actions,
last-admin guard toast, invite generation, revoke flow, empty states.
AdminTabs.test.ts updated to assert 5 tabs and Users active state.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Public /register form: username, optional display name, password +
confirm, optional invite token. Submits to POST /api/auth/register
via the new register() helper in auth/store.svelte.ts. On success
the server sets the session cookie and the frontend redirects to /;
on error the page shows a code-specific message (invite_required,
invite_invalid, username_taken, etc.).
Login page gets a "Don't have an account? Register" link below the
form for symmetry.
Tests cover form rendering, password-mismatch client-side rejection,
the happy-path submit + redirect, and the invite-invalid error
mapping.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Five admin endpoints under existing RequireAdmin middleware:
- GET /api/admin/invites — list active + recently-redeemed
- POST /api/admin/invites — generate 24h invite, returns
{token, expires_at, ...}. Audits ActionInviteCreate.
- DELETE /api/admin/invites/{token} — revoke unredeemed invite.
Audits ActionInviteRevoke.
- GET /api/admin/users — list all users (id, username,
display_name, is_admin, created_at).
- PUT /api/admin/users/{id}/admin — toggle is_admin with
last-admin guard. Audits ActionPromoteAdmin / ActionDemoteAdmin.
Last-admin guard counts admins, refuses demotion of the sole
admin with 409 'last_admin'. Race window between count and update
is acceptable for v1 — worst case is 'no admins left,' which the
env-driven bootstrap or CLI reset can recover from. Common path
('admin demotes themselves') is now blocked.
Adds ListUsers, CountAdmins, UpdateUserAdmin sqlc queries to
users.sql.
Tests cover: invite create/list/delete round-trip, non-admin gets
403, user list, promote happy path, last-admin demotion refused,
two-admins demotion allowed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
POST /api/auth/register accepts {username, password, invite_token?,
display_name?} and creates a user. Race-safe first-admin path: when
the users table is empty, the SQL query (CreateUserFirstAdminRace)
inserts is_admin computed from a SELECT NOT EXISTS subquery — no
serializable isolation needed; concurrent empty-state registrations
both end up admin (benign).
Past first-admin, registration_settings.mode dictates: 'invite_only'
(default) requires a valid unredeemed unexpired invite token, which
is atomically claimed via RedeemInvite (rows-affected returns from
sqlc's :execrows directive). 'open' mode skips the invite check.
On success: hashes password (bcrypt), mints session token + cookie
matching handleLogin's shape, mints a separate api_token for
Subsonic clients, audits ActionRegister + ActionInviteRedeem
(best-effort — a failed audit write does NOT fail the user-facing
operation).
Validation: 3-32 char usernames (alphanumeric + underscore +
hyphen), 8-char minimum password. Duplicate username surfaces as
409.
Tests cover: first-user-becomes-admin, invite-only requires token,
valid token redeems + non-admin role, invalid token 400, open mode
skips check, duplicate username 409, password too short 400,
username format 400, race scenario asserts at-least-one-admin.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Schema for user management U1: display_name on users; user_invites;
registration_settings singleton (default 'invite_only'); audit_log.
Plus the internal/audit package centralizing the action-name
vocabulary and JSON metadata marshaling so handlers don't repeat
boilerplate.
Race-safe first-admin uses a query-shape primitive
(CreateUserFirstAdminRace's WHERE NOT EXISTS subquery) rather than
a schema-level constraint. Concurrent empty-state registrations
both see 'no users yet' and both insert as admin — fine, having
two admins from the start is benign; what matters is at-least-one.
users.username uniqueness arbitrates if the two callers picked the
same username.
CreateUser signature gains display_name (nullable); existing
bootstrap call sites pass nil. The audit package declares the full
U1+U2+U3 action vocabulary upfront so subsequent slices are purely
additive on the caller side.
Tests cover audit Write with + without metadata and that every
declared action constant persists correctly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Click on the For-You tile's play button now triggers a synchronous
refresh, then enqueues + auto-plays the freshly-built playlist.
Two-click-target tile pattern: tile BODY click still navigates to
the playlist detail page (existing behavior); the play button is
the new generate-and-play action.
Behavior is gated on playlist.system_variant === 'for_you' — every
other playlist (user, Discover, Songs-like-X) keeps the existing
"fetch detail, enqueue, play" flow. The atomic-replace BuildSystem-
Playlists creates a new playlist_id, so the play handler uses the
refresh response's id for the follow-up getPlaylist call rather
than the stale tile prop's id.
Empty-library response (playlist_id: null) is a no-op — clicking
play in a degenerate-empty library doesn't error, it just doesn't
start playback. Caches are invalidated post-refresh so the home
view re-fetches and the tile re-renders with the new id, avoiding
the corner case of a click-then-tile-body 404.
Tests cover the refresh-and-play happy path, the empty-library
no-op, and that non-For-You playlists keep the existing flow.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
POST /api/playlists/system/for-you/refresh re-runs the system
playlist build synchronously for the calling user, then returns
the freshly-built For-You playlist's id, track count, and the
track IDs in playlist position order.
The frontend tile play button (next task) calls this endpoint
on click and enqueues the returned track_ids directly — one
roundtrip, no follow-up "list tracks" call needed for playback
to start. Same authenticated-user-only posture as the Discover
refresh from D-T3.
Returns playlist_id=null and track_ids=[] when the build
succeeded but the user's library yielded no eligible candidates
(degenerate empty-library). Returns 500 on actual build failure.
Tests cover 200 with shape (track_count matches len(track_ids))
and 401 without auth.
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.
bvandeusen