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

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) {

lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts

@@ -0,0 +1,59 @@
+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";
+
+vi.mock("@/lib/supabase/socials/selectSocials", () => ({
+  selectSocials: 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(selectSocials).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(selectSocials).toHaveBeenCalledWith({ profileUrlContains: SPOTIFY_ID });
+  });
+
+  it("returns null when no social carries that Spotify id", async () => {
+    vi.mocked(selectSocials).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(selectSocials).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(selectSocials).mockRejectedValue(new Error("db down"));
+
+    expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull();
+  });
+});

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");
+  });
 });

lib/valuation/findCanonicalArtistBySpotifyId.ts

@@ -0,0 +1,38 @@
+import { selectSocials } from "@/lib/supabase/socials/selectSocials";
+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<string | null> {
+  try {
+    const socials = (await selectSocials({ profileUrlContains: 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;
+  }
+}

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;