From 18dbf01c322ca750d19bb1b5fe3605b2100861e5 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 28 Jul 2026 22:07:35 -0500 Subject: [PATCH] fix(valuation): prefer an artist the account already rosters for a Spotify id api#791 stopped the valuation minting a new artist row, but the chat add flow creates its own row (+ Spotify social) BEFORE the fire-and-forget valuation runs. When the global lookup then resolved a DIFFERENT canonical, insertAccountArtistId linked that one too and the account showed two roster entries again -- reproduced live on chat#1900's preview after api#791 merged, caught by Sweets in the artist sidebar. findCanonicalArtistBySpotifyId now collects every artist linked to the Spotify id and prefers one the requesting account already rosters; the global resolution is unchanged when the account rosters none of them. Co-Authored-By: Claude Opus 5 (1M context) --- .../findCanonicalArtistBySpotifyId.test.ts | 38 +++++++++++++++++++ .../findCanonicalArtistBySpotifyId.ts | 33 ++++++++++++---- lib/valuation/linkSearchedArtistToAccount.ts | 2 +- 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts b/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts index e991e1f6..7ce890a5 100644 --- a/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts +++ b/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; import { selectSocials } from "@/lib/supabase/socials/selectSocials"; import { selectAccountSocials } from "@/lib/supabase/account_socials/selectAccountSocials"; +import { selectAccountArtistIds } from "@/lib/supabase/account_artist_ids/selectAccountArtistIds"; vi.mock("@/lib/supabase/socials/selectSocials", () => ({ selectSocials: vi.fn(), @@ -9,12 +10,16 @@ vi.mock("@/lib/supabase/socials/selectSocials", () => ({ vi.mock("@/lib/supabase/account_socials/selectAccountSocials", () => ({ selectAccountSocials: vi.fn(), })); +vi.mock("@/lib/supabase/account_artist_ids/selectAccountArtistIds", () => ({ + selectAccountArtistIds: vi.fn(), +})); const SPOTIFY_ID = "0xPoVNPnxIIUS1vrxAYV00"; describe("findCanonicalArtistBySpotifyId", () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(selectAccountArtistIds).mockResolvedValue([] as never); }); // Artists are canonical and shared (chat#1866); account_artist_ids is the @@ -56,4 +61,37 @@ describe("findCanonicalArtistBySpotifyId", () => { expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull(); }); + // The chat add flow creates its own artist row (+ Spotify social) BEFORE the + // fire-and-forget valuation runs. If the global lookup then resolves a + // DIFFERENT canonical, insertAccountArtistId links that one too and the + // account shows two roster entries again — reproduced live on chat#1900 + // after api#791 merged. An artist the account already rosters must win. + it("prefers an artist the requesting account already rosters", async () => { + vi.mocked(selectSocials).mockResolvedValue([ + { id: "social-new" }, + { id: "social-old" }, + ] as never); + vi.mocked(selectAccountSocials).mockImplementation((async (args: { socialId: string }) => { + if (args.socialId === "social-new") return [{ account_id: "client-created-1" }]; + return [{ account_id: "old-canonical-1" }]; + }) as never); + vi.mocked(selectAccountArtistIds).mockResolvedValue([ + { artist_id: "client-created-1" }, + ] as never); + + const found = await findCanonicalArtistBySpotifyId(SPOTIFY_ID, "acct-1"); + + expect(found).toBe("client-created-1"); + expect(selectAccountArtistIds).toHaveBeenCalledWith(["acct-1"]); + }); + + it("still resolves the global canonical when the account rosters nothing for the id", async () => { + vi.mocked(selectSocials).mockResolvedValue([{ id: "social-old" }] as never); + vi.mocked(selectAccountSocials).mockResolvedValue([{ account_id: "old-canonical-1" }] as never); + vi.mocked(selectAccountArtistIds).mockResolvedValue([ + { artist_id: "unrelated-artist" }, + ] as never); + + expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID, "acct-1")).toBe("old-canonical-1"); + }); }); diff --git a/lib/valuation/findCanonicalArtistBySpotifyId.ts b/lib/valuation/findCanonicalArtistBySpotifyId.ts index 6f5c3126..c3b58ad6 100644 --- a/lib/valuation/findCanonicalArtistBySpotifyId.ts +++ b/lib/valuation/findCanonicalArtistBySpotifyId.ts @@ -1,36 +1,53 @@ import { selectSocials } from "@/lib/supabase/socials/selectSocials"; import { selectAccountSocials } from "@/lib/supabase/account_socials/selectAccountSocials"; +import { selectAccountArtistIds } from "@/lib/supabase/account_artist_ids/selectAccountArtistIds"; /** * 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). + * many accounts roster the same artist (chat#1866). The lookup is global, but + * **an artist the requesting account already rosters wins**: the chat add flow + * creates its own artist row (+ Spotify social) before the fire-and-forget + * valuation runs, so resolving a different canonical here would link a second + * roster entry — the exact double this exists to prevent (chat#1889 row 8, + * reproduced live on chat#1900 after api#791). * * 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. + * @param accountId - The requesting account; used only to prefer an + * already-rostered artist, never to scope the search. * @returns The artist's account id, or null when none exists yet. */ export async function findCanonicalArtistBySpotifyId( spotifyArtistId: string, + accountId?: string, ): Promise { try { const socials = (await selectSocials({ profileUrlContains: spotifyArtistId })) ?? []; if (socials.length === 0) return null; + const linkedArtistIds: string[] = []; 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; + for (const link of links) { + if (link.account_id && !linkedArtistIds.includes(link.account_id)) { + linkedArtistIds.push(link.account_id); + } + } } + if (linkedArtistIds.length === 0) return null; - return null; + if (accountId) { + const rostered = await selectAccountArtistIds([accountId]); + const rosteredIds = new Set(rostered.map(row => row.artist_id)); + const alreadyRostered = linkedArtistIds.find(id => rosteredIds.has(id)); + if (alreadyRostered) return alreadyRostered; + } + + return linkedArtistIds[0]; } 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 236c7cbd..0c243588 100644 --- a/lib/valuation/linkSearchedArtistToAccount.ts +++ b/lib/valuation/linkSearchedArtistToAccount.ts @@ -38,7 +38,7 @@ export async function linkSearchedArtistToAccount(params: { try { if (!artistName) return null; - const canonicalId = await findCanonicalArtistBySpotifyId(spotifyArtistId); + const canonicalId = await findCanonicalArtistBySpotifyId(spotifyArtistId, accountId); if (canonicalId) { const alreadyRostered = await selectAccountArtistId(accountId, canonicalId); if (!alreadyRostered) {