Uh oh!
There was an error while loading. Please reload this page.
feat: nullable songs.artwork_url for the artist page V2 - #58
Conversation
Persists Apple Music artwork per song (recoupable/app#1968) so the public profile serves artwork without calling Apple on every build. Populated lazily by the api write-through; null until resolved.
Updates to Preview Branch (feat/songs-artwork-url) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe migration adds a nullable ChangesSongs artwork metadata
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk:⚪ Minimal · up to This PR adds a nullable songs artwork field through a localized, rerunnable migration; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="supabase/migrations/20260819120000_songs_artwork_url.sql">
<violation number="1" location="supabase/migrations/20260819120000_songs_artwork_url.sql:5">
P2: When the api lazily writes `artwork_url`, the existing `set_updated_at` BEFORE UPDATE trigger on `songs` bumps `updated_at` too. That timestamp is meant to reflect song-content changes, but a background cache write-through will now look like a fresh content update to anything ordering or filtering by `updated_at`. If that mislabeling matters for consumers, have the write-through preserve `updated_at` (set `updated_at = songs.updated_at` in the UPDATE) or use a dedicated cache column so it isn't conflated.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| -- so the public profile does not hit Apple on every page build. Populated | ||
| -- lazily by the api's fetch-on-miss write-through; null until resolved. | ||
| alter table public.songs | ||
| add column if not exists artwork_url text; |
There was a problem hiding this comment.
P2: When the api lazily writes artwork_url, the existing set_updated_at BEFORE UPDATE trigger on songs bumps updated_at too. That timestamp is meant to reflect song-content changes, but a background cache write-through will now look like a fresh content update to anything ordering or filtering by updated_at. If that mislabeling matters for consumers, have the write-through preserve updated_at (set updated_at = songs.updated_at in the UPDATE) or use a dedicated cache column so it isn't conflated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/migrations/20260819120000_songs_artwork_url.sql, line 5:
<comment>When the api lazily writes `artwork_url`, the existing `set_updated_at` BEFORE UPDATE trigger on `songs` bumps `updated_at` too. That timestamp is meant to reflect song-content changes, but a background cache write-through will now look like a fresh content update to anything ordering or filtering by `updated_at`. If that mislabeling matters for consumers, have the write-through preserve `updated_at` (set `updated_at = songs.updated_at` in the UPDATE) or use a dedicated cache column so it isn't conflated.</comment>
<file context>
@@ -0,0 +1,8 @@
+-- so the public profile does not hit Apple on every page build. Populated
+-- lazily by the api's fetch-on-miss write-through; null until resolved.
+alter table public.songs
+ add column if not exists artwork_url text;
+
+comment on column public.songs.artwork_url is
</file context>
sweetmantech
commented
Aug 19, 2026
Preview verification — the migration run on an ephemeral PostgresNo shadow database is reachable from this machine (no Docker, no Documented vs actual
Note: adding a nullable column with no default is a metadata-only change in Postgres 11+ — no table rewrite, so the brief Merge orderdocs#304 ✅ merged → this PR → api#842 (which writes this column). Ready to merge; after it applies, remember the house rule of confirming |
| add column if not exists artwork_url text; | ||
| comment on column public.songs.artwork_url is | ||
| 'Apple Music artwork URL for the song''s release, resolved lazily from the batch ISRC lookup. Null until first resolution.'; |
There was a problem hiding this comment.
Why is apple music hard coded in the db note? Why couldn't the artwork be pulled from spotify or another streaming platform?
There was a problem hiding this comment.
Good catch — the provider is an implementation detail of the api's resolver, not a schema contract, so it didn't belong in the column comment. Fixed in 6dc8ea0: the comment now reads "Artwork URL for the song's release, resolved lazily by the api from a streaming provider. Null until first resolution." The column just stores a URL; any provider's artwork can land here.
Why the api resolves from Apple Music today rather than Spotify: the batch ISRC→artwork lookup already exists in the api (getAppleSongsByIsrc, shipped for the catalog work) with credentials configured, and it returns artwork for up to 25 ISRCs per request with no per-track search. Spotify can absolutely be a second source later (its track objects carry album art too) — that's an api-side change with zero migration needed, which is exactly why the schema stays source-agnostic.
Re-verified the revised file on the ephemeral Postgres 14: applies cleanly, idempotent re-run, comment persists verbatim.
The column stores an artwork URL; which provider resolves it is the api's call, not schema contract. Apple Music is today's resolver, mentioned only as the current state.
Uh oh!
There was an error while loading. Please reload this page.
Row 2 of recoupable/chat#1968.
One migration: nullable
artwork_url textonpublic.songs, with a column comment. No backfill — the api PR (row 3) populates it lazily via a fetch-on-miss write-through from the Apple batch ISRC lookup, per the decision recorded on the issue.add column if not existskeeps the migration re-runnable.Merge order
After docs#304, before the api PR — the api writes this column.
Summary by cubic
Persists per-song artwork by adding a nullable
artwork_urlonpublic.songs, so Artist page V2 serves artwork without calling a streaming provider on every build. Previously we didn’t store artwork; now the API lazily resolves and writes the URL on first lookup (source-agnostic, provider chosen by the API; Apple Music today).Review and rollout
artwork_url textwith a column comment; idempotent viaadd column if not exists.recoupable/docs#304and before the API PR that writes this column.Written for commit 6dc8ea0. Summary will update on new commits.
Summary by CodeRabbit