Skip to content

feat: track client cover size preference and prefetch on album/song responses#691

Open
brian-doherty wants to merge 1 commit into
sentriz:masterfrom
brian-doherty:feat/cover-size-preference
Open

feat: track client cover size preference and prefetch on album/song responses#691
brian-doherty wants to merge 1 commit into
sentriz:masterfrom
brian-doherty:feat/cover-size-preference

Conversation

@brian-doherty

Copy link
Copy Markdown
Contributor

Summary

  • Adds a ClientCoverSizePreference DB table (with migration) that records the last cover art size requested by each user+client pair, updated on every getCoverArt call
  • Extracts ensureCoverCached from ServeGetCoverArt so the resize+save logic is reusable
  • Wraps the resp handler in New() with a local closure that calls prefetchCovers after any successful album/song response — no changes to individual handlers required
  • prefetchCovers looks up the stored size preference for the requesting client and pre-warms the cover cache for all album/track IDs in the response (deduplicated, each in its own goroutine)
  • Artist covers are excluded from prefetch since they require external Last.fm HTTP requests

Test plan

  • First request for getCoverArt records size preference in DB
  • Subsequent getAlbum/getAlbumList/search3 etc. responses trigger background cache warming at the stored size
  • Cover is served from cache on next getCoverArt call without re-encoding
  • No regression on cold-cache cover serving

🤖 Generated with Claude Code

@brian-doherty
brian-doherty force-pushed the feat/cover-size-preference branch 2 times, most recently from c3332c4 to ff596f1 Compare May 6, 2026 19:00
@sentriz

sentriz commented May 16, 2026

Copy link
Copy Markdown
Owner

thanks Brain this is interesting, though I have a couple comments

  • if we record the last cover size preference for a client, what happens when a client doesn't request a size and they have a preference set? to they get the original back?
  • endpoints like getArtists without pagination will try cache for just me with a modest library size will presumably try cache ~10000 covers without a limit it looks like. might kill some devices like a raspberrypi
  • the middleware check reads the response and collects ids is a little bit awkward. and creates a coupling from two places. maybe its better to have the spec.NewXFromY funcs receive a map[string]struct{} where they can add the cover ids that they "own". no coupling then, and a little less magic without the middleware

@sentriz
sentriz force-pushed the master branch 2 times, most recently from c00760e to 9f706b9 Compare May 22, 2026 18:01
@brian-doherty

Copy link
Copy Markdown
Contributor Author
  1. Right now, it uses and records the default cover size. That means that if the client fetches without a size, their saved size is changed to the default. I think that's OK, but I can change it to not save a cover size if you don't like that.
  2. We could add a max prefetch parameter of some sort to limit this, but are the config options getting out of control? Or maybe exclude calls without a page size instead?
  3. Good idea. I'll look at that once you reply on 1 and 2.

Thanks!

…esponses

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@brian-doherty
brian-doherty force-pushed the feat/cover-size-preference branch from 7c27ca2 to bc0d39b Compare May 24, 2026 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants