From 232751834903c793cd870b4006bc7dec952e1239 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 28 Jul 2026 17:43:08 -0500 Subject: [PATCH 1/3] fix(valuation): reuse the canonical artist for a Spotify id, never mint 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) --- .../socials/selectSocialsBySpotifyArtistId.ts | 31 +++++++++ .../findCanonicalArtistBySpotifyId.test.ts | 65 +++++++++++++++++++ .../linkSearchedArtistToAccount.test.ts | 37 +++++++++++ .../findCanonicalArtistBySpotifyId.ts | 38 +++++++++++ lib/valuation/linkSearchedArtistToAccount.ts | 18 +++++ 5 files changed, 189 insertions(+) create mode 100644 lib/supabase/socials/selectSocialsBySpotifyArtistId.ts create mode 100644 lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts create mode 100644 lib/valuation/findCanonicalArtistBySpotifyId.ts diff --git a/lib/supabase/socials/selectSocialsBySpotifyArtistId.ts b/lib/supabase/socials/selectSocialsBySpotifyArtistId.ts new file mode 100644 index 000000000..a8fd23983 --- /dev/null +++ b/lib/supabase/socials/selectSocialsBySpotifyArtistId.ts @@ -0,0 +1,31 @@ +import { Tables } from "@/types/database.types"; +import supabase from "../serverClient"; + +/** + * Selects socials whose `profile_url` points at a given Spotify artist id. + * + * Matches on the id rather than a full-URL equality check because + * `profile_url` is stored inconsistently — with and without a scheme + * (`open.spotify.com/artist/{id}` vs `https://open.spotify.com/artist/{id}`), + * and sometimes with a `?si=` query — so `selectSocials({ profile_url })` + * (an exact `eq`) misses real matches. + * + * @param spotifyArtistId - The Spotify artist id to look for. + * @returns The matching socials, newest first. + * @throws Error if the query fails. + */ +export async function selectSocialsBySpotifyArtistId( + spotifyArtistId: string, +): Promise[]> { + const { data, error } = await supabase + .from("socials") + .select("*") + .ilike("profile_url", `%${spotifyArtistId}%`) + .order("updated_at", { ascending: false }); + + if (error) { + throw new Error(`Failed to fetch socials by spotify artist id: ${error.message}`); + } + + return data || []; +} diff --git a/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts b/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts new file mode 100644 index 000000000..995690d80 --- /dev/null +++ b/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; +import { selectSocialsBySpotifyArtistId } from "@/lib/supabase/socials/selectSocialsBySpotifyArtistId"; +import { selectAccountSocials } from "@/lib/supabase/account_socials/selectAccountSocials"; + +vi.mock("@/lib/supabase/socials/selectSocialsBySpotifyArtistId", () => ({ + selectSocialsBySpotifyArtistId: vi.fn(), +})); +vi.mock("@/lib/supabase/account_socials/selectAccountSocials", () => ({ + selectAccountSocials: vi.fn(), +})); + +const SPOTIFY_ID = "0xPoVNPnxIIUS1vrxAYV00"; + +describe("findCanonicalArtistBySpotifyId", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + // Artists are canonical and shared (chat#1866); account_artist_ids is the + // join that lets many accounts roster one artist. So the lookup is global, + // NOT scoped to the requesting account — otherwise every account mints its + // own copy of the same Spotify artist (chat#1889 row 8). + it("finds the artist globally, not scoped to any account", async () => { + vi.mocked(selectSocialsBySpotifyArtistId).mockResolvedValue([ + { id: "social-1" }, + ] as never); + vi.mocked(selectAccountSocials).mockResolvedValue([ + { account_id: "canonical-artist-1" }, + ] as never); + + const found = await findCanonicalArtistBySpotifyId(SPOTIFY_ID); + + expect(found).toBe("canonical-artist-1"); + expect(selectSocialsBySpotifyArtistId).toHaveBeenCalledWith(SPOTIFY_ID); + }); + + it("returns null when no social carries that Spotify id", async () => { + vi.mocked(selectSocialsBySpotifyArtistId).mockResolvedValue([] as never); + + const found = await findCanonicalArtistBySpotifyId(SPOTIFY_ID); + + expect(found).toBeNull(); + expect(selectAccountSocials).not.toHaveBeenCalled(); + }); + + it("returns null when the social exists but no artist is linked to it", async () => { + vi.mocked(selectSocialsBySpotifyArtistId).mockResolvedValue([ + { id: "social-1" }, + ] as never); + vi.mocked(selectAccountSocials).mockResolvedValue([] as never); + + expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull(); + }); + + // Never fail a valuation over a dedup lookup: falling back to creating an + // artist is strictly better than a 500. + it("returns null when the lookup throws", async () => { + vi.mocked(selectSocialsBySpotifyArtistId).mockRejectedValue( + new Error("db down"), + ); + + expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull(); + }); +}); diff --git a/lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts b/lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts index 968edc3ac..5e262f5f8 100644 --- a/lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts +++ b/lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts @@ -3,9 +3,21 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { linkSearchedArtistToAccount } from "../linkSearchedArtistToAccount"; import { createArtistInDb } from "@/lib/artists/createArtistInDb"; import { updateArtistSocials } from "@/lib/artist/updateArtistSocials"; +import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; +import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId"; +import { insertAccountArtistId } from "@/lib/supabase/account_artist_ids/insertAccountArtistId"; vi.mock("@/lib/artists/createArtistInDb", () => ({ createArtistInDb: vi.fn() })); vi.mock("@/lib/artist/updateArtistSocials", () => ({ updateArtistSocials: 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(), +})); const accountId = "550e8400-e29b-41d4-a716-446655440000"; const spotifyArtistId = "4q3ewBCX7sLwd24euuV69X"; @@ -17,6 +29,9 @@ describe("linkSearchedArtistToAccount", () => { vi.clearAllMocks(); vi.mocked(createArtistInDb).mockResolvedValue({ account_id: createdArtistId } as never); vi.mocked(updateArtistSocials).mockResolvedValue([] as never); + vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue(null); + vi.mocked(selectAccountArtistId).mockResolvedValue(null as never); + vi.mocked(insertAccountArtistId).mockResolvedValue(undefined as never); }); it("creates the searched artist, attaches its Spotify social, and returns the new id", async () => { @@ -57,4 +72,26 @@ describe("linkSearchedArtistToAccount", () => { linkSearchedArtistToAccount({ accountId, spotifyArtistId, artistName }), ).resolves.toBeNull(); }); + // Artists are canonical and shared (chat#1866). Creating a second one for a + // Spotify id that already exists is what duplicated every cold-start signup's + // roster (chat#1889 row 8). + it("rosters the existing canonical artist instead of creating a duplicate", async () => { + vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue("canonical-1"); + + const result = await linkSearchedArtistToAccount({ accountId, spotifyArtistId, artistName }); + + expect(createArtistInDb).not.toHaveBeenCalled(); + expect(insertAccountArtistId).toHaveBeenCalledWith(accountId, "canonical-1"); + expect(result).toBe("canonical-1"); + }); + + it("does not re-link a canonical artist the account already rosters", async () => { + vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue("canonical-1"); + vi.mocked(selectAccountArtistId).mockResolvedValue({ id: "link-1" } as never); + + const result = await linkSearchedArtistToAccount({ accountId, spotifyArtistId, artistName }); + + expect(insertAccountArtistId).not.toHaveBeenCalled(); + expect(result).toBe("canonical-1"); + }); }); diff --git a/lib/valuation/findCanonicalArtistBySpotifyId.ts b/lib/valuation/findCanonicalArtistBySpotifyId.ts new file mode 100644 index 000000000..7f6346537 --- /dev/null +++ b/lib/valuation/findCanonicalArtistBySpotifyId.ts @@ -0,0 +1,38 @@ +import { selectSocialsBySpotifyArtistId } from "@/lib/supabase/socials/selectSocialsBySpotifyArtistId"; +import { selectAccountSocials } from "@/lib/supabase/account_socials/selectAccountSocials"; + +/** + * The canonical artist account for a Spotify artist id, if one already exists. + * + * Artists are canonical and shared — `account_artist_ids` is the join that lets + * many accounts roster the same artist (chat#1866). So this lookup is + * deliberately **global**, not scoped to the requesting account: scoping it + * per-account is what lets every signup mint its own copy of the same Spotify + * artist, which is the duplicate `linkSearchedArtistToAccount` was producing + * (chat#1889 row 8). + * + * Best-effort: never throws. Failing this lookup falls back to creating an + * artist, which is strictly better than failing the valuation. + * + * @param spotifyArtistId - The Spotify artist id to resolve. + * @returns The artist's account id, or null when none exists yet. + */ +export async function findCanonicalArtistBySpotifyId( + spotifyArtistId: string, +): Promise { + try { + const socials = await selectSocialsBySpotifyArtistId(spotifyArtistId); + if (socials.length === 0) return null; + + for (const social of socials) { + const links = await selectAccountSocials({ socialId: social.id }); + const linked = links.find(link => link.account_id); + if (linked?.account_id) return linked.account_id; + } + + return null; + } catch (error) { + console.error("Error resolving canonical artist by spotify id:", error); + return null; + } +} diff --git a/lib/valuation/linkSearchedArtistToAccount.ts b/lib/valuation/linkSearchedArtistToAccount.ts index 5009d7707..236c7cbdb 100644 --- a/lib/valuation/linkSearchedArtistToAccount.ts +++ b/lib/valuation/linkSearchedArtistToAccount.ts @@ -1,5 +1,8 @@ import { createArtistInDb } from "@/lib/artists/createArtistInDb"; import { updateArtistSocials } from "@/lib/artist/updateArtistSocials"; +import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; +import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId"; +import { insertAccountArtistId } from "@/lib/supabase/account_artist_ids/insertAccountArtistId"; /** * Fallback roster attach for the valuation flow: when the ISRC → song_artists @@ -9,6 +12,12 @@ import { updateArtistSocials } from "@/lib/artist/updateArtistSocials"; * roster it can confirm. Server-side, account-scoped version of the marketing * funnel's old client-side linkArtistToAccount. * + * Reuses the canonical artist when one already carries this Spotify id, and + * only creates when none does. Artists are canonical and shared — + * `account_artist_ids` is the join that lets many accounts roster the same + * artist (chat#1866) — so creating unconditionally minted a duplicate for + * every account whose first artist was added this way (chat#1889 row 8). + * * Link-only: the caller resolves the Spotify profile once and enriches the * returned artist separately (runValuationHandler → enrichSearchedArtistProfile), * so enrichment covers both the canonical and fallback paths (chat#1881 2a). @@ -29,6 +38,15 @@ export async function linkSearchedArtistToAccount(params: { try { if (!artistName) return null; + const canonicalId = await findCanonicalArtistBySpotifyId(spotifyArtistId); + if (canonicalId) { + const alreadyRostered = await selectAccountArtistId(accountId, canonicalId); + if (!alreadyRostered) { + await insertAccountArtistId(accountId, canonicalId); + } + return canonicalId; + } + const created = await createArtistInDb(artistName, accountId); if (!created?.account_id) return null; From 41279e32e9d32513d3dc1f2da4233056278bd38f Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 28 Jul 2026 17:43:30 -0500 Subject: [PATCH 2/3] style: prettier formatting on findCanonicalArtistBySpotifyId test --- .../__tests__/findCanonicalArtistBySpotifyId.test.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts b/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts index 995690d80..98be0f65f 100644 --- a/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts +++ b/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts @@ -22,9 +22,7 @@ describe("findCanonicalArtistBySpotifyId", () => { // NOT scoped to the requesting account — otherwise every account mints its // own copy of the same Spotify artist (chat#1889 row 8). it("finds the artist globally, not scoped to any account", async () => { - vi.mocked(selectSocialsBySpotifyArtistId).mockResolvedValue([ - { id: "social-1" }, - ] as never); + vi.mocked(selectSocialsBySpotifyArtistId).mockResolvedValue([{ id: "social-1" }] as never); vi.mocked(selectAccountSocials).mockResolvedValue([ { account_id: "canonical-artist-1" }, ] as never); @@ -45,9 +43,7 @@ describe("findCanonicalArtistBySpotifyId", () => { }); it("returns null when the social exists but no artist is linked to it", async () => { - vi.mocked(selectSocialsBySpotifyArtistId).mockResolvedValue([ - { id: "social-1" }, - ] as never); + vi.mocked(selectSocialsBySpotifyArtistId).mockResolvedValue([{ id: "social-1" }] as never); vi.mocked(selectAccountSocials).mockResolvedValue([] as never); expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull(); @@ -56,9 +52,7 @@ describe("findCanonicalArtistBySpotifyId", () => { // Never fail a valuation over a dedup lookup: falling back to creating an // artist is strictly better than a 500. it("returns null when the lookup throws", async () => { - vi.mocked(selectSocialsBySpotifyArtistId).mockRejectedValue( - new Error("db down"), - ); + vi.mocked(selectSocialsBySpotifyArtistId).mockRejectedValue(new Error("db down")); expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull(); }); From d899d0eaf61e2d1bceef6244ae8677a93d911a14 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 28 Jul 2026 21:14:49 -0500 Subject: [PATCH 3/3] refactor(supabase): fold the Spotify-id social lookup into selectSocials (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) --- lib/supabase/socials/selectSocials.ts | 11 +++++++ .../socials/selectSocialsBySpotifyArtistId.ts | 31 ------------------- .../findCanonicalArtistBySpotifyId.test.ts | 16 +++++----- .../findCanonicalArtistBySpotifyId.ts | 4 +-- 4 files changed, 21 insertions(+), 41 deletions(-) delete mode 100644 lib/supabase/socials/selectSocialsBySpotifyArtistId.ts diff --git a/lib/supabase/socials/selectSocials.ts b/lib/supabase/socials/selectSocials.ts index ee04075b0..9cf37bf9a 100644 --- a/lib/supabase/socials/selectSocials.ts +++ b/lib/supabase/socials/selectSocials.ts @@ -4,6 +4,13 @@ import supabase from "../serverClient"; type SelectSocialsParams = { id?: string; profile_url?: string; + /** + * Substring match (ilike) on profile_url. Use for Spotify-id lookups: + * profile_url is stored inconsistently — with and without a scheme, + * sometimes with a ?si= query — so the exact-match `profile_url` param + * misses real matches (chat#1889 row 8). + */ + profileUrlContains?: string; }; /** @@ -26,6 +33,10 @@ export async function selectSocials( query = query.eq("profile_url", params.profile_url); } + if (params.profileUrlContains) { + query = query.ilike("profile_url", `%${params.profileUrlContains}%`); + } + const { data, error } = await query; if (error) { diff --git a/lib/supabase/socials/selectSocialsBySpotifyArtistId.ts b/lib/supabase/socials/selectSocialsBySpotifyArtistId.ts deleted file mode 100644 index a8fd23983..000000000 --- a/lib/supabase/socials/selectSocialsBySpotifyArtistId.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { Tables } from "@/types/database.types"; -import supabase from "../serverClient"; - -/** - * Selects socials whose `profile_url` points at a given Spotify artist id. - * - * Matches on the id rather than a full-URL equality check because - * `profile_url` is stored inconsistently — with and without a scheme - * (`open.spotify.com/artist/{id}` vs `https://open.spotify.com/artist/{id}`), - * and sometimes with a `?si=` query — so `selectSocials({ profile_url })` - * (an exact `eq`) misses real matches. - * - * @param spotifyArtistId - The Spotify artist id to look for. - * @returns The matching socials, newest first. - * @throws Error if the query fails. - */ -export async function selectSocialsBySpotifyArtistId( - spotifyArtistId: string, -): Promise[]> { - const { data, error } = await supabase - .from("socials") - .select("*") - .ilike("profile_url", `%${spotifyArtistId}%`) - .order("updated_at", { ascending: false }); - - if (error) { - throw new Error(`Failed to fetch socials by spotify artist id: ${error.message}`); - } - - return data || []; -} diff --git a/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts b/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts index 98be0f65f..e991e1f6c 100644 --- a/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts +++ b/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts @@ -1,10 +1,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; -import { selectSocialsBySpotifyArtistId } from "@/lib/supabase/socials/selectSocialsBySpotifyArtistId"; +import { selectSocials } from "@/lib/supabase/socials/selectSocials"; import { selectAccountSocials } from "@/lib/supabase/account_socials/selectAccountSocials"; -vi.mock("@/lib/supabase/socials/selectSocialsBySpotifyArtistId", () => ({ - selectSocialsBySpotifyArtistId: vi.fn(), +vi.mock("@/lib/supabase/socials/selectSocials", () => ({ + selectSocials: vi.fn(), })); vi.mock("@/lib/supabase/account_socials/selectAccountSocials", () => ({ selectAccountSocials: vi.fn(), @@ -22,7 +22,7 @@ describe("findCanonicalArtistBySpotifyId", () => { // NOT scoped to the requesting account — otherwise every account mints its // own copy of the same Spotify artist (chat#1889 row 8). it("finds the artist globally, not scoped to any account", async () => { - vi.mocked(selectSocialsBySpotifyArtistId).mockResolvedValue([{ id: "social-1" }] as never); + vi.mocked(selectSocials).mockResolvedValue([{ id: "social-1" }] as never); vi.mocked(selectAccountSocials).mockResolvedValue([ { account_id: "canonical-artist-1" }, ] as never); @@ -30,11 +30,11 @@ describe("findCanonicalArtistBySpotifyId", () => { const found = await findCanonicalArtistBySpotifyId(SPOTIFY_ID); expect(found).toBe("canonical-artist-1"); - expect(selectSocialsBySpotifyArtistId).toHaveBeenCalledWith(SPOTIFY_ID); + expect(selectSocials).toHaveBeenCalledWith({ profileUrlContains: SPOTIFY_ID }); }); it("returns null when no social carries that Spotify id", async () => { - vi.mocked(selectSocialsBySpotifyArtistId).mockResolvedValue([] as never); + vi.mocked(selectSocials).mockResolvedValue([] as never); const found = await findCanonicalArtistBySpotifyId(SPOTIFY_ID); @@ -43,7 +43,7 @@ describe("findCanonicalArtistBySpotifyId", () => { }); it("returns null when the social exists but no artist is linked to it", async () => { - vi.mocked(selectSocialsBySpotifyArtistId).mockResolvedValue([{ id: "social-1" }] as never); + vi.mocked(selectSocials).mockResolvedValue([{ id: "social-1" }] as never); vi.mocked(selectAccountSocials).mockResolvedValue([] as never); expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull(); @@ -52,7 +52,7 @@ describe("findCanonicalArtistBySpotifyId", () => { // Never fail a valuation over a dedup lookup: falling back to creating an // artist is strictly better than a 500. it("returns null when the lookup throws", async () => { - vi.mocked(selectSocialsBySpotifyArtistId).mockRejectedValue(new Error("db down")); + vi.mocked(selectSocials).mockRejectedValue(new Error("db down")); expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull(); }); diff --git a/lib/valuation/findCanonicalArtistBySpotifyId.ts b/lib/valuation/findCanonicalArtistBySpotifyId.ts index 7f6346537..6f5c31267 100644 --- a/lib/valuation/findCanonicalArtistBySpotifyId.ts +++ b/lib/valuation/findCanonicalArtistBySpotifyId.ts @@ -1,4 +1,4 @@ -import { selectSocialsBySpotifyArtistId } from "@/lib/supabase/socials/selectSocialsBySpotifyArtistId"; +import { selectSocials } from "@/lib/supabase/socials/selectSocials"; import { selectAccountSocials } from "@/lib/supabase/account_socials/selectAccountSocials"; /** @@ -21,7 +21,7 @@ export async function findCanonicalArtistBySpotifyId( spotifyArtistId: string, ): Promise { try { - const socials = await selectSocialsBySpotifyArtistId(spotifyArtistId); + const socials = (await selectSocials({ profileUrlContains: spotifyArtistId })) ?? []; if (socials.length === 0) return null; for (const social of socials) {