feat(artists): POST /api/artists resolve-or-create by spotify_artist_id (chat#1889 row 8) #793
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||
| import { resolveOrCreateArtist } from "@/lib/artists/resolveOrCreateArtist"; | ||
| import { createArtistInDb } from "@/lib/artists/createArtistInDb"; | ||
| import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; | ||
| import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId"; | ||
| import { insertAccountArtistId } from "@/lib/supabase/account_artist_ids/insertAccountArtistId"; | ||
| import { selectAccountWithSocials } from "@/lib/supabase/accounts/selectAccountWithSocials"; | ||
| import { updateArtistSocials } from "@/lib/artist/updateArtistSocials"; | ||
|
|
||
| vi.mock("@/lib/artists/createArtistInDb", () => ({ createArtistInDb: vi.fn() })); | ||
| vi.mock("@/lib/valuation/findCanonicalArtistBySpotifyId", () => ({ | ||
| findCanonicalArtistBySpotifyId: vi.fn(), | ||
| })); | ||
| vi.mock("@/lib/supabase/account_artist_ids/selectAccountArtistId", () => ({ | ||
| selectAccountArtistId: vi.fn(), | ||
| })); | ||
| vi.mock("@/lib/supabase/account_artist_ids/insertAccountArtistId", () => ({ | ||
| insertAccountArtistId: vi.fn(), | ||
| })); | ||
| vi.mock("@/lib/supabase/accounts/selectAccountWithSocials", () => ({ | ||
| selectAccountWithSocials: vi.fn(), | ||
| })); | ||
| vi.mock("@/lib/artist/updateArtistSocials", () => ({ updateArtistSocials: vi.fn() })); | ||
|
|
||
| const SPOTIFY_ID = "0xPoVNPnxIIUS1vrxAYV00"; | ||
| const created = { id: "new-1", account_id: "new-1", name: "Del Water Gap" }; | ||
|
|
||
| describe("resolveOrCreateArtist", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| vi.mocked(createArtistInDb).mockResolvedValue(created as never); | ||
| vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue(null); | ||
| vi.mocked(selectAccountArtistId).mockResolvedValue(null as never); | ||
| vi.mocked(selectAccountWithSocials).mockResolvedValue({ | ||
| id: "canonical-1", | ||
| name: "Del Water Gap", | ||
| } as never); | ||
| }); | ||
|
|
||
| it("creates and attaches the Spotify social when no canonical exists", async () => { | ||
| const result = await resolveOrCreateArtist({ | ||
| name: "Del Water Gap", | ||
| accountId: "acct-1", | ||
| spotifyArtistId: SPOTIFY_ID, | ||
| }); | ||
|
|
||
| expect(createArtistInDb).toHaveBeenCalledWith("Del Water Gap", "acct-1", undefined); | ||
| expect(updateArtistSocials).toHaveBeenCalledWith("new-1", { | ||
| SPOTIFY: `https://open.spotify.com/artist/${SPOTIFY_ID}`, | ||
| }); | ||
| expect(result).toEqual({ artist: created, created: true }); | ||
|
There was a problem hiding this comment. P2: Test "returns the created artist when the social attach fails" verifies resilience to a rejected updateArtistSocials but doesn't assert the social attach was attempted. If a regression causes the function to silently skip updateArtistSocials (no call), the mock rejection never fires and the test passes trivially because it only checks the return value. Add Prompt for AI agents |
||
| }); | ||
|
|
||
| // One canonical artist per Spotify id (chat#1889, decision 2026-07-29): | ||
| // when it exists, link it to the account — never mint a second row. | ||
| it("links and returns the existing canonical instead of creating", async () => { | ||
| vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue("canonical-1"); | ||
|
|
||
| const result = await resolveOrCreateArtist({ | ||
| name: "Del Water Gap", | ||
| accountId: "acct-1", | ||
| spotifyArtistId: SPOTIFY_ID, | ||
| }); | ||
|
|
||
| expect(createArtistInDb).not.toHaveBeenCalled(); | ||
| expect(updateArtistSocials).not.toHaveBeenCalled(); | ||
| expect(insertAccountArtistId).toHaveBeenCalledWith("acct-1", "canonical-1"); | ||
| expect(result.created).toBe(false); | ||
| expect(result.artist).toMatchObject({ id: "canonical-1", account_id: "canonical-1" }); | ||
| }); | ||
|
|
||
| it("does not re-link a canonical the account already rosters", async () => { | ||
| vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue("canonical-1"); | ||
| vi.mocked(selectAccountArtistId).mockResolvedValue({ id: "link-1" } as never); | ||
|
|
||
| const result = await resolveOrCreateArtist({ | ||
| name: "Del Water Gap", | ||
| accountId: "acct-1", | ||
| spotifyArtistId: SPOTIFY_ID, | ||
| }); | ||
|
|
||
| expect(insertAccountArtistId).not.toHaveBeenCalled(); | ||
| expect(result.created).toBe(false); | ||
| }); | ||
|
|
||
| it("plain create path is untouched when no spotify id is given", async () => { | ||
| const result = await resolveOrCreateArtist({ name: "X", accountId: "acct-1" }); | ||
|
|
||
| expect(findCanonicalArtistBySpotifyId).not.toHaveBeenCalled(); | ||
| expect(updateArtistSocials).not.toHaveBeenCalled(); | ||
| expect(result).toEqual({ artist: created, created: true }); | ||
| }); | ||
|
|
||
| // Enrichment is non-fatal (same contract as chat#1892): the row exists by | ||
| // the time the social attach runs, so a failed attach must not fail the add. | ||
| it("returns the created artist when the social attach fails", async () => { | ||
| vi.mocked(updateArtistSocials).mockRejectedValue(new Error("nope")); | ||
|
|
||
| const result = await resolveOrCreateArtist({ | ||
| name: "Del Water Gap", | ||
| accountId: "acct-1", | ||
| spotifyArtistId: SPOTIFY_ID, | ||
| }); | ||
|
|
||
| expect(result).toEqual({ artist: created, created: true }); | ||
| }); | ||
| }); | ||
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import { createArtistInDb, type CreateArtistResult } from "@/lib/artists/createArtistInDb"; | ||
| import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; | ||
| import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId"; | ||
| import { insertAccountArtistId } from "@/lib/supabase/account_artist_ids/insertAccountArtistId"; | ||
| import { selectAccountWithSocials } from "@/lib/supabase/accounts/selectAccountWithSocials"; | ||
| import { updateArtistSocials } from "@/lib/artist/updateArtistSocials"; | ||
|
|
||
| export interface ResolveOrCreateArtistParams { | ||
| name: string; | ||
| accountId: string; | ||
| organizationId?: string; | ||
| /** When present, resolve-or-create the canonical artist for this Spotify id. */ | ||
| spotifyArtistId?: string; | ||
| } | ||
|
|
||
| export interface ResolveOrCreateArtistResult { | ||
| artist: CreateArtistResult | null; | ||
| /** false when an existing canonical was linked instead of created (→ 200, not 201). */ | ||
| created: boolean; | ||
| } | ||
|
|
||
| /** | ||
| * Creates an artist — or, when `spotifyArtistId` names an artist that already | ||
| * exists, links that canonical to the account instead of minting a duplicate. | ||
| * | ||
| * One canonical artist per Spotify id (chat#1889, decision 2026-07-29): | ||
| * artists are shared across accounts via `account_artist_ids`, so creation is | ||
| * the only place dedup can be enforced. Every add path minting its own row is | ||
| * what made downstream canonical lookups ambiguous and doubled rosters. | ||
| * | ||
| * The social attach on the create path is non-fatal (same contract as the | ||
| * chat-side add, chat#1892): the row exists by then, and a failed attach must | ||
| * not fail the add — the social is still fixable in verify-socials. | ||
| */ | ||
| export async function resolveOrCreateArtist( | ||
| params: ResolveOrCreateArtistParams, | ||
| ): Promise<ResolveOrCreateArtistResult> { | ||
| const { name, accountId, organizationId, spotifyArtistId } = params; | ||
|
|
||
| if (spotifyArtistId) { | ||
| const canonicalId = await findCanonicalArtistBySpotifyId(spotifyArtistId); | ||
|
There was a problem hiding this comment. P1: A non-empty Prompt for AI agentsThere was a problem hiding this comment. P1: Concurrent requests can both observe no canonical and create separate artist accounts because the lookup and creation are not atomic; the later social upsert does not make the artist rows unique. An atomic database resolve/create operation with a unique Spotify identity is needed to preserve the one-canonical-per-ID contract. Prompt for AI agents |
||
| if (canonicalId) { | ||
| const alreadyRostered = await selectAccountArtistId(accountId, canonicalId); | ||
|
There was a problem hiding this comment. P2: Concurrent reuse requests can race between the roster existence check and insert, causing duplicate Prompt for AI agents |
||
| if (!alreadyRostered) { | ||
| await insertAccountArtistId(accountId, canonicalId); | ||
| } | ||
| const artist = await selectAccountWithSocials(canonicalId); | ||
| return { | ||
| artist: artist ? { ...artist, account_id: artist.id } : null, | ||
| created: false, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| const created = await createArtistInDb(name, accountId, organizationId); | ||
|
Comment on lines
+40
to
+55
There was a problem hiding this comment. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bash
rg -n -C 5 'findCanonicalArtistBySpotifyId|createArtistInDb|selectAccountArtistId|insertAccountArtistId' lib
rg -n -C 3 'spotify|account_artist_ids|unique' supabase libRepository: recoupable/api Length of output: 50371 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Files containing resolveOrCreateArtist:"
fd -a 'resolveOrCreateArtist\.ts$' . | sed 's#^\./##'
file="$(fd 'resolveOrCreateArtist\.ts$' . | head -n 1)"
echo "---- $file ----"
wc -l "$file"
ast-grep outline "$file" --view expanded || true
sed -n '1,180p' "$file" | cat -n
echo "---- Supabase table definitions around artists/socials/account_artist_ids ----"
rg -n -C 8 'artists|spotify_id|account_artist|account_social|i[._-]?new|index|unique|constraint|account_socials' supabase lib | head -n 260Repository: recoupable/api Length of output: 23226 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Candidate artifact files:"
git ls-files | rg '(^|/)(artist|artists|account_artist|artist_social|account_social|account_socials|spotify).*(sql|ts)$|supabase/.*/.*(artist|spotify|social).*' | head -n 300
for f in $(git ls-files | rg '(^|/)(artist|artists|spotify|socials?).*\.(sql|ts)$' | head -n 80); do
echo "---- $f ----"
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
doneRepository: recoupable/api Length of output: 50371 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Locate Supabase schema and migrations:"
git ls-files | rg '(^|/)(supabase|migrations|.*\.sql$)' | head -n 200
echo "---- Files directly named around artists/socials/account_artist_ids sql ----"
git ls-files | rg '(artist|artists|account_artist|artist_social|account_social|socials).*\.sql$' | sort
echo "---- Relevant SQL snippets ----"
for f in $(git ls-files | rg '(artist|artists|account_artist|artist_social|account_social|socials).*\.sql$' | sort | head -n 80); do
echo "---- $f ----"
rg -n -C 8 'artists|spotify|unique|unique .*spotify|account_artist|account_social|unique .*account.*social|constraint.*account.*social|i[._-]?new|create table' "$f" || true
doneRepository: recoupable/api Length of output: 11060 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- candidate value/spotify canonical files ----"
git ls-files | rg '(^|/)findCanonicalArtistBySpotifyId\.ts$|spotify|artist' | head -n 200
echo "---- implementation files ----"
for f in $(git ls-files | rg '(^|/)findCanonicalArtistBySpotifyId\.ts$|(^|/)createArtistInDb\.ts$|(^|/)(insertAccountArtistId|selectAccountArtistId|updateArtistSocials|insertAccountSocial|socials).*\.ts$' | sort); do
echo "---- $f ----"
wc -l "$f"
ast-grep outline "$f" --view expanded || true
sed -n '1,220p' "$f" | cat -n
done
echo "---- schema searches for SPOTIFY/social/account_artist constraints ----"
rg -n -C 6 'SPOTIFY|account_socials|socials|account_artist_ids|spotify|create table|constraint|unique|create unique|i[_-]?new|unique constraint|unique' lib/supabase/lib/supabase | head -n 350Repository: recoupable/api Length of output: 50370 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- target implementation files ----"
for f in \
lib/valuation/findCanonicalArtistBySpotifyId.ts \
lib/artists/createArtistInDb.ts \
lib/supabase/account_artist_ids/selectAccountArtistId.ts \
lib/supabase/account_artist_ids/insertAccountArtistId.ts \
lib/supabase/accounts/selectAccountWithSocials.ts \
lib/supabase/account_socials/insertAccountSocial.ts \
lib/supabase/socials/selectSocials.ts \
lib/supabase/socials/upsertSocials.ts
do
if [ -f "$f" ]; then
echo "---- $f ----"
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
else
echo "MISSING: $f"
fi
done
echo "---- schema/migration SQL files ----"
git ls-files \*.{sql|ddl}| head -n 200
for f in $(git ls-files \*.{sql|ddl} | head -n 200); do
echo "---- $f: relevant schema matches ----"
rg -n -C 5 'create table|account_socials|socials|account_artist_ids|spotify|unique constraint|unique|SPOTIFY|profile_url' "$f" | head -n 180
doneRepository: recoupable/api Length of output: 12821 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- all SQL files ----"
git ls-files | rg '\.sql$' | sort
echo "---- schema/migration relevant constraints ----"
for f in $(git ls-files | rg '\.sql$' | sort); do
matches=$(rg -n -C 5 'account_socials|socials|account_artist_ids|spotify|unique constraint|unique|SPOTIFY|profile_url' "$f" || true)
if [ -n "$matches" ]; then
echo "---- $f ----"
echo "$matches"
fi
done
echo "---- deterministic race-order probe ----"
python3 - <<'PY'
sequence = [
("A", "findCanonical", None),
("B", "findCanonical", None),
("A", "createArtistInDb", "artist-A"),
("B", "createArtistInDb", "artist-B"),
("A", "updateArtistSocials", "artist-A"),
("B", "updateArtistSocials", "artist-B"),
]
for step in sequence:
print(f"{step[0]}: {step[1]} -> {step[2]}")
print("\nTwo artist rows exist after interleaved createArtistInDb calls, each with same Spotify social: duplicate canonical artist rows.")
PYRepository: recoupable/api Length of output: 176 Make canonical artist lookup, social attach, and roster linking atomic.
Enforce one canonical Spotify artist in the schema, use an atomic upsert/transaction for create + social attach, and use an idempotent or conflict-safe insert for 🤖 Prompt for AI Agents |
||
|
|
||
| if (created && spotifyArtistId) { | ||
| try { | ||
| await updateArtistSocials(created.account_id, { | ||
| SPOTIFY: `https://open.spotify.com/artist/${spotifyArtistId}`, | ||
| }); | ||
| } catch { | ||
| // Non-fatal by design — see docstring. | ||
| } | ||
| } | ||
|
|
||
| return { artist: created, created: true }; | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: Test 'passes organization_id to createArtistInDb' title now references a function that is no longer the public API, and the assertion no longer verifies organizationId was forwarded. The old test explicitly checked the org UUID in the third arg; the new assertion only checks name and accountId, so it would pass even if organizationId were dropped. Either add organizationId to the objectContaining matcher or rename the test to reflect what it actually validates.
Prompt for AI agents