Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions hooks/useAddSpotifyArtist.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,31 @@

import { useState } from "react";
import { usePrivy } from "@privy-io/react-auth";
import { useQueryClient } from "@tanstack/react-query";
import { toast } from "sonner";
import { addSpotifyArtist } from "@/lib/artists/addSpotifyArtist";
import { runValuation } from "@/lib/valuation/runValuation";
import { useArtistProvider } from "@/providers/ArtistProvider";
import { useOrganization } from "@/providers/OrganizationProvider";
import useCatalogs from "@/hooks/useCatalogs";
import type { SpotifyArtistSearchResult } from "@/types/spotify";

/**
* Adds a Spotify-searched artist to the roster and refreshes + selects it.
* Wraps `addSpotifyArtist` with auth + the shared roster (ArtistProvider),
* mirroring `useAddRosterArtist`.
*
* On the **first** artist add (the account has no catalog yet) it also
* fire-and-forget kicks `POST /api/valuation` for that artist's Spotify id —
* seeding the onboarding catalog in the background so the "Claim your catalog"
* step is already complete by the time the user reaches it (chat#1867).
*/
export function useAddSpotifyArtist() {
const { getAccessToken } = usePrivy();
const { getArtists } = useArtistProvider();
const { selectedOrgId } = useOrganization();
const catalogsQuery = useCatalogs();
const queryClient = useQueryClient();
const [isAdding, setIsAdding] = useState(false);

const add = async (artist: SpotifyArtistSearchResult): Promise<boolean> => {
Expand All @@ -33,6 +43,25 @@ export function useAddSpotifyArtist() {
);
await getArtists(created.account_id);
toast.success(`${artist.name} added to your roster`);

// Seed the onboarding catalog from the first artist's discography.
// Only when we know there is no catalog yet (empty, not just unloaded),
// and fire-and-forget so the dialog closes immediately — the catalog
// materializes in ~20s and the sequence advances on the next landing.
const catalogs = catalogsQuery.data?.catalogs;

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: Two rapid sequential artist adds can trigger concurrent valuations before the first one completes. Since isAdding resets to false right after the artist-add API returns (valuation runs fire-and-forget in the background), the UI allows adding another artist while the first valuation is still in flight. At that point the catalogs query hasn't been invalidated yet, so it still reports an empty catalog — causing a second POST /api/valuation to fire. Consider tracking a separate isValuating ref or skipping the valuation if one is already pending to avoid duplicate catalog creation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useAddSpotifyArtist.ts, line 51:

<comment>Two rapid sequential artist adds can trigger concurrent valuations before the first one completes. Since `isAdding` resets to false right after the artist-add API returns (valuation runs fire-and-forget in the background), the UI allows adding another artist while the first valuation is still in flight. At that point the catalogs query hasn't been invalidated yet, so it still reports an empty catalog — causing a second `POST /api/valuation` to fire. Consider tracking a separate `isValuating` ref or skipping the valuation if one is already pending to avoid duplicate catalog creation.</comment>

<file context>
@@ -33,6 +43,25 @@ export function useAddSpotifyArtist() {
+      // Only when we know there is no catalog yet (empty, not just unloaded),
+      // and fire-and-forget so the dialog closes immediately — the catalog
+      // materializes in ~20s and the sequence advances on the next landing.
+      const catalogs = catalogsQuery.data?.catalogs;
+      if (catalogs && catalogs.length === 0) {
+        toast.promise(
</file context>

if (catalogs && catalogs.length === 0) {

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: Fresh onboarding can still miss catalog seeding when the user selects an artist before useCatalogs finishes loading. The undefined loading state fails this guard and nothing reruns after the catalog query becomes empty; defer/retry the seed once the query resolves, or prevent selection until this check is known.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useAddSpotifyArtist.ts, line 52:

<comment>Fresh onboarding can still miss catalog seeding when the user selects an artist before `useCatalogs` finishes loading. The `undefined` loading state fails this guard and nothing reruns after the catalog query becomes empty; defer/retry the seed once the query resolves, or prevent selection until this check is known.</comment>

<file context>
@@ -33,6 +43,25 @@ export function useAddSpotifyArtist() {
+      // and fire-and-forget so the dialog closes immediately — the catalog
+      // materializes in ~20s and the sequence advances on the next landing.
+      const catalogs = catalogsQuery.data?.catalogs;
+      if (catalogs && catalogs.length === 0) {
+        toast.promise(
+          runValuation(accessToken, artist.id).then(() => {
</file context>

toast.promise(
runValuation(accessToken, artist.id).then(() => {
queryClient.invalidateQueries({ queryKey: ["catalogs"] });
Comment on lines +53 to +55

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: The "Your catalog is ready" success toast fires before the catalog data refresh finishes. The .then() callback calls queryClient.invalidateQueries() but doesn't return its promise, so the outer promise resolves as soon as the invalidation is scheduled — not when it actually completes. This means there's a brief window where the toast claims the catalog is ready but the React Query cache hasn't been refreshed yet. If the user navigates to the catalog page right after the toast, they could see stale/empty data momentarily. Return the invalidation promise from the .then() callback so the toast waits for the full invalidation roundtrip.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useAddSpotifyArtist.ts, line 53:

<comment>The "Your catalog is ready" success toast fires before the catalog data refresh finishes. The `.then()` callback calls `queryClient.invalidateQueries()` but doesn't return its promise, so the outer promise resolves as soon as the invalidation is *scheduled* — not when it actually completes. This means there's a brief window where the toast claims the catalog is ready but the React Query cache hasn't been refreshed yet. If the user navigates to the catalog page right after the toast, they could see stale/empty data momentarily. Return the invalidation promise from the `.then()` callback so the toast waits for the full invalidation roundtrip.</comment>

<file context>
@@ -33,6 +43,25 @@ export function useAddSpotifyArtist() {
+      // materializes in ~20s and the sequence advances on the next landing.
+      const catalogs = catalogsQuery.data?.catalogs;
+      if (catalogs && catalogs.length === 0) {
+        toast.promise(
+          runValuation(accessToken, artist.id).then(() => {
+            queryClient.invalidateQueries({ queryKey: ["catalogs"] });
</file context>
Suggested change
toast.promise(
runValuation(accessToken, artist.id).then(() => {
queryClient.invalidateQueries({ queryKey: ["catalogs"] });
runValuation(accessToken, artist.id).then(() => {
return queryClient.invalidateQueries({ queryKey: ["catalogs"] });
}),

}),
Comment on lines +53 to +56

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the target file and locate the referenced code.
git ls-files hooks/useAddSpotifyArtist.ts
wc -l hooks/useAddSpotifyArtist.ts
cat -n hooks/useAddSpotifyArtist.ts | sed -n '1,220p'

# Search for related patterns in the repository for context.
rg -n "toast\.promise|invalidateQueries\(\{ queryKey: \[\"catalogs\"\] \}\)" hooks . || true

Repository: recoupable/chat

Length of output: 4193


Return the invalidation promise from then. toast.promise currently resolves as soon as runValuation finishes, not after queryClient.invalidateQueries completes, so the success toast can fire before the catalog refresh and any invalidation error is dropped.

Proposed fix
-          runValuation(accessToken, artist.id).then(() => {
-            queryClient.invalidateQueries({ queryKey: ["catalogs"] });
-          }),
+          runValuation(accessToken, artist.id).then(() =>
+            queryClient.invalidateQueries({ queryKey: ["catalogs"] }),
+          ),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
toast.promise(
runValuation(accessToken, artist.id).then(() => {
queryClient.invalidateQueries({ queryKey: ["catalogs"] });
}),
toast.promise(
runValuation(accessToken, artist.id).then(() =>
queryClient.invalidateQueries({ queryKey: ["catalogs"] }),
),
🤖 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 `@hooks/useAddSpotifyArtist.ts` around lines 53 - 56, Update the promise chain
in the toast.promise call within the artist valuation flow so the then callback
returns queryClient.invalidateQueries({ queryKey: ["catalogs"] }). This keeps
toast.promise pending until catalog invalidation completes and propagates any
invalidation error.

{
loading: `Valuing ${artist.name}'s catalog…`,
success: "Your catalog is ready",
error:
"Couldn't value the catalog automatically — you can claim it later",
},
);
}
return true;
} catch (error) {
toast.error(
Expand Down
8 changes: 4 additions & 4 deletions lib/ai/__tests__/organizeModels.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ describe("organizeModels", () => {
it("splits featured and non-featured gateway models", () => {
const models = [
{
id: "openai/gpt-5.2",
name: "gpt-5.2",
specification: { specificationVersion: "v2", provider: "openai", modelId: "gpt-5.2" },
id: "openai/gpt-5.5",
name: "gpt-5.5",
specification: { specificationVersion: "v2", provider: "openai", modelId: "gpt-5.5" },
},
{
id: "some/other-model",
Expand All @@ -22,7 +22,7 @@ describe("organizeModels", () => {
] as never;

const result = organizeModels(models);
expect(result.featuredModels.map((m) => m.id)).toContain("openai/gpt-5.2");
expect(result.featuredModels.map((m) => m.id)).toContain("openai/gpt-5.5");
expect(result.otherModels.map((m) => m.id)).toEqual(["some/other-model"]);
});
});
41 changes: 41 additions & 0 deletions lib/valuation/__tests__/runValuation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { runValuation } from "@/lib/valuation/runValuation";
import { getClientApiBaseUrl } from "@/lib/api/getClientApiBaseUrl";

vi.mock("@/lib/api/getClientApiBaseUrl", () => ({
getClientApiBaseUrl: vi.fn(),
}));

describe("runValuation", () => {
beforeEach(() => {
vi.clearAllMocks();
vi.mocked(getClientApiBaseUrl).mockReturnValue("https://api.example.com");
});

it("POSTs the spotify_artist_id to /api/valuation with bearer auth", async () => {
global.fetch = vi
.fn()
.mockResolvedValue({ ok: true }) as unknown as typeof fetch;

await runValuation("tok", "0xPoV");

expect(global.fetch).toHaveBeenCalledWith(
"https://api.example.com/api/valuation",
expect.objectContaining({
method: "POST",
headers: expect.objectContaining({ Authorization: "Bearer tok" }),
body: JSON.stringify({ spotify_artist_id: "0xPoV" }),
}),
);
});

it("throws on a non-ok response", async () => {
global.fetch = vi.fn().mockResolvedValue({
ok: false,
status: 402,
text: vi.fn().mockResolvedValue("insufficient credits"),
}) as unknown as typeof fetch;

await expect(runValuation("tok", "0xPoV")).rejects.toThrow("402");
});
});
29 changes: 29 additions & 0 deletions lib/valuation/runValuation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { getClientApiBaseUrl } from "@/lib/api/getClientApiBaseUrl";

/**
* Kicks `POST /api/valuation` for a Spotify artist (api#776): resolves the
* artist's releases, measures play counts, and materializes an account-owned
* catalog + value band. Used fire-and-forget on the first artist add to seed
* the onboarding catalog so "Claim your catalog" is already complete.
*
* @param accessToken - Privy bearer for the owning account.
* @param spotifyArtistId - The Spotify artist id to value.
*/
export async function runValuation(
accessToken: string,
spotifyArtistId: string,
): Promise<void> {
const res = await fetch(`${getClientApiBaseUrl()}/api/valuation`, {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bearer ${accessToken}`,
},
body: JSON.stringify({ spotify_artist_id: spotifyArtistId }),
});
Comment on lines +16 to +23

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== file outline ==\n'
ast-grep outline lib/valuation/runValuation.ts --view expanded || true

printf '\n== file contents ==\n'
cat -n lib/valuation/runValuation.ts

printf '\n== references ==\n'
rg -n "runValuation\\(|valuation.*toast|toast.*valuation|AbortController|signal:" lib -S

Repository: recoupable/chat

Length of output: 1797


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "runValuation\\(" .
rg -n "AbortController|signal:|timeout|setTimeout\\(" lib src app .

Repository: recoupable/chat

Length of output: 4364


🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline hooks/useAddSpotifyArtist.ts --view expanded || true
cat -n hooks/useAddSpotifyArtist.ts | sed -n '1,140p'

Repository: recoupable/chat

Length of output: 3677


Bound the valuation request. lib/valuation/runValuation.ts:16-23 can hang indefinitely here, which leaves the toast.promise loading state stuck with no recovery path. Add an AbortSignal timeout or accept a caller-provided signal and fail fast on abort.

🤖 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/runValuation.ts` around lines 16 - 23, Update the valuation
request in runValuation to use an AbortSignal timeout or caller-provided signal,
ensuring the fetch aborts and rejects after a bounded duration. Preserve the
existing request payload and headers while allowing the surrounding
toast.promise flow to recover from the failure.


if (!res.ok) {
const detail = await res.text().catch(() => "");
throw new Error(`Valuation failed: HTTP ${res.status} ${detail}`.trim());
}
}
Loading