golangci-lint surfaced both on 9c7dec6:
- errcheck on bare 'defer tx.Rollback(ctx)' in 2 test files
- gofmt -s wanted tighter map-key alignment in library_sync.go +
library_sync_test.go (auto-fixed by 'gofmt -s -w')
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
internal/sync/changes_test.go imports github.com/stretchr/testify/require,
moving testify from indirect to direct. Also pulled in testify's own
indirects (go-spew, go-difflib) and promoted pgerrcode to direct.
Caught by CI go vet on 6fb6729.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five sites wired:
- Create / Update / Delete playlist: pool-bound LogChange after success
- AppendTracks: tx-bound LogChange per inserted playlist_track row
- RemoveTrack: tx-bound LogChange after the delete + lookup the
track_id pre-delete so the composite key is still resolvable
The two tx-bound paths (AppendTracks, RemoveTrack) treat LogChange as
required — failure rolls back the playlist mutation too. The pool-bound
paths Warn on failure to match the scanner / likes pattern (mutation
already committed; missed log row recovers on next mutation).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
All 6 like/unlike sites (track/album/artist × like/unlike) now emit a
library_changes row via the shared logLikeChange helper. Best-effort:
LogChange failures Warn but don't fail the HTTP response — a missed
log row is recovered at the next mutation on the same entity.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires sync.LogChange into the library mutation sites so /api/library/sync
reflects upserts and deletes.
Architectural pivot: LogChange's signature is now (ctx, dbq.DBTX, ...) so
it works with both *pgxpool.Pool and pgx.Tx. The scanner doesn't run
mutations in explicit transactions, so it pool-binds; delete.go matches.
Tx-bound callers (likes/playlists in subsequent commits) keep atomicity.
Also: sync.FormatUUID centralizes the pgtype.UUID → canonical string
conversion that both the scanner and the sync handler need; library_sync.go
now uses it instead of a local copy.
Best-effort logging on scanner failures (Warn, don't fail the scan): a
LogChange error after a successful upsert is rare and self-healing — the
next scan that touches the entity re-emits the change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Returns batched upserts + deletes since the supplied cursor. Empty cursor
returns full snapshot; subsequent calls pull deltas. Per-user entities
(likes, playlists) are scoped to the authed user. Composite-key entities
(likes, playlist_tracks) use stable string ids encoded by sync.EncodeLikeID
/ sync.EncodePlaylistTrackID.
Behavior:
204 No Content - no changes since cursor
200 OK - JSON syncResponse {cursor, upserts, deletes}
410 Gone - cursor older than oldest log row; client must reset
401 / 500 - standard envelope errors
Adds sqlc queries GetAlbumsByIDs, GetTracksByIDs, GetPlaylistsByIDs to
mirror the existing GetArtistsByIDs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New package internal/sync. LogChange writes a library_changes row inside
the supplied tx. Encode helpers produce stable composite ids for like_*
and playlist_track entries. Subsequent commits wire LogChange into the
scanner / likes / playlists services.
Also: dbtest.dataTables now includes library_changes so test isolation
holds across runs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Append-only change log for library entities. Every mutation on
artists/albums/tracks/likes/playlists/playlist_tracks will write a row
in the same transaction as the mutation itself (wired in subsequent
commits). Powers the Flutter delta-sync endpoint (#357).
- 0025_library_changes migration (up + down)
- internal/db/queries/library_changes.sql (Insert, GetSince, MaxCursor, MinCursor)
- regenerated dbq from sqlc
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MobileNavDrawer: Svelte 5 + jsdom binds `inert` as a property, not always
an attribute. Mirror the QueueDrawer.test.ts pattern that accepts either.
Integrations Save tests: Per commit bca8622 (Save runs Test first), the
two existing Save tests need to mock testLidarrConnection before clicking
Save — otherwise the Test step returns undefined, fails the ok check,
and putLidarrConfig is never reached. The newer 'Lidarr first-time
setup' suite already does this; just bringing the older two tests in
line. Also wrapped the assertions in waitFor since the put-config call
now resolves async after the test promise.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI vitest run on 3f8a2c5 surfaced 17 failures across 5 test files; this
commit addresses 15 that are this batch's responsibility.
1. RowActionsMenu now accepts ariaLabel on RowAction. Defaults to label.
Admin pages (requests/quarantine/users) pass per-row aria-labels
matching the pre-batch buttons ("Approve Geogaddi", "Resolve Roygbiv",
"Make alice admin", etc.) so screen readers + tests find them.
2. PlayerBar.test.ts — anchored regex /^(play|pause)$/i so the new
"Player options" overflow ⋮ doesn't also match /play|pause/i.
3. MobileNavDrawer.test.ts — added vi.mock for $app/state, $app/navigation,
and $lib/auth/store.svelte (mirrors Shell.test.ts pattern). Without
these, SvelteKit's notifiable_store helper isn't bootstrapped in
vitest and the suite fails to load.
The 2 remaining vitest failures are in /admin/integrations Save flow
(putLidarrConfig spy not called). Untouched by this batch and the
recent "Save runs Test first" refactor (bca8622) appears related —
flagging for operator verification, not chasing as a regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bind:this requires an Identifier or MemberExpression — a ternary
like {i === 0 ? firstNavLink : undefined} is invalid. Replaced
with a single bind on the <aside> root, then querySelector('nav a')
to locate the first nav link at focus time. Same behaviour, valid
Svelte.
Caught by CI svelte-check on 268e12a (failed before the @const
fix landed in 1536860):
src/lib/components/MobileNavDrawer.svelte:91:13
https://svelte.dev/e/bind_invalid_expression
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Svelte 5 requires {@const} to be a direct child of certain blocks
({#snippet}, {#if}, {#each}, etc.). Placing them inside <li>
bodies broke the build with const_tag_invalid_placement. Moved
the RowAction const declarations up so they sit between {#each}
and the <li> opener; the each-binding (u/r) is in scope for the
entire each block body, so the consts behave identically.
Caught by local docker build on dev:
src/routes/admin/users/+page.svelte:259:12
https://svelte.dev/e/const_tag_invalid_placement
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Without padding, max-w-md (448px) modals touch the viewport edges
on a 375px screen. p-4 on the overlay clamps them inside the safe
area. Covers all routes that use the shared <Modal> component
(discover track-confirm, quarantine typed-DELETE, users
password-reset, etc.).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- HorizontalScrollRow arrows bump 32px → 40px on coarse pointers
(touch screens) for finger-friendly hits; mouse pointers unchanged.
- AlphabeticalGrid divider letter is now <h3> so screen readers
announce it as a section heading. Margin reset to preserve layout.
- Modal overlay gets p-4 so max-w-md modals stay inside the safe
area at 375px viewport. Covers all routes that use the shared
<Modal> component (discover track-confirm, quarantine
typed-DELETE, users password-reset, etc.).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Requests: primary Approve, secondary [Override, Reject].
Quarantine: primary Resolve, secondary [Play, Delete file]; the
Delete-via-Lidarr conditional stays inline above md (preserves the
disabled-with-tooltip semantics) and hides below md to avoid
crowding — the operator can still trigger it from the desktop view.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three classes:
1. album_cover_cache.dart docstring used <applicationCacheDirectory>
and <albumId> which the analyzer reads as unintended HTML. Wrapped
the path in backticks and used {} placeholders.
2. settings AppearanceSection used RadioListTile.groupValue + onChanged,
deprecated in Flutter 3.32+. Wrapped the radios in a RadioGroup
ancestor (the new API) so individual tiles only declare value +
activeColor. Test updated to read groupValue off the RadioGroup
ancestor.
3. theme_extension.dart factories built FabledSwordTheme(...) without
const, even though all args are static const tokens. Added const to
the outer constructor calls in both .dark() and .light() — this
propagates const to the inner TextStyle(...) calls automatically.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a playlist has zero tracks, the detail screen previously showed
just the header with no indication of what to do next. Adds an inline
hint mirroring the web's copy: "No tracks yet. Add some via the
\"Add to playlist…\" entry on any track row."
Implementation: itemCount goes from tracks.length + 1 to
(tracks.isEmpty ? 2 : tracks.length + 1) and the builder emits the
hint at index 1 when tracks is empty. Header still renders at index 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to ccebd98 — the search_screen edit failed in that commit
because the file hadn't been read in the session. "No matches." →
"No matches for that query." Same scope as the larger sweep commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five terse "No X." strings replaced with the longer actionable forms
the web SPA uses on equivalent surfaces:
- Library Artists/Albums tabs: "No artists yet — scan a library folder
via the server's config." (mirrors web /library/artists, /albums)
- Library Liked tab: "No liked artists, albums, or tracks yet."
(consolidates web's per-section copy since Flutter shows all three
in one tab)
- Search no-results: "No matches for that query." (web has per-section
copy that doesn't fit Flutter's combined view)
- Discover no-results: "Nothing to add for that search yet." (mirrors
web /discover)
Hidden tab keeps its current actionable hint (web's "Nothing hidden
yet." is terser but lacks the "use a track's menu to flag" guidance).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four cases: dark factory pulls from FabledSwordDarkTokens, light from
FabledSwordLightTokens, flat tokens (accent / moss / etc.) are shared
between both, and the fromTokens() back-compat alias returns the dark
theme.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three RadioListTiles between Profile and Password sections. Tap →
themeModeProvider.notifier.set(...) → MaterialApp rebuilds with the
new theme via the existing themeMode wiring.
System has the only subtitle ("Match the device setting") — the
light/dark options are self-explanatory.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MaterialApp now provides both light and dark themes plus a
themeMode bound to themeModeProvider. ThemeMode.system rebuilds
the tree automatically when the OS toggles brightness — no manual
listener needed.
While themeModeProvider is loading (storage read in flight), falls
back to AppThemeMode.system, which inherits the OS setting.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AsyncNotifier wrapping flutter_secure_storage's "theme_mode" key.
Returns AppThemeMode.system as the default. set(mode) writes the
value + updates state.
Used by MinstrelApp (next commit) to drive MaterialApp.themeMode and
by the Settings appearance section to persist the operator's pick.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both builders construct a ThemeData with the matching brightness +
ColorScheme.dark/.light + the corresponding FabledSwordTheme
extension. buildThemeData() stays as a back-compat alias returning
the dark theme so the existing widget-test suite keeps using it
without churn.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both factories produce the same field shape (semantic role-based
names like obsidian/iron/parchment) with palette values from the
new FabledSwordDarkTokens or FabledSwordLightTokens generated
classes. fromTokens() stays as a back-compat alias returning the
dark theme so existing call sites keep working.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gen_tokens.dart now produces FabledSwordDarkTokens (dark surface),
FabledSwordLightTokens (light surface), and FabledSwordFlatTokens
(colors that don't change + radii + fonts). The original
FabledSwordTokens class stays as a back-compat alias re-exporting
the dark surface + flat values; any code that reads
FabledSwordTokens.obsidian etc. continues to work during migration.
Source: shared/fabledsword.tokens.json (already had dark/light/flat
splits — this was the generator catching up).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds optional actions: bool = true param. When true (default), renders
the 3-dot menu trigger at the end of the row. Album detail / Search /
History / Liked tabs all consume TrackRow so they pick up the menu
transitively.
Pair with c8baaa5 which wired the button into the other 3 surfaces;
this commit completes the wire-up by including TrackRow itself (its
write was missed in the previous commit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- TrackRow gains an actions: bool = true param; renders the button
after duration. Album detail + Search + History/Liked tabs all
consume TrackRow so they pick up the menu transitively.
- CompactTrackCard (home Most-played) gets the button at the end of
its inner row.
- _PlaylistTrackRow in playlist_detail_screen gets the button next
to duration; skipped for unavailable rows (no track id).
- NowPlayingScreen renders the button next to the title with
hideQueueActions: true (Play next / Add to queue suppressed since
the menu's track IS the playing one). artistId is left empty since
it's not stashed in MediaItem.extras — Go-to-artist will route to
/artists/ which is benign for v1; a follow-up could stash it.
Closes Tier 1 follow-up #2 (track-level actions menu).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Modal bottom sheet listing the caller's user-created playlists (system
playlists like For-You are filtered out — they're not user-mutable).
Tap a row to pop with the playlist id; consumer (TrackActionsSheet)
then calls PlaylistsApi.appendTracks.
Empty-state copy: "You haven't created any playlists yet."
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Modal bottom sheet with five reason chips (mirroring server vocabulary
bad_rip / wrong_file / wrong_tags / duplicate / other) plus an optional
notes field. Returns (reason, notes) on Hide, null on Cancel. Default
selection is bad_rip.
Used by TrackActionsSheet's Hide action.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 3-dot trigger and the modal-bottom-sheet menu it opens. 7 actions:
Play next, Add to queue, Like/Unlike, Add to playlist, Go to album,
Go to artist, Hide/Unhide. hideQueueActions: true suppresses the
first two for the Now Playing surface.
Sub-sheets (HideTrackSheet, AddToPlaylistSheet) land in subsequent
commits — this commit imports them speculatively, so CI between this
and the next two commits will fail. Resolved by Tasks 5 + 6.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three small data-layer additions for the upcoming track-actions menu:
- PlaylistsApi.appendTracks(playlistId, trackIds) wraps
POST /api/playlists/{id}/tracks for the "Add to playlist" action.
- audio_handler gains playNext (insertAudioSource at currentIndex+1)
and enqueue (addAudioSource) — both also push the audio_service
queue notifier so the queue-screen UI stays in sync.
- The AudioSource construction was extracted into a private
_buildAudioSource helper so setQueueFromTracks / playNext / enqueue
share one source-building path.
- PlayerActions exposes playNext / enqueue for menu use.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AsyncNotifier wrapping /api/quarantine/mine with optimistic flag /
unflag mutations and an isHidden(trackId) convenience for menu state.
Mirrors the LikedIdsController pattern: optimistic update, rollback
on server error.
Used by the next slice's TrackActionsSheet to power Hide / Unhide.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Dio client for /api/quarantine — flag a track with a reason + optional
notes, unflag by track id. Mirrors the server's user-scoped quarantine
endpoints (separate from /admin/quarantine).
Used by the track-actions menu's Hide/Unhide action in the next slice
of commits.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Audio handler accepts an AlbumCoverCache via configure() and uses it
to populate MediaItem.artUri with a file:// URI to a locally cached
album cover. Lock screen, Bluetooth car displays, Wear OS, and CarPlay
now show the album cover for the currently playing track instead of
the system's generic music icon.
Flow:
- _toMediaItem stashes album_id in MediaItem.extras
- After setQueueFromTracks pushes the queue + initial mediaItem, fires
_loadArtForCurrentItem async (doesn't block playback)
- Subscribes to _player.currentIndexStream so track advances trigger
the same loader for the new current item
- _loadArtForCurrentItem early-returns if cache is null, no current
item, no album_id, or artUri already set; otherwise calls
cache.getOrFetch and pushes an updated MediaItem with artUri set
- Race guard: if the user skipped to another track while the fetch was
in flight, the result is discarded
Closes the visible "generic music icon on lock screen" gap operator
flagged when first-testing on a real device.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the Riverpod provider (constructed with dioFactory pulling from
the authenticated dioProvider) and extends PlayerActions.playTracks
to forward the cache to audio_handler.configure(). The handler
signature change lands in the next commit.
Intentional intermediate-broken state: audio_handler.configure
doesn't yet accept coverCache. Resolved by the next task in the same
push.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fetches album cover bytes via the authenticated dio and writes to
<applicationCacheDirectory>/album_covers/<albumId>.jpg. Returns the
local path so MediaItem.artUri can point at a file:// URI — Android's
MediaSession framework fetches artUri itself without our Bearer
header, so a pre-fetched local file is the workaround.
Concurrent callers for the same albumId dedupe to one fetch via an
in-flight Future map. Any failure (network / 4xx / 5xx / disk full /
empty id) returns null without throwing — playback continues with
the system's generic music icon, same UX as today.
No eviction — covers are tiny, OS clears cache when space is tight.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Already a transitive dep via flutter_secure_storage and flutter_cache_manager.
Promoting to direct so we can import it from album_cover_cache (next
commit) for the lock-screen artwork cache directory.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three info-level lints from CI:
- playlist_card.dart doc comment: "/playlists/<id>" → "/playlists/{id}"
in backticks, since `<id>` was flagged as unintended HTML.
- compact_track_card_test.dart: home: Scaffold(...) is now const, which
propagates const to CompactTrackCard and makes the inner [_track]
implicitly const.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Expands the existing single test into four cases:
- 4 placeholder cards in Playlists row when no system playlists exist
- Empty-state copy for each section verbatim against the strings
in HomeScreen
- Real For-You card renders with the system badge when present
- Existing rollups (Recently added / Rediscover) still render correctly
alongside the new providers
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewrites HomeScreen body to match the web home layout:
- Playlists row: For-You + 3 Songs-like + user playlists, with
PlaylistPlaceholderCard for empty slots driven by
systemPlaylistsStatusProvider
- Recently added: 50 albums in 2 rows of 25 sharing a scroll controller
- Rediscover: separate scrollers for albums (square) and artists
(circular) — same condition as web's two-scroller branch
- Most played: 75 tracks in 3 rows of 25 using new CompactTrackCard
- Last played: existing single row layout preserved
Empty states use design-system understated-mythic copy mirrored
verbatim from web. Loading state uses DelayedLoading so brief network
blips don't flash a spinner.
Closes the visible parity gap the operator hit when first opening the
Flutter app on a real device.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lets multiple stacked HorizontalScrollRow instances render under a
single visible heading by passing title: "" to the second-row-onward
instances. Combined with the existing shared-controller support, this
gives us multi-row scrollers without a new widget.
Used in the next commit's home rewrite for Recently added (2 rows)
and Most played (3 rows).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bvandeusen