34 Commits

Author SHA1 Message Date
Deluan Quintão
ff033d8db6
chore(deps): upgrade to Go 1.27 (#5990)
* build: upgrade to Go 1.27

Bumps the toolchain in go.mod, both golang base images in the Dockerfile, and
the devcontainer VARIANT. CI needs no change, as the workflows resolve the
version through go-version-file: go.mod.

Tests, race tests, build and vet all pass on go1.27.0.

* build: upgrade golangci-lint to v2.13.0

v2.13.0 is the first release built with Go 1.27, so it can lint a module whose go
directive is 1.27. It also enables gosec's G404 on math/rand/v2, which flags the
three rand.Shuffle call sites. Shuffle order is not a security decision, and the
crypto-backed alternative in utils/random costs 25x and allocates per swap, so the
call sites are annotated rather than the rule excluded, keeping G404 active for the
cases where it would matter.

* chore(deps): update Go dependencies to latest versions

Signed-off-by: Deluan <deluan@navidrome.org>

* build: bump golangci-lint to v2.13.2

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-08-30 12:43:15 -04:00
Deluan Quintão
59448e9283
fix(scrobbler): back off when a provider asks us to, instead of retrying per play (#6028)
* feat(agents): retry-later error type with optional server delay

Add agents.ErrRetryLater and agents.RetryLaterError, which carries the
delay requested by an external service (e.g. ListenBrainz's
X-RateLimit-Reset-In). scrobbler.ErrRetryLater becomes an alias of the new
sentinel, so existing errors.Is checks and the plugin error-string protocol
keep working unchanged. Groundwork for honoring server-requested retry
delays across scrobbling, metadata agents and artwork.

Song.Equals tests moved to song_test.go to enable external test package.

* fix(scrobbler): honor backoff window and server-requested retry delay

ListenBrainz 429s were decoded into a typed error that classified as
unrecoverable, silently discarding the scrobble (a JSON-bodied 429 was
measured live). The client now maps any 429 to agents.RetryLaterError,
carrying X-RateLimit-Reset-In when present (capped at 1h). Last.fm error 29
(rate limit) is now retryable like 11/16. The buffer's drain loop no longer
lets wake signals bypass an active backoff window - new plays enqueue but
drain only when the window closes - and the wait honors the server delay
via max(backoff, retryIn).

* feat(agents): skip cooling-down agents in aggregate calls

When an agent reports retry-later, remember a per-agent cooldown deadline
(the server-requested delay, or 1 minute when unspecified) and skip that
agent in all aggregate metadata calls until it passes. A round that found
no data but skipped or saw a throttled agent returns ErrRetryLater instead
of ErrNotFound, so callers cannot mistake rate limiting for a definitive
'no data' answer.

* feat(artwork): honor server-requested retry delay when rescheduling

When an external image lookup fails with a retry-later error carrying a
delay (e.g. a 429 with X-RateLimit-Reset-In), the chain trace carries the
largest such hint back to the worker, which reschedules the item at
max(exponential backoff, server delay) instead of backoff alone.

* feat(plugins): retry-later with optional delay for scrobbler and agent plugins

Scrobbler plugins can now return scrobbler(retry_later:N) to request a
retry in N seconds (capped at 1h); the bare token keeps its old meaning.
Metadata-agent plugins, which had no error vocabulary at all, gain the
parallel agent(retry_later[:N]) token, mapped to agents.RetryLaterError so
the aggregate's cooldown and the artwork worker honor plugin throttling
the same way as built-in agents.

* fix: address whole-branch review findings for retry-later handling

Narrow the aggregate's throttled rule to the spec sentence: core.Agents returns
ErrRetryLater only when no agent answered at all (all skipped-cooling or
retry-later). An agent that does not implement the called method now returns an
internal errUnsupported instead of ErrNotFound, so it counts as "did not run" —
without that, the always-appended local agent would answer for biography, URL
and images and make ErrRetryLater unreachable.

Wire the consequence in core/external: a throttled round no longer stamps
ExternalInfoUpdatedAt (artist and album), so the empty result is not cached for
the TTL, and TopSongs maps ErrRetryLater to the same empty-200 the not-found
path already produced instead of a new client-facing error.

Move the Last.fm code-29 mapping into the client's central error construction so
every metadata path produces RetryLaterError, and map ListenBrainz's body-level
code 429 (sent with a non-429 HTTP status) the same way.

Clamp server- and plugin-requested delays in seconds before scaling to a
Duration, in all three parse sites: a header of 18446744074 wrapped past 2^64 and
came out as a 0.29s delay.

Also: extract the artwork worker's reschedule computation into retryDelay() and
cover both it and the trace RetryIn wiring with tests; collapse the double regex
call in mapScrobblerError; drop capabilities.ScrobblerErrorRetryLaterIn (ndpgen
never emits funcs, so plugin authors could not reach it); regenerate the PDKs so
MetadataAgentError reaches the Go and Rust SDKs; de-flake the cooldown tests
(long RetryIn for the skip case, separate expiry spec); and cover the max()
retry-delay aggregation across users in the scrobble buffer.

* refactor: dedupe retry-later parsing and simplify error collection

- Add agents.NewRetryLater and agents.RetryLaterFromSeconds, with a single
  1h cap, replacing the parse+clamp+multiply logic and the maxRetryInSeconds
  constant duplicated across listenbrainz, plugins and the agent adapter.
- Move HTTP header parsing to httpclient.RetryAfter, so the transport layer
  owns it and stays domain-agnostic; drop retryInFromHeaders from the
  ListenBrainz client. Covered by a new Ginkgo table in that package.
- Collapse the two near-identical plugin retry_later regexes into one
  parseRetryLater(prefix, msg) shared by the agent and scrobbler adapters.
- Fold the duplicated noteRetryIn snippet from fetchArtistImage and
  fetchAlbumImage into recordAgent, which already branched on the same
  isTransientExternal condition.
- Replace the atomic.Bool + note() closure in populateArtistInfo with
  errgroup's own error collection; the group carries no context, so a
  returned error does not cancel its siblings.
- Reuse recoveringScrobbler for the per-user delay test instead of a third
  double, and switch fakeScrobbler's mutex-guarded error to the
  atomic.Pointer idiom already used in the same package.

* refactor(listenbrainz): keep rate-limit header parsing in the adapter

The X-RateLimit-Reset-In header is ListenBrainz's own convention, not a
shared one: Last.fm sends no rate-limit headers at all and reports its
limit as a body code, and no other integration in tree sends Retry-After.
A parser in utils/httpclient implied a uniformity across services that
does not exist, so it moves back next to the only client that can know
which header its service sends.

* refactor(agents): collapse the retry-later sentinel and error into one type

ErrRetryLater is now the zero-delay RetryLaterError rather than a separate
errors.New value, so errors.Is and errors.AsType both match the sentinel and
every delay-carrying variant. That removes the trap where a bare sentinel
silently skipped the AsType path, and lets every consumer read the delay off
the error directly: the RetryIn accessor and the two constructors are gone,
with the policy cap applied where untrusted input is parsed.

* refactor(agents): split the cooldown store from the per-dispatch tally

The cooldown map and mutex become a cooldowns value with active/park, holding
no knowledge of errors; agentAttempts records one dispatch's outcomes and owns
the classification that noteAgentError used to hide behind a bool. The three
dispatch loops now touch a single object: skip folds the cooldown check and the
throttled flag into one call, so the store never appears in the loops.

* refactor(agents): share one dispatch loop between the agent call helpers

callAgentMethod and callAgentSliceMethod ran identical loops, differing only in
how they test a result for emptiness: a slice cannot be compared against its
zero value, so the two could not share a constraint. Both now delegate to
callAgent, which takes that test as a parameter. Keeping the loop in one place
matters more than the lines saved: it holds the cooldown skip, the attempt
recording and the empty-dispatch verdict, and a fix applied to one copy but not
the other would be silent.

* test: cover the two retry-later paths a mutation could break silently

Both gaps were proven, not guessed: making the artwork worker pass 0 instead
of the collected hint left all 386 specs green, and replacing the default
agent cooldown with 0 left the agents suite green. The worker test drives a
throttled image agent through drain and asserts the persisted retry_at, and
the cooldown test parks an agent that asked to be retried without naming a
delay, which is what Last.fm does on every rate limit.

* refactor(artwork): carry the external failure as an error, not a flag plus a trace field

The retry delay was riding on ChainTrace, a diagnostic that gets persisted, while
the very same signal — an external source faulted — already travelled by value as
resolution.extError. That was two mechanisms for one idea, and it put control-flow
state inside a serializable trace.

resolution.extError and chainState.extErr become the error itself, so a caller
checks err != nil for the fault and errors.AsType for the delay the provider asked
for. The agent loops return that error last, per convention, and longerRetry keeps
whichever failure wants the longer wait. ChainTrace goes back to holding only steps
and no longer imports core/agents.

* fix(artwork): check the resolve error before reading its resolution

Reading res.extError before the err check was safe only because every error path
in resolve returns a bare resolution{}; a future path returning a partly-filled
one would have been read silently. The failure path now returns no delay
explicitly.

* test(artwork): assert the delay acquire reports, not just its downstream effect

acquire's retry delay was only covered through the worker's persisted retry_at,
one layer away from where the value is computed. Both outcomes are now pinned at
the processor: a plain failure asks for nothing, a throttled provider's delay is
passed through.

* refactor: share the retry-seconds parse and drop the backoff deadline arithmetic

The clamp-before-scaling invariant lived in two parsers and was independently
re-tested in three files with the same magic number; a fix applied to one copy
would have left the others wrapping a huge value down to a fraction of a second.
It moves to agents.ParseRetryIn.

The buffer tracked an absolute retryDeadline only to re-arm a timer that was
already armed for the same instant; a backingOff flag says the same thing without
the arithmetic. The plugin token regex now carries its capability in the pattern
instead of capturing and comparing, so another capability's token in the same
message cannot mask it. resolution.extError becomes extErr, matching its
chainState counterpart.

* fix(agents): keep the longer cooldown when parks overlap

Calls to one agent overlap, so a short cooldown could land after a long one
started and cut it short. park now keeps whichever deadline is later, matching
the rule longerRetry already applies on the artwork side. No in-tree provider
can currently produce two different delays for the same agent, so this is
hardening rather than a fix for observed behaviour.

* fix(agents): parse the retry delay at a fixed width

strconv.Atoi parses into the native int, so on the 32-bit targets we ship
(linux/386, windows/386, three ARM variants) a delay above MaxInt32 seconds
overflowed and became unspecified instead of being capped. No provider sends a
68-year delay, so this is not user-visible, but the overflow tests asserted the
cap and would have failed on those architectures, where tests never run.

* fix(plugins): anchor the retry_later regex to a word boundary

Prevents a superstring like useragent(retry_later) from matching the
agent capability token.
2026-08-29 17:28:29 -04:00
Deluan Quintão
f08b5297ee
feat(ui): add Refresh Metadata to the album and artist context menus (#6036)
* refactor(artwork): move artworkItemName into core/artwork as ItemName

* feat(external): add RefreshInfo to force an external info refresh

RefreshInfo re-fetches and re-saves external info for one artist or
album, bypassing the TTL check that UpdateArtistInfo/UpdateAlbumInfo
use. It is synchronous; callers that must not block detach it themselves.

Also makes MockArtistRepo/MockAlbumRepo.UpdateExternalInfo persist to
Data (previously a no-op) and adds the new method to the e2e noopProvider,
both required so the interface addition compiles and is observable in tests.

* feat(external): broadcast RefreshResource after external info is saved

populateArtistInfo and populateAlbumInfo now emit the same RefreshResource
event the artwork worker uses, so the UI learns about both foreground and
background metadata refreshes.

* feat(nativeapi): replace artwork refresh endpoint with metadata refresh

* feat(ui): add refreshMetadata to the data provider

* feat(ui): add a Refresh Metadata item to the album and artist context menus

* fix(ui): re-fetch artist info when the record is refreshed

* test: fix mislabeled spec, add kind-gate negative case, guard nil mock maps

- Rename the RefreshInfo spec that claimed to cover the save-failure/broadcast
  path: SetError(true) fails Get too, so it only proves RefreshInfo bails out
  early at getArtist.
- Add a spec proving playlist refreshes skip the external-info step, since
  that asymmetry (al/ar only) was documented but unasserted.
- Add lazy nil-map init to MockAlbumRepo/MockArtistRepo.UpdateExternalInfo so
  a composite-literal-constructed mock doesn't panic on first save.

* test: relocate discArtworkName specs from cmd to core/artwork

artworkItemName moved into core/artwork as ItemName in an earlier commit, but
its disc-name specs stayed behind in cmd/artwork_test.go, reaching across
packages. Move them to core/artwork/item_name_test.go where the code now lives.

* fix(ui): shape refreshMetadata like a react-admin response

react-admin validates custom dataProvider methods and rejects any response
without a `data` key, so the raw httpClient promise made every click surface
an error toast instead of the success message. The unit test mocked
useDataProvider, which skips that validation.

Also folds "which kinds have external info" into external.HasInfo so the
handler stops restating it, drops the nil-broker guard that only existed for
tests, and delegates the mocks' UpdateExternalInfo to Put.

* refactor(external): unexport infoKinds

Only HasInfo is used outside the package, so the slice itself does not need
to be exported.

* refactor(artwork): fold ItemName into housekeeping.go next to Refresh

ItemName exists to guard Refresh from ids that would orphan a queue row, and
both callers invoke them back to back. A separate file hid that pairing; it was
only split out to keep the move out of cmd/ legible in review.

* fix(nativeapi): return 500 when the refresh lookup fails for a non-ErrNotFound reason

A transient repository error told the admin the id did not exist, and the error
was dropped without a log line, so nothing pointed at the real cause.

Also drops the inherited claim that clearing artwork state shows a placeholder.
Reads fall back to local resolution, so that only holds when there is no local art.

* fix(ui): move Refresh Metadata above Get Info in the context menu

Menu order follows key insertion order in the options object, so the new spec
pins the position rather than leaving it to be shuffled by the next addition.
2026-08-25 23:59:40 -04:00
Deluan Quintão
757ca783d3
fix(instant-mix): top short mixes up instead of returning what the first source found (#5951)
* fix(instant-mix): top short mixes up instead of returning what the first source found

SimilarSongs returned the first non-empty source's tracks, however few. For a
thinly-represented artist that meant a 3-track mix no matter the requested count:
the artist agent found nothing, the similar-artists fallback matched 3 library
tracks, and `len(res) > 0` kept seed-track sampling from ever running.

Clients treat that as a failed mix and retry with a bigger limit forever. Finamp
cycles limit 34 through 472 and starts over, ~1 request every 2s indefinitely,
each one re-hitting Last.fm, Deezer and AudioMuse.

Sources are now chained rather than raced: each one tops the mix up until it
holds count tracks, so the agent's picks, the similar-artists fallback and
seed-track sampling all contribute instead of the first one winning outright.

* refactor(external): move similar-songs code to its own file

provider.go held two distinct concerns: artist/album external metadata and the
similar-songs mix pipeline. The mix code was already one contiguous block, and
maxSeeds, maxSimilarSongs and dedupByID were used by nothing else.

Moved SimilarSongs and its helpers to provider_similarsongs.go, matching the
existing provider_similarsongs_test.go. Pure code motion: the moved block is
byte-for-byte unchanged and provider.go has no additions, only deletions.

* fix(instant-mix): dedup before deciding a mix is full

topUp measured res before deduplicating it. Matcher.MatchSongs deliberately
re-emits a library track when the same input song repeats, and the similar-artists
fallback can reach one track through several artists, so len(res) could equal count
while holding fewer unique tracks. That returned a mix with duplicates in it and
stopped the top-up early; the caller then deduplicated and handed back a short mix,
which is the client retry loop this branch set out to fix.

Deduplicate first, so the length check counts what the client will actually receive.

* perf(instant-mix): skip a fallback once the mix is already full

The artist path nested one topUp inside another, so the inner one measured only
similarSongsFallback's own result against the full count. With 49 agent matches and
one fallback match for count=50 the mix was already full, yet seed-track sampling
still ran and fired up to five GetSimilarSongsByTrack calls whose results the outer
topUp then truncated away.

topUp now takes the sources as a variadic list and re-checks the accumulated mix
before each one, so a later, costlier source only runs while the mix is still short.
That also flattens the artist case: the agent, the similar-artists fallback and
seed-track sampling are now three peers in one chain instead of two nested calls.

* fix(instant-mix): count distinct tracks when picking the fallback mix

similarSongsFallback stopped after count picks from the weighted chooser, but a
track can sit in that chooser once per artist listing it in their top songs, and
Pick removes the entry it returns. Repeats therefore consumed pick slots and left
unique candidates stranded, so the batch could come back short of count. On the
track path this is the only source, so that short mix reached the client and kept
the retry loop alive.

Track the ids already picked and keep drawing until count distinct tracks are held
or the chooser is empty.

* fix(instant-mix): match the whole agent response before trimming

MatchSongs was capped at count, and it re-emits a track when the same song repeats,
so [A, A, B] with count=2 returned [A, A] and never reached B. topUp then shrank
that to [A] and, with an empty or overlapping fallback, the mix stayed short even
though B had been available all along. seedMix already matched its full merged set
for this reason; mixFromAgent now does the same and leaves the trim to topUp.

Also drop the capacity hint on the picked-ids map. It was sized from the caller's
count, which CodeQL flags as an allocation sized by user input (go/uncontrolled-
allocation-size). SimilarSongs clamps count to maxSimilarSongs long before this
point, so the hint bought nothing worth the alert.

* refactor(instant-mix): tidy the mix chain and its specs

Quality pass over the new code, no behaviour change:

- topUp: drop the first-vs-last error bookkeeping (the value is only read when the
  mix is empty, so the distinction is unobservable) and the redundant nil guard
  (dedupByID returns nil for an empty result, so both branches already agreed).
- mixFromAgent: assign through the if-scoped err instead of a second error name.
- Hoist the similar-artists fallback closure written verbatim in two switch arms.
- Use map[string]struct{} in the pick loop, matching dedupByID in the same file.
- Trim three comments back within budget; two restated the line below them and one
  carried commit-message rationale.
- Tests: add an ids() helper for the ID assertion repeated seven times, and fold
  the track-entity stub block copied into three specs into stubTrackEntity. The
  block hard-coded .Twice() on GetEntityByID, which pinned an implementation
  detail no spec asserts.

* revert(instant-mix): inline the similar-artists fallback closure again

Hoisting it to a shared artistFallback var moved the call away from the arm that
uses it and saved nothing: each arm reads better spelling out its own sources.
2026-08-13 18:50:34 -04:00
Deluan Quintão
6c3e7e268b
feat(instant-mix): support album, playlist and genre sources (#5948)
* feat(agents): local agent genre-hint similar songs fallback

* feat(external): playlist instant mix via seed-track sampling

* test(external): cover playlist mix never-empty fallback and maxSeeds cap

Adds coverage for the empty-match seed fallback and the maxSeeds
call cap on GetSimilarSongsByTrack, per code review finding.

* feat(external): genre instant mix via seed-track sampling

* feat(external): album instant mix falls back to AudioMuse track similarity

* feat(external): artist instant mix falls back to seed-track sampling

* fix(jellyfin): route genre seeds through instant mix instead of empty

* feat(jellyfin): add /Albums/{id}/Similar route for albumMix radio

* perf(external): bound playlist seed sampling to a random N

samplePlaylistTracks loaded an entire playlist's joined rows just to keep
5 random seeds; push the bound and randomization into the query instead,
matching the other samplers (GetRandom/GetAllByTags with Max).

Fixing this surfaced a real bug: resetSeededRandom's SEEDEDRAND rewrite
assumed every table's id is TEXT, but playlist_tracks.id is an INTEGER
position, so the random sort silently dropped every row. Cast the id to
TEXT before hashing (no-op for the other, TEXT-id tables).

Also trims a changelog-flavored comment and a duplicated rationale in
server/jellyfin/similar_test.go.

* refactor(external): parallelize seed mix and dedup mix helpers

Run the up-to-5 per-seed GetSimilarSongsByTrack calls concurrently (errgroup),
route the four container cases through a shared seedMix helper, flatten the
genre lookup, and sample playlist seeds without forcing a smart-playlist
rebuild. Share the media-file->Song mapping in the local agent.

* perf(agents): use the indexed genre filter for local similarity

Replace GetAllByTags (a json_tree scan of every media_file row) with the
media_file_tags semi-join from #5940, deriving the seed's genre tag ids
locally since they hash from (name, value).

Also carry the library id and the recording MBID on the returned songs:
the matcher resolves by id first and looks up mbz_recording_id, so the
release-track id it got before matched nothing and the local fallback
silently returned no songs.

* refactor: drop redundant MBID and fold mixFromSeeds into seedMix

The local agent returns library tracks, so the id alone resolves them in the
matcher's first phase; the MBID was never consulted. mixFromSeeds had no
caller other than seedMix.

* docs: trim redundant comments

* fix(jellyfin): adopt the GUID id codec in the merged similar routes

getSimilarAlbums still used resolveItemID/DecodeID, which #5942 replaced with
itemIDParam; its tests passed raw ids that the strict codec now rejects.

* fix(external): guard non-positive counts and blend every seed

A negative Subsonic count reached matched[:count] and panicked. The matcher
also keeps input order and stops at count, so seed-grouped results let the
first seed fill the whole mix; interleaving gives every seed a share.

Drops the duplicate playlist-track mock in favour of tests.MockPlaylistTrackRepo,
which pages like the real repository and records the query options.

* fix(external): refresh smart playlists before sampling seeds

A smart playlist materializes no playlist_tracks until it is evaluated, so
sampling without the refresh mixed an empty seed set. The refresh is a no-op
for regular playlists, inside the refresh delay, and for non-owners.

* fix(external): skip missing tracks and a nil playlist-track repo when sampling

Tracks() logs and returns a nil repository when its own lookup fails, so the
chained GetAll panicked. Seeds can also reach the mix verbatim when the agents
find nothing, so a missing file would surface as an unplayable entry.

* fix(jellyfin): never report the seed album as its own similar album

The sampled-seed fallback returns the album's own tracks, which similarAlbums
mapped straight back to the requested album, often as the only result.

* test(agents): assert the genre predicate instead of relying on the mock

MockMediaFileRepo ignores QueryOptions.Filters, so the spec passed even with
no genre filter at all. It now checks the generated predicate carries the
seed's own tag id, the indexed join and the missing exclusion.

* fix(external): clamp the requested count before it becomes a query limit

Subsonic passes the client's count through unbounded. At MaxInt64 the local
agent's count+1 overflows negative, and GetRandom omits the SQL limit unless
Max is positive, so one request would hydrate every matching track. 500 is
what the widest caller (similarAlbums, limit*5) legitimately asks for.

* fix(external): deduplicate playlist seeds by media file

A playlist can hold the same file at several positions, so sampling its rows
could seed the mix twice: a wasted agent call, and a duplicate track whenever
the seed fallback kicks in.

* fix(external): drop tracks two seeds both recommend

The matcher re-emits a track when two inputs are identical, so overlapping
recommendations took several slots in the mix. Match the whole merged set and
dedup before trimming. Playlist sampling now over-fetches before its own
dedup, so repeated positions cannot collapse the seed count.

* test(external): make the seed-blend assertion independent of the shuffle

It matched four tracks and kept two at random, so both could come from the
first seed once in six runs. Keeping three of the four makes a seed-two track
unavoidable.

* fix(external): seed artist mixes from every credited role

media_file.artist_id is the deprecated primary artist, so an artist credited
only on the album, as on compilations, sampled no seeds at all. Use the same
participant filter the artist listings use.

* refactor(external): drop the now-vestigial seed interleaving

Matching the whole merged set removed the early truncation the interleave
guarded against, and the shuffle before the trim makes input order irrelevant.
Its comment described the old behaviour.

* test(agents): give the id-mapping fixture a matching genre

The related track carried no genre, so the real query would never return it;
the spec only passed because the mock ignores QueryOptions.Filters.

* test(agents): drop the MBID from the id-mapping fixture

Local agent candidates are non-missing library rows, so the matcher always
resolves them in its id phase and never reads the MBID. The field guarded a
regression that could not change behaviour.

* test(agents): remove unnecessary comment about MBID in GetArtistTopSongs test

* fix(jellyfin): only let a not-found entity fall through in getInstantMix

Discarding the error conflated a genre id, which never resolves, with a real
lookup failure, which then made a provider call that fails the same way.

* test: pin the invariants the specs only appeared to cover

The missing filter was asserted by substring, so flipping it to true passed
everywhere, including the spec named for it. Matching the whole merged set,
the local agent's over-fetch, and its no-genres early return had no coverage
at all; each is now pinned by a spec that fails when the code is broken.

* test: make the remaining specs say what they actually guard

The playlist-track spec named a sort whitelist it does not exercise; it guards
the integer-id CAST, so it now asserts no rows are dropped. The maxSeeds cap
passed with either bound removed, and the over-fetch was pinned by its literal
value rather than the duplicate positions it exists for. Also drops setup the
count guard returns before reaching.

* fix(external): fall back when the agent's picks are not in this library

A non-empty answer whose songs are all absent locally matched nothing and was
returned as-is, so the mix came back empty with sampleable source tracks
sitting right there.

* refactor(external): name the agent-then-fallback flow once

Each entity case repeated the same error and emptiness plumbing around the
matcher. mixFromAgent states it once and each case supplies only what differs:
how to ask, and what to do when the answer is unusable.
2026-08-12 23:02:13 -04:00
Deluan Quintão
944ca3100f
feat(artwork): new artwork pipeline with background resolution and Low Quality Image Placeholders (#5847)
* feat(artwork): add artwork, item_artwork and artwork_queue tables

* feat(artwork): add artwork models, repository interfaces and mocks

* feat(artwork): implement artwork repository

* feat(artwork): implement item_artwork repository with batched hydration

* feat(artwork): implement artwork_queue repository

* feat(artwork): add content-addressed originals store

* feat(artwork): add artwork prune (orphan cleanup)

* fix(artwork): never sweep files on transient DB errors during prune

* refactor(artwork): fold originals package into core/artwork as ImageStore

* refactor(artwork): merge item artwork state into ArtworkRepository

* fix(artwork): chunk unbounded IN clauses and restore interface docs

* refactor(artwork): apply simplify-pass cleanups

Internal item_artwork sqlRepository helper, toSQLArgs upserts, batched queue enqueue, EnqueueStaleAbsent moved to queue repo, snapshot-based prune sweep, mock/real semantics aligned.

* fix(artwork): address review findings on prune/sweep races and mock fidelity

Sweep now honors an mtime grace window (in-flight acquisitions and temp files), reacquired orphans reset the prune grace window, and the queue mock implements real stale-absent semantics.

* fix(artwork): atomic orphan deletion and timestamp semantics from review

DeleteOrphans re-checks age+references at delete time, PutItemArtwork defaults attempted_at, queue mock timestamps mirror SQL.

* fix(artwork): guard orphan file removal with the prune grace window

Duplicate ImageStore writes refresh the file mtime and Remove skips files newer than the cutoff, so overlapping acquisitions cannot lose their store files to a concurrent prune.

* fix(artwork): rewrite vanished duplicates and sweep stale mime variants

Write falls through to a real write when the liveness touch fails, and sweep retention now matches the recorded mime's extension so obsolete variants are reclaimed.

* fix(artwork): index artwork_queue in dequeue order

The previous leading retry_at range column forced a temp B-tree sort of the whole eligible set on every DequeueBatch; ordering the index by (priority DESC, enqueued_at) lets scans stop after the batch size.

* fix(artwork): honor the orphan cutoff in the repository mock

The mock's DeleteOrphans now applies createdBefore like the SQL implementation, and a new spec covers a freshly reacquired row surviving prune.

* fix(artwork): reject malformed hashes in ImageStore operations

Known-absent states carry an empty hash and malformed persisted hashes could panic path sharding or inject separators; Write/Open/Remove now return an error for anything but 16 lowercase hex chars.

* fix(artwork): mock PutImage refreshes created_at like the SQL repository

Prune specs now age fixtures directly instead of seeding stale timestamps through the upsert.

* fix(artwork): store backing-file provenance per item, not per hash

* feat(artwork): import blurhash encoder from #5797

* feat(artwork): add worker-side artwork resolvers

* fix(artwork): propagate playlist tile failures and dedupe external step

* feat(artwork): add acquisition processor

Resolves one queue item end to end: hash/dedup, decode + 128px thumbnail
blurhash, place bytes (store vs source file), and persist found/absent/
failed state for the worker (Task 4) to act on.

* style(artwork): tighten processor comments to budget

* feat(artwork): add acquisition worker service

* feat(artwork): enqueue artwork resolution from scan and CRUD paths

* feat(artwork): artwork backfill, fingerprint re-resolution and scheduled jobs

* test(artwork): leak/soak coverage and deferred assertions

* fix(artwork): propagate transient artist image errors to the worker

callGetImage swallowed all agent errors, so an agent outage surfaced as ErrNotFound and the worker settled artist artwork as a definitive absent (and reset the breaker). Add an additive ArtistImageResult path that returns the underlying agent error on transient failure while keeping ArtistImage byte-identical for existing callers; the worker's artist external step uses it via fromArtistExternalResult.

* fix(artwork): resolve full playlist source chain

resolvePlaylist only built the generated grid, dropping the uploaded-image, sidecar and ExternalImageURL sources the old reader_playlist.go chain serves. Port the full chain before the grid fallback: uploaded (upload), sidecar (folder), and ExternalImageURL routed through extGate with the same extError semantics as the other external steps. Also rewires the artist external step onto ArtistImageResult.

* fix(artwork): purge dangling queue rows and guard concurrent re-enqueues

Queue rows for deleted entities failed forever (Get -> ErrNotFound -> failed -> capped retries, unbounded). Add ArtworkQueueRepository.PurgeDangling, called from Prune next to the item_artwork purge. Separately, the found/absent path unconditionally deleted the dequeued row, erasing a concurrent scan re-enqueue; switch to DeleteIfUnchanged, which deletes only while retry_at still matches the dequeued value (verified retry_at is the column an Enqueue upsert resets).

* style(artwork): fix comment accuracy and budget; fingerprint ArtistImageFolder

Correct the inverted workerDeps.extGate comment, trim over-budget doc comments, and add conf.Server.ArtistImageFolder to the resolution fingerprint so an image-folder change re-resolves artist artwork.

* fix(artwork): treat missing local playlist cover as definitive, not transient

A playlist ExternalImageURL pointing at a local file that fails to open was
routed through extError, causing failed/48h-retry loops that burn a rate
limiter token forever instead of falling through to the generated grid.

* refactor(artwork): deduplicate purge loop, backfill table, and extGate alias

* fix(artwork): cap resolved image reads

A user-editable ExternalImageURL can point at an arbitrarily large endpoint;
a fast server could make the worker buffer hundreds of MB inside the 5s HTTP
timeout. Bound the read to a fixed 20MB cap (no config knob) via io.LimitReader
and fail the item if it is exceeded.

* fix(artwork): retry higher-priority external art after fallback hit

With CoverArtPriority="external,cover.jpg", a transient external failure
followed by a folder hit dropped the external error: the worker recorded
found and deleted the queue row, so the configured higher-priority external
art was never retried. Carry extError onto the fallback resolution and add
an outcomeFoundStale that persists+serves the art but reschedules via
MarkFailed, giving the external source another chance. When external later
answers definitively-not-found, the hit is not stale and the row is deleted.

* fix(artwork): treat playlist cover URL 404 as definitive miss

The playlist ExternalImageURL step used sources.go's fromURL, which maps any
non-200 to a generic error, so a stale URL returning 404/410 was classified
transient: infinite backoff plus it counted toward the circuit breaker,
blocking valid external work. Add a local fetch in resolve.go that maps
404/410 to model.ErrNotFound (definitive) while keeping other non-200s
transient. sources.go is left untouched.

* test(artwork): move soak test into the Ginkgo suite

* test(artwork): make leak and permission tests pass on linux

goleak now ignores notify's nonrecursive-tree goroutines (linux uses
inotify, which spawns dispatch+internal instead of darwin's recursive
dispatch), and the read-only-dir prune spec skips under root, where
permission bits cannot make Remove fail.

* fix(artwork): reject decompression-bomb dimensions before decoding

* fix(artwork): keep fresh re-enqueues ahead of stale failure backoff

* fix(artwork): include M3U external art flag in the config fingerprint

* fix(artwork): resolve private playlists with an admin context

* test(artwork): convert non-synctest timing tests to Ginkgo specs

TestArtworkBackoffSchedule and TestArtworkWorkerRunNoLeak needed no real
*testing.T (no synctest), so move them into worker_test.go as Ginkgo
specs. TestArtworkBreakerHalfOpen stays plain since testing/synctest
requires a real *testing.T, matching core/scrobbler's precedent.

* fix(artwork): store backing-file provenance per item, not per hash

* fix(artwork): apply image limits to playlist tile decoding

decodeTile ran image.Decode on every sampled album's resolved bytes
before processItem's maxImageBytes/maxImagePixels guards applied,
letting an oversized or decompression-bomb tile fully decode
unbounded. Enforce both caps inside decodeTile itself.

* refactor(artwork): reuse auth.WithAdminUser and dedupe image cap guards

* perf(artwork): fetch only IDs for backfill enumeration

Backfill enumerated every album, artist, playlist and radio via GetAll
and mapped out just the ID. GetAll materializes full entities (library
joins, participant/stats/tags JSON, annotation, artwork hydration), so on
a large library it loaded tens of thousands of heavy structs only to read
one field each — spiking transient RSS to ~1GB during the one-time
upgrade backfill, a memory risk on small NAS/Pi hardware.

Add GetAllIDs to the album, artist, playlist and radio repositories: it
reuses each repo's base row-set filter (library visibility, artist
content join, playlist userFilter) but projects only id, skipping the
heavy columns and post-processing. A per-repo parity test asserts
GetAllIDs returns exactly the same id set as GetAll.

Verified on a 727MB / 29k-artist production DB copy: peak RSS during
backfill dropped from ~1012MB to ~89MB, file descriptors flat, same
36,138 items enqueued.

* feat(artwork): promote worker concurrency and external rate to real configs

The artwork worker's drain speed was governed by two hidden Dev flags,
DevArtworkWorkerConcurrency and DevArtworkExternalRPS, both defaulting to 2.
On a large library's one-time backfill the external rate limiter is the real
ceiling: every art-less item waits on it before the (rate-limited) external
lookup, so the drain crawls at ~RPS items/sec while local-art items are
unaffected.

Promote both to documented, supported options: ArtworkWorkerConcurrency
(default 4) sets local-resolution parallelism, ArtworkExternalMaxRPS
(default 2, 0 = unlimited) caps external-agent lookups to stay polite to
Last.fm/Deezer/etc. Operators can now trade first-backfill speed against
external-API rate limits. The old Dev names still map for backward compat.

* fix(deezer): never return empty-image-id placeholder pictures

* feat(agents): enumerate enabled image-retriever agents per capability

* feat(artwork): worker fetches agent images directly with per-agent rate limits and breakers

* fix(artwork): treat agent not-found as breaker success

* feat(model): content-hash artwork id suffix and hydratable per-entity image state

* feat(persistence): hydrate artwork hash and absence onto entity pages

* feat(artwork): resolve media_file embedded art in the worker, invalidate on rescan

* feat(artwork): broadcast refresh events when artwork lands

* fix(artwork): broadcast refresh for stale-found artwork too

* feat(artwork): state-backed serving path with provisional read-through

* feat(server): serve artwork from persisted state with content-hash caching

* feat(subsonic): content-hash coverArt ids, omit artwork on known-absent

* refactor(artwork): delete the legacy reader chain, cache warmer, and provider image methods

* feat(artwork): precache on acquisition, bump on upload/radio changes, manual re-resolve API

* test(artwork): end-to-end coverage for the serving cutover

* chore(artwork): generic 500 bodies on refresh endpoint, trim stale test comments

* fix(artwork): request read-through must not reset the failure backoff

The provisional read-through and dangling re-enqueue used Enqueue, whose upsert
resets retry_at, so any browse of an unresolved entity that was backing off after
an external failure made it immediately eligible again — defeating the exponential
backoff during a provider outage. Add EnqueueBump, which raises priority but leaves
an existing row's retry_at intact, and route the serving path through it. Scan and
manual re-resolve keep Enqueue's reset (a detected change wants immediate retry).

* fix(artwork): keep an eligible track's cover requestable when its album is absent

An embedded-eligible track with no resolved item_artwork row inherited the album's
ImageAbsent, so when the album resolved absent (e.g. CoverArtPriority without
'embedded') the track's coverArt was omitted permanently — the client never
requested it, so the lazy mediafile path never resolved it — even though the
serving path would extract and serve the track's own embedded art. Hydration now
never copies the album's absence onto an eligible-but-unresolved track.

* fix(artwork): validate each agent image URL before picking the largest

bestImageURL selected the largest by size and only then parsed it, so a malformed
largest URL (e.g. a bad percent-escape) returned nil and shadowed a valid smaller
candidate, contradicting the documented skip-unparseable behavior. Parse per
candidate and compare sizes only among URLs that parse.

* fix(artwork): fall back to disc art, not the album, for multi-disc tracks

serveMediaFile delegated an absent/ineligible track straight to AlbumCoverArtID,
skipping the disc-specific lookup that MediaFile.CoverArtID (and the deleted legacy
reader) use. On multi-disc albums with per-disc images that served the album cover
instead of the configured disc artwork. Delegate through DiscCoverArtID.

* fix(artwork): enqueue uploaded artwork only after the filename is persisted

SetImage cleared state and enqueued the bump before the caller stored the new
filename, so a worker drain in that window could resolve against the old (already
deleted) file and settle absent, leaving the upload unused until a later scan. Move
the invalidate+enqueue into EnqueueArtwork, which each caller now invokes after the
entity Put.

* fix(artwork): keep multi-disc tracks requestable when the album is absent

Round-1's hydration fix still copied the album's known-absent onto a non-eligible
(or own-absent) track, but MediaFile.CoverArtID routes a multi-disc track to disc
art, which resolves provisionally and is never known-absent. Marking it absent made
Subsonic omit coverArt so clients never requested a valid disc image. Only mark a
single-disc track absent, and only when its own art won't resolve.

* fix(artwork): serve a local playlist ExternalImageURL as a file-backed reference

A local ExternalImageURL was resolved through the external step and labelled
external, so placeBytes copied it into the content-addressed store and dropped its
path/mtime — replacing the file never tripped the staleness check. Classify local
references as file-backed (resolved in place, even on the request path) and keep
store-backed behaviour only for http(s) URLs.

* fix(artwork): requeue playlist cover when its track set changes

A generated-grid cover went stale after track mutations: nothing re-resolved the
playlist's artwork, and the request path deliberately never rebuilds the grid, so
serveEntity kept returning the old grid hash indefinitely. Enqueue pl artwork from
refreshCounters (the choke point for every track-set change); no clear, so the old
cover keeps serving until the worker rebuilds.

* fix(artwork): open library-backed artwork through its on-disk root

A library configured with a file:// path stored absRoot as the raw URI, so Abs
produced strings like file:/music/cover.jpg that os.Open/os.Stat reject — folder,
upload and embedded art were treated as dangling on every request, looping forever.
Normalize a file:// path to its parsed OS path (the same root os.DirFS uses);
non-local schemes are left unchanged (out of scope, per the artwork-musicfs TODO).

* fix(artwork): only use disc resolution for multi-disc albums

DiscCoverArtID returns a dc- id for any track with DiscNumber>0, so serveDisc ran
the full DiscArtPriority chain even for single-disc albums, where a stray disc*/
embedded image could shadow higher-priority album art. Gate disc resolution on the
album having more than one disc, matching the legacy reader; single-disc tracks
serve album art directly.

* fix(artwork): invalidate artwork when an uploaded image is deleted

Deleting an artist/radio/playlist upload cleared the filename but left the found
item_artwork row and its hash, so lists kept advertising the deleted cover's
hash-suffixed immutable URL and clients could display it indefinitely. Call
EnqueueArtwork after the delete-side Put, symmetric with upload, so the state is
cleared and re-resolved to the next source (or absent).

* fix(artwork): restore synthetic-artist guard and unicode normalization in agent lookups

Moving agent calls into the worker bypassed two behaviors of the aggregate provider:
Agents.GetArtistImages' guard for Unknown/Various Artists (a direct retriever call
could assign an unrelated image to a synthetic artist), and auxAlbum/auxArtist.Name's
DevPreserveUnicodeInExternalCalls normalization (records with typographic quotes/dashes
missed exact-name searches). Re-apply both before enumerating retrievers.

* fix(artwork): enforce entity visibility on the Subsonic getCoverArt path

serveEntity reads persisted item_artwork by id, bypassing the library and private-
playlist filters that the legacy entity-load applied. On the authenticated Subsonic
path a user could fetch artwork for an inaccessible album or someone else's private
playlist by guessing an id. getCoverArt now resolves the underlying entity through
the request-scoped (filtered) repositories and serves the placeholder when it is not
visible, so existence isn't leaked and the always-an-image invariant holds. The
public share (JWT-authorized) and Jellyfin (admin) paths are intentionally untouched.

* fix(artwork): version the artwork ETag with the served representation

The ETag was the pixel hash of the original image, so a CoverArtQuality or
EnableWebPEncoding change altered the resized bytes without changing the ETag —
revalidating clients got a spurious 304 and kept the old encoding. Resized responses
now carry a representation ETag (hash + size + square + encode settings) used for the
ETag header and If-None-Match, while the immutable decision stays on the pixel hash
(URLs remain pixel-identity per the spec, so hash-suffixed clients keep zero-request
caching). Full-size originals fall back to the pixel hash as before.

* fix(artwork): don't stamp the album hash onto multi-disc tracks

The hydration fallback assigned a found album hash to every fallback track, but a
multi-disc track's CoverArtID emits a dc- id served from disc-specific art whose hash
is unknown at hydration time. Advertising dc-..._<albumHash> gave clients a content-
version that never changes when the disc image does, breaking id-based refresh. Only
stamp the album hash for single-disc tracks (DiscNumber == 0); multi-disc tracks stay
unhashed and rely on the correct ETag returned by the served response.

* fix(artwork): enqueue new empty playlists by id, and refresh on absent outcomes

Two worker/enqueue fixes from review:
- playlistRepository.Put assigned the generated id to the caller's Playlist but passed
  the stale copy (empty id) to refreshCounters, enqueueing a pl|"" row the worker
  failed until the daily dangling purge while the real playlist went unresolved. Set
  the id on the copy before enqueueing.
- The drain refresh batch only included found/foundStale, so a cover removed by a scan
  (found -> absent) never notified clients, leaving the old immutable image displayed.
  Broadcast absent outcomes too; precache still only warms found/foundStale.

* fix(artwork): honor disabled per-track art at serve time; use nanosecond mtime provenance

Two serving-correctness fixes from review:
- serveMediaFile served a persisted mf embedded image even after EnableMediaFileCoverArt
  was turned off (the setting isn't in the config fingerprint, so found rows aren't
  reprocessed). Direct mf- URLs now honor the setting at serve time and fall back to
  disc/album art.
- The file-backed staleness check compared whole-second mtimes, so a same-second content
  replacement (two writes in one second, or timestamp-preserving tools) could serve
  different bytes under the old hash + immutable policy. RefMtime is now unix-nanoseconds
  (no schema change; int64 column), detecting sub-second changes where the filesystem
  records them.

* fix(artwork): preserve the drive when normalizing Windows file:// library paths

url.Parse puts the volume of file://C:/Music in Host, not Path, so localOSRoot dropped
it and returned /Music — os.Open/os.Stat then failed and folder/embedded art on Windows
looped as dangling. Rejoin the host volume, matching core/storage/local's newLocalStorage.

* fix(artwork): clamp negative sizes to full-size; convert imghttp test to Ginkgo

- A negative size (Subsonic size / Jellyfin maxwidth accept signed ints) reached
  resizeStaticImage, where the square path builds image.NewNRGBA(Rect(0,0,size,size))
  — a giant rectangle that panics/OOMs. Clamp size<0 to 0 (full-size) at the Service
  entry. Positive sizes were already clamped to the original.
- imghttp used a plain func Test with a table; convert to a Ginkgo DescribeTable with
  the suite entry point in imghttp_suite_test.go (AGENTS.md test-framework requirement).

* test(artwork): use renamed ArtworkWorkerConcurrency in e2e tests

* feat(artwork): carry blurhash through item image hydration

* feat(nativeapi): expose artwork hash, absence and blurhash

* feat(artwork): hydrate the parent album's artwork state onto tracks

* test(artwork): add hydrateArtwork regression guard for AlbumImage wiring

Drives hydrateArtwork itself (not applyItemImage directly) over tracks that
take each of the loop's continue branches, so a future edit moving the
AlbumImage fill below a continue would fail loudly instead of passing silently.

* feat(jellyfin): version album and artist image tags by content hash

* fix(jellyfin): trim primaryImageTag comment to why-only, within budget

* feat(jellyfin): emit real blurhashes and drop the synthesized fallback

* feat(ui): version cover art urls by content hash and skip absent art

* feat(ui): add BlurHashCanvas placeholder component

* fix(ui): clear stale blurhash pixels and assert the draw path in tests

Clear the canvas before each decode attempt so a hash change that fails
to decode doesn't leave the previous frame's pixels on screen once this
wires into a list that recycles items. Also strengthen the specs to
assert createImageData/putImageData were actually invoked (and with
what), instead of only checking that a <canvas> element exists.

* feat(ui): show the blurhash while an album cover loads

* fix(artwork): hydrate cursor streams via an id pre-pass

The album, artist and playlist GetCursor built their own select and never
called hydrateArtwork, so every Jellyfin list endpoint (all six stream via
GetCursor) emitted entity-id image tags and no blurhash. Only GetAll
hydrated, which is why Subsonic and the native API were unaffected.

Each cursor now resolves its ordered/filtered/paginated id set with the
cheap id-only GetAllIDs query, then streams those ids in chunks through the
repo's existing GetAll, which already hydrates and applies the full select.
Max/Offset are consumed by the pre-pass alone; the chunk query carries only
the caller's filters, Sort and Order.

This also removes a pre-existing deep-pagination cost: keeping OFFSET out of
the joined query makes the pre-pass a covering index scan instead of paying
the library and annotation joins for every skipped row. Benchmarked on a
synthetic 100k-album DB with the real schema, page=500 at offset 90,000:
3.9ms via the id pre-pass, 52.5ms for the current shape, 192.7ms for a naive
join. An unpaginated full stream costs ~24% more, which is the trade.

GetAllIDs gains the annotation join whenever the caller's filters or sort
reference an annotation column (same gate CountAll uses), otherwise
Filters=IsFavorite and SortBy=PlayCount would fail in the pre-pass. The
playlist pre-pass repeats GetAll's columns so ORDER BY keeps resolving to
playlist.name rather than the joined user.name.

* fix(jellyfin): hydrate artwork on the song cursor

Jellyfin's listSongs streamed media files via GetCursor, which never
hydrates artwork, so songs emitted entity-id image tags and no blurhash.

media_file now uses the same id pre-pass as the other three cursors
(album/artist/playlist), for consistency, but on a separate method,
GetCursorWithArtwork: GetCursor itself must stay untouched, since it's
also the scanner's hot path and the scanner never reads artwork.

Measured on 1,000,000 tracks, the pre-pass over all ids costs +41.8 MB
heap and +298 ms versus GetCursor's bounded +0.0 MB. The Jellyfin path
is paginated, though, so in practice it only ever pre-passes a page's
worth of ids, not the full library, and doesn't pay that cost.

* feat(jellyfin): emit a song's own cover art when it differs from the album's

Real Jellyfin fills ImageTags from each item's own images before falling back
to the parent album, and Finamp checks imageTags.Primary before AlbumId. Our
mapper read only the album's image, so a track with distinct embedded art
silently showed the album cover.

Emit exactly one entry under ImageBlurHashes.Primary: Go marshals
map[string]string in sorted key order rather than insertion order, so a
second entry could pair the wrong blurhash with the image imageId resolves
to, and Finamp pins that pairing in its cache for 365 days.

* test(persistence): scope the GetCursorWithArtwork full-stream spec to tie-free ids

The fixture has title ties (e.g. three "Antenna" tracks), so the unscoped
positional comparison against GetAll only passed because SQLite's tie order
happened to coincide between the full scan and the pre-pass's id IN (...)
fetch. Scope it to onlySongs like the sibling ordering specs already do.

* refactor(artwork): route song own-art through primaryImageTag; align chunk size

Cleanups surfaced by /simplify: the song mapper's own-art branch reimplemented
primaryImageTag's tag+blurhash-map construction (and its one-entry invariant) — route
it through the helper so that invariant lives in one place. Tie artworkChunkSize to a
whole multiple of artworkBatchSize so a cursor page re-chunks into even hydration
batches. Hoist a duplicated imageLoading && blurHash boolean in the album grid.

* fix(ui): serve the placeholder for known-absent art instead of a broken icon

getCoverArtUrl returned '' for an imageAbsent record, so <img src={undefined}> rendered as the browser's broken-image icon on every absent cover. The server already serves a proper placeholder for absent art, so build the url and let it render.

* feat(ui): show the blurhash as the loading placeholder across cover surfaces

Add a shared CoverImage component (useImageUrl blob cache + blurhash + fade) and render the blurhash while a cover loads on the list thumbnails (CoverArtAvatar, radio) and the artist/album/playlist detail pages. The detail pages now go through CoverImage instead of a plain CardMedia, so their images come from the in-memory blob cache and survive React remounts without re-fetching. BlurHashCanvas gains an optional style prop.

* refactor(ui): unify list cover surfaces onto the shared CoverImage component

Route the album grid, CoverArtAvatar (artist/playlist lists) and the radio list's cover field through CoverImage instead of each carrying its own useImageUrl + blurhash-overlay wiring. CoverImage gains a default object-fit: cover. Radio keeps its uploaded-image gate and the generic radio placeholder for stations with no art.

* fix(ui): address CoverImage review findings

Restructure CoverImage so the size/shape lives on the root and the blurhash + image are absolute fills: the <img> mounts only once its blob is ready, so an unresolved cover never flashes a broken <img>. Add a fit prop (default cover) so album/playlist detail keep their letterbox instead of being cropped by a hardcoded object-fit. Remove the orphaned coverLoading styles and an unused subsonic import; add a CoverImage unit test.

* perf(ui): only refetch already-loaded records on SSE refresh

The artwork worker broadcasts a RefreshResource event per resolved chunk,
carrying every id in the chunk. useResourceRefresh was doing a getMany for
all of them, so any open list/detail page fetched hundreds of artists it
was not displaying. Filter the event ids to records already in the store;
the rest load fresh (with their new artwork) when navigated to.

* refactor(artwork): scale worker concurrency with CPU count

ArtworkWorkerConcurrency now defaults to max(2, NumCPU()/2) instead of a
fixed 4, mirroring MaxOpenConns: local resolution scales with the host but
stays at half the SQLite pool so it never starves the scanner/UI. External
RPS stays a fixed 2 — it gates third-party API calls and is bounded by their
tolerance, not the host, so it must not scale with CPUs.

Also drop the DevArtworkWorkerConcurrency/DevArtworkExternalRPS deprecated
aliases: those names were never released, so there is nothing to migrate.

* feat(artwork): re-queue an absent cover when its page is viewed

serveEntity now schedules a Bump recheck for an entity whose art was recorded
absent, so viewing a missing cover re-triggers resolution (e.g. after an
external source that was down during the scan comes back), matching the
request-time bump that already covers never-resolved entities.

Throttled by attempted_at against requestRecheckAge (1h) so repeatedly opening
a genuinely-absent page can't hammer external services. EnqueueBump preserves
an existing failed-state backoff via MAX(priority,...) and inserts a fresh,
immediately-eligible recheck for a settled-absent row (whose queue row was
already deleted).

* refactor(artwork): move ImageUploadService to artwork.Uploader

Relocate the image-upload service from core to core/artwork as
artwork.Uploader, co-locating it with the resolver/worker/serving that own
the artwork state it invalidates. MaxImageUploadSize moves too — its only
callers are the two image-upload handlers — which lets core/image_upload.go
be deleted entirely.

Extract the shared "clear resolved state + re-queue at Bump" invalidation
into artwork.Refresh and fold nativeapi's refreshArtwork handler onto it,
removing the duplicated DeleteForItem+Enqueue block that had drifted into
three places.

The wire provider moves from core's set to artwork's; the
playlists.ImageUploadService binding moves to the top-level injector so
core/artwork stays unaware of playlists. Behavior is unchanged.

* refactor(artwork): thread model.Kind through the artwork API

Entity-level artwork queries now take a typed model.Kind instead of a bare
prefix string. GetItemArtwork, DeleteForItem(s), GetInfoForItems,
EnqueueStaleAbsent, hydrateItemImages, enqueueBackfillKind and artwork.Refresh
convert to the prefix string only at the two real boundaries: the SQL
item_kind column (kind.Prefix() inside each repo) and external string inputs
(a new model.ParseKind for the nativeapi URL param, which also validates it).

The Backfill/stale-absent kind slices, the resolve.go dispatch switch, and the
kind→resource / kind→table lookup maps now use the Kind vars directly. The
queue lifecycle methods (MarkFailed/Delete*) keep string kinds — they operate
on a dequeued item's raw ItemKind column, which stays a string field, always
populated via kind.Prefix().

Removes every bare "al"/"ar"/… prefix literal from non-test code (27 -> 0);
behavior is unchanged.

* tune(artwork): drop backoff base from 5m to 15s

The exponential retry (base × 4^attempts, cap 48h) started at 5 minutes, so a
single transient failure — a timeout under load, an external blip — parked a
cover for 5 minutes even though a retry seconds later would have resolved it.
Start at 15s instead: transient failures recover almost immediately (15s → 1m
→ 4m → 16m …), while persistent failures still escalate to the 48h cap (now at
the 8th attempt instead of the 5th).

* tune(artwork): 5s backoff base + 12h give-up, drop the cap

Retry backoff now starts at 5s (was 15s) so a transient failure recovers on
essentially the next drain, and jitter widens to ±40% so a wave of correlated
failures doesn't re-clump into one poll.

Add a 12h give-up budget measured from enqueued_at: once the next backoff would
land past it, the worker stops retrying instead of grinding at a cap forever. A
bare failure settles absent (handed to the 24h stale-absent sweep, and still
recoverable on a page view); a found-stale keeps its already-served art. The
budget bounds the tail, so the separate 48h backoffCap is removed.

* fix(lastfm): match album.getInfo on name+artist only, not MBID

Last.fm's album.getInfo by MBID is unreliable: a correct MBID can return a
different album, or none. Observed with black midi's "7-eleven" (whose correct
MBID returned a FLEETWOOD release) and both missing The Chats albums (one MBID
404s, the other resolves to a different self-titled release). The worker then
recorded covers absent — or would fetch the wrong art — even though the correct
cover is on Last.fm by name+artist.

Stop passing the MBID to album.getInfo; query by name+artist only, which also
drops the now-dead error-6 MBID-retry fallback. The low-level client keeps its
MBID support for other callers; only the album lookup changes.

* fix(lastfm): return agents.ErrNotFound on error 6 (not found)

Last.fm returns error 6 for a missing artist/album — a definitive negative —
but the agent returned the raw *lastFMError, so the artwork worker treated
every not-found as a real fault: it counted toward the per-source circuit
breaker (5 in a row opens it, fast-failing all Last.fm calls including valid
ones) and was retried as a transient error instead of settling absent. On a
first scan of a library with many artists Last.fm lacks, this stalled valid
cover lookups and left entities churning in backoff.

Translate error 6 to the shared agents.ErrNotFound at the agent boundary
(callAlbumGetInfo / callArtistGetInfo), matching how the Deezer agent maps its
client's not-found, and log it at Debug instead of Error — which also removes
the not-found log spam.

* feat(artwork): log external image-lookup failures at debug

The worker's res.reader==nil && extError branch returned outcomeFailed with no
log, so a failing external cover lookup (agent error, dead image URL, download
timeout) was undiagnosable. Log the agent, entity, and underlying error at the
fetch site where it's in hand — this surfaced a Last.fm album.getInfo returning
an image URL that itself 404s.

* fix(artwork): treat a 404/410 image URL as not-found, not a transient fault

An agent (notably Last.fm's album.getInfo) can advertise a cover URL that is
itself dead — a 404. sources.go's fromURL returned a generic error for any
non-200, so a dead URL was treated as a transient failure: it churned in
backoff and counted toward the circuit breaker, stalling valid lookups.

Map 404/410 to model.ErrNotFound in fromURL so a dead URL settles absent, and
collapse the near-identical fetchPlaylistImageURL (which already did this for
M3U covers) into it.

* feat(artwork): make artwork re-resolution targeted, not blunt

Two gaps in when the pipeline re-resolves artwork:

The recheck job only requeued absent-state rows (hash=''), so an entity that was
never processed — added between scans, or on a server with the scanner disabled —
had no periodic safety net and stayed without artwork indefinitely. Add
EnqueueMissing(kind): a SQL set-difference enqueueing entities with no
item_artwork row at Recheck priority (ON CONFLICT DO NOTHING, so it never
disturbs a queued row). Run it once at startup and hourly alongside the
stale-absent recheck. Rename staleAbsentKinds -> recheckKinds accordingly.

Conversely, the config fingerprint included consts.Version, which embeds the git
SHA and so changed on every build, re-enqueueing every entity in the library
(~34k here) and re-querying external agents at the configured RPS for anything
without local art. Replace it with an explicit artworkEpoch constant, bumped
deliberately when resolution semantics change. The cases that motivated the
version input — absent art becoming available — are already covered by the
stale-absent and missing-row rechecks; only a corrected wrong-pick needs the
epoch. A test guards against reintroducing the version.

* test(artwork): restore resolution edge-case e2e coverage

The serving cutover removed the album/disc/artist/mediafile/playlist/radio e2e
specs that documented the folder-selection rules and guarded the #5376/#5456/
#5451/#5457 regressions; nothing replaced them, so compareImageFiles and the
parent-fallback logic were left untested.

Restore them driving the real pipeline: a real scanner populates the folder
graph from an in-memory library, the real Worker drains the queue, and the real
Service serves. Folder-backed art is file-backed (served via os.Open, which the
in-memory FS can't satisfy) so its selection is asserted on the persisted state
row; store-backed and real-disk sources are asserted byte-for-byte. Single-disc
disc resolution now serves album art directly, so only multi-disc disc scenarios
are ported.

* fix(artwork): run disc resolution for single-disc albums too

ed4178a6 gated serveDisc on len(album.Discs) > 1, claiming parity with the legacy
reader. The legacy reader has no such gate: artwork.go dispatches every dc- id to
newDiscArtworkReader, whose Reader() walks DiscArtPriority unconditionally.

The gate also lost art. For a single-disc album whose only image is disc1.jpg,
the disc request skipped the chain and fell through to album art, which does not
match CoverArtPriority — so tracks tagged disc 1 (whose CoverArtID is a dc- id)
served nothing at all, where before they served disc1.jpg.

A single disc can legitimately have its own cover, distinct from the album's, and
DiscArtPriority is what expresses that preference. Drop the gate and restore the
single-disc e2e scenarios that covered it.

* fix(artwork): register the GIF decoder in core/artwork

The deleted artwork.go carried blank imports for image/gif and
x/image/webp. WebP came back via resize.go's gen2brain/webp, which
self-registers, but GIF did not: core/artwork claims GIF support in
mimeForFormat and extForMime while relying on an unrelated server
package to have imported the decoder.

The guard lives in the e2e suite because that test binary has no other
image/gif importer; a spec in core/artwork would pass regardless, since
animation_test.go imports the package non-blank.

* fix(artwork): never record absent after a local I/O failure

Local sources swallowed their open errors, so a stale NFS/SMB mount was
indistinguishable from "this entity has no artwork": the chain returned
no reader, processItem took the absent branch, and the upsert replaced a
good content hash with the empty string. Clients then saw a placeholder
until the 1h request recheck or the 24h stale-absent sweep, and the
orphaned bytes became eligible for the next prune.

A candidate the resolver knows about — a file in the folder listing, a
track's own audio file — failing to open is not evidence of absence, so
it now forces a retry the same way an external agent error does.

* fix(artwork): keep served art when the retry budget runs out

Exhausting the 12h budget called writeAbsent unconditionally, so an
entity whose art was already resolved and serving lost it to a long
upstream outage: the hash went empty, clients fell back to the
placeholder, and the now-unreferenced bytes were freed by the next
prune even though nothing about the image had changed.

Exhaustion means the source stayed unreachable, not that the cover
disappeared, so absent is now recorded only when there is nothing to
keep.

* fix(artwork): restart the retry budget on re-enqueue

The conflict clause updated only priority and retry_at, so a row that
already existed kept its original attempts and enqueued_at. The worker
measures the 12h give-up budget from enqueued_at, so any row that had
been pending across a longer gap — a server left off, an upgrade, a
laptop asleep — gave up on its very first attempt and settled absent.

The manual re-resolve endpoint is the sharpest case: it clears artwork
state and re-queues, but inherited the old row's spent window, so a
deliberate retry got one shot. A fresh request now gets a fresh budget,
with the mock updated to match.

* fix(artwork): keep not-found distinct from artwork-absent

GetOrPlaceholder folded model.ErrNotFound in with ErrUnavailable, so an
id matching no entity returned 200 and the placeholder PNG. That made
the ErrorDataNotFound branch in getCoverArt unreachable: Subsonic went
from error 70 to a successful placeholder, and Jellyfin's Primary image
endpoint from 404 to 200. Neither is ours to change.

An entity with no art and an id with no entity are different answers;
only the first is a placeholder.

* fix(artwork): make the prune sweep cancellable

Sweep walked the whole store with no context, and RunPrune holds the
prune write lock for its full duration. In-flight acquisitions park on
the read lock, drain's WaitGroup never returns, and Run never reaches
its ctx.Err() check — so a SIGTERM during a daily prune over a large
store on slow storage waits out the container's grace period and dies
mid-remove.

* fix(artwork): hydrate the tracks reached through a playlist

loadTracks and the playlist-track cursor were the only entity-page
paths that never hydrated artwork state, so a song reached through a
playlist behaved differently from the same song in the songs list:
Subsonic emitted a hashless coverArt id, which imghttp downgrades to
no-cache, and advertised art even for known-absent albums; Jellyfin
emitted AlbumPrimaryImageTag as the bare album id — a tag that never
changes when the cover does — and no blurhash at all.

The media-file hydration moves next to the other hydration helpers so
both paths share one implementation rather than growing a third.

* feat(ui): cross-fade the cover over its blurhash

The blurhash unmounted the moment the blob arrived, so the placeholder
vanished a frame before the image painted. The image now mounts
transparent and fades in over the blurhash, which stays behind it until
the fade completes. A blob already cached when the instance mounts
skips the fade, so a remount does not re-animate.

* fix(ui): retire the blurhash on a timer, not transitionend

Under prefers-reduced-motion the img rule sets transition:none, so
toggling opacity fires no transitionend and the handler that unmounts
the blurhash never ran. The placeholder stayed mounted for the life of
the component — visible in the letterbox bars wherever the cover is
rendered with fit="contain", and a live canvas per tile everywhere else.

One duration constant now drives both the CSS transition and the timer,
so they cannot drift.

* fix(artwork): stop advertising a hash for bytes that won't be served

Hydration stamped the album's hash and blurhash onto an embedded-eligible
track that had no state row yet. Serving takes provisionalEmbedded for
exactly that case and returns the track's own embedded image, so the id
carried a content-version belonging to a different picture: every such
request fell back to no-cache instead of immutable, and a client keying
its cover cache on the blurhash paired the album's with the track's art.

AlbumCoverArtID had the mirror-image problem, building the album id from
the track's own ItemImage. It happened to work only because hydration
overwrote ImageHash in precisely the fallback cases; a track with its own
resolved art would have stamped that hash onto the album's id.

* perf(artwork): precache from the bytes just acquired

Warming the resize cache re-read the two rows and the file the
acquisition had just written, so every acquired image cost two extra
queries and a second full read of a file whose bytes were still in
memory. processItem now hands back what it persisted and precache warms
from that, under the same cache key the serving path computes.

Resolving the admin user also moves behind the empty-queue check: it is
needed only to resolve private playlists, so an idle server no longer
runs a user lookup on every poll.

* perf(artwork): keep the worker pool fed across a drain

The pool was fed from a batch sized to the pool itself (2x concurrency)
with a WaitGroup barrier before the next dequeue, so one item burning
its external timeout idled every other slot until it finished. The
legacy cache warmer had no such barrier: it streamed through a pipeline
of 4.

Dequeuing well past the pool keeps the slots fed for the whole pass at
no extra cost, since DequeueBatch does not mark rows taken and was
already one query per pass. Acquiring a slot now also observes
cancellation, so a larger batch cannot delay shutdown.

* perf(artwork): drain local and external artwork in separate pools

A first backfill enqueues artists before albums at a single priority, so
the drain took them in that order. Artists resolve through a
rate-limited agent, and gate() waits for its permit while holding a
worker slot, so the whole pool sat asleep in the limiter with every
album queued behind it.

Measured on a 96k-track library (29,115 artists to 6,949 albums, 4:1):
zero albums resolved in seven minutes, and roughly 3.3 hours before the
first album cover would have appeared. Splitting the drain gives each
class its own slots: albums now finish in under eight minutes while
artists trickle at the same 2/s they were always limited to.

The two budgets are carved out of MaxOpenConns so a second pool cannot
take connections the scanner and the UI need. Dequeue filters by kind,
and the drain index leads with item_kind so each pool seeks to its own
work instead of scanning past the other's backlog.

* fix(artwork): treat an unreadable upload as a failure, not a miss

resolveLocalFile swallowed every os.Open error, so uploads, playlist
sidecars, a local M3U image and the artist image folder still had the
bug that was fixed for folder and embedded sources: a permission or
transient I/O error on a file that exists read as "no image here". The
worker then settled the item absent and dropped its queue row.

Uploads outrank every other source, so an unreadable one now stops the
chain rather than letting a lower-priority image be persisted in its
place. A genuinely missing file stays a clean miss.

Reported by Codex on #5847.

* fix(artwork): make the pool-split worker test race-clean

CI runs the suite under -race, which the local `make test` does not, so
this only showed up there: 230 specs passed and the detector still
failed the run.

The drain-pools spec started Run and never waited for it, so pool
goroutines outlived the spec and raced the config snapshot Ginkgo
restores on cleanup. It now cancels, unparks the blocked lookups and
waits for Run to return.

Two test doubles also had to become concurrency-safe, since the spec is
the first to resolve several artists at once: fakeImageAgent's call
counters, and MockAlbumRepo.GetAll, which records the last query options
on a read path. MockDataStore's lazy accessors get the same treatment —
only MediaFile was guarded before, and two pools now reach them
concurrently. ArtworkQueue takes an unlocked helper for its internal
Artwork call, since repoMu is not reentrant.

* perf(artwork): stop reading and hashing disc art on every request

The resize-cache key was the content hash, which cannot be computed
without reading the file, so a warm cache never prevented the I/O: every
sized disc request read up to 20MB and hashed it before the lookup. The
legacy reader keyed on the id and the album's mtime and touched the file
only on a miss.

Disc art has no state row and therefore no stored hash, so the key is
that same identity — id, album mtime, DiscArtPriority — and the
selection chain now runs only when the cache misses. Full-size requests
stream the source instead of buffering and hashing it.

This matters more now that single-disc albums keep running the disc
chain, which puts every disc-tagged track without embedded art on this
path.

* fix(artwork): key disc art on folder image changes, not just the album

The identity cache key used album.UpdatedAt alone, which a replaced
disc image does not necessarily move — the sized response would then
serve the old image indefinitely. The legacy reader folded ImportedAt
and the folder's ImagesUpdatedAt into its key for exactly this reason,
and loadAlbumFoldersPaths already returns that timestamp; the disc
reader was discarding it.

* fix(artwork): return undispatched items when a drain is cancelled

claim() reserves the whole batch before dispatch, but the cancellation
path returned without releasing what it had not yet started, leaving
those items in the in-flight set permanently — no later drain could
claim them again.

Harmless until the batch grew past the pool size; now a cancel strands
up to a full batch. The e2e harness cancels mid-drain after every
acquire, so it surfaced there first: one spec timed out waiting for an
item that had been claimed and abandoned, and the suite went from 59s
to 87s on CI.

* fix(jellyfin): make a track's own cover reachable for Jellyfin clients

Media files are never enqueued — the scanner drops their state without
queueing them and the recheck kinds exclude them — so an unresolved
track keeps an empty ImageHash and SongToBaseItem always fell through to
AlbumPrimaryImageTag. Finamp then asks for the album image, nothing ever
requests mf-, and the read-through that resolves the track never fires:
the own-cover branch was unreachable for Jellyfin-only users.

An eligible, unresolved, not-known-absent track now advertises its id as
the Primary tag, which is what makes the client ask. The request serves
the embedded art and queues the track so the worker persists a real
hash. No blurhash is sent, since none exists yet and a fake would be
cached against that tag forever.

Serving gains the album fallback that made this safe to advertise: an
eligible track whose frame will not extract now falls back the way
CoverArtID does instead of answering with a placeholder.

Reported by Codex on #5847.

* fix(artwork): treat an unreadable artist-folder image as a failure

Third and last site in this class: findImageInFolder logged and skipped
an image the glob had already matched, so a permissions or mount failure
during the artist-folder traversal read as "no image here" and let
processItem settle the artist absent, discarding any artwork already
resolved.

A matched-but-unreadable file now propagates through fromArtistFolder
and lands as localError, the same as album folder art, embedded art and
uploads. A folder with no match stays a definitive miss.

Also normalizes the e2e path assertions with filepath.ToSlash: the
stored SourcePath is OS-native, so the forward-slash suffixes failed
all 23 folder specs on Windows.

Reported by Codex on #5847.

* fix(artwork): give full-size disc art a real ETag

Regression from 04d5a556. Keying the resize cache on identity meant the
disc response no longer carried a content hash, and the full-size branch
set no ETag either — so WriteImageHeaders fell back to the empty hash
and emitted `ETag: ""` for every full-size disc image. Since ifNoneMatch
compares the unquoted value, a client echoing that back matched, and got
a 304 even after the image was replaced.

The full-size branch now carries the same identity validator the sized
branch uses, so it moves when the folder's images change.

WriteImageHeaders is hardened against the class as well: an empty
validator is no validator, so it is neither emitted nor matched.

Reported by Codex on #5847.

* test(artwork): inject the unreadable source instead of chmod

os.Chmod cannot revoke read access on Windows — it only toggles the
read-only attribute — so the findImageInFolder spec opened the file
happily and failed there, and the upload spec passed for the wrong
reason: outcomeFailed came from the 1-byte payload failing to decode,
not from the source being unreadable.

findImageInFolder takes an fs.FS, so the failure is now injected and
the spec is filesystem-independent. The upload path goes through
os.Open directly and has nothing to inject, so it skips on Windows
rather than pretend to cover it.

* fix(artwork): compare the pixel cap without multiplying

Defence in depth rather than a live hole: the reported crafted PNG
(0xffffffff square) never reaches the multiplication, because
image/png rejects it at DecodeConfig, and the largest dimensions any
supported format can declare — 2^30-1 for PNG, 16-bit for JPEG and
GIF, 14-bit for WebP — cannot overflow the int64 product.

decodeCapped is format-agnostic though, so the guard should not depend
on a decoder's own limits staying where they are. Comparing by division
holds for any dimensions a decoder might report, and non-positive ones
are now rejected outright.

* refactor(ui): rename cover artwork components

* fix(ui): remove Artwork rendering gate

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(cache): re-fetch when a cache entry outlives its data file

fscache's Remove drops the in-memory entry, releases the lock, and only then
unlinks - blocking until every outstanding reader closes. A Get landing in that
window re-creates the file at the same path under a fresh entry, and the deferred
unlink deletes those new bytes. The entry survives pointing at nothing, and since
a present entry is treated as a hit, every later Get for that key returns ENOENT
for the rest of the process's life. Only a restart, which rebuilds the map from
disk, cleared it.

Get now drops such an entry and retries once, so a vanished data file costs one
re-fetch instead of poisoning the key permanently. This also covers a file
disappearing for reasons unrelated to that race, such as external deletion or a
restored backup.

Specs cover an in-process entry, one adopted at startup, and the deferred-removal
race itself.

* fix(artwork): log why a sized cover fell back to the placeholder

serveHash routed every non-cancel cache error into dangling(), which returns
ErrUnavailable and is then rendered as a placeholder at 200 OK. A cache-layer
fault was therefore indistinguishable from an album genuinely having no artwork,
and left no trace: a broken resize cache silently served placeholders for a
quarter of the library while the logs stayed clean.

Log the error before falling back, so the cause is recoverable from the logs.

* fix(artwork): precache the cover variant the UI actually requests

precache built its resizedItem without setting square, so it warmed
h-<hash>.<size>.false.<quality>. The list surfaces - album grid, artwork avatars,
playlist and radio details - all request square covers, so the warmed entry was
never read and every grid cover stayed a cold miss on first view.

Set square on the precache item so the key matches the request path. The existing
specs asserted the '.300.false.' key and were updated accordingly.

* fix(cache): give a re-created cache entry its own file

Create re-opened the path with O_TRUNC, which shrinks the file out from under an
older stream that may still be serving readers. stream.Reader then hits EOF from
the OS at the new, shorter length while the broadcaster still reports the original
size, so Wait() reports 'more data exists' and the reader retries forever - a tight
pread loop that burns a core and never releases its handle, which in turn blocks
Stream.Remove() indefinitely.

Unlink first and create with O_EXCL so the new entry gets a fresh inode. Existing
readers keep their descriptor on the old inode, see its full contents, and reach a
clean EOF.

Note this does not address the deferred unlink deleting the re-created file, which
is handled separately by the re-fetch in fileCache.Get.

* fix(cache): keep the in-place truncate on windows

Unlinking before re-creating fixes the premature-EOF spin on unix, but Windows
refuses to remove a file another handle still has open and returns a sharing
violation. Because Create surfaces that error, every cache miss on a path with a
live reader would have failed outright - worse than the spin it was meant to fix.

Split the create behind a build tag: unix unlinks for a fresh inode, Windows keeps
truncating in place and stays exposed to the spin, which is the behaviour it
already had. The unix-only spec is skipped there.

* fix(artwork): shape the blurhash placeholder to the artwork's aspect ratio

The UI decoded every blurhash into a 32x32 bitmap and stretched it to fill its
container, so a non-square cover showed a full-box blur that collapsed into a
letterboxed image the moment it loaded. On the album detail page the
placeholder overhung the image by a third of the box height.

A blurhash string carries no aspect ratio of its own, so the dimensions have to
come from the server. artwork.width/height were already stored and read by
nobody; they now surface on ItemImage as imageWidth/imageHeight, hydrated
through the join that was already in place. Existing rows already carry them,
so no migration or rescan is needed.

A square request is padded rather than cropped, which aspect-fits the content
inside the square the server returns. That made the grid a second instance of
the same bug, so `square` now implies contain for the image as well as the
placeholder, instead of the two renderers reading it differently.

* feat(jellyfin): expose PrimaryImageAspectRatio

Real Jellyfin carries width/height of the Primary image on BaseItemDto
(MediaBrowser.Model/Dto/BaseItemDto.cs), attaching it only when the request's
Fields asks for it (DtoService.cs ContainsField). The dimensions are now on
model.ItemImage for the web UI's blurhash placeholder, so the adapter can
report the same thing for free.

Gated behind Fields to match, and omitted rather than defaulted when the item
has no image or unknown dimensions: real Jellyfin falls back to a per-type
default of 1 for music, but a wrong ratio mis-shapes a client's placeholder,
and we only lack dimensions when the artwork is genuinely unresolved.

primaryImageTag becomes primaryImage, returning the tag, blurhashes and ratio
together, so the choice of which image is Primary is made once per mapper
rather than the same ItemImage being threaded through two calls.

ArtistToBaseItem and PlaylistToBaseItem take Fields now, like the album and
song mappers already did.

* refactor(model): give ItemImage an AspectRatio method

The zero/absent guards around width/height are ItemImage's own invariant, not
the Jellyfin adapter's. Moving them onto the model keeps one definition for
every consumer, so a second one cannot quietly disagree about what an unknown
ratio means.

* fix(ui): anchor detail pages to the top when opened from a list

React Router keeps the previous page's scroll offset, so opening an album from
a scrolled list started the detail page mid-song-list. Artist pages had the
same bug; it just shows less because the artist list is rarely long enough to
scroll far.

Keyed on the record id rather than mount, so detail-to-detail navigation (an
album's artist link) resets too, and so the scroll waits for the record instead
of firing against an empty page.

* fix(ui): stop the Random album grid collapsing on every keystroke

The grid was replaced by a spinner whenever the random list was loading, so
each search keystroke collapsed it to spinner height and back. That blanket
blanking is the flicker commit 9e559311a removed for every other list; random
kept the exception so a re-roll would not flash the roll it is replacing.

Blank on a seed change instead of on any load: a re-roll gets a new seed and
still blanks, while a search keeps the seed and leaves the grid in place. The
seed is tracked from empty rather than from the current value because a re-roll
redirects and remounts the grid, which would otherwise look already-settled
with the previous roll still on screen.

Same rule for the pagination, which was hidden on the same condition.

* refactor(artwork): move fingerprint property key const to `consts` package

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(tests): enhance database handling with resettable tables and truncation

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(artwork): narrow the prune lock and drop redundant in-flight tracking

The prune read-lock wrapped all of processItem, including external fetches under
their own timeout. Since a pending RWMutex writer blocks new readers, one prune
arriving behind a slow provider stalled every subsequent item in both drain
pools. Extract persist() so the lock covers only the window it protects: store
placement plus the two row writes.

The in-flight set guarded against a queue row appearing twice in one batch, but
artwork_queue's primary key makes that impossible, drains are serial per pool,
and the pools' kind lists are disjoint. Removing it also retires the cancellation
unwind loop that existed only to release those claims.

* fix(artwork): never settle absent for a kind no recheck job revisits

The 12h retry budget hands a bare failure to the periodic stale-absent
sweep, which is what makes the resulting absent row recoverable. Media
files are deliberately excluded from that sweep -- they resolve embedded
only, at scan or on view -- so exhausting the budget on a transient read
error recorded a "this track has no cover" verdict that nothing would
ever revisit.

Settle absent only for kinds a recheck job covers. Without a row the
track stays unresolved, so the next view re-enqueues it.

* refactor(artwork): remove dead plumbing from the serving path

artworkReader.LastUpdated had no callers: invalidation rides entirely on
the cache key, so the interface member, the resizedItem field and its
four assignments were vestigial. Reader's second return value was
likewise discarded at all three call sites.

resizedItem.Key duplicated representationTag's format string over
identical inputs, where drift would serve a wrong-keyed entry under a
right-looking validator; it now derives from it. newResizedItem had one
caller and a doc comment claiming a sharing with worker.precache that
never existed -- precache builds its own literal.

Also unexport Prune, which no caller outside the package used while
RunPrune documented itself as the only sanctioned path, drop a
single-call placeholder wrapper, and delete five fakeFolderRepo fields
no spec ever set.

* refactor(artwork): drop queue repository methods only tests called

MarkFailed and Delete had no production caller: the worker only ever uses
MarkFailedIfUnchanged and DeleteIfUnchanged, which refuse to act on a row
a concurrent scan re-enqueued. Keeping the unconditional pair meant the
interface offered the racy variant under the more obvious name.

MarkFailedIfUnchanged does not build on MarkFailed, so nothing in the
implementation needed them either. The repository tests used them to put
a row into a backed-off state; they now do that directly.

* test(artwork): pin that a request never fetches or samples album art

resolveItemLocal's guard against the remote ExternalImageURL fetch and
the 2x2 grid had no coverage: deleting it left the whole suite green
while putting synchronous network calls on the request path.

The worker resolving the same playlist is asserted alongside, so the
spec cannot pass by simply resolving nothing.

* refactor(artwork): give the resolver a receiver and one capability field

The resolve* chain walkers each took seven parameters -- ds, agents,
ffmpeg, gate, localOnly -- while the package already had workerDeps
bundling the same collaborators for processItem. They are now methods on
a resolver.

The external capability is one nilable field instead of three values
that had to agree. Previously a local-only resolution passed agents=nil,
gate=denyGate and localOnly=true, and only the localOnly check actually
protected anything: the external branch dereferences agents in the loop
header, before the gate closure runs, so denyGate could never fire. It
is deleted. A nil ext now both marks the resolution local-only and
removes the agents there were to dereference, and newLocalResolver takes
no parameter that could supply one.

The playlist tile loop hardcoded localOnly=false, safe only because an
early return 22 lines above it made that unreachable; it now inherits
the resolver's capability.

* refactor(artwork): funnel every served representation through one path

serveHash, serveBytes and serveDisc each hand-copied the same five steps
-- test for full size, stream or build a resizedItem, call the cache,
wrap with a validator -- with a different error policy bolted on. The
ETag rule was restated at four sites and applied inconsistently.

serveSource now states it once: full size streams open() directly, and
an ETag is attached only when the bytes are resized or there is no hash
to validate against. Each caller keeps just its own error policy, and
serveBytes folds into its single caller.

Two behavior changes fall out, both narrowing an aborted request's blast
radius: serveDisc propagates context.Canceled instead of falling back to
a full album resolution, and serveHash's full-size path propagates it
instead of going dangling, which would have enqueued a re-resolution for
a request nobody is waiting on.

* style(artwork): use one log prefix, spelled the way the codebase does

The package logged under three spellings of its own name -- "artwork: "
lowercase, "Prune: " and one "Artwork: " -- and the lowercase ones
carried lowercase message text, against 568 capitalized to 48 lowercase
elsewhere.

Prefixing itself is the convention here (Scanner:, API:, Watcher:) and
it earns its place: DevLogSourceLine is off by default, so without it a
line does not say which subsystem emitted it. So this normalizes the
spelling rather than dropping the prefix. Error strings stay lowercase
and unprefixed per Go convention.

* refactor(artwork): give workerDeps only what the processor uses

The bag carried cache, which processItem never reads and only the
worker's precache uses, and carried agents/ffmpeg/gate solely to
reconstruct a resolver on every queue item. cache and ffmpeg move to
Worker, where precache actually uses them, and the resolver is built
once in NewWorker.

persist's hash parameter was redundant: decodeArtwork sets Hash and
GetImage selects it, so art.Hash already holds it on both paths.

The type itself now lives beside Worker, which owns it, rather than in
the file of the function it is passed to.

* refactor(artwork): make acquisition a processor with its own receiver

workerDeps was a parameter bag threaded into two free functions that
nothing outside the worker calls. It becomes the processor type, with
processItem and persist as acquire and persist methods on it, and Worker
holds one collaborator instead of reaching through a bag.

Kept as a separate type rather than folding onto Worker: acquisition
takes a queue item and returns bytes, while Worker.process settles the
queue row around it. That boundary is what keeps retry policy out of the
image pipeline, and what lets the acquisition specs build a three-field
value instead of a Worker with drain pools, gates, a broker and a
real on-disk cache.

* refactor(artwork): collect the external gate contract in one file

gateFunc, passthroughGate and isTransientExternal sat in agent_images.go
while every implementation lived in worker.go: extGate, breaker,
Worker.gate, gateFor. isTransientExternal even carries a comment saying
it must stay consistent with breaker.record, which was in the other
file -- a rule spanning two files with only a comment holding it
together.

Pure move into gate.go: no symbol added or removed.

* refactor(artwork): put the whole playlist grid in playlist_cover.go

decodeTile and assembleTiles were in resolve.go while the geometry they
depend on -- rect, fillCenter, tileSize -- was in playlist_cover.go and
used nowhere else, so one file held the grid's helpers and another its
assembly.

Pure move.

* refactor(artwork): move resizedItem next to the interface it implements

resizedItem is the only implementation of artworkReader, which is
declared in image_cache.go, and it is used by the worker's precache as
well as the serving path -- so worker.go was reaching into serving.go
for a cache type. representationTag stays in serving.go, where the
HTTP validator belongs.

Pure move.

* refactor(artwork): fold Refresh into housekeeping

refresh.go was a 21-line file for one function that clears artwork state
and enqueues -- the same thing Backfill, EnqueueStaleAbsentAll and
EnqueueMissingAll already do next door.

Pure move.

* refactor(artwork): accumulate priority-chain state in one place

Every source in the album and artist chains repeated the same five
lines: stamp the accumulated external failure onto a hit, or OR the
local fault into the running total on a miss. Each new source was a
chance to forget the OR, which is how the local-I/O-settles-absent bug
happened.

chainState.try does both, so each case drops to three lines and the
omission is no longer expressible. Semantics are unchanged: a hit still
carries extErr only. Also drops localErr from resolvePlaylist, which
declared it but never assigned it.

* refactor(artwork): expose housekeeping through the Worker

scheduleArtworkHousekeeping received a *artwork.Worker and then called
CreateDataStore() for a second handle onto the state that Worker already
owns, because Backfill, EnqueueStaleAbsentAll and EnqueueMissingAll were
free functions taking a DataStore.

They are now Worker methods over unexported implementations, the same
shape prune/RunPrune already uses: one public path, and the specs keep
calling the plain function with a mock store instead of standing up a
Worker. Fingerprint is unexported too -- nothing outside the package
used it.

* refactor(artwork): name the service for its domain, not its role

Every other core interface is named for what it is -- Playlists,
Library, Scrobbler, MediaStreamer -- while this one was artwork.Service,
the only X.Service in the tree. It becomes artwork.Artwork/NewArtwork,
and serving.go follows the type to artwork.go.

ProvideImageStore was likewise the only "func Provide" in the repo; it
is GetImageStore now, matching GetImageCache beside it. cmd/wire_gen.go
regenerated via make wire.

* docs(artwork): give each Worker housekeeping method its own godoc

The three methods shared one comment attached to Backfill, so godoc
rendered the other two undocumented and the one it did show described
the group rather than the call. Each now opens with its own name and
says what that call does, including Backfill's bool return.

* refactor(artwork): move fakeFolderRepo to artwork_suite_test.go

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(artwork): unexport artwork.HashImage function

* fix(log): stop ShortDur eating significant trailing zeros

TrimSuffix(s, "0s") was meant to turn "4h0m0s" into "4h", but it strips
any trailing "0s"/"0m" -- so 10s logged as "1", 20s as "2", 1m30s as
"1m3", 2h30m as "2h3", and a zero duration as the empty string. Every
elapsed/duration field in the app was affected.

The suffix now has to include the preceding unit, so only a whole
zero-valued component is dropped. The existing table only covered values
that dodge the bug (4m, 4h, 4m3s); added the ones that don't.

* feat(artwork): prefix every log message and time the slow steps

Prefix: 22 messages still logged unprefixed, so a line from this package
was indistinguishable from any other subsystem's. All 40 now carry
"Artwork: ", matching Scanner:/API:/Watcher: -- which earns its place
because DevLogSourceLine is off by default.

Timing on what can actually be slow: total per acquisition (on every
exit, failures included), the read that also covers the provider
download, hashing, decode+blurhash, resize, drain batch, precache,
prune, backfill, and the external agent call -- with the rate-limiter
wait counted separately, since a throttled agent and a slow one look
identical from the drain.

Debug coverage for states that were previously silent: dedup hit vs
decode, settling absent, serving a lower-priority source after an
external failure, retry scheduling with attempts and budget left, giving
up when the budget runs out, breaker open/close per agent, provisional
read-through, dangling state rows, and the mtime mismatch that makes art
appear to vanish. outcome gained a String() so it reads as a name.

* refactor(artwork): log decoded dimensions as fields, not a formatted string

fmt.Sprintf ran on every newly-decoded image even with Debug off, since
Go evaluates log arguments regardless of level. Separate width/height
fields also query better than a "300x300" string.

Correction to e0f1acd1a's message: it said "all 40" messages carry the
prefix. The package has 68 log call sites and 62 distinct messages; 40
was only what the test suite happened to exercise. The sweep itself was
complete -- zero unprefixed messages remain.

* fix(subsonic): stop serving artwork for a deleted radio

artworkAccessible checked every kind except radio, which fell through to
the default "no per-user access control" branch and returned true without
a lookup. Radio deletion removes only the entity row -- item_artwork and
the uploaded file survive until the next prune -- so the old ra- id kept
serving the removed radio's image for up to a day.

This is a regression against master, where newRadioArtworkReader loaded
the radio first and returned its error, so deletion took effect at once.
Radios stay globally visible; only existence is checked.

CreateMockedRadioRepo left Data nil, so its own Put panicked on first
use; initialized it.

Reported by Codex on #5847.

* fix(artwork): stop serving artwork for entities that no longer exist

Artwork state and its bytes outlive a deleted entity until the next
prune (@daily), and the serving path consulted only item_artwork, so a
Subsonic id or a signed public token kept serving a removed entity's
image in the meantime. Master's readers loaded the entity first, so this
was a regression.

The check goes in serveHash, the one path that can hand back a found
row's bytes: absent rows are already unavailable, and the provisional
and disc paths load their entity to resolve at all. Doing it there
instead of per-handler also settles who owns the invariant. Subsonic had
worked around it with artworkAccessible, whose comment described the
service "bypassing the library and private-playlist filters"; that
workaround is now deleted, since the service resolves through the
request-scoped repositories and enforces the filters itself.

Because those repositories are ctx-scoped, each caller says what it
wants by what it passes: Subsonic hands over the request context and so
gets visibility as well as existence, while the public image route
elevates like the Jellyfin one already did -- a token is the
authorization there, and a visibility check would hide a shared private
playlist, the very case shares exist for.

Two supporting fixes. albumRepository.Exists and mediaFileRepository
.Exists used the plain exists() helper, which applies no library filter,
so they reported rows in libraries the caller cannot see; they now count
through applyLibraryFilter as CountAll and artistRepository.Exists
already do. Neither had a production caller. RadioRepository gained the
Exists it lacked.

Tests follow the layers: the service refuses a found row whose entity is
gone, the repositories hide rows the caller may not see, and the
handlers only assert the context they hand over. Jellyfin needed no
change -- resolveArtworkID probes the entity tables, so a deleted item
yields an empty artwork id -- but that protection was incidental and
untested, so it is pinned now.

* fix(playlist): rebuild the generated cover only when the tracks change

The enqueue sat in refreshCounters, which Put also reaches for an
ordinary metadata update, so renaming a playlist or editing its comment
re-resolved the cover. The 2x2 grid samples albums with random(), so
that silently handed the playlist a different cover for an edit that
touched no tracks -- and refetched remote artwork to do it.

It now happens where the track set actually changes: addTracks (which
Put-with-tracks and updatePlaylist both funnel through) and renumber
(reached from removeOrphans). Creation still enqueues even with no
tracks, since an imported m3u can carry an ExternalImageURL.

Reported by Codex on #5847.

* style(artwork): trim verbose comments to the 1-2 line budget

Comments only; no executable code changed. Verified by comparing the Go
token stream of every touched file before and after: identical.

Removes 375 of the 1104 comment lines this branch added, targeting content
that belongs in a commit message or PR body rather than in the code:
rejected alternatives ("DeleteIfUnchanged, not Delete", "Waking all beats
routing by kind"), refactor history ("as the legacy reader did"), issue
references (#5798, #5597, #5376), benchmark numbers (~400ms, ~16k allocs),
and four persistence doc comments that duplicated the interface godoc in
model/artwork.go verbatim.

Comments predating this branch are left untouched.

The ASCII fixture trees in the e2e suites are deliberately kept above the
line budget: they diagram the fixture layout with its expected outcomes,
and every pre-existing block in those files carries one.

* test(artwork): make the artists-first backfill assertion non-vacuous

The follow-up loop could never fail: once the first "ar" index is asserted
to be 0, every non-"ar" element is necessarily at an index greater than 0.
A sequence like ["ar", "al", "ar"] passed both assertions, which is exactly
the interleaving the check exists to forbid.

Assert the partition directly instead: nothing after the first non-artist
call may be an artist. Verified by mutation — enqueueing artists a second
time after albums now fails the spec.

* refactor: use stdlib slices/maps and utils helpers in artwork code

Mechanical cleanups, no behavior change:

- 15 copies of the same id-extraction loop collapse to slice.Map (5 repo
  mocks, the scanner's track sweep, 4 wantIDs assertions) and slice.ToMap
  (6 index-by-id loops in the hydration specs).
- disc.go built a map[string]bool purely to dedup folder ids and then
  walked it back into a slice; slice.Unique says that directly.
- folders_artist.go's image filter is slice.Filter over model.IsImageFile.
- mock_artwork_repo deleted from a map while ranging it; maps.DeleteFunc
  states the intent.
- sort.Slice -> slices.SortFunc + cmp.Or; math.Min/Max -> builtin min/max;
  make+copy -> bytes.Clone; strings.Split -> SplitSeq on a per-request
  path; three-clause pixel loops -> for range.
- Reuse utils.BaseName where a stem was recomputed by hand. Not at
  playlist_cover.go:27: that path is a full OS path and utils.BaseName
  uses path.Base, which does not split backslashes.
- Drop a dead nil-guard in agents.go: getAgent returns a bare nil
  interface, and a type assertion on nil already yields ok == false.

cmp.Or was rejected for the gate fallback (func types are not comparable,
does not compile) and for ItemArtwork.AttemptedAt (cmp.Or compares
time.Time with ==, which includes loc; IsZero does not).

* refactor(persistence): collapse the four hydrateArtwork copies into one generic

album/artist/playlist/radio each carried the same eight lines, differing
only in element type and model.Kind. hydrateItems takes a ref callback
yielding an item's id and the ItemImage to fill, which is all that varied.

The len()==0 guards drop out: hydrateItemImages already short-circuits an
empty id list, and both loops are no-ops on an empty slice. applyItemImage
stays as-is; hydrateMediaFileArtwork and its own specs still use it.

* perf(scanner): stop reprocessing album and artist artwork on every full scan

A full scan re-imports every track, so the per-entity artwork enqueue in
persistChanges fired for every album and artist in the library — measured as
100% of both on a live instance, and ~8.5k artists / ~16.7k Deezer fetches on a
96k-file library. Re-importing a track is no evidence the art changed: artist
art has no track-content source at all, and the albums re-derived to identical
hashes across three consecutive scans.

Enqueue through EnqueueIfMissing on a full scan, which anti-joins item_artwork
so only entities that never resolved are queued. Incremental scans keep the
unconditional Enqueue, where a re-import does mean the file changed. Doing this
at the enqueue site rather than skipping it wholesale keeps a first scan filling
in covers as it walks, instead of stalling every cover until the scan ends.

EnqueueMissing grows a priority argument and runs once at end of scan, so an
entity phase 1 never saw still resolves, at Scan priority rather than dropping
to Recheck behind the backfill.

* refactor(artwork): derive blurhash components inside Encode

Components had a single caller, which only ever fed it the bounds of the image
it then passed to Encode. Exporting it gave callers two ways to get it wrong —
components mismatched with the image, or out of the 1..9 range — in exchange for
a knob nobody turned.

Derive them from img.Bounds() at the top of Encode and unexport the helper. The
counts must come from the pre-downscale bounds: downscale's integer rounding can
shift the ratio across a component boundary, and the hash is a client-side cache
key. Verified byte-identical over 18 hashes spanning 9 aspect ratios.

The out-of-range validation goes with it, being unreachable once the counts are
always derived. The aspect-ratio table now asserts through Encode's size flag,
which encodes (x-1)+(y-1)*9.

* refactor(ui): replace the blurhash package with a local decoder

The UI pulled in the `blurhash` dependency for one function, `decode`, called
from a single component. The decoder is ~80 lines of well-specified arithmetic,
so carrying a dependency for it costs more in supply chain and bundle than it
saves.

Equivalence was proven against the package before removing it: 84 hashes — three
real ones plus every component count from 1x1 to 9x9 — decoded at six sizes,
compared byte for byte, plus parity on which malformed inputs throw. Those pixel
values are now pinned in the spec, so drift from the reference algorithm fails.

The punch parameter is dropped rather than reproduced: no caller passes one, and
the package applies `punch | 1`, which silently turns a punch of 2 into 3.

* refactor: simplify the artwork enqueue and blurhash paths

Cleanup pass over the three preceding commits.

Tabulate the cosine terms in the UI blurhash decoder instead of calling Math.cos
per pixel per component: 248us -> 75us for a 32x32 decode, and an album grid
mounts one decoder per tile. Output is unchanged, which the pinned pixel specs
enforce. The Go encoder already tabulated the same terms.

Drop the dead paths that deriving components inside Encode left behind: the
zero-size guard in components, the post-downscale empty check, and the
no-AC-factor branch, which cannot be reached now that the counts are always at
least 1x9. The empty-image check moves ahead of the derivation, where it belongs.

In the queue mock, look up item_artwork by its existing iaKey rather than
scanning the map, and hold the lock across EnqueueIfMissing through a shared
unlocked helper instead of releasing it mid-operation. Extract the duplicated
drain-and-resolve block in the scanner specs into one helper.

* fix(artwork): refresh songs when their album's artwork changes

A track with no art of its own is served its album's, so hydration copies the
album's hash onto the track record. When an album resolution changed, the worker
broadcast only an `album` refresh, leaving song and now-playing surfaces holding
the previous hash-suffixed URL until something else refetched them.

Pair the album refresh with a song one. The dependent id list is unbounded — an
album has arbitrarily many tracks and a drained batch arbitrarily many albums —
so this refreshes the resource as a whole via the protocol's existing wildcard
rather than enumerating ids.

Reported by Codex on #5847.

* feat(persistence): log SQLite result codes on failed statements

SQLite reuses one message for errors that need different responses: "database
is locked" is both SQLITE_BUSY, which busy_timeout retries, and
SQLITE_BUSY_SNAPSHOT, which it can never retry because the transaction's read
snapshot is already stale. Reading only the message, the two are
indistinguishable, and a lock error seen in the wild could not be diagnosed
without guessing which one it was.

Add db.ErrorCodes to unwrap a sqlite3.Error and report its result and extended
result codes, and include them in the SQL error log. The helper lives in db
because that package already owns the driver, so persistence does not need to
import it. Constraint, readonly and disk-full errors share messages the same
way, so this applies to every failed statement, not just locks.

* fix(ui): keep the Random grid blank while a refresh re-rolls

Refresh bumps the list version, which changes the random seed and remounts the
grid. useRollChanged tracked the seed on screen in a ref inside the grid, so the
remount started it empty and adopted the new seed on the first render, while the
refetch had not begun and the store still held the previous roll. The grid
painted the old albums for the length of the request and swapped when the new
roll arrived.

Move the ref up to AlbumList, which a refresh does not remount, and pass it to
the grid and the pagination. The seed on screen then survives the remount, so a
refresh reads as a re-roll and blanks until the new roll lands. A search
keystroke keeps the seed and still leaves the grid in place.

* fix(ui): keep the album grid working outside the Random list

Hoisting the shown-seed ref into AlbumList made the prop mandatory in practice:
ArtistShow renders the same grid through ReferenceManyField and passes no seed
tracking, so useRollChanged dereferenced undefined and the artist page died with
"Cannot read properties of undefined (reading 'current')".

Own a ref in the grid when none is passed. A caller with no roll to track then
behaves as it did before, while the Random list keeps the ref that has to outlive
the refresh remount.

* refactor(config): make the artwork tuning options dev flags

ArtworkWorkerConcurrency and ArtworkExternalMaxRPS become
DevArtworkWorkerConcurrency and DevArtworkExternalMaxRPS, joining the
other DevArtwork* flags. Their defaults should not need tuning, so they
do not belong in the documented, user-facing option set.

* refactor(artwork): name the hashed image store folder for how it is addressed

The content-addressed store sat in artwork/store/, which did not
distinguish it from the artist/, playlist/ and radio/ upload folders
beside it — those hold artwork too. It is now artwork/hashed/, naming the
one property that sets it apart, and the path comes from a consts entry
rather than a bare literal, matching how the sibling folders are built.

Deliberately not under cache/: that folder holds resizes that rebuild
offline from a local source, while this one holds the only local copy of
externally fetched images, whose re-fetch depends on a third party still
serving them and whose bytes back the stored blurhash and dimensions.

No migration: anything left in the old artwork/store/ is orphaned and
re-resolved into the new location.

* refactor(artwork): call the album e2e helpers by their real names

album_test.go aliased expectAlbumFolderCover and expectAlbumAbsent to
shorter local names, which cost a lookup to resolve and hid the prefix
that distinguishes them from the artist and playlist helpers.

* test(artwork): cover the album-root and artist-folder path arithmetic

The unit specs for these helpers lived in the readers #5856 patched, both
deleted here, leaving the album-root promotion and the artist-folder climb
covered only end-to-end. A layout can show which image won but not why, so
these pin the parts behind it: that the parent is fetched only when it could
qualify as an album root, that a failed fetch propagates or degrades, and
that commonDir keeps a shared name fragment from reading as a shared folder.

Each spec was checked against a mutant: removing the library-root guard, the
other-album audio check, the single-folder short circuit, the single-root
climb, or commonDir's separator all turn one red.

Master's remaining specs were dropped as redundant — they cover sources this
branch already exercises under different names.

* fix(ui): stop artwork refreshes reloading the whole page

Resolving an album broadcast song:["*"], because a track with no art of its
own is served its album's and the dependent ids are unbounded server-side.
useResourceRefresh checked for that wildcard across every resource in the
payload, not just the ones a component shows, so the album grid called
refresh() — a full page reload — for every drained batch. Upgrading a large
library reloaded the grid thousands of times.

The client knows what the server cannot: which tracks are loaded, and their
albumId. So the fan-out moves there, the wildcard check is scoped to watched
resources, and the backend now names only what it resolved.

The playlist views watch playlistTrack too: their rows carry albumId but are
keyed by playlist entry, so a song refresh never reached them — previously
masked by the wildcard's page reload.

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(ui): stop the album grid blanking its covers on refresh

Cover takes its height from react-measure, which reports nothing until it
re-measures. That was harmless while the cover class sat on the img itself,
which has intrinsic size; it now sits on the Artwork root, whose img fills it
absolutely and so lends no height. A refresh remounts the tiles, and for the
frame before measurement lands the box collapsed to zero and every cover
vanished.

Give the box its own aspect ratio as a floor. The measured height still wins
once it arrives.

* refactor(artwork): drop the unused Bump/wake path from the worker

Worker.Bump had no production callers. Every real bump writes the queue row
through the repository instead: artwork.Refresh (the refresh endpoint and the
uploader), service.enqueue (read-through and stale-absent views), and
radio_repository. None of them wake a pool.

Bump was also the only sender to drainPool.wake, so the select case it fed was
unreachable outside tests, and runPool read as if a bump were picked up
promptly when it always waited for the 5s poll.

The e2e and unit suites used Bump only as a driver, never as the subject, so
they now enqueue with EnqueueBump and exercise the path production takes.
EnqueueBump rather than Refresh because Refresh also clears the state row,
which would change semantics for the re-bump-after-state and dedup specs.

Both harnesses start Run after enqueueing, so a fresh pool drains on its first
iteration and the wake never affected them: core/artwork/e2e averaged 3.26s
before and 3.30s after over 5 runs, within noise.

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(artwork): unexport what nothing outside the package calls

PlaceholderFor had no callers at all. Its comment offered it to callers that
must not consult persisted state, but no such caller was ever written, so it
is removed rather than unexported.

entityExists and encode83 are only reached from inside their own packages;
their tests are package-internal (artwork) or never touch them (blurhash_test).

ImageStore stays exported: server/subsonic/e2e constructs one to wire up the
worker, and that suite cannot move into package artwork.

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(persistence): name the backoff-preserving enqueue for what it does

EnqueueBump was named for a priority, but its behaviour is preserving an
existing row's retry_at; the priority comes from the item. Its one caller
passes ArtworkPriorityScan, which read like a bug at the call site and is not.

The three DO NOTHING inserts each repeated the INSERT prefix, the column list
and the conflict clause. They now share insertIfNotQueued, with the CTE that
EnqueueIfMissing needs passed as a prefix, and the column list lives in one
slice used by both squirrel and the raw SQL.

EnqueueIfMissing had no test against real SQL, only a mock mirroring the
anti-join by hand, so the rewritten statement had nothing verifying it. Two
specs now cover it: items with a state row are skipped, and an already-queued
row keeps its priority.

Signed-off-by: Deluan <deluan@navidrome.org>

* test(thumbhash): vendor the reference and generate golden vectors

* test(thumbhash): add a faithful reference port as the differential oracle

* fix(thumbhash): re-condition alpha fixture and assert solid.png header

alpha.png varied only alpha over a constant color, so compositing atop the
average canceled L/P/Q exactly like solid.png, leaving it just as float-noise
unstable and silently dropping alpha-path coverage. Vary R/G/B with position
too so alpha.png carries real signal and reproduces byte-exactly, then
replace the blanket solid.png skip with a precise assertion on its
well-conditioned header bytes and quantized-zero scales.

Also apply a range-over-int modernizer hint in reference_test.go.

* test(thumbhash): dither the fixtures so no DCT coefficient is degenerate

The gradient fixtures are smooth analytic ramps, and alpha.png's alpha channel varied
along x only. Both make whole families of DCT coefficients mathematically zero: 13 of
alpha.png's 38 AC nibbles, and at least one in every other ramp fixture.

A zero coefficient normalizes to exactly the 0.5 midpoint, i.e. 15*f = 7.5, so which of
nibble 7 or 8 it quantizes to is decided by ~1e-16 of float rounding noise. Those nibbles
are therefore unstable across any two summation orders -- the vendored JS and the Go port
already disagreed on solid.png for this reason -- and they carry no signal, so a bug that
transposed two of them would be invisible.

A deterministic +/-4 dither, plus an alpha ramp that varies in x and y, leaves every
coefficient at least 4.8e-5 from the tie boundary: ~1e9 times the observed inter-
implementation noise. solid.png and tiny.png regenerate byte-identical and keep their
existing carve-outs.

* feat(thumbhash): add a two-pass separable ThumbHash encoder

Encode replaces the reference's four w*h scratch arrays (~320 KB at 100x100) and its 40
re-reads of every pixel with a single pixel pass that folds each row into per-frequency
sums, then a second fold over rows. The transform drops from O(terms * pixels) to
O(nx * pixels + terms * h), nx <= 7.

Verified against the vendored JS goldens and, differentially, against the literal Go port
on 500 randomized images. On the fixtures the two implementations' AC coefficients agree
to 8e-15; the separable inner loop reassociates the additions, so exact agreement holds
only where a coefficient is not sitting on a quantization tie, which is why the fixtures
are now dithered.

* test(thumbhash): fuzz the opaque path and drop an ill-conditioned golden

The 500-image randomized differential filled every byte randomly, so hasAlpha
(avgA < w*h) was true for all 500 images: the 7x7 no-alpha layout, terms(7,7),
nx=7 and the `if hasAlpha` false branch were never fuzzed. Force full opacity on
alternating iterations, which splits the run 250/250 with the seed and iteration
count unchanged. All 500 still match the reference port.

tiny.png is 1x1, so it has no non-zero AC content: 12 of its 37 AC coefficients
sit exactly on the round(15*f) = .5 tie and 24 are within 1e-12. It passed only
because a single pixel admits no summation reassociation. Give it the same
header-only carve-out solid.png already had, in both test files. The four
well-conditioned fixtures keep strict full byte equality.

Also document the divergence class on Encode itself rather than only in test
comments, add a sub-image regression spec, and use the max builtin over
math.Max. The toNRGBA Rect.Min gate turns out to protect nothing — SubImage
re-slices Pix so Pix[0] is the Rect.Min pixel and the loops read it correctly
either way — so its comment, which claimed the opposite, is corrected. The gate
is kept for now; removing it is a separate call.

* test(thumbhash): benchmark against blurhash at the pipeline input size

* feat(artwork): persist a thumbhash alongside the blurhash

The artwork table is content-addressed, so this costs one row per unique image
rather than per entity. Measured on a 25k-image library: +0.6MB on a 727MB DB.

The column is added to the existing add_artwork_tables migration rather than a
new one, since #5847 has not been released and the table it extends ships in
that same PR.

* feat(artwork): hydrate and expose thumbHash on every entity

ItemArtworkInfo.Image() is the single projection every hydration branch goes
through, so adding the field there covers albums, artists, playlists and both
the own-art and inherited media-file branches.

* feat(artwork): encode blurhash and thumbhash from one 100px thumbnail

thumbnailSize drops 128 -> 100 so a single CatmullRom scale feeds both encoders;
thumbhash hard-rejects anything larger and a second downscale would cost more
than shrinking the shared one.

decodeArtwork on a 1000x1000 JPEG, before -> after:
  15.53ms -> 15.45ms/op, 5878122 -> 5014796 B/op, 37 -> 75 allocs/op

Time is a wash because the JPEG decode dominates; the 863KB drop is the smaller
thumbnail more than paying for thumbhash's added work.

This rewrites every blurhash value, so it must land before #5847 reaches a
release: Finamp keys its cover cache and download dedup on that value.

* feat(ui): use thumbhash for the cover loading placeholder

Adds a local decoder rather than the thumbhash npm package, matching the local
blurhash decoder it replaces. Pixels are pinned against evanw/thumbhash's
reference decoder via the vendored copy already in the encoder's testdata.

Unlike a blurhash, a thumbhash carries its own approximate aspect, so a record
with no dimensions now falls back to that instead of to a square.

The blurHash API field stays: Jellyfin clients and third-party native clients
still consume it.

* perf(artwork): hand both hash encoders one NRGBA thumbnail

makeThumbnail emitted a premultiplied *image.RGBA, so thumbhash allocated and
un-premultiplied a full copy on every image while blurhash paid nothing. It now
emits *image.NRGBA, which thumbhash wants as-is, and blurhash reads that type
directly, premultiplying per pixel to keep its output identical.

thumbhash.Encode at the pipeline's 100x100 input:
  134200 -> 95300 ns/op, 56188 -> 15163 B/op, 37 -> 35 allocs/op
decodeArtwork on a 1000x1000 JPEG:
  5014796 -> 4973800 B/op, exactly the conversion that is gone

The scaler costs the same into either destination (7.33ms vs 7.34ms measured),
so no time is traded for this.

* test(artwork): drop the decodeArtwork benchmark

It existed to capture the before/after baseline for the 128->100 thumbnail
change, which it has served. As an ongoing guard it is misleading: a 1000x1000
JPEG decode is ~97% of the 15ms it measures, so the two encoders it would be
reached for are ~2.6% of the signal. It reported a phantom 2.8% regression for
the NRGBA thumbnail change that isolating the scaler disproved.

The per-encoder benchmarks in blurhash/ and thumbhash/ cover the part that
actually changes.

* test(artwork): benchmark both hash encoders from one file

benchImage and BenchmarkEncodeAtInputSize existed in both encoder packages, and
blurhash's copy had drifted into a third inline duplicate once both benches
moved to NRGBA input.

The head-to-head now lives in core/artwork, which already imports both encoders
and can pin the input to the real thumbnailSize constant. The gradient builder
moves to tests, which both packages already import via their suite bootstrap.

thumbhash keeps a shipped-vs-reference benchmark, since referenceEncode is
test-only and cannot be reached from core/artwork.

* test(thumbhash): drop the shipped-vs-reference benchmark

The two-pass rewrite's advantage over the naive port is already recorded in its
commit message; carrying the benchmark to re-derive it has no ongoing use.

reference_test.go stays: besides the benchmark baseline it holds the
differential oracle the golden-vector and randomised specs assert against.

* test(thumbhash): assert against reference goldens, drop the Go port

The reference port existed to be a differential oracle, but its packing half was
a verbatim copy of production pack(), so it was not as independent as it looked.
Only the 500-image randomised spec needed it; the six PNG fixtures cannot reach
random sizes, aspects, or both coefficient layouts.

gen_generated.mjs now emits 300 vectors from evanw's actual JS. Their pixels are
a pure function of their index, so Go rebuilds them byte-for-byte and only the
hashes are committed (16KB). The oracle is now the reference itself rather than
a hand transcription of it.

Also from the cleanup pass:
- maxCX/maxCY scanned every term to recover a bound that is just the widest
  coefficient region
- tests.GradientImage duplicated generateGradientImage three files away
- BenchmarkHashEncodersAtInputSize was also BenchmarkHashEncoders/*/100x100
- processor had two internal test files with no rule for which gets a new spec
- three blurhash specs differing only by alpha became a DescribeTable
- the shared mock's GetInfoForItems projection had drifted from the real query

* refactor(model): keep the blurhash off native JSON and fold its specs

Nothing on the native API consumes blurHash: the web UI reads thumbHash, and
Jellyfin's mappers take the Go field directly rather than through this
serialization. It was shipping ~40 unused bytes on every row of every list
response, on a surface that accretes clients once published.

The JSON specs were five marshal-and-check-keys blocks over the same struct;
they collapse to one populated case, one bare case, and a guard that the
blurhash stays off the wire.

* refactor(persistence): simplify the artwork dangling-row purge helper

purgeDangling took a bound executeSQL method value and a table name only because
its two callers live on unrelated types. Neither parameter was load-bearing:
executeSQL is a value receiver on sqlRepository that never reads tableName, and
both callers already hold an sqlRepository whose tableName is the table being
purged. Taking that struct collapses both parameters into the receiver.

itemArtworkSQL wrapped sqlRepository without adding a single method, so the items
field becomes a plain sqlRepository.

danglingItemArtworkKinds is also read by EnqueueAllMissing, which enqueues
entities that have no artwork row yet - neither a purge nor anything specific to
item_artwork. Renamed to artworkOwnerTables to describe both of its uses.

* fix(artwork): keep sweeping past a store file that cannot be removed

Sweep returned the os.Remove error straight out of WalkDir, so a single
unremovable file abandoned the rest of the walk. Every later orphan, stale mime
variant and abandoned temp file then survived until the next prune, which would
abort at the same place.

Warn and carry on instead. The orphan-file loop in prune already had this
resilience; Sweep is where it belongs, since it is the only step that reaches
stray files and superseded variants.

* refactor(artwork): let the sweep reclaim orphan files instead of prune

Deleting unreferenced artwork rows took five round trips: snapshot the orphan
hashes, fetch their mimes, delete, re-fetch to see which rows actually went, then
remove each file. The last four exist only to work out which files to delete -
but the sweep that runs moments later already derives exactly that from the
database, since GetAllMimes is read after the delete and Sweep removes any file
whose hash has no row, under the same mtime guard. The sweep is also the more
thorough of the two: it reclaims stale mime variants of an orphan hash, which the
per-hash removal never looked at.

Delete the rows in one predicate-only statement and let the sweep follow. The
orphan predicate now runs once per prune instead of once for the snapshot plus
once per 200-hash delete chunk, and with no hash list to bind there is nothing
left to chunk.

GetOrphanHashes and GetImages had no other caller and are gone. Dropping them
also removes the mock's OrphanHashes lever, which let a spec declare a hash
orphaned while item_artwork still referenced it - a state the SQL repository
could never produce, since both predicates were always identical.

* refactor(model): name the artwork mime lookup for what it is keyed by

GetAllMimes reads as a collection of mime types; it returns a hash -> mime index
over every stored artwork. GetMimeByHash names the key, matching the convention
the other map-returning repository methods already follow (CountBySuffix,
CountByClient).

* refactor(artwork): split the repository's deletes from its purges by name

Six removal methods across the two artwork repositories used Delete and Purge
interchangeably, so neither prefix told you anything. There is a real distinction
underneath: Delete methods are caller-directed and take the rows to remove, while
Purge methods are garbage collection - the repository finds rows whose referent
is gone by joining against the owning table, and returns a count because the
caller cannot know one. The signatures already followed that split; only the
names did not.

DeleteOrphans was the odd one out, taking a grace cutoff rather than a target and
returning a count, so it becomes PurgeOrphans. PurgeDanglingItemArtwork loses the
Artwork it repeats from its own interface and keeps the Items that says which of
that interface's two tables it means.

DeleteForItem was DeleteForItems with one id - squirrel emits = for a scalar and
IN for a slice against the same predicate - so its one caller passes a slice
instead. Dangling and Orphan stay distinct: dangling is a row whose entity is
gone, orphan is an image no state row references.

* refactor(artwork): drop the orphaned store remover, aggregate sweep failures

Review follow-ups to the prune rework.

ImageStore.Remove lost its only production caller when the sweep took over
orphan-file reclamation. It carried its own copy of the mtime guard that now
lives solely in Sweep, so the two could drift, and it stood as an invitation to
grow a second reclaim path. Its specs either duplicated Sweep's own coverage or
tested nothing else, so they go with it; the one unrelated test that used it to
delete a file calls os.Remove directly.

Sweep warned once per file it could not remove, which is fine for a stray
permission bit and useless for a store mounted read-only - a large library would
emit one line per aged file on every prune, and prune would still report success.
Count the failures instead and warn once, with the count and the last error, so a
systemic failure is legible without drowning the log.

Also: the orphan log line counts rows, not files, and now says so; and the
blocked-removal spec asserts its two fixtures really do land in different shards,
since the assertion is vacuous if they collide.

* fix(ui): show the placeholder when a refresh swaps in an uncached cover

A cover whose hash changed under a mounted grid went blank for the whole fetch,
instead of falling back to its thumbhash. Artwork decided 'this blob was already
cached, skip the placeholder' once at mount and never revisited it, so every
later url for that record inherited the answer.

The fix has to live in useImageUrl. imgUrl is state cleared in an effect, so on
the render where url changes it still holds the previous url's blob - any caller
inspecting it reads the new url as already cached. Only the hook can see
cache.get(url) for the url actually being rendered, so it now reports fromCache
and resyncs its state during render rather than one render later.

Two shapes were tried and rejected against a live server before this one: a ref
recomputed per url, which a render-phase resync discards without rolling back the
ref, and the same tracking in Artwork state, which still samples imgUrl before
React re-runs the component. Verified on a running instance with cover-art
responses held open: the thumbhash now covers the whole window and stays under
the image until the fade ends.

* fix(ui): keep the placeholder for a cover whose fetch failed

useImageUrl caches a failed fetch as an entry with no blob, so treating every
cached entry as instantly painted suppressed the placeholder on remount: the
cover became an empty box instead of falling back to its thumbhash. Only a
cached blob counts as instant.

Caught by Codex on the previous commit, which introduced fromCache with the
looser predicate.

* feat(artwork): capture each image's dominant colour

A flat colour is the one placeholder a client can paint with no decoding at all,
so it can cover the frames before a thumbhash canvas even renders. Extract it
from the same 100px thumbnail the two hash encoders already share, and expose it
as dominantColor on the native API.

Dominant means presence, not salience: the largest cluster wins, so a white
sleeve reports white. That is what a placeholder needs. An accent colour is the
opposite question and is deliberately not answered here - it depends on the
client's theme, and by the time a client needs one it has the cover pixels and
can compute whatever suits it.

Bins at 4 bits per channel, then merges perceptually-near bins in Oklab before
picking the winner: a gradient splits across adjacent bins and would otherwise
lose to a smaller flat region. Measured at 246us per image on the weakest target
hardware we care about (Celeron N5105), against ~15.8 images/s for the pipeline
around it.

The column goes into the feature's original migration rather than a new one:
computing it later would mean re-decoding every image in the library.

* fix(artwork): ignore image candidates that cannot be fetched

bestImageURL only rejected URLs that url.Parse itself refuses, which is almost
nothing: relative paths and any scheme parse cleanly. A candidate like
images/big.jpg or ftp://host/big.jpg therefore won on size, and since the
function returns a single URL whose failure ends that agent's turn, the agent's
valid smaller images were never tried.

Two details made it easy to hit rather than theoretical. Size is frequently 0 for
every candidate, and only a strictly larger one replaces the first, so an
unfetchable entry in first position sticks. And these strings come straight from
plugins, where a relative path is an ordinary mistake.

Accept only absolute http/https URLs with a host.

* revert(ui): move detail-page scroll handling to its own branch

Reverts 40153bd43. Scroll position carrying over between pages is stock React
Router behaviour rather than something this branch introduced, so the fix does
not belong in an artwork PR that is otherwise ready for review.

The work continues on fix/scroll-restoration, off master, where it grew into
per-route restoration: a new page starts at the top and going back returns to
where you were, which the scroll-to-top could not do.

* fix(ui): fade cover art consistently, and shorten the fade to 150ms

The <img> mounts with its blob URL already set, so onLoad often fires in the
same frame the element is inserted. The opacity:0 start state never gets
painted, the transition has no value to animate from, and the cover pops in
instead of fading. Only tiles whose decode happened to straddle a paint
boundary actually faded, which made a grid load look ragged: 4 to 6 of 18
covers faded, varying between runs.

Deferring the decoded flag by two animation frames guarantees the hidden state
is painted first. Measured on a live grid afterwards, 17 of 18 covers fade, the
18th having a cached blob and taking the intentional instant path, and the tail
after the last byte arrives drops from ~440ms to ~163ms.

The duration also drops from 500ms to 150ms. That 500ms mirrored master's fade,
but master paints no placeholder behind the image and needs a slow blend to
cover a blank tile. Here a thumbhash already fills the gap, so a long crossfade
only delays the grid settling, measured at 145-190ms slower to full opacity
than master.

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-08-09 15:03:27 -04:00
Kendall Garner
b40b41584a
feat(subsonic): Implement OpenSubsonic topSongsByArtistId extension (#5853)
* implement topSongsByArtistId extension

* do not look up by artist name if empty

---------

Co-authored-by: Deluan Quintão <deluan@navidrome.org>
2026-08-02 12:39:51 -04:00
Deluan Quintão
bd9fa1c602
feat(listenbrainz): match collaboration top-songs via all credited artist MBIDs (#5670)
* refactor(agents): drop single Artist fields from Song, keep only Artists

Song now represents credited artists solely via the Artists slice; the
single Artist/ArtistMBID fields and the ArtistList passthrough are removed.
Equals continues to hash the whole value.

* refactor(lastfm,listenbrainz): build Song.Artists in built-in agents

Last.fm and ListenBrainz now populate the Artists slice directly. For
ListenBrainz top songs, all credited artist MBIDs are mapped (the combined
display name on the first credit plus MBID-only collaborators) instead of
keeping only the first MBID, feeding the matcher's per-MBID specificity.

* refactor(external): set Song.Artists in top-songs enrichment

getMatchingTopSongs now seeds an Artists entry from the known artist when a
song carries none, replacing the single Artist/ArtistMBID writes.

* refactor(plugins): fold single-artist SongRef into Song.Artists

SongRef keeps its single Artist/ArtistMBID fields as part of the plugin wire
contract; songRefToAgentSong now folds them into a one-element Artists list
when a plugin sends no artists array.

* refactor(matcher): read Song.Artists directly

groupQueries consumes s.Artists now that ArtistList is gone; test inputs
build the Artists slice.

* fix(matcher): keep MBID-only artists as identity signals

Review follow-up: an artist credited only by MBID (empty ID and name) was
dropped before resolution in four places, defeating the multi-MBID matching
path this PR adds.

- matcher.groupQueries: treat a non-empty MBID as a usable artist signal
- external.getMatchingTopSongs: backfill the primary credit's name/MBID when
  the agent left them empty
- plugins.songRefToAgentSong: fold a single-artist SongRef when only ArtistMBID
  is set (not just when Artist name is set)
- listenbrainz.topSongArtists: return nil instead of an empty-name placeholder
  when neither name nor MBIDs are present

* fix(external): only backfill top-song artist onto an unnamed credit

Review follow-up (codex P2): the previous backfill stamped the queried
artist's MBID onto Artists[0] whenever it was empty, even when that credit
already named a different (e.g. featured) artist — producing a mismatched
name+MBID pair that could mis-rank matches. Now only an unnamed first credit
is filled (it is, by construction, the queried artist); an already-named
credit is left untouched.

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-06-26 17:06:14 -04:00
Deluan Quintão
803b385920
fix(matcher): match by artist credit so artist-MBID specificity works and collaborators match (#5637)
* refactor(matcher): carry resolved artist MBID on sanitizedTrack

* feat(matcher): two-phase artist resolution for title matching

* fix(matcher): wire phase-1 artist mock in consumer tests; guard back-map dupes

- External tests for title-fallback paths now register a second `.Maybe()`
  artistRepo.GetAll expectation for the matcher's phase-1 artist-resolution
  call, and supply RoleArtist Participants on phase-2 track mocks so the
  back-map routes tracks to their query buckets correctly.
- Back-map loop extracted into buildTracksByQuery helper; guard ensures each
  track is appended at most once per query bucket even when it credits multiple
  resolved artists in that bucket, preventing duplicate scoring.
- matchTitlePhase2 helper: removed redundant squirrel.Sqlizer type assertion
  (ranging over squirrel.And already yields Sqlizer).

* perf(matcher): use non-correlated IN subquery for phase-2 lookup

* refactor(matcher): decompose two-phase title matching into named steps

matchByTitle had grown to ~98 lines holding four jobs and eleven locals. Split
it into a thin orchestrator over named phases:

- groupQueriesByArtist: build the per-artist query buckets.
- resolveArtists / resolvedArtists: phase 1, with the three parallel maps
  (byQuery / mbid / allIDs) folded into one type that owns artist-ID routing and
  track bucketing. Artist ownership is now two direct map lookups (by order name,
  by MBID) instead of the prior O(artists x queries) nested scan.
- fetchTracksCreditedTo: phase 2, using squirrel.Placeholders instead of a
  hand-rolled placeholder string.
- bucketTracks / scoring loop: unchanged behavior.

Pure restructuring; the suite, race, consumers, and the real-DB benchmark are all
unchanged. Also dedupes the phase-1 MBID filter as a side effect.

* refactor(matcher): apply simplify cleanups to two-phase resolution

Behavior-preserving cleanups from a /simplify pass:
- struct{} sets instead of bool-valued sets (codebase convention)
- move newSanitizedTrack after the dedup guard in bucketTracks, so a track
  credited to multiple resolved artists is sanitized only when actually bucketed
- pre-size the phase-1/phase-2 maps; drop the dead nil-guard in own()
- slice.Map for the []string->[]any arg conversion
- document why the raw media_file_artists SQL stays in this layer, and that
  bucketTracks relies on the bulk participants JSON carrying artist IDs
- extract an artistParticipants() test helper, collapsing 57 four-level
  Participants literals (test file -211 lines net)

* docs(matcher): rename inner title-matching steps to avoid 'phase' clash

The matcher's top-level strategies (ID/MBID/ISRC/Title) are already called phases.
Reusing 'phase 1/2' for the two steps inside title matching (resolve artists,
fetch their tracks) was confusing. Drop the ordinals and let the function names
(resolveArtists, fetchTracksCreditedTo) and prose describe the steps. Renamed the
matchTitlePhase2 test helper to matchTracksByArtistQuery. Comment-only; no behavior
change.

* fix(matcher): own MBID-resolved artists for every aliased query

Address bot review on PR #5637. When several agent queries share one
ArtistMBID under different sanitized names (agent aliases), the resolver
kept only the last query name per MBID, so the others never owned the
resolved artist and their songs fell through. Track all query names per
MBID instead; add a RED-proven test for the two-alias case.

Also invert byQuery into a reverse artist-ID -> query-name index in
bucketTracks, dropping the per-participant scan over all queries, and
guard fetchTracksCreditedTo against an empty artist-ID slice. Fix the
artist mock to forward variadic options with Called(options...) so
QueryOptions-shaped matchers receive the same argument shape as real
calls.

* docs(matcher): trim comments that restated the code

Cut three doc comments down to their why: groupQueriesByArtist, own, and
bucketTracks no longer restate what the signature and body already show.
Condense fetchTracksCreditedTo's rationale from two paragraphs to one,
keeping the role='artist', non-correlated-IN, and layer-boundary notes.
2026-06-21 11:05:58 -04:00
Deluan Quintão
6486a27634
refactor(matcher): index-space resolution + batched title lookups (#5635)
* refactor(matcher): resolve matches in song-index space

* test(matcher): pin per-index duration matching for duplicate title+artist songs

* refactor(matcher): drop unreachable specificity sentinel

* refactor(matcher): hoist PreferStarred read out of scoring loop

* docs(matcher): correct config field references in MatchSongs doc

* fix(matcher): log swallowed per-artist DB error in title matching

* fix(matcher): fail title matching when all artist lookups error

* refactor(matcher): simplify loaders and test helpers

* docs(matcher): move algorithm docs to package-level doc.go

* docs(matcher): focus examples on fuzzy matching behavior

* refactor(matcher): store index in dedup map and harden test helper

* fix(matcher): keep exact-phase matches when all title lookups fail

* perf(matcher): batch title-phase artist lookups into one query

matchByTitle issued one GetAll per distinct artist, run serially. On a large
library a batch of similar-songs spans dozens of artists, and profiling against
a 95k-track library showed the matcher was ~90% bound in that serial query loop
(a 100-song batch fired ~89 separate multi-join queries, taking ~6s).

Replace the loop with a single 'order_artist_name IN (...)' query, then group the
returned tracks by artist in memory and score each song against its bucket. This
cuts a 100-song batch from ~6s to ~0.4s (roughly 14x) with less memory.

Grouping keys on order_artist_name (the field the query filters on, matching how
the per-artist queries are keyed), falling back to the sanitized Artist when it
is unset. Because there is now a single query, matchByTitle is all-or-nothing
like the ID/MBID/ISRC loaders: the per-artist best-effort skip is gone, while
resolveMatches still preserves exact-phase matches when the title query fails.

* refactor(matcher): group batched title matches by order_artist_name

After batching the title-phase lookups into one query, the returned tracks must
be grouped back to their artist. Key on MediaFile.OrderArtistName — the exact
field the query filters on — so collaboration/"feat." tracks (whose display
Artist differs from the sort artist) bucket correctly, with a sanitized-Artist
fallback when it is unset.

OrderArtistName is deprecated in favor of Participants, but the bulk GetAll path
does not hydrate participant detail (the rich artist fields come only from the
per-record GetWithParticipants JOIN), so the participant order name is empty here
and the column is the only populated source.

Also adds a TODO in computeSpecificityLevel: its artist-MBID levels read the
deprecated, unpopulated MediaFile.MbzArtistID column, so they never fire today.
2026-06-21 03:45:56 -04:00
Deluan
efe9291db0 refactor: multiple syntax updates for Go 1.26
Signed-off-by: Deluan <deluan@navidrome.org>
2026-05-19 18:02:36 -03:00
Deluan Quintão
259c1a9484
feat(subsonic): add sonicSimilarity extension as plugin capability (#5419)
* feat(plugins): add sonicSimilarity capability types

Defines the SonicSimilarity plugin capability interface with
GetSonicSimilarTracks and FindSonicPath methods, along with
their request/response types.

* feat(sonic): add core sonic similarity service

Implements the Sonic service with HasProvider, GetSonicSimilarTracks,
and FindSonicPath, delegating to the PluginLoader and using the
Matcher for index-preserving library resolution.

* test(sonic): add sonic service unit tests

Covers HasProvider, GetSonicSimilarTracks, and FindSonicPath with
mock plugin loader and provider, verifying error propagation and
successful match resolution via the library matcher.

* feat(matcher): add MatchSongsToLibraryMap for index-preserving matching

Adds a new method alongside MatchSongsToLibrary that returns a
map[int]MediaFile keyed by input song index rather than a flat slice,
enabling callers to correlate similarity scores back to the original
position in the results.

* fix(sonic): check provider availability before MediaFile lookup

Avoids unnecessary DB call when no plugin is available, and ensures
the correct error path is tested.

* feat(plugins): add sonic similarity adapter

Adds SonicSimilarityAdapter implementing sonic.Provider, bridging the
plugin system to the core sonic service via Extism plugin functions.
Reuses existing songRefsToAgentSongs helper for SongRef conversion.

* feat(plugins): add LoadSonicSimilarity to plugin manager

Adds Manager.LoadSonicSimilarity method following the pattern of
LoadLyricsProvider, enabling the core sonic service to load a
SonicSimilarityAdapter from a named plugin.

* feat(subsonic): add sonicMatch response type

Add SonicMatch struct with Entry and Similarity fields, and a SonicMatches slice to the Subsonic response struct. These types support the OpenSubsonic sonicSimilarity extension for returning similarity-scored track results.

* feat(subsonic): add getSonicSimilarTracks and findSonicPath handlers

Add two new Subsonic API handlers for the sonicSimilarity OpenSubsonic extension: GetSonicSimilarTracks returns similarity-scored tracks similar to a given song, and FindSonicPath returns a path of tracks connecting two songs. Both handlers delegate to the sonic core service and map results to SonicMatch response types.

* feat(subsonic): advertise sonicSimilarity extension when plugin available

Update GetOpenSubsonicExtensions to conditionally include the sonicSimilarity extension only when a sonic similarity plugin provider is available. The nil guard ensures backward compatibility with tests that pass nil for the sonic field. Also update the existing test to pass the new nil parameter.

* feat(subsonic): wire sonic similarity service into router

Add the sonic.Sonic service to the Router struct and New() constructor, register the getSonicSimilarTracks and findSonicPath routes, and wire sonic.New and its PluginLoader binding into the Wire dependency injection graph. Update all existing test call sites to pass the new nil parameter. Regenerate wire_gen.go.

* fix(e2e): add sonic parameter to subsonic.New call in e2e tests

* test(subsonic): add sonicSimilarity extension advertisement tests

Restructures the GetOpenSubsonicExtensions test into two contexts: one verifying the baseline 5 extensions are returned when no sonic similarity plugin is configured, and one verifying that the sonicSimilarity extension is advertised (making 6 total) when a plugin loader reports an available provider. Adds a mockSonicPluginLoader to satisfy the sonic.PluginLoader interface without requiring a real plugin.

* feat(subsonic): add nil guard and e2e tests for sonic similarity endpoints

Handlers return ErrorDataNotFound when no sonic service is configured,
preventing nil panics. E2e tests verify both endpoints return proper
error responses when no plugin is available.

* fix(subsonic): return HTTP 404 when no sonic similarity plugin available

Endpoints are always registered but return 404 when no provider is
available, rather than a subsonic error code 70.

* refactor: clean up sonic similarity code after review

Extract shared helpers to reduce duplication across the sonic similarity
implementation: loadAllMatches in matcher consolidates the 4-phase
matching pipeline, songRefToAgentSong eliminates per-iteration slice
allocation in the adapter, sonicMatchResponse deduplicates response
building in handlers, and a package-level constant replaces raw
capability name strings in core/sonic.

* fix empty response shapes

Signed-off-by: Deluan <deluan@navidrome.org>

* test(plugins): add testdata plugin and e2e tests for sonic similarity

Add a test-sonic-similarity WASM plugin that implements both
GetSonicSimilarTracks and FindSonicPath via the generated sonicsimilarity
PDK. The plugin returns deterministic test data with decreasing similarity
scores and supports error injection via config. Adapter tests verify the
full round-trip through the WASM plugin including error handling. Also
includes regenerated PDK code from make gen.

* docs: update README to include new capabilities and usage examples for plugins

Signed-off-by: Deluan <deluan@navidrome.org>

* test(e2e): enhance sonic similarity tests with additional scenarios and mock provider

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: address PR review feedback for sonic similarity

Fix incorrect field names in README documentation ({from, to} →
{startSong, endSong}) and remove unnecessary XML serialization test
from e2e suite since OpenSubsonic endpoints only use JSON.

* refactor: rename Matcher methods for conciseness

Rename MatchSongsToLibrary to MatchSongs and MatchSongsToLibraryMap to
MatchSongsIndexed. The Matcher receiver already establishes the "to
library" context, making that suffix redundant, and "Indexed" better
describes the intent (preserving input ordering) than "Map" which
describes the data structure.

* refactor: standardize variable naming for media files in sonic path methods

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: simplify plugin loading by introducing adapter constructors

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-04-27 17:50:09 -04:00
Deluan Quintão
7e083e0795
fix: split html sanitization from plaintext handling (#5403)
* fix: split html sanitization from plaintext handling

Add a dedicated SanitizeHTML helper for HTML-rendered values so entity-encoded markup is decoded before bluemonday sanitization. Use the new helper for the login welcome message and artist biographies while preserving SanitizeText semantics for lyrics and other plaintext callers. Add regression coverage for both helpers and the serveIndex welcomeMessage path.

* docs: add SanitizeText and SanitizeHTML godoc

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: preserve plain text in artist biographies

Revert artist biography storage to SanitizeText so entity-encoded plain text remains decoded for Subsonic consumers. This avoids double-escaping values like R&B in XML responses while keeping the new welcomeMessage HTML sanitization in place, and adds a regression test covering the biography storage behavior.

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-04-23 17:53:28 -04:00
Deluan Quintão
3b7d3f4383
feat(matcher): add Matcher.PreferStarred option to bias fuzzy matcher toward starred/high-rated tracks (#5387)
* matcher: update godoc for matcher config scoring order

* conf: log deprecated SimilarSongsMatchThreshold option

* conf: enable matcher prefer-starred by default
2026-04-19 12:54:41 -04:00
Deluan Quintão
52e47b896a
refactor: extract song-to-library matcher to core/matcher package (#5348)
* refactor: extract matchSongsToLibrary to core/matcher package

Move the song-to-library matching algorithm from core/external into its own
core/matcher package. The Matcher struct exposes a single public method
MatchSongsToLibrary that implements a multi-phase matching algorithm
(ID > MBID > ISRC > fuzzy title+artist). Includes pre-sanitization
optimization for the fuzzy matching loop.

No behavioral changes — the algorithm is identical to the version in
core/external/provider_matching.go.

* refactor: inject matcher.Matcher via Wire instead of creating it inline

Add *matcher.Matcher as a dependency of external.NewProvider, wired via
Google Wire. Update all provider test files to pass matcher.New(ds).
This eliminates tight coupling so future consumers can reuse the matcher
without depending on the external package.

* refactor: remove old provider_matching files

Delete core/external/provider_matching.go and its tests. All matching
logic now lives in core/matcher/.

* test(matcher): restore test coverage lost in extraction

Port back 23 specs that existed in the old provider_matching_test.go
but were dropped during the extraction. Covers specificity levels,
fuzzy matching thresholds, fuzzy album matching, duration matching,
and deduplication edge cases.

* test(matcher): extract matchFieldInAnd/matchFieldInEq helpers

The four inline mock.MatchedBy closures in setupAllPhaseExpectations
all followed the same squirrel.And -> squirrel.Eq -> field-name-check
pattern. Extract into two small helpers to reduce duplication and
make the setup functions read as a concise list of phase expectations.

* refactor(matcher): address PR #5348 review feedback

- sanitizedTrack now holds *model.MediaFile instead of a value copy. Since
  MediaFile is a large struct (~74 fields), this avoids the per-track copy
  into sanitized[] and a second copy when findBestMatch assigns the winner.
  loadTracksByTitleAndArtist updated to iterate by index and pass &tracks[i].

- loadTracksByISRC now sorts results (starred desc, rating desc, year asc,
  compilation asc) so that when multiple library tracks share an ISRC the
  most relevant one is picked deterministically, matching the sort order
  already used by loadTracksByTitleAndArtist.

- Restored the four worked examples (MBID Priority, ISRC Priority,
  Specificity Ranking, Fuzzy Title Matching) in the MatchSongsToLibrary
  godoc that were dropped during the extraction.

- matcher_test.go: tests now enforce expectations via AssertExpectations in
  a DeferCleanup. The old setupAllPhaseExpectations helper was replaced
  with per-phase helpers (expectIDPhase/expectMBIDPhase/expectISRCPhase +
  allowOtherPhases) so each test deterministically verifies which matching
  phases fire. This surfaced (and fixes) a latent issue copilot flagged:
  the old .Once() expectations were not actually asserted, so tests would
  silently pass even when phases short-circuited unexpectedly.
2026-04-12 16:47:22 -04:00
Deluan Quintão
0f6a076dca
fix(artwork): refresh stale artist image URLs on expiry (#5267)
* fix(external): refresh stale artist image URLs on expiry

ArtistImage() was serving cached image URLs from the database
indefinitely, ignoring ExternalInfoUpdatedAt. When users changed agent
configuration (e.g. disabling Deezer), old URLs persisted because only
the UpdateArtistInfo code path checked the TTL.

Now ArtistImage() checks the expiry and enqueues a background refresh
when the cached info is stale, matching the pattern used by
refreshArtistInfo(). The stale URL is still returned immediately to
avoid blocking clients.

Fixes #5266

* test: add expired artist image info test with log assertion

Verify that ArtistImage() enqueues a background refresh when cached
info is expired, by capturing log output and checking for the expected
debug message. Also asserts the stale URL is returned immediately
without calling the agent.

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: only enqueue refresh when returning a stale cached URL

Move the expiry check to the else branch so we only enqueue a
background refresh when a cached image URL exists and is being
returned. This avoids doubling external API calls when the URL is
empty (synchronous fetch) but ExternalInfoUpdatedAt is old.

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-03-30 09:35:02 -04:00
Deluan Quintão
69e7d163fc
remove built-in Spotify integration (#5197)
* refactor: remove built-in Spotify integration

Remove the Spotify adapter and all related configuration, replacing
the built-in integration with the plugin system. This deletes the
adapters/spotify package, removes Spotify config options (ID/Secret),
updates the default agents list from "deezer,lastfm,spotify" to
"deezer,lastfm", and cleans up all references across configuration,
metrics, logging, artwork caching, and documentation. Users with
Spotify config options will now see a warning that the options are
no longer available.

* feat: add ListenBrainz to list of default agents

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-03-15 13:18:54 -04:00
Deluan Quintão
4ddb0774ec
perf(artwork): improve image serving performance with WebP encoding and optimized pipeline (#5181)
* test(artwork): add benchmark helpers for generating test images

* test(artwork): add image decode benchmarks for JPEG/PNG at various sizes

* test(artwork): add image resize benchmarks for Lanczos at various sizes

* test(artwork): add image encode benchmarks for JPEG quality levels and PNG

* test(artwork): add full resize pipeline benchmark (decode+resize+encode)

* test(artwork): add tag extraction benchmark for embedded art

* test(cache): add file cache benchmarks for read, write, and concurrent access

* test(artwork): add E2E benchmarks for artwork.Get with cache on/off and concurrency

* fix(test): use absolute path for tag extraction benchmark fixture

* test(artwork): add resize alternatives benchmark comparing resamplers

* perf(artwork): switch to CatmullRom resampler and JPEG for square images

Replace imaging.Lanczos with imaging.CatmullRom for image resizing
(30% faster, indistinguishable quality at thumbnail sizes). Stop forcing
PNG encoding for square images when the source is JPEG — JPEG is smaller
and faster to encode. Square images from JPEG sources went from 52ms to
10ms (80% improvement). Add sync.Pool for encode buffers to reduce GC
pressure under concurrent load.

* perf(artwork): increase cache warmer concurrency from 2 to 4 workers

Resize is CPU-bound, so more workers improve throughput on multi-core
systems. Doubled worker count to better utilize available cores during
background cache warming.

* perf(artwork): switch to xdraw.ApproxBiLinear and always encode as JPEG

Replace disintegration/imaging with golang.org/x/image/draw for image
resizing. This eliminates ~92K allocations per resize (from imaging's
internal goroutine parallelism) down to ~20, reducing GC pressure under
concurrent load.

Always encode resized artwork as JPEG regardless of source format, since
cover art doesn't need transparency. This is ~5x faster than PNG encode
and produces much smaller output (e.g. 18KB JPEG vs 124KB PNG).

* perf(artwork): skip external API call when artist image URL is cached

ArtistImage() was always calling the external agent (Spotify/Last.fm)
to get the image URL, even when the artist already had URLs stored in
the database. This caused every artist image request to block on an
external API call, creating severe serialization when loading artist
grids (5-20 seconds for the first page).

Now use the stored URL directly when available. Artists with no stored
URL still fetch synchronously. Background refresh via UpdateArtistInfo
handles TTL-based URL updates.

* perf(artwork): increase getCoverArt throttle from NumCPU/3 to NumCPU

The previous default of max(2, NumCPU/3) was too aggressive for artist
images which are I/O-bound (downloading from external CDNs), not
CPU-bound. On an 8-core machine this meant only 2 concurrent requests,
causing a staircase pattern where 12 images took ~2.4s wall-clock.

Bumping to max(4, NumCPU) cuts wall-clock time by ~50% for artist image
grids while still preventing unbounded concurrency for CPU-bound resizes.

* perf(artwork): encode resized images as WebP instead of JPEG

Switch from JPEG to WebP encoding for resized artwork using gen2brain/webp
(libwebp via WASM, no CGo). WebP produces ~74% smaller output at the same
quality with only ~25% slower full-pipeline encode time (cached, so only
paid once per artwork+size).

Use NRGBA image type to preserve alpha channel in WebP output, and
transparent padding for square canvas instead of black.

Also removes the disintegration/imaging dependency entirely by replacing
imaging.Fill in playlist tile generation with a custom fillCenter function
using xdraw.ApproxBiLinear.

* perf(artwork): switch from ApproxBiLinear to BiLinear scaling for improved image processing

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(configuration): rename CoverJpegQuality to CoverArtQuality and update references

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(artwork): add DevJpegCoverArt option to control JPEG encoding for cover art

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(artwork): remove redundant transparent fill and handle encode errors in resizeImage

Removed a no-op draw.Draw call that filled the NRGBA canvas with
transparent pixels — NewNRGBA already zero-initializes to fully
transparent. Also added an early return on encode failure to avoid
allocating and copying potentially corrupt buffer data before returning
the error.

* fix(configuration): reorder default agents (deezer is faster)

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(test): resolve dogsled lint warning in tag extraction benchmark

Use all return values from runtime.Caller instead of discarding three
with blank identifiers, which triggered the dogsled linter.

* fix(artwork): revert cache key format

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(configuration): remove deprecated CoverJpegQuality field and update references to CoverArtQuality

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-03-13 09:35:59 -04:00
Maximilian
a704e86ac1
refactor: run Go modernize (#5002) 2026-02-08 09:57:30 -05:00
Deluan Quintão
36252823ce
fix(agents): deduplicate mismatched songs in similar songs matching (#4956)
* feat(agents): enhance song matching by removing unwanted duplicates while preserving identical entries

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: consolidate duplicate checks

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-01-30 15:25:00 +01:00
Deluan Quintão
1afcf7775b
feat: add ISRC matching for similar songs (#4946)
* feat: add ISRC support to similar songs matching and plugin interface

Add ISRC (International Standard Recording Code) as a high-priority
identifier in the provider matching algorithm, alongside MBID. The
matching pipeline now uses four strategies in priority order:
ID > MBID > ISRC > Title+Artist fuzzy match.

- Add ISRC field to agents.Song struct
- Add ISRC field to plugin capability SongRef (Go, Rust PDKs)
- Add loadTracksByISRC using json_tree query on tags column
- Integrate ISRC into matchSongsToLibrary, selectBestMatchingSongs,
  and buildTitleQueries

https://claude.ai/code/session_01Dd4mTq1VQZag4RNjCVusiF

* chore: regenerate plugin schema after ISRC addition

Run `make gen` to update the generated YAML schema for the
metadata agent capability with the new ISRC field on SongRef.

https://claude.ai/code/session_01Dd4mTq1VQZag4RNjCVusiF

* feat(mediafile): add GetAllByTags method to MediaFileRepository for tag-based retrieval

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(provider): speed up track matching by incorporating prior matches in ISRC and MBID lookups

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
Co-authored-by: Claude <noreply@anthropic.com>
2026-01-27 14:54:29 -05:00
Deluan Quintão
5db585e1b1
refactor: use duration as a soft ranking signal instead of hard cutoff in track matching (#4944)
* refactor: integrate duration into matchScore instead of using pre-filter

Duration matching was handled as a binary pre-filter with fallback,
inconsistent with how title, specificity, and album are scored via the
matchScore system. Move duration into matchScore as a boolean field
ranked between title similarity and specificity level, making all
match criteria use the same hierarchical comparison.

https://claude.ai/code/session_01BWJ5aAzbQRvwjB7PvUcNYs

* refactor: remove findBestMatchInTracks function and integrate its logic into findBestMatch

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: use duration proximity score instead of boolean match

Replace the binary durationMatch bool with a continuous durationProximity
float64 (0.0-1.0) using 1/(1+diff). This removes the hard 3-second
tolerance cutoff, so closer durations are always preferred over farther
ones without an arbitrary cliff edge.

https://claude.ai/code/session_01BWJ5aAzbQRvwjB7PvUcNYs

* style: fix gofmt alignment in matchScore struct

https://claude.ai/code/session_01BWJ5aAzbQRvwjB7PvUcNYs

---------

Signed-off-by: Deluan <deluan@navidrome.org>
Co-authored-by: Claude <noreply@anthropic.com>
2026-01-27 11:12:18 -05:00
Deluan Quintão
fda35dd8ce
feat(plugins): add similar songs retrieval functions and improve duration consistency (#4933)
* feat: add duration filtering for similar songs matching

Signed-off-by: Deluan <deluan@navidrome.org>

* test: refactor expectations for similar songs in provider matching tests

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add functions to retrieve similar songs by track, album, and artist

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(plugins): support uint32 in ndpgen

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(plugins): update duration field to use seconds as float instead of milliseconds as uint32

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: add helper functions for Rust's skip_serializing_if with numeric types

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(provider): enhance track matching logic to fallback to title match when duration-filtered tracks fail

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-01-26 18:28:41 -05:00
Deluan Quintão
772d1f359b
feat: add similar songs functionality in agents, and Instant Mix (song-based) to UI (#4919)
* refactor: rename ArtistRadio to SimilarSongs for clarity and consistency

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: implement GetSimilarSongsByTrack and related functionality for song similarity retrieval

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: enhance GetSimilarSongsByTrack to include artist and album details and update tests

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: enhance song matching by implementing title and artist filtering in loadTracksByTitleAndArtist

Signed-off-by: Deluan <deluan@navidrome.org>

* test: add unit tests for song matching functionality in provider

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: extract song matching functionality into its own file

Signed-off-by: Deluan <deluan@navidrome.org>

* docs: clarify similarSongsFallback function description in provider.go

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: initialize result slice for songs with capacity based on response length

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: simplify agent method calls for retrieving images and similar songs

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: simplify agent method calls for retrieving images and similar songs

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: remove outdated comments in GetSimilarSongs methods

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: use composite key for song matches to handle duplicates by title and artist

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: consolidate expectations setup for similar songs tests

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: add instant mix action to song context menu and update translations

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(provider): handle unknown entity types in GetSimilarSongs

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: move playSimilar action to playbackActions and streamline song processing

Signed-off-by: Deluan <deluan@navidrome.org>

* format

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: enhance instant mix functionality with loading notification and shuffle option

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: implement fuzzy matching for similar songs based on configurable threshold

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: implement track matching with multiple specificity levels

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: enhance track matching by implementing unified scoring with specificity levels

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: enhance deezer top tracks result with album

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: enhance track matching with fuzzy album similarity for improved scoring

Signed-off-by: Deluan <deluan@navidrome.org>

* docs: document multi-phase song matching algorithm with detailed scoring and prioritization

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-01-25 16:16:43 -05:00
Deluan
7b9bc1c5ac refactor: move agent files to adapters for consistency
Signed-off-by: Deluan <deluan@navidrome.org>
2026-01-14 19:33:54 -05:00
Deluan
9d95ef7b3f fix: specify media_file.id in track loading query to improve accuracy
Signed-off-by: Deluan <deluan@navidrome.org>
2026-01-12 08:15:46 -05:00
Deluan
55966ba5ec feat(agents): add ID field to Artist and Song structs with direct matching
Add ID field to Artist and Song structs in the agents package. When resolving
similar artists and top songs, the provider now uses a three-phase lookup:
1. Direct ID match (if agent returns internal Navidrome IDs)
2. MBID exact match (if MusicBrainz ID is available)
3. Fuzzy name/title match (existing behavior)

This enables agents to return more precise matches when they have access to
internal database IDs, while maintaining backward compatibility with
name-based matching.
2026-01-11 17:06:25 -05:00
Deluan Quintão
13f6eb9a11
feat: make Unicode handling in external API calls configurable (#4277)
* feat: make Unicode handling in external API calls configurable

- Add DevPreserveUnicodeInExternalCalls config option (default: false)
- Refactor external provider to use NameForExternal() method on auxArtist
- Remove redundant Name field from auxArtist struct
- Update all external API calls (image, URL, biography, similar, top songs, MBID) to use configurable Unicode handling
- Add comprehensive tests for both Unicode-preserving and normalized behaviors
- Refactor tests to use constants and improved structure with BeforeEach blocks

Fixes issue where Spotify integration failed to find artist images for artists with Unicode characters (e.g., en dash) in their names.

Signed-off-by: Deluan <deluan@navidrome.org>

* address comments

Signed-off-by: Deluan <deluan@navidrome.org>

* avoid calling str.Clean multiple times

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: apply Unicode handling pattern to auxAlbum

Extended the configurable Unicode handling to album names, matching the
pattern already implemented for artist names. This ensures consistent behavior
when DevPreserveUnicodeInExternalCalls is enabled for both artist and album
external API calls.

Changes:
- Removed Name field from auxAlbum struct, added Name() method with Unicode logic
- Updated getAlbum, UpdateAlbumInfo, populateAlbumInfo, and AlbumImage functions
- Added comprehensive tests for album Unicode handling (preserve and normalize)
- Fixed typo in artist image test description

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2025-12-02 13:08:30 -05:00
Kendall Garner
91e7f7b5c9
fix(server): ensure that similar artists retrieved from provider are no more than limit (#4267)
* fix(provider): ensure that similar artists retreived from provider are no more than limit

* add overlimit multiplier
2025-06-29 12:19:29 -04:00
Bastiaan van der Plaat
93040b3f85
feat(agents): Add Deezer API artist image provider agent (#4180)
* feat(agents): Add Deezer API artist image provider agent

* fix(agents): Use proper naming convention of consts

* fix(agents): Check if json test data can be read

* fix(agents): Use underscores for unused function arguments

* fix(agents): Move int literal to deezerArtistSearchLimit const

* feat: add Deezer configuration option to disable it.

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
2025-06-28 17:50:06 -04:00
Deluan Quintão
28bbd00dcc
refactor: rename SimilarSongs to ArtistRadio (#4248) 2025-06-25 18:21:14 -04:00
Deluan Quintão
f1fc2cd9b9
feat(plugins): experimental support for plugins (#3998)
* feat(plugins): add minimal test agent plugin with API definitions

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: add plugin manager with auto-registration and unique agent names

Introduced a plugin manager that scans the plugins folder for subdirectories containing plugin.wasm files and auto-registers them as agents using the directory name as the unique agent name. Updated the configuration to support plugins with enabled/folder options, and ensured the plugin manager is started as a concurrent task during server startup. The wasmAgent now returns the plugin directory name for AgentName, ensuring each plugin agent is uniquely identifiable. This enables dynamic plugin discovery and integration with the agents orchestrator.

* test: add Ginkgo suite and test for plugin manager auto-registration

Added a Ginkgo v2 suite bootstrap (plugins_suite_test.go) for the plugins package and a test (manager_test.go) to verify that plugins in the testdata folder are auto-registered and can be loaded as agents. The test uses a mock DataStore and asserts that the agent is registered and its AgentName matches the plugin directory. Updated go.mod and go.sum for wazero dependency required by plugin WASM support.

* test(plugins): ensure test WASM plugin is always freshly built before running suite; add real-plugin Ginkgo tests. Add BeforeSuite to plugins suite to build plugins/testdata/agent/plugin.wasm using Go WASI build command, matching README instructions. Remove plugin.wasm before build to guarantee a clean build. Add full real-plugin Ginkgo/Gomega tests for wasmAgent, covering all methods and error cases. Fix manager_test.go to use pointer to Manager. This ensures plugin tests are always run against a freshly compiled WASM binary, increasing reliability and reproducibility.

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): implement persistent compilation cache for WASM agent plugins

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): implement instance pooling for wasmAgent to improve resource management

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): enhance logging for wasmAgent and plugin manager operations

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): implement HttpService for handling HTTP requests in WASM plugins

Also add a sample Wikimedia plugin

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): standardize error handling in wasmAgent and MinimalAgent

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: clean up wikimedia plugin code

Standardized error creation using 'errors.New' where formatting was not needed. Introduced a constant for HTTP request timeouts. Removed commented-out log statement. Improved code comments for clarity and accuracy.

* refactor: use unified SPARQLResult struct and parser for SPARQL responses

Introduced a single SPARQLResult struct to represent all possible SPARQL response fields (sitelink, wiki, comment, img). Added a parseSPARQLResult helper to unmarshal and check for empty results, simplifying all fetch functions and improving type safety and maintainability.

* feat(plugins): improve error handling in HTTP request processing

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: background plugin compilation, logging, and race safety

Implemented background WASM plugin compilation with concurrency limits, proper closure capture, and global compilation cache to avoid data races. Added debug and warning logs for plugin compilation results, including elapsed time. Ensured plugin registration is correct and all tests pass.

* perf: implement true lazy loading for agents

Changed agent instantiation to be fully lazy. The Agents struct now stores agent names in order and only instantiates each agent on first use, caching the result. This preserves agent call order, improves server startup time, and ensures thread safety. Updated all agent methods and tests to use the new pattern. No changes to agent registration or interface. All tests pass.

* fix: ensure wasm plugin instances are closed via runtime.AddCleanup

Introduced runtime.AddCleanup to guarantee that the Close method of WASM plugin instances is called, even if they are garbage collected from the sync.Pool. Modified the sync.Pool.New function in manager.go to register a cleanup function for each loaded instance that implements Close. Updated agent.go to handle the pooledInstance wrapper containing the instance and its cleanup handle. Ensured cleanup.Stop() is called before explicitly closing an instance (on error or agent shutdown) to prevent double closing. This fixes a potential resource leak where instances could be GC'd from the pool without proper cleanup.

* refactor: break down long functions in plugin manager and agent

Refactored plugins/manager.go and plugins/agent.go to improve readability and reduce function length. Extracted pool initialization logic into newPluginPool and background compilation/agent factory logic into precompilePlugin/createAgentFactory in manager.go. Extracted pool retrieval/validation and cleanup function creation into getValidPooledInstance/createPoolCleanupFunc in agent.go.

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): rename wasmAgent to wasmArtistAgent

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(api): add AlbumMetadataService with AlbumInfo and AlbumImages requests

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugin): rename MinimalAgent for artist metadata service

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(api): implement wasmAlbumAgent for album metadata service with GetAlbumInfo and GetAlbumImages methods

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): simplify wasmAlbumAgent and wasmArtistAgent by using wasmBasePlugin

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add support for ArtistMetadataService and AlbumMetadataService in plugin manager

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): enhance plugin pool creation with custom runtime and precompilation support

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): implement generic plugin pool and agent factory for improved service handling

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): reorganize plugin management

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): improve function signatures for clarity and consistency

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): implement background precompilation for plugins and agent factory creation

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): include instanceID in logging for better traceability

Signed-off-by: Deluan <deluan@navidrome.org>

* test(plugins): add tests for plugin pre-compilation and agent factory synchronization

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add minimal album test agent plugin for AlbumMetadataService

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): rename fake artist and album test agent plugins for metadata services

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(makefile): add Makefile for building plugin WASM binaries

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add FakeMultiAgent plugin implementing Artist and Album metadata services

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): remove log statements from FakeArtistAgent and FakeMultiAgent methods

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: split AlbumInfoRetriever and AlbumImageRetriever, update all usages

Split the AlbumInfoRetriever interface into two: AlbumInfoRetriever (for album metadata) and AlbumImageRetriever (for album images), to better separate concerns and simplify implementations. Updated all agents, providers, plugins, and tests to use the new interfaces and methods. Removed the now-unnecessary mockAlbumAgents in favor of the shared mockAgents. Fixed a missing images slice declaration in lastfm agent. All tests pass except for known ignored persistence tests. This change reduces code duplication, improves clarity, and keeps the codebase clean and organized.

* feat(plugins): add Cover Art Archive AlbumMetadataService plugin for album cover images

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: remove wasm module pooling

it was causing issues with the GC and the Close methods

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: rename metadata service files to adapter naming convention

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: unify album and artist method calls by introducing callMethod function

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: unify album and artist method calls by introducing callMethod function

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: handle nil values in data redaction process

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: add timeout for plugin compilation to prevent indefinite blocking

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: implement ScrobblerService plugin with authorization and scrobbling capabilities

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: simplify generalization

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: tests

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: enhance plugin management by improving scanning and loading mechanisms

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: update plugin creation functions to return specific interfaces for better type safety

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: enhance wasmBasePlugin to support specific plugin types for improved type safety

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: implement MediaMetadataService with combined artist and album methods

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: improve MediaMetadataService plugin implementation and testing structure

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: add tests for Adapter Media Agent and improve plugin documentation

Signed-off-by: Deluan <deluan@navidrome.org>

* docs: add README for Navidrome Plugin System with detailed architecture and usage guidelines

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: enhance agent management with plugin loading and caching

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: update agent discovery logic to include only local agent when no config is specified

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: encapsulate agent caching logic in agentCache struct\n\nReplaced direct map/mutex usage for agent caching in Agents with a dedicated agentCache struct. This improves readability, maintainability, and testability by centralizing TTL and concurrency logic. Cleaned up comments and ensured all linter and test requirements are met.

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: correct file extension filter in goimports command

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: use defer to unlock the mutex

Signed-off-by: Deluan <deluan@navidrome.org>

* chore: move Cover Art Archive AlbumMetadataService plugins to an example folder

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: handle errors when creating media metadata and scrobbler service plugins

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: increase compilation timeout to one minute

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: add configurable plugin compilation timeout

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: implement plugin scrobbler support in PlayTracker

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: add context management and Stop method to buffered scrobbler

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: add username field to scrobbler requests and update logging

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: data race in test

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: rename http proto files to host and update references

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: remove unused plugin registration methods from manager

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: extend plugin manifests and implement plugin management commands

Signed-off-by: Deluan <deluan@navidrome.org>

* Update utils/files.go

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>

* fix for code scanning alert no. 43: Arbitrary file access during archive extraction ("Zip Slip")

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

* feat: add plugin dev workflow support

Added new CLI commands to improve plugin development workflow: 'plugin dev' to create symlinks from development directories to plugins folder, 'plugin refresh' to reload plugins without restarting Navidrome, enhanced 'plugin remove' to handle symlinked development plugins correctly, and updated 'plugin list' to display development plugins with '(dev)' indicator. These changes make the plugin development workflow more efficient by allowing developers to work on plugins in their own directories, link them to Navidrome without copying files, refresh plugins after changes without restart, and clean up safely.

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): implement timer service with register and cancel functionality - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): implement timer service with register and cancel functionality - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): implement timer service with register and cancel functionality - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): implement timer service with register and cancel functionality

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: lint errors

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(README): update documentation to include TimerCallbackService and its functionality

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add InitService with OnInit method and initialization tracking - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add tests for InitService and plugin initialization tracking

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): expand documentation on plugin system implementation and architecture

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: panic

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): redirect plugins' stderr to logs

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add safe accessor methods for TimerService

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add plugin-specific configuration support in InitRequest and documentation

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add TimerCallbackService plugin adapter and integration

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): rename services for consistency and clarity

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add mutex for configuration access and clone plugin config

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(tests): remove configtest dependency to prevent data races in integration tests

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): remove PluginName method from WASM plugin implementations and update LoadPlugin to accept service type

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): implement instance pooling for wasmBasePlugin to improve performance - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add wasmInstancePool for managing WASM plugin instances with TTL and max size

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(plugins): correctly pass error to done function in wasmBasePlugin

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): rename service types to capabilities for consistency

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): simplify instance management in wasmBasePlugin by removing error handling in closure

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): update wasmBasePlugin and wasmInstancePool to return errors for better error handling

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): rename InitService to LifecycleManagement for consistency

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): fix instance ID logging in wasmBasePlugin

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): extract instance ID logging to a separate function in wasmBasePlugin, to avoid vet error

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): make timers be isolated per plugin

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): make timers be isolated per plugin

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): rename HttpServiceImpl to httpServiceImpl for consistency and improve logging

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add config service for plugin-specific configuration management

Signed-off-by: Deluan <deluan@navidrome.org>

* Update plugins/manager.go

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>

* Update plugins/manager.go

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>

* feat(crontab): implement crontab service for scheduling and canceling jobs

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(singleton): fix deadlock issue when a constructor calls GetSingleton again

Signed-off-by: Deluan <deluan@navidrome.org> (+1 squashed commit)
Squashed commits:
[325a96ea2] fix(singleton): fix deadlock issue when a constructor calls GetSingleton again

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(scheduler): implement Scheduler for one-time and recurring job scheduling, merging CrontabService and TimerService

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(scheduler): race condition in the scheduleOneTime and scheduleRecurring methods when replacing jobs with the same ID

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(scheduler): consolidate job scheduling logic into a single helper function

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugin): rename GetInstance method to Instantiate for clarity

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add WebSocket service for handling connections and messages

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(crypto-ticker): add WebSocket plugin for real-time cryptocurrency price tracking

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(websocket): enhance connection management and callback handling

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(manager): only create one adapter instance for each adapter/capability pair

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(websocket): ensure proper resource management by closing response body and use defer to unlocking mutexes

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: flaky test

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugin): refactor WebSocket service integration and improve error logging

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugin): add SchedulerCallback support and improve reconnection logic

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: test panic

Signed-off-by: Deluan <deluan@navidrome.org>

* docs: add crypto-ticker plugin example to README

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(manager): add LoadAllPlugins and LoadAllMediaAgents methods with slice.Map integration

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(api): add Timestamp field to ScrobblerNowPlayingRequest and update related methods

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(websocket): add error field to response messages for better error handling

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(cache): implement CacheService with string, int, float, and byte operations

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(tests): update buffered scrobbler tests for improved scrobble verification and use RWMutex in mock repo

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(cache): simplify cache service implementation and remove unnecessary synchronization

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(tests): add build step for test plugins in the test suite

Signed-off-by: Deluan <deluan@navidrome.org>

* wip

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(scheduler): implement named scheduler callbacks and enhance Discord plugin integration

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(rpc): enhance activity image processing and improve error handling in Discord integration

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(discord): enhance activity state with artist list and add large text asset

Signed-off-by: Deluan <deluan@navidrome.org>

* fix tests

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(artwork): implement ArtworkService for retrieving artwork URLs

Signed-off-by: Deluan <deluan@navidrome.org>

* Add playback position to scrobble NowPlaying (#4089)

* test(playtracker): cover playback position

* address review comment

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>

* fix merge

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: remove unnecessary check for empty slice in Map function

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: update reflex.conf to include .wasm file extension

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(scanner): normalize attribute strings and add edge case tests for PID calculation

Relates to https://github.com/navidrome/navidrome/issues/4183#issuecomment-2952729458

Signed-off-by: Deluan <deluan@navidrome.org>

* test(ui): fix warnings (#4187)

* fix(ui): address test warnings

* ignore lint error in test

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(server): optimize top songs lookup (#4189)

* optimize top songs lookup

* Optimize title matching queries

* refactor: simplify top songs matching

* improve error handling and logging in track loading functions

Signed-off-by: Deluan <deluan@navidrome.org>

* test: add cases for fallback to title matching and combined MBID/title matching

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(ui): playlist details overflow in spotify-based themes (#4184)

* test: ensure playlist details width

* fix(test): simplify expectation for minWidth in NDPlaylistDetails

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(test): test all themes

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>

* chore(deps): update TagLib to version 2.1 (#4185)

* chore: update cross-taglib

* fix(taglib): add logging for TagLib version

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>

* test: verify agents fallback (#4191)

* build(docker): downgrade Alpine version from 3.21 to 3.19, oldest supported version.

This is to reduce the image size, as we don't really need the latest.

Signed-off-by: Deluan <deluan@navidrome.org>

* fix tests

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(runtime): implement pooled WASM runtime and module for better instance management

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(discord-plugin): adjust timer delay calculation for track completion

Signed-off-by: Deluan <deluan@navidrome.org>

* resolve PR comments

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): implement cache cleanup by size functionality

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(manager): return error from getCompilationCache and handle it in ScanPlugins

Signed-off-by: Deluan <deluan@navidrome.org>

* fix possible rce condition

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(docs): update README to include Cache and Artwork services

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(manager): add permissions support for host services in custom runtime - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(manifest): add permissions field to plugin manifests - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* test(permissions): implement permission validation and testing for plugins - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add unauthorized_plugin to test permission enforcement - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(docs): add Plugin Permission System section to README - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(manifest): add detailed reasons for permissions in plugin manifests - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(permissions): implement granular HTTP permissions for plugins - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(permissions): implement HTTP and WebSocket permissions for plugins - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: unexport all plugins package private symbols

Signed-off-by: Deluan <deluan@navidrome.org>

* update docs

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: rename plugin_lifecycle_manager

Signed-off-by: Deluan <deluan@navidrome.org>

* docs: add discord-rich-presence plugin example to README

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: add support for PATCH, HEAD, and OPTIONS HTTP methods

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: use folder names as unique identifiers for plugins

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: read config just once, to avoid data race in tests

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: rename pluginName to pluginID for consistency across services

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: use symlink name instead of folder name for plugin registration

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: update plugin output format to include ID and enhance README with symlink usage

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: implement shared plugin discovery function to streamline plugin scanning and error handling

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: show plugin permissions in `plugin info`

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: add JSON schema for Navidrome Plugin manifest and generate corresponding Go types - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: implement typed permissions for plugins to enhance permission handling

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: refactor plugin permissions to use typed schema and improve validation - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: update HTTP permissions handling to use typed schema for allowed URLs - WIP

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: remove unused JSON schema validation for plugin manifests

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: remove unused fields from PluginPackage struct in package.go

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: update file permissions in tests and remove unused permission parsing function

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: refactor test plugin creation to use typed permissions and remove legacy helper

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: add website field to plugin manifests and update test cases

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: permission schema to use basePermission structure for consistency

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: enhance host service management by adding permission checks for each service

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: reorganize code files

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: simplify custom runtime creation by removing compilation cache parameter

Signed-off-by: Deluan <deluan@navidrome.org>

* doc: add WebSocketService and update ConfigService for plugin-specific configuration

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: implement WASM loading optimization to enhance plugin instance creation speed

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: rename custom runtime functions and update related tests for clarity

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: enhance plugin structure with compilation handling and error reporting

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: improve logging and context tracing in runtime and wasm base plugin

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: enhance runtime management with scoped runtime and caching improvements

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: implement EnsureCompiled method for improved plugin compilation handling

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: implement cached module management with TTL for improved performance

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: replace map with sync.Map

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: adjust time tolerance in scrobble buffer repository tests to avoid flakiness

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: enhance image processing with fallback mechanism for improved error handling

Signed-off-by: Deluan <deluan@navidrome.org>

* docs: review test plugins readme

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: set default timeout for HTTP client to 10 seconds

Signed-off-by: Deluan <deluan@navidrome.org>

* feat: enhance wasm instance pool with concurrency limits and timeout settings

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(discordrp): implement caching for processed image URLs with configurable TTL

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
2025-06-22 20:45:38 -04:00
Deluan Quintão
bc733540f9
refactor(server): optimize top songs lookup (#4189)
* optimize top songs lookup

* Optimize title matching queries

* refactor: simplify top songs matching

* improve error handling and logging in track loading functions

Signed-off-by: Deluan <deluan@navidrome.org>

* test: add cases for fallback to title matching and combined MBID/title matching

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2025-06-08 11:44:44 -04:00
Deluan Quintão
58367afaea
refactor: external_metadata -> external.Provider (#3903)
* tests for TopSongs

Signed-off-by: Deluan <deluan@navidrome.org>

* convert to Ginkgo

Signed-off-by: Deluan <deluan@navidrome.org>

* consolidate tests

Signed-off-by: Deluan <deluan@navidrome.org>

* rename external metadata -wip

Signed-off-by: Deluan <deluan@navidrome.org>

* rename external metadata to extdata.Provider

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor tests - wip

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor test helpers

Signed-off-by: Deluan <deluan@navidrome.org>

* remove reflection

Signed-off-by: Deluan <deluan@navidrome.org>

* use mock.Mock

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor

Signed-off-by: Deluan <deluan@navidrome.org>

* fix

Signed-off-by: Deluan <deluan@navidrome.org>

* receive Agents interface in Provider constructor

Signed-off-by: Deluan <deluan@navidrome.org>

* use mock for Agents

Signed-off-by: Deluan <deluan@navidrome.org>

* tests for SimilarSongs

Signed-off-by: Deluan <deluan@navidrome.org>

* remove duplication

Signed-off-by: Deluan <deluan@navidrome.org>

* ArtistImage tests

Signed-off-by: Deluan <deluan@navidrome.org>

* AlbumImage tests

Signed-off-by: Deluan <deluan@navidrome.org>

* fix provider error handling

Signed-off-by: Deluan <deluan@navidrome.org>

* UpdateAlbumInfo tests - wip

Signed-off-by: Deluan <deluan@navidrome.org>

* UpdateAlbumInfo tests - wip

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor

Signed-off-by: Deluan <deluan@navidrome.org>

* UpdateArtistInfo tests - wip

Signed-off-by: Deluan <deluan@navidrome.org>

* clean up

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor

Signed-off-by: Deluan <deluan@navidrome.org>

* fix test descriptions

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: rename extdata package to external

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2025-04-08 21:11:09 -04:00