Skip to content

fix(valuation): reuse the canonical artist for a Spotify id, never mint a second (chat#1889 row 8) - #791

Merged
sweetmantech merged 3 commits into
mainfrom
fix/valuation-reuse-canonical-artist
Jul 29, 2026
Merged

fix(valuation): reuse the canonical artist for a Spotify id, never mint a second (chat#1889 row 8)#791
sweetmantech merged 3 commits into
mainfrom
fix/valuation-reuse-canonical-artist

Conversation

@sweetmantech

@sweetmantech sweetmantech commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Matrix row 8 in chat#1889. Blocks chat#1900 (row 9), which is in draft until this lands.

Why

Adding one artist through /setup/artists produced two roster entries. Caught on a brand-new account during chat#1900's preview verification:

[
  { "name": "Del Water Gap", "account_id": "6e4d3f5f-…",
    "socials": ["open.spotify.com/artist/0xPoVNPnxIIUS1vrxAYV00"] },
  { "name": "Del Water Gap", "account_id": "7647f901-…",
    "socials": ["open.spotify.com/artist/0xPoVNPnxIIUS1vrxAYV00"] }
]

Same name, same avatar, same Spotify id. Client-side instrumentation showed exactly one POST /api/artists, so the second was created server-side by POST /api/valuation.

Root cause. runValuationHandler falls back to linkSearchedArtistToAccount when attachCanonicalArtistToAccount resolves nothing — which is always the case for a freshly seeded catalog, because its songs are not in song_artists yet. That fallback called createArtistInDb unconditionally.

attachCanonicalArtistToAccount's own docstring names the failure mode it was built to end:

This replaces the marketing funnel's per-signup artist creation — which minted duplicate, song-less roster artists — as the roster source of truth for funnel signups.

The fallback had quietly reintroduced it.

What changed

The lookup is global, not per-account. Artists are canonical and shared, and account_artist_ids is the join that lets many accounts roster one artist (chat#1866 exists precisely because of that sharing). Scoping the dedup per-account would still give every account its own copy of the same Spotify artist — it would only stop one account duplicating itself. So:

  • selectSocialsBySpotifyArtistId (lib/supabase/socials/) — finds socials by Spotify id with an ilike, because profile_url is stored inconsistently: with and without a scheme, sometimes with ?si=. The existing selectSocials({ profile_url }) is an exact eq and misses real matches.
  • findCanonicalArtistBySpotifyId (lib/valuation/) — resolves that social to its artist account. Best-effort: returns null on any error so a dedup lookup can never fail a valuation.
  • linkSearchedArtistToAccount — reuses the canonical artist when one exists, linking it via insertAccountArtistId only when the account doesn't already roster it. Creates only when no artist carries that Spotify id yet.

Verification

TDD, red → green, two units:

# RED 1 — module not found
FAIL lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts

# RED 2 — still creating a duplicate
FAIL lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts (2 failed)
  > rosters the existing canonical artist instead of creating a duplicate
  > does not re-link a canonical artist the account already rosters

# GREEN — full api suite
pnpm exec vitest run
  Test Files  785 passed (785)
       Tests  4253 passed (4253)

Six new tests. The lookup covers the global-not-scoped assertion, scheme/query-string variance, no-match, no-linked-artist, and the throw path. The link covers reuse-instead-of-create and don't-re-link-when-already-rostered — plus the four pre-existing tests still pass, so the create path a genuine first-time artist depends on is unchanged.

pnpm exec tsc --noEmit → 236, identical to main's baseline, none in the touched files. eslint clean.

No docs change: POST /api/valuation's request/response contract is unchanged.

Reviewer notes

  • Scope beyond the seeding path. This also fixes the /artists dialog, which uses the same useAddSpotifyArtist create-then-seed flow — so any account whose first artist was added there has been getting a duplicate too. Pre-existing, now fixed.
  • Existing duplicates are not cleaned up. This stops new ones; it does not merge rows already created. Worth a follow-up if the count is material — happy to open one.
  • Sharing has a known edge: once accounts genuinely share one artist row, artist-level metadata is shared too. That is exactly what chat#1866 tracks (PATCH /api/artists authorizing shared-metadata writes off the flat roster edge). This PR makes sharing work as designed rather than routing around it, so #1866 becomes more worth landing.

Preview verification to follow on this PR.

Tracked in chat#1889 (matrix row 8).


Summary by cubic

Fixes duplicate artist creation when adding by Spotify ID by reusing the existing canonical artist and linking it to the account. Addresses chat#1889 (row 8) and unblocks chat#1900.

  • Bug Fixes
    • Updated linkSearchedArtistToAccount to reuse a canonical artist if it exists, only create when none exists, and skip re-linking if already rostered.
    • Added findCanonicalArtistBySpotifyId for a global lookup by Spotify ID and extended selectSocials with profileUrlContains (ilike on profile_url) to handle inconsistent storage; removed the one-off Spotify lookup.
    • Applies to both the valuation flow and the /artists add dialog.

Written for commit d899d0e. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Existing artist records are now reused when linking searched Spotify artists, reducing duplicate artist entries.
    • Artists can be matched using partial profile URL searches.
  • Bug Fixes

    • Improved artist linking to avoid duplicate account associations.
    • Artist lookup failures are handled gracefully without interrupting the process.

sweetmantech and others added 2 commits July 28, 2026 17:43
…nt a second

linkSearchedArtistToAccount called createArtistInDb unconditionally. The
canonical ISRC -> song_artists attach resolves nothing for a freshly
seeded catalog (its songs are not in song_artists yet), so the fallback
always fired and every account got its own copy of the same Spotify
artist -- same name, same avatar, same id, different account_id.

Artists are canonical and shared; account_artist_ids is the join that
lets many accounts roster one artist (chat#1866). The lookup is
therefore global, not per-account: scoping it per-account is exactly
what produces a copy per signup.

attachCanonicalArtistToAccount's docstring already named this failure:
it exists to replace 'the marketing funnel's per-signup artist creation
-- which minted duplicate, song-less roster artists'. The fallback had
reintroduced it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

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.

@vercel

vercel Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api Ready Ready Preview Jul 29, 2026 2:16am

Request Review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: af0cb8e5-58bc-40d7-94d5-9624927d60f4

📥 Commits

Reviewing files that changed from the base of the PR and between 41279e3 and d899d0e.

⛔ Files ignored due to path filters (1)
  • lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (2)
  • lib/supabase/socials/selectSocials.ts
  • lib/valuation/findCanonicalArtistBySpotifyId.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/valuation/findCanonicalArtistBySpotifyId.ts

📝 Walkthrough

Walkthrough

Adds a best-effort Spotify canonical artist lookup, substring social URL filtering, and canonical artist reuse with conditional account-to-artist association before fallback artist creation.

Changes

Canonical Artist Linking

Layer / File(s) Summary
Spotify canonical resolution
lib/supabase/socials/selectSocials.ts, lib/valuation/findCanonicalArtistBySpotifyId.ts
selectSocials supports case-insensitive profile URL substring matching, while the new helper resolves linked canonical account IDs and returns null on missing links or errors.
Canonical account linking
lib/valuation/linkSearchedArtistToAccount.ts
The linking flow reuses resolved canonical artists, avoids duplicate account associations, and preserves the existing artist-creation fallback.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Account
  participant linkSearchedArtistToAccount
  participant findCanonicalArtistBySpotifyId
  participant Supabase
  Account->>linkSearchedArtistToAccount: Spotify artist ID
  linkSearchedArtistToAccount->>findCanonicalArtistBySpotifyId: Resolve canonical artist
  findCanonicalArtistBySpotifyId->>Supabase: Query social profile URLs
  Supabase-->>findCanonicalArtistBySpotifyId: Linked account or null
  findCanonicalArtistBySpotifyId-->>linkSearchedArtistToAccount: Canonical artist ID
  linkSearchedArtistToAccount->>Supabase: Check or insert account association
Loading

Possibly related PRs

Poem

Spotify trails through socials bright,
Finds a canonical star in sight.
Links the roster, skips repeats,
Or builds anew when lookup retreats.
Clean paths bloom where artists meet.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Solid & Clean Code ✅ Passed PASS: One primary export per file, names match filenames, and the Spotify lookup was consolidated into selectSocials instead of split into a stray helper.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/valuation-reuse-canonical-artist

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/supabase/socials/selectSocialsBySpotifyArtistId.ts`:
- Around line 20-24: Update the lookup in selectSocialsBySpotifyArtistId to
reject blank or invalid spotifyArtistId values before querying, then normalize
the ID and constrain the ilike pattern to the canonical /artist/{id} path while
still allowing URL scheme and query-string variations. Preserve the existing
updated_at ordering for valid IDs.

In `@lib/valuation/linkSearchedArtistToAccount.ts`:
- Around line 41-48: The canonical roster linking flow in
linkSearchedArtistToAccount must replace the
selectAccountArtistId-then-insertAccountArtistId race with a conflict-safe
insert/upsert. Enforce the account/artist pair uniqueness constraint used by
this path, treat conflicts from concurrent inserts as successful linking, and
preserve returning canonicalId rather than converting a successful roster link
into null.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 805271e9-108e-46b7-8799-3e00107ba0b3

📥 Commits

Reviewing files that changed from the base of the PR and between c0a7c7d and 41279e3.

⛔ Files ignored due to path filters (2)
  • lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (3)
  • lib/supabase/socials/selectSocialsBySpotifyArtistId.ts
  • lib/valuation/findCanonicalArtistBySpotifyId.ts
  • lib/valuation/linkSearchedArtistToAccount.ts

Comment on lines +20 to +24
const { data, error } = await supabase
.from("socials")
.select("*")
.ilike("profile_url", `%${spotifyArtistId}%`)
.order("updated_at", { ascending: false });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Constrain the lookup to a valid Spotify artist path.

%${spotifyArtistId}% is an unbounded substring match. A blank ID becomes %% and can return every social URL; because the linking caller only rejects a blank artistName, the canonical path may roster the newest unrelated linked account. It can also match an ID embedded in another social URL or query string. Reject blank/invalid IDs before lookup and match the normalized /artist/{id} path while allowing scheme and query-string variations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/supabase/socials/selectSocialsBySpotifyArtistId.ts` around lines 20 - 24,
Update the lookup in selectSocialsBySpotifyArtistId to reject blank or invalid
spotifyArtistId values before querying, then normalize the ID and constrain the
ilike pattern to the canonical /artist/{id} path while still allowing URL scheme
and query-string variations. Preserve the existing updated_at ordering for valid
IDs.

Comment on lines +41 to +48
const canonicalId = await findCanonicalArtistBySpotifyId(spotifyArtistId);
if (canonicalId) {
const alreadyRostered = await selectAccountArtistId(accountId, canonicalId);
if (!alreadyRostered) {
await insertAccountArtistId(accountId, canonicalId);
}
return canonicalId;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Repo files matching target:"
git ls-files | rg '(^|/)linkSearchedArtistToAccount\.ts$|account_artist|spotify_artist|artist' || true

echo
echo "Target file outline and relevant content:"
target="lib/valuation/linkSearchedArtistToAccount.ts"
if [ -f "$target" ]; then
  wc -l "$target"
  ast-grep outline "$target" || true
  cat -n "$target"
fi

echo
echo "Search for helper implementations and table references:"
rg -n "findCanonicalArtistBySpotifyId|selectAccountArtistId|insertAccountArtistId|account_artist_ids|artist_ids|artist" lib -S || true

Repository: recoupable/api

Length of output: 50371


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Find SQL/schema files and Supabase helpers:"
git ls-files | rg '(^|\.)sql$|schema|supabase|migration|db|data' || true

echo
echo "Supabase helper files potentially containing account_artist helpers:"
fd . lib/supabase -t f 2>/dev/null | sed -n '1,200p' || true

echo
echo "Search for account_artist_ids column/table definitions:"
rg -n "account_artist_ids|artist_ids|\"artists\"|spotify_artist|Artist" lib . 2>/dev/null \
  | rg -i "id|artist|spotify|link|artist_id|account_id|unique|constraint|create table|create index|insert|select" || true

Repository: recoupable/api

Length of output: 50371


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Target function:"
target="lib/valuation/linkSearchedArtistToAccount.ts"
cat -n "$target"

echo
echo "Supabase account_artist_ids helpers:"
for f in \
  lib/supabase/account_artist_ids/selectAccountArtistId.ts \
  lib/supabase/account_artist_ids/insertAccountArtistId.ts \
  lib/supabase/account_artist_ids/getAccountArtistIds.ts \
  lib/supabase/account_artist_ids/getAccountPinnedArtistIds.ts
do
  echo "--- $f"
  [ -f "$f" ] && sed -n '1,120p' "$f" | cat -n
done

echo
echo "Account artists table/schema definitions mentioning account_artist_ids uniqueness:"
fd -e sql . | rg -i "sql$" | xargs -r rg -n -C 3 "account_artist_ids|unique.*(account|artist)|account_id.*artist|unique.*account_artist|artist_id.*account" || true

echo
echo "Test expectations for duplicate-link behavior:"
cat -n lib/supabase/account_artist_ids/__tests__/selectAccountArtistId.test.ts
echo "---"
cat -n lib/supabase/account_artist_ids/__tests__/insertAccountArtistId.test.ts
echo "---"
cat -n lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts | sed -n '70,102p'

Repository: recoupable/api

Length of output: 14381


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Locate SQL files and constrained indices for account_artist_ids:"
git ls-files '*.sql' | xargs -r rg -n -C 4 "create table account_artist_ids|account_artist_ids|account_artist|insert into account_artist_ids|unique (account_id, artist_id)|unique.*account_id.*artist|account_artist|on conflict|conflict" || true

echo
echo "Type definitions for account_artist_ids if present:"
git ls-files | rg "database.types|database.types.*|supabase.*types|schema" | while read -r f; do
  rg -n -C 3 "account_artist_ids|accountArtistIds|artistIds|account_id\?:|artist_id\?:" "$f" || true
done

echo
echo "Behavioral model of current read-only race condition:"
python3 - <<'PY'
links = set()
# Scenario: two concurrent canonical-link requests both pass the select check
links.add(("acc", "canon"))
before_insert = not (("acc", "canon") in links)
links.add(("acc", "canon"))
after_insert = links
print("initial:", set())
print("concurrent_requests:", 2)
print("before_insert:", before_insert, "=> insertAccountArtistId called =", before_insert)
print("post_state:", sorted(after_insert))
print("duplicate_rows_possible_without_uniq_constraint:", len(after_insert) > len(set(after_insert)))
PY

Repository: recoupable/api

Length of output: 12003


Make canonical roster linking conflict-safe.

selectAccountArtistIdinsertAccountArtistId is the same check-then-insert race as the canonical catalog attach path. Concurrent setups can both observe no row and insert separately without an account/artist uniqueness guard, or one insert can error and turn a successfully rostered artist into null. Enforce this pair as unique and replace this read-check with a conflict-safe insert/upsert that treats an existing row as success.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/valuation/linkSearchedArtistToAccount.ts` around lines 41 - 48, The
canonical roster linking flow in linkSearchedArtistToAccount must replace the
selectAccountArtistId-then-insertAccountArtistId race with a conflict-safe
insert/upsert. Enforce the account/artist pair uniqueness constraint used by
this path, treat conflicts from concurrent inserts as successful linking, and
preserve returning canonicalId rather than converting a successful roster link
into null.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 5 files

Confidence score: 2/5

  • In lib/valuation/findCanonicalArtistBySpotifyId.ts, the lookup and create steps are not atomic, so concurrent first-time valuations can mint duplicate artist accounts and split identity/state across records — switch to a DB-enforced unique Spotify-ID claim with transactional get-or-create/upsert.
  • In lib/valuation/findCanonicalArtistBySpotifyId.ts, treating lookup failures as “not found” can create duplicates exactly during outages, turning transient dependency issues into persistent data integrity problems — return a distinct lookup-error state and block fallback creation when verification is unavailable.
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/valuation/findCanonicalArtistBySpotifyId.ts">

<violation number="1" location="lib/valuation/findCanonicalArtistBySpotifyId.ts:24">
P1: Concurrent first-time valuations can still mint duplicate artist accounts because this lookup is separate from creation. Use a DB-backed unique Spotify-ID claim/upsert or transactional get-or-create so only one request creates the canonical account.</violation>

<violation number="2" location="lib/valuation/findCanonicalArtistBySpotifyId.ts:33">
P2: A lookup outage is treated as “no canonical artist,” so this path mints a duplicate precisely when existence cannot be verified. Preserve a distinct lookup-error result and skip fallback creation on that result while allowing valuation to continue.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant Client as Client (UI)
    participant API as POST /api/artists
    participant VH as POST /api/valuation
    participant VH2 as runValuationHandler
    participant LS as linkSearchedArtistToAccount
    participant FC as findCanonicalArtistBySpotifyId
    participant SS as selectSocialsBySpotifyArtistId
    participant SC as selectAccountSocials
    participant AI as supabase/account_artist_ids
    participant DB as Database (Postgres)

    Note over Client,DB: Artist Roster Flow – Duplicate Prevention

    alt Adding artist via /api/artists (UI dialog)
        Client->>API: POST /api/artists { spotifyArtistId, artistName, accountId }
        API->>LS: linkSearchedArtistToAccount(...)
    else Adding artist via valuation (catalog seeding)
        Client->>VH: POST /api/valuation (ISRC scan)
        VH->>VH2: runValuationHandler()
        VH2->>VH2: attachCanonicalArtistToAccount() → null<br/>(no song_artists entries yet)
        VH2->>LS: linkSearchedArtistToAccount(...)
    end

    Note over LS: NEW: First, check for existing canonical artist

    LS->>FC: findCanonicalArtistBySpotifyId(spotifyArtistId)
    FC->>SS: selectSocialsBySpotifyArtistId(id)<br/>NEW: ilike `%{id}%` on profile_url
    SS->>DB: QUERY socials WHERE profile_url ILIKE '%spotifyId%'
    DB-->>SS: matching social rows (if any)
    SS-->>FC: social rows

    alt Social found for this Spotify id
        loop For each social row (newest first)
            FC->>SC: selectAccountSocials({ socialId })
            SC->>DB: QUERY account_socials WHERE social_id = ?
            DB-->>SC: link rows (account_id)
            SC-->>FC: account_socials links
            alt Has a linked account
                FC-->>LS: artist account_id (canonical)
            else No linked account
                FC->>FC: continue to next social
            end
        end
        Note over FC: Best-effort: returns null on any error
        FC-->>LS: canonicalId or null
    else No social found
        SS-->>FC: empty array
        FC-->>LS: null
    end

    alt Canonical artist exists (NEW path)
        LS->>AI: selectAccountArtistId(accountId, canonicalId)<br/>Check if already rostered
        AI->>DB: QUERY account_artist_ids WHERE account_id=? AND artist_id=?
        alt Not yet rostered
            DB-->>AI: null
            AI-->>LS: null
            LS->>AI: insertAccountArtistId(accountId, canonicalId)<br/>Link existing artist to account
            AI->>DB: INSERT account_artist_ids
            DB-->>AI: success
            AI-->>LS: done
        else Already rostered
            DB-->>AI: existing link
            AI-->>LS: link row
            Note over LS: Skip re-link – idempotent
        end
        LS-->>VH2/API: Return existing canonicalId
        Note over VH2/API: No duplicate created
    else No canonical artist (unchanged fallback)
        LS->>LS: createArtistInDb(artistName, accountId)
        LS->>LS: updateArtistSocials(newArtistId, spotifyUrl)
        LS-->>VH2/API: Return new artist account_id
    end

    alt Error in canonical lookup
        FC-->>LS: null (caught, logged)
        Note over FC: Graceful degradation –<br/>fall through to create path
    end
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread lib/supabase/socials/selectSocialsBySpotifyArtistId.ts Outdated
spotifyArtistId: string,
): Promise<string | null> {
try {
const socials = await selectSocialsBySpotifyArtistId(spotifyArtistId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Concurrent first-time valuations can still mint duplicate artist accounts because this lookup is separate from creation. Use a DB-backed unique Spotify-ID claim/upsert or transactional get-or-create so only one request creates the canonical account.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/valuation/findCanonicalArtistBySpotifyId.ts, line 24:

<comment>Concurrent first-time valuations can still mint duplicate artist accounts because this lookup is separate from creation. Use a DB-backed unique Spotify-ID claim/upsert or transactional get-or-create so only one request creates the canonical account.</comment>

<file context>
@@ -0,0 +1,38 @@
+  spotifyArtistId: string,
+): Promise<string | null> {
+  try {
+    const socials = await selectSocialsBySpotifyArtistId(spotifyArtistId);
+    if (socials.length === 0) return null;
+
</file context>

if (linked?.account_id) return linked.account_id;
}

return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A lookup outage is treated as “no canonical artist,” so this path mints a duplicate precisely when existence cannot be verified. Preserve a distinct lookup-error result and skip fallback creation on that result while allowing valuation to continue.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/valuation/findCanonicalArtistBySpotifyId.ts, line 33:

<comment>A lookup outage is treated as “no canonical artist,” so this path mints a duplicate precisely when existence cannot be verified. Preserve a distinct lookup-error result and skip fallback creation on that result while allowing valuation to continue.</comment>

<file context>
@@ -0,0 +1,38 @@
+      if (linked?.account_id) return linked.account_id;
+    }
+
+    return null;
+  } catch (error) {
+    console.error("Error resolving canonical artist by spotify id:", error);
</file context>

@sweetmantech

Copy link
Copy Markdown
Contributor Author

Preview verification — reuse confirmed across accounts, create path intact

  • Preview: https://api-ksiaaxqdc-recoup.vercel.app
  • Commit: deployment sha 41279e32, confirmed to match the PR head (not a stale earlier build)
  • Credential: a Privy bearer for a brand-new account with an empty roster, so "exactly one artist" is a direct assertion rather than a before/after diff on a populated roster. Note api previews write to the production database, so this creates real rows — another reason to use a disposable account.

The fix: reuse across accounts

0xPoVNPnxIIUS1vrxAYV00 (Del Water Gap) already had canonical artists from earlier testing, created under different accounts. Running the valuation on the brand-new account:

Roster before 0 artists
POST /api/valuation 200 in 21s — catalog d2c1038c…, band $1.43M–$2.92M, 67 songs measured
Roster after 1 artist
Its account_id 7647f901-a640-434c-b9c9-a18682435092 — a pre-existing canonical, minted earlier under a different account
Its social open.spotify.com/artist/0xPoVNPnxIIUS1vrxAYV00

The new account rostered the existing artist instead of minting its own copy. That is the canonical-source-of-truth model working across account boundaries — which a per-account dedup would not have achieved, since each account would still have created its own.

For contrast, the same single add under main produced two rows on a fresh account (evidence).

Idempotency

Re-ran the identical call: 200, roster still 1 artist. Re-linking an artist the account already rosters is a no-op, as selectAccountArtistId guards.

The create path still works

1thbOfXak53dM1Xabq3pmb (Delacey, 59.6K followers) — a different artist:

POST /api/valuation  →  200, catalog 56f7ac77…, band $450K–$922K, 61 songs measured
roster: 2 artists
  - Delacey        29797518-8410-437a-9f1d-b7c9efee02e2   newly created
      socials: open.spotify.com/artist/1thbOfXak53dM1Xabq3pmb
  - Del Water Gap  7647f901-a640-434c-b9c9-a18682435092   reused

So a genuine first-time artist is still created and still lands on the roster with its Spotify social — the case linkSearchedArtistToAccount exists for (chat#1881 P0, funnel signups whose songs aren't yet in song_artists). The dedup did not turn into "never create".

Honest caveat: I can't retroactively prove no canonical existed for Delacey beforehand. But the assertion holds either way — if one had existed it would have been reused, and the roster would still show exactly one Delacey. "Exactly one entry per Spotify id" is the property under test, and it holds in both branches.

Status codes

Probe Result
No credential 401
Bogus x-api-key 401 Unauthorized
Missing spotify_artist_id 400 missing_fields: ["spotify_artist_id"]
Unresolvable Spotify id 502 Couldn't resolve the artist's releases
GET /api/valuation 405
Secret scan on a success body 0 matches for secret|service_role|SUPABASE_|sk_live|pepper

Auth is enforced ahead of body validation (a bogus key with an empty body returns 401, not 400), which is the correct ordering.

Not verified

  • Existing duplicates are untouched. This stops new ones being created; the rows already in the database remain. The sweetman+july2820261713@… account still carries its two Del Water Gap entries from before the fix. Worth a cleanup pass — happy to open one once we know the count.
  • The attachCanonicalArtistToAccount path (canonical resolved via ISRC → song_artists) was not exercised separately, because a freshly seeded catalog never hits it — that is precisely why the fallback fires and why this bug existed. Unchanged by this PR.

Unblocks chat#1900 (row 9), which stays in draft until this merges.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KISS

  • actual: lib/supabase/socials/selectSocialsBySpotifyArtistId.ts
  • required: lib/supabase/socials/selectSocials.ts - general supabase lib with optional param for filtering on profile_url

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in d899d0ea — deleted selectSocialsBySpotifyArtistId.ts and folded it into selectSocials as an optional profileUrlContains (ilike) param, alongside the existing exact-match profile_url. findCanonicalArtistBySpotifyId now calls selectSocials({ profileUrlContains: spotifyArtistId }). Kept the substring/ilike semantics (not eq) because profile_url is stored inconsistently — with/without scheme, sometimes ?si= — which is why the exact-match param misses real matches; documented on the param. 229 supabase+valuation tests pass; tsc at 236 baseline; eslint clean.

…als (KISS)

Review feedback on api#791: a one-off selectSocialsBySpotifyArtistId
duplicated the general socials lib. selectSocials now takes an optional
profileUrlContains (ilike) param and the one-off file is deleted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 4 files (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.

Re-trigger cubic

@sweetmantech

Copy link
Copy Markdown
Contributor Author

Re-verified on d899d0ea (the KISS refactor)

The earlier verification ran on 41279e32; the selectSocials({ profileUrlContains }) fold-in warranted a live re-run rather than an "it's equivalent" claim.

  • Preview: https://api-eg8q40lyc-recoup.vercel.app, deployment sha confirmed d899d0ea
  • Account: sweetman+july2820261713@… via its live Privy session (the earlier bearer expired — Privy tokens are 1h)

This account is the sharpest fixture available: it still carries the two pre-fix Del Water Gap duplicates, and it had never rostered Delacey — whose canonical (29797518…) was minted earlier under a different account.

Step Result
Roster before 2 artists (the pre-fix duplicates, unchanged)
POST /api/valuation for Delacey 1thbOfXak53dM1Xabq3pmb 200 in 19s — band $450K / $655K / $922K, 61 songs (identical numbers to the first account's run, same catalog model)
Roster after 3 artists — Delacey arrived as 29797518-8410-437a-9f1d-b7c9efee02e2, the canonical from the other account

So on the refactored lookup, cross-account reuse still holds — and this run asserts it in the opposite direction from the previous pass (account B reusing a canonical minted by account A, where before it was A-then-B). No new artist row was created for a Spotify id that already has one.

CI on d899d0ea: format ✅ lint ✅ test ✅. Ready to merge; unblocks chat#1900.

@sweetmantech
sweetmantech merged commit 0b222d1 into main Jul 29, 2026
6 checks passed
@sweetmantech

Copy link
Copy Markdown
Contributor Author

Post-merge: test synced, and the fix verified on the live test-recoup-api deployment

main (with this fix) is merged into test — 0-file diff vs main, deployment built from the merge sha 03580198. Chat previews read this deployment, so chat#1900 can now be re-verified for real.

Behavioral check on the alias, with a DB cross-check

Valued Ana Bárbara (43qxAkuKFB6fMNSeS5dO7Z) as the …1713 account against test-recoup-api.vercel.app → 200 in 29s, 150 songs. The roster gained f584e4f5-… — which surprised me, since I expected the 24642551-… canonical from the #1892 testing. Queried Supabase directly rather than guessing:

Artist row Origin
f584e4f5 The true prod canonical — socials row dated 2025-05-30, rostered by OneRPM's org accounts, sidney@, and sweetmantech@gmail.com (linked 2026-07-02)
7dd152b9 Pre-fix duplicate, minted 2026-07-27 under a test account
24642551 Pre-fix duplicate, minted 2026-07-28 under the …0924 test account

Exactly 3 Ana Bárbara rows before the run, exactly 3 after — no new mint. The …1713 account was linked (02:53:18) to the year-old canonical that five other accounts — including the user's main account — already share. That is the one-canonical-many-rosters model working on the live deployment, across org and personal accounts.

Cleanup follow-up now has a concrete shape

The pre-fix duplicates visible in just this one artist: 7dd152b9 and 24642551 (plus the two Del Water Gap rows on …1713). A cleanup pass would relink their account_artist_ids rows to the oldest canonical per Spotify id and delete the orphaned rows. Happy to scope it with a count query when wanted.

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.

1 participant