Uh oh!
There was an error while loading. Please reload this page.
feat(catalogs): claim-time canonical-artist roster attach + Spotify socials at ingest (chat#1850 P1) - #768
Merged
Conversation
…ocials at ingest (chat#1850 P1) POST /api/catalogs now resolves the claimed snapshot's canonical artist through the songs graph (measured ISRCs -> song_artists -> dominant artist account) and links it to the claiming account's roster, on both first claims and idempotent re-claims. This replaces the marketing funnel's per-signup POST /api/artists, which minted a duplicate, song-less roster artist per signup. Forward fix: capture enrichment (linkSongsToArtists) now attaches the Spotify artist profile URL (case preserved) to resolved artist accounts that lack one, so canonicals are findable by Spotify id going forward. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a canonical-artist attachment feature that links accounts to their dominant artist based on claimed song ISRCs, wired into catalog creation and idempotent snapshot re-claim paths. Also adds Spotify social profile enrichment for artists linked during song-to-artist linking. ChangesCanonical Artist and Spotify Socials Enrichment
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant createCatalogHandler
participant createSnapshotCatalog
participant attachCanonicalArtistToAccount
participant selectSongArtistsBySongs
participant getDominantSongArtist
createCatalogHandler->>createSnapshotCatalog: create catalog with ISRCs
createSnapshotCatalog->>attachCanonicalArtistToAccount: accountId, isrcs
attachCanonicalArtistToAccount->>selectSongArtistsBySongs: fetch song-artist links
attachCanonicalArtistToAccount->>getDominantSongArtist: determine dominant artist
attachCanonicalArtistToAccount->>attachCanonicalArtistToAccount: insert association if absent
Note over createCatalogHandler: Idempotent re-claim path
createCatalogHandler->>createCatalogHandler: select snapshot measurements, dedupe ISRCs
createCatalogHandler->>attachCanonicalArtistToAccount: accountId, isrcs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
This was referenced Jul 8, 2026
There was a problem hiding this comment.
1 issue found across 14 files
Confidence score: 3/5
- In
lib/supabase/song_artists/selectSongArtistsBySongs.ts, the chunkedselect("*")calls are unpaginated, so larger song batches can hit the Supabase/PostgREST per-request row cap and silently dropsong_artistsrows; merging as-is risks incomplete artist mappings in user-facing results — add explicit pagination (or reduce chunk/query shape) and verify full-row retrieval before merging.
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="lib/supabase/song_artists/selectSongArtistsBySongs.ts">
<violation number="1" location="lib/supabase/song_artists/selectSongArtistsBySongs.ts:20">
P2: Each chunked `select("*")` on `song_artists` is unpaginated. Supabase/PostgREST caps rows per request (commonly 1000), and exceeding that limit silently truncates the result without raising `error`. If a 200-ISRC chunk maps to more rows than the cap — plausible for collaborative catalogs — `getDominantSongArtist` will count an incomplete graph and may attach the wrong canonical artist. Consider bounding the chunk size by expected `song_artists` cardinality, adding `.range()` pagination within each chunk, or moving the resolution to a single SQL query/RPC/CTE that returns only the dominant artist id.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Client (UI / API)
participant CreateCat as createCatalogHandler
participant Cat as createSnapshotCatalog
participant AttachArtist as attachCanonicalArtistToAccount
participant SelectSongArt as selectSongArtistsBySongs
participant GetDom as getDominantSongArtist
participant SelAccArt as selectAccountArtistId
participant InsAccArt as insertAccountArtistId
participant SupaSong as song_artists (DB)
participant SupaAcc as account_artist_ids (DB)
participant LinkSongs as linkSongsToArtists
participant AttachSocials as attachSpotifySocialsToArtists
participant SelSoc as selectAccountSocials
participant UpdSoc as updateArtistSocials
participant SupaSoc as account_socials (DB)
Note over Client,UpdSoc: Claim-time canonical-artist roster attach
alt First claim via createSnapshotCatalog
CreateCat->>Cat: Create snapshot catalog with songs
Cat->>AttachArtist: attachCanonicalArtistToAccount(accountId, isrcs)
Note over AttachArtist: Best-effort: never throws
AttachArtist->>SelectSongArt: selectSongArtistsBySongs(isrcs)
SelectSongArt->>SupaSong: SELECT * FROM song_artists WHERE song IN (isrcs)
SupaSupaSong-->>SelectSongArt: song_artist rows
SelectSongArt-->>AttachArtist: artist links
AttachArtist->>GetDom: getDominantSongArtist(links)
Note over GetDom: Deterministic: most links wins, ties by lowest artist id
GetDom-->>AttachArtist: dominant artist id
alt Has dominant artist
AttachArtist->>SelAccArt: selectAccountArtistId(accountId, artistId)
SelAccArt->>SupaAcc: Check if link exists
SupaAcc-->>SelAccArt: existing or null
alt No existing link
AttachArtist->>InsAccArt: insertAccountArtistId(accountId, artistId)
InsAccArt->>SupaAcc: INSERT new roster link
SupaAcc-->>InsAccArt: success
end
AttachArtist-->>Cat: artist id
else No links
AttachArtist-->>Cat: null
end
Cat-->>CreateCat: catalog created with artist on roster
else Re-claim via createCatalogHandler
CreateCat->>CreateCat: Detect existing catalog (idempotent)
CreateCat->>CreateCat: selectSongMeasurements to get ISRCs
CreateCat->>AttachArtist: attachCanonicalArtistToAccount(accountId, isrcs)
Note over AttachArtist: Heals pre-fix claims on next click
AttachArtist->>SelectSongArt: selectSongArtistsBySongs(isrcs)
SelectSongArt->>SupaSong: chunked SELECT
SupaSupaSong-->>SelectSongArt: artist links
AttachArtist->>GetDom: getDominantSongArtist(links)
AttachArtist->>SelAccArt: Check existing
alt Not already linked
AttachArtist->>InsAccArt: Insert roster link
end
AttachArtist-->>CreateCat: artist id
CreateCat-->>Client: existing catalog + artist attached
end
Note over LinkSongs,UpdSoc: Spotify socials at ingest (forward fix)
LinkSongs->>LinkSongs: Enrich songs with Spotify artist names
LinkSongs->>LinkSongs: Collect spotifyArtistId by normalized name
LinkSongs->>AttachSocials: attachSpotifySocialsToArtists([{artistAccountId, spotifyArtistId}])
Note over AttachSocials: Best-effort per artist
loop For each unique artist account
AttachSocials->>SelSoc: selectAccountSocials(accountId)
SelSoc->>SupaSoc: Check existing socials
SupaSoc-->>SelSoc: social rows
alt No existing Spotify artist social
AttachSocials->>UpdSoc: updateArtistSocials(accountId, {SPOTIFY: url})
Note over UpdSoc: Case-preserved id (no lowercasing)
UpdSoc->>SupaSoc: INSERT/UPDATE social
else Already has Spotify artist social
AttachSocials->>AttachSocials: Skip (never clobber)
end
end
AttachSocials-->>LinkSongs: done
LinkSongs-->>LinkSongs: Continue with song-artist linking
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const rows: Tables<"song_artists">[] = []; | ||
| for (let i = 0; i < songs.length; i += CHUNK_SIZE) { | ||
| const chunk = songs.slice(i, i + CHUNK_SIZE); | ||
| const { data, error } = await supabase.from("song_artists").select("*").in("song", chunk); |
There was a problem hiding this comment.
P2: Each chunked select("*") on song_artists is unpaginated. Supabase/PostgREST caps rows per request (commonly 1000), and exceeding that limit silently truncates the result without raising error. If a 200-ISRC chunk maps to more rows than the cap — plausible for collaborative catalogs — getDominantSongArtist will count an incomplete graph and may attach the wrong canonical artist. Consider bounding the chunk size by expected song_artists cardinality, adding .range() pagination within each chunk, or moving the resolution to a single SQL query/RPC/CTE that returns only the dominant artist id.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/supabase/song_artists/selectSongArtistsBySongs.ts, line 20:
<comment>Each chunked `select("*")` on `song_artists` is unpaginated. Supabase/PostgREST caps rows per request (commonly 1000), and exceeding that limit silently truncates the result without raising `error`. If a 200-ISRC chunk maps to more rows than the cap — plausible for collaborative catalogs — `getDominantSongArtist` will count an incomplete graph and may attach the wrong canonical artist. Consider bounding the chunk size by expected `song_artists` cardinality, adding `.range()` pagination within each chunk, or moving the resolution to a single SQL query/RPC/CTE that returns only the dominant artist id.</comment>
<file context>
@@ -0,0 +1,31 @@
+ const rows: Tables<"song_artists">[] = [];
+ for (let i = 0; i < songs.length; i += CHUNK_SIZE) {
+ const chunk = songs.slice(i, i + CHUNK_SIZE);
+ const { data, error } = await supabase.from("song_artists").select("*").in("song", chunk);
+
+ if (error) {
</file context>
17 tasks
ContributorAuthor
Preview verification — 2026-07-08, |
| # | Check | Documented / expected | Actual (live preview) | Status |
|---|---|---|---|---|
| 1 | First claim: POST /api/catalogs {snapshot} | 200; catalog created; songs_added = measurement count; canonical artist attached to roster | 200; catalog 2255ff65…; songs_added: 67; account_artist_ids 17 → 18 rows, added row = canonical 7647f901… | ✅ |
| 2 | No duplicate artist mint on claim | Claim never creates artist accounts | All 5 "Del Water Gap"-named accounts predate the claim (canonical + 4 known funnel dupes); 0 new rows | ✅ |
| 3 | Re-claim idempotency (same snapshot) | 200; same catalog returned; songs_added: 0; no duplicate roster row | 200; same id 2255ff65…; songs_added: 0; roster unchanged at 18 | ✅ |
| 4 | Healing path (pre-fix claims repair on re-claim) | Attach re-runs on the snapshot.catalog branch | Deleted the account_artist_ids row → re-POST → same catalog + canonical link re-attached (canonical_relinked: 1, roster back to 18) | ✅ |
| 5 | Hero's call: GET /api/catalogs/2255ff65…/measurements?artist_account_id=7647f901…&limit=1 | Artist-scoped aggregate equals unscoped for a fully-linked catalog | measured_song_count: 67, total_streams: 557,734,241, valuation.mid: $2,058,990, artist_account_id echoed — identical to unscoped (prod repro this morning returned 0/$0 via the funnel-minted duplicate) | ✅ |
| 6 | 400 — non-UUID snapshot | snapshot must be a valid UUID | HTTP 400, {"missing_fields":["snapshot"],"error":"snapshot must be a valid UUID"} | ✅ |
| 7 | 401 — no credentials | Exactly one of x-api-key / Authorization required | HTTP 401, {"error":"Exactly one of x-api-key or Authorization must be provided"} | ✅ |
| 8 | 403 — snapshot owned by another account | Snapshot belongs to a different account | HTTP 403, exact message (probed with walkthrough-owned 9e6f9f2c…; rejected before any write) | ✅ |
| 9 | 404 — unknown snapshot id | Snapshot not found | HTTP 404, exact message (00000000-0000-4000-8000-000000000000) | ✅ |
No secret/env value echoed in any response body.
Not exercised live
- Spotify socials at ingest (
attachSpotifySocialsToArtists): needs a real capture-enrichment run to observe; rests on this PR's unit tests, including the case-preservation assertions. - The full fresh-funnel signup (OTP → auto-run → claim) — that's the post-deploy prod walkthrough already planned on chat#1859.
Side observation (pre-existing, not this PR)
For the same Privy bearer, POST /api/accounts resolved account eb2ffb4a… (empty email) while the claim path (validateAuthContext → getOrCreateAccountIdByAuthToken, email-keyed) resolved fb678396… — two different accounts for one credential. Didn't affect these tests once the fixture was pointed at the email-keyed account, but it looks like a latent identity split worth a look outside this PR.
Cleanup
All fixture rows removed after the run (1 roster link, 2 catalogs, 2 account_catalogs, 67 catalog_songs, 67 song_measurements, 1 snapshot); the caller's roster verified back to its 17-artist baseline.
Verdict: docs ↔ API ↔ reality agree on every path tested — good to merge (then deploy before marketing#46).
🤖 Generated with Claude Code
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Uh oh!
There was an error while loading. Please reload this page.
3 tasks
This was referenced Jul 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The api half of chat#1850 P1 — two changes:
POST /api/catalogs(snapshot path) now resolves the claimed songs' canonical artist through the songs graph — measured ISRCs →song_artists→ the dominant artist account (verified deterministic on the repro: 67/67 links to one account) — and inserts it into the claiming account'saccount_artist_idswhen absent. Runs on first claims (createSnapshotCatalog) AND idempotent re-claims (createCatalogHandler), so pre-fix claims heal on the next click. Best-effort: a failed attach never fails the claim. This replaces the marketing funnel's per-signupPOST /api/artistsas the roster source of truth.linkSongsToArtists, the api#684 path) has Spotifytrack.artistsids in hand when auto-creating canonical artist accounts, but attached no socials — making canonicals unfindable by Spotify id. It now attacheshttps://open.spotify.com/artist/{id}(case preserved — ids are case-sensitive base62; the legacy lowercasing corruption is a different write path and is not repeated here) via the sameupdateArtistSocialslib pathPATCH /api/artistsuses, skipping artists that already have a Spotify artist social.New files (SRP, one exported function each):
lib/supabase/song_artists/selectSongArtistsBySongs.ts(chunked batch select),lib/songs/getDominantSongArtist.ts(pure resolution, deterministic tie-break),lib/catalog/attachCanonicalArtistToAccount.ts,lib/songs/attachSpotifySocialsToArtists.ts.Tests (TDD RED→GREEN)
pnpm exec tsc --noEmit: 200 errors — exactly the pre-existing baseline, none in touched files.pnpm lintclean.Verification
4b934253…filtered by the canonical roster artist returns 67 songs / 557,513,905 streams viaget_catalog_measurements_aggregate.Links
Merge order
main.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes