- 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>
Production list-playlists handler reuses Service.List + in-memory kind
filter so other users' public playlists can surface alongside the
caller's filtered own. Comment clarifies the divergence so future
readers don't expect this query to be wired into the API path.
Push kind='system' guard into each of the five Service edit methods
(Update, Delete, AppendTracks, RemoveTrack, Reorder) immediately after
the ownership check; map the typed error to 403 in writePlaylistErr;
add table-driven test covering all five verbs against a seeded system
playlist.
Adds ?kind= filter (user|system|all, default user) to GET /api/playlists
so system mixes don't leak into slice-1 SPA views. Lazy background build
fires when kind=system|all and the caller's run row is stale (>24h).
Propagates Kind/SystemVariant/SeedArtistID through PlaylistRow and
playlistRowView wire types.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds internal/db/queries/system_playlists.sql with 12 named queries
covering active-user enumeration, per-user run tracking (claim/finish/fail),
seed-artist/track/cover selection, and playlist CRUD by kind.
Runs sqlc generate to emit dbq/system_playlists.sql.go; also updates
playlists.sql.go and models.go to reflect the new kind/system_variant/
seed_artist_id columns added in migration 0015.
Note: plan specified a.cover_path for PickTopAlbumCoverForArtistByUser
but albums uses cover_art_path — corrected in the query file.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code-quality review flagged convention drift from sibling library
pages. Aligning so the history page matches the same shape:
- queryStore + $derived($queryStore) pattern (single subscription
point) instead of $query.X everywhere.
- h1 uses font-display text-2xl font-medium per FabledSword design
system (weights 400/500 only — font-semibold drifted to 600).
- h2 uses font-medium + z-10 for sticky-header layering.
- InfiniteScrollSentinel uses its `enabled` prop (which exists; the
earlier spec note claiming otherwise was wrong) so the observer
isn't recreated on every fetch cycle.
- Loading more… / End of history footers added to match albums.
- onRetry passes query.refetch by reference, not wrapped.
- Dropped redundant `as HistoryEvent[]` cast.
Also fixed test case 4 which trivially passed regardless of the
conditional gate's correctness — now queries the sentinel's actual
DOM root (div[aria-hidden="true"].h-px) and asserts presence/absence.
Added a positive twin test for hasNextPage=true.
Implements the listening-history endpoint for M7 #365: returns a user's
play events newest-first, excluding skipped events and quarantined tracks,
with offset/limit pagination and a has_more heuristic.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CI test-web was blocking on:
- QueueDrawer.test close-button query: getByLabelText(/close queue/i)
matched BOTH the backdrop button (aria-label="Close queue backdrop")
and the close button (aria-label="Close queue"). Switched to exact
string match.
- QueueDrawer.test inert assertion: Svelte 5 + jsdom uses property
binding for `inert`, not attribute, so hasAttribute('inert') returns
false. Check the DOM property (with attribute fallback for
robustness) instead.
- queue-row-math.ts -0 normalization: Math.round(-0.5) returns -0,
and Object.is(-0, 0) is false — vitest's .toBe(0) fails. Added
`|| 0` to normalize -0 to +0 at the function boundary so consumers
never see -0.
- player/store.svelte.ts user import guard: persistence $effect.root
reads `user.value?.id` at module-init, but in test files where
auth/store.svelte.ts is imported transitively (auth/store.test,
playlists/playlists.test), the player module loads via auth's
import chain before auth's `user` binding is assigned. `user?.value`
guards against that init-order race. The proper fix is to invert
the auth↔player import dep (filed as I2 follow-up under #375).
CI svelte-check was blocking on:
- 1 ERROR I introduced in the previous cleanup: QueueDrawer.test.ts
passed { hidden: true } to getByLabelText, but that option only
exists on getByRole. Fixed by switching to a direct
document.querySelector for the aria-hidden assertion.
- 13 WARNINGS pre-existing in the codebase from M7 #352/#372/#349
era, never surfaced because earlier CI runs failed before reaching
type-check. Now that CI gets that far, they accumulate. Cleared:
- FlagPopover, RemoveTrackPopover, AddToPlaylistMenu, TrackMenu:
interactive role divs gain tabindex="-1" + onkeydown that stops
propagation and closes on Escape (functional, not just warning-
suppression).
- FlagPopover state-from-props: $state(untrack(() => prop ?? default))
for explicit initial-snapshot semantics; const isUpdate switched
to $derived so it reacts to prop changes.
- PlaylistTrackRow drag-div: role="listitem" added.
- playlists/+page.svelte: autofocus replaced with bind:this + $effect.
CI svelte-check failed on 5 type errors caused by the slice using
duration_ms/album_name fields that don't exist on TrackRef (the real
shape uses duration_sec + album_id/album_title). Fixed across
QueueDrawer, QueueDrawer.test, QueueTrackRow.test, persisted.test,
and the inline `state.current = null` in PlayerBar.test (TrackRef |
undefined, not | null).
Final whole-slice review concerns also rolled in:
- C1 (Critical): persisted position was restored to UI but never
seeked into the audio element — first timeupdate snapped _position
back to 0. Added a one-shot _pendingRestorePosition module ref +
consumePendingRestorePosition() export; layout's loadedmetadata
handler now seeks once on restore.
- I1 (Important): throttled-write effect now wraps _queue / _index
reads in untrack so the dependency tracking matches the design
intent. Position remains the only tracked dep.
- I4 (Important): drawer gets inert={!queueDrawerOpen} so its inner
buttons drop out of tab order when closed (aria-hidden alone
doesn't do that). Removed redundant role="complementary" from
<aside> (implied by the element). New test asserts inert binding.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wrap the _position read inside untrack() in the immediate-write $effect
so it no longer registers as a reactive dependency, preventing the effect
from firing on every 4Hz position tick. Also adds three missing
restoreQueue unit tests with vi.mock for the auth store.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
M2 — Add TODO(#375) tripwire comments where the dark/light theme-color
hex pair is duplicated outside tokens.json (vite.config.ts and
applyMetaThemeColor.svelte.ts). Refactoring is out of scope for #363;
the comments are stop signs for the next person who edits these.
M5 — The SPA never reads window.__MINSTREL__.description. The OG meta
description is server-rendered and complete on its own. Drop the dead
inline-script field and the corresponding helpers in branding.ts.
Server-side BrandingConfig.Description stays — it's still used for
<meta name="description"> and og:description.
Copy nit — Playlist detail page title becomes "Playlist · Foo"
matching the singular form already used by Artist · / Album ·.
Nine golangci-lint failures accumulated across M7 #352, #372, and
6d1709c that were never surfaced because test-go never ran green long
enough to reach the lint step. The first push to land cleanly through
vet (the M7 #363 slice) ran lint and exposed them.
- errcheck: discard the error on defer Close() in collage.go,
collage_test.go, and server_test.go.
- gofmt -s: re-run on config.go and playlists_test.go.
- goimports: move the stray "time" import from the local-module group
to stdlib in server_test.go.
- revive unused-parameter: rename ctx to _ on stubScanner.Scan and
fakeLidarrUnmonitorer.UnmonitorTrack (test stubs); on
tracks.Service's adminID, add //nolint:revive directive rather than
renaming because the HTTP handler passes admin.ID (a real authed-user
UUID) and the existing comment already flags it as reserved for the
audit-log follow-up.
The legacy SMARTMUSIC_ prefix dates from when the project was specced
as "smart-music" before being renamed to Minstrel. Hard cutover to
MINSTREL_* across config.go, tests, config.example.yaml, README, and
docker-compose. Also renames SMARTMUSIC_CONFIG in cmd/minstrel/main.go
which was missed in the original audit.
Also seeds Auth.AdminBootstrap.Username = "admin" in Default() so
MINSTREL_DATABASE_URL is the only env var required for a fresh prod
deployment — the bootstrap auto-generates a password and prints it
to stderr. Drops the now-redundant MINSTREL_AUTH_ADMIN_USERNAME=admin
line from the README quickstart and docker-compose.
M1 follow-up from the #363 final review: guards
MINSTREL_BRANDING_APP_NAME and _DESCRIPTION against empty-string
overrides — empty would have shipped <title></title> to share-preview
crawlers.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add browser tab titles to all 18 static page routes using the pageTitle()
helper from $lib/branding. Titles follow the Minstrel · Section format.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Agent-authored design specs and implementation plans are kept local
on the working machine; the canonical record lives in commit messages
and the running code. The 48 historical files remain in git history
and can be retrieved via git checkout <sha> -- docs/superpowers/ if
ever needed; they just stop accruing on the dev tip.
Per-route titles, MINSTREL_APP_NAME via Go template injection of the
embedded index.html, OG/Twitter meta + theme-color follow-through for
the dark/light toggle, and an operator-first README rewrite. PWA /
service worker / install icons explicitly out of scope (Flutter #356
+ Tauri post-v1 cover mobile and desktop). OG image ships as a
generated placeholder; hand-designed artwork drops in later.
Web tokens.json was reshaped for the SPA theme toggle (#362) with
colors split into dark/light/flat blocks. gen_tokens.dart still
expected a flat string map and threw _Map<String,dynamic> cast
errors in the flutter CI workflow. Read colors.dark + colors.flat
and merge into the existing flat namespace; light variants will be
emitted when the Flutter client gains a theme toggle as part of
#356 parity work. Adds hyphen→camelCase normalization so the new
on-action token surfaces as onAction.
Those actions don't have meaningful semantics for the track you're
already listening to. New TrackMenu prop hideQueueActions gates the
pair; PlayerBar passes it true. Other mount sites (track rows in
lists) keep the full 8-entry menu.
- AddToPlaylistMenu: anchor submenu bottom to parent bottom so it
grows upward (top-0 → bottom-0) — kebab row is usually near the
bottom of the track menu and the submenu kept extending off-screen.
- HorizontalScrollRow: scrollbar-width: none + ::-webkit-scrollbar
display:none. Page arrows already drive the scroll; the scrollbar
was just consuming vertical real estate.
Spec table's "Dark (current)" column now matches the actual
tokens.json values. Added Tailwind text-action-fg alias as the
mechanism for the non-flipping --fs-on-action token. Updated
audit notes to use the real Tailwind class (text-text-primary)
rather than the raw token (text-fs-parchment).
bvandeusen
