feat(onboarding): seed the catalog when the first artist is added in setup (chat#1889 row 8) - #1900
feat(onboarding): seed the catalog when the first artist is added in setup (chat#1889 row 8)#1900sweetmantech wants to merge 3 commits into
Conversation
…setup /setup/artists added artists through useAddRosterArtist, which creates and enriches but never values. useAddSpotifyArtist -- already used by the /artists dialog -- also kicks POST /api/valuation when the account has no catalog yet, which is what makes the payoff reachable: without it an account onboarded through setup finished with no catalog, so chat#1895's reward could only ever render its 'Claim your catalog' fallback, and that CTA lands on 'No catalogs found' with nothing to click. Points AddArtistForm at that hook and deletes useAddRosterArtist, whose enrichment duplicated addSpotifyArtist (identical createRosterArtist then saveArtist with image + profileUrls, and it predates chat#1892). Carries chat#1892's non-fatal-enrichment guard into addSpotifyArtist first: it awaited saveArtist bare, so a failed PATCH reported failure over an artist POST /api/artists had already created and a retry duplicated it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughOnboarding artist selection now uses the Spotify artist hook and payload. Spotify enrichment failures no longer fail artist creation. Setup valuation distinguishes catalog loading from the intermediate catalog-measuring state. ChangesSpotify onboarding and valuation readiness
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AddArtistForm
participant useAddSpotifyArtist
participant addSpotifyArtist
AddArtistForm->>useAddSpotifyArtist: add(selected Spotify artist)
useAddSpotifyArtist->>addSpotifyArtist: create and enrich artist
addSpotifyArtist-->>useAddSpotifyArtist: return created or enriched artist
useAddSpotifyArtist-->>AddArtistForm: update addition state
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7d544d66f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| */ | ||
| const AddArtistForm = () => { | ||
| const { addArtist, isAdding } = useAddRosterArtist(); | ||
| const { add, isAdding } = useAddSpotifyArtist(); |
There was a problem hiding this comment.
When /setup/artists is opened directly and the user selects an artist before useCatalogs() has resolved, this swapped-in hook reports success but never starts runValuation: useAddSpotifyArtist only seeds when catalogsQuery.data?.catalogs is already a defined empty array, while ConfirmRosterStep renders this form without waiting for that query. That leaves cold-start setup users in the same no-catalog fallback this change is meant to eliminate, so the add flow should wait/refetch until catalog state is known before deciding whether to seed.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 7 files
Confidence score: 3/5
- In
components/Onboarding/AddArtistForm.tsx, moving touseAddSpotifyArtistappears to make catalog seeding contingent oncatalogsQuery.data?.catalogsalready being defined, so ifConfirmRosterSteprenders before that query resolves the seed path may be skipped and onboarding can fail or behave inconsistently for users—guard this by handling the loading/undefined state explicitly or delaying the action until catalogs are resolved.
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="components/Onboarding/AddArtistForm.tsx">
<violation number="1" location="components/Onboarding/AddArtistForm.tsx:26">
P2: Switching AddArtistForm to useAddSpotifyArtist means catalog seeding now depends on catalogsQuery having already resolved (`catalogsQuery.data?.catalogs` being a defined empty array). If ConfirmRosterStep renders this form before that query settles (e.g. when /setup/artists is opened directly), the add will report success but skip kicking off `runValuation`, leaving the account without a catalog — the exact dead-end this PR is trying to fix. Consider waiting for/refetching catalog state before deciding whether to seed.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| */ | ||
| const AddArtistForm = () => { | ||
| const { addArtist, isAdding } = useAddRosterArtist(); | ||
| const { add, isAdding } = useAddSpotifyArtist(); |
There was a problem hiding this comment.
P2: Switching AddArtistForm to useAddSpotifyArtist means catalog seeding now depends on catalogsQuery having already resolved (catalogsQuery.data?.catalogs being a defined empty array). If ConfirmRosterStep renders this form before that query settles (e.g. when /setup/artists is opened directly), the add will report success but skip kicking off runValuation, leaving the account without a catalog — the exact dead-end this PR is trying to fix. Consider waiting for/refetching catalog state before deciding whether to seed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/AddArtistForm.tsx, line 26:
<comment>Switching AddArtistForm to useAddSpotifyArtist means catalog seeding now depends on catalogsQuery having already resolved (`catalogsQuery.data?.catalogs` being a defined empty array). If ConfirmRosterStep renders this form before that query settles (e.g. when /setup/artists is opened directly), the add will report success but skip kicking off `runValuation`, leaving the account without a catalog — the exact dead-end this PR is trying to fix. Consider waiting for/refetching catalog state before deciding whether to seed.</comment>
<file context>
@@ -15,16 +15,19 @@ import type { SpotifyArtistSearchResult } from "@/types/spotify";
*/
const AddArtistForm = () => {
- const { addArtist, isAdding } = useAddRosterArtist();
+ const { add, isAdding } = useAddSpotifyArtist();
const [isOpen, setIsOpen] = useState(false);
</file context>
Seeding creates the catalog seconds after the first artist is added, but its measurements land later. In that window /setup/valuation could not redirect (a catalog exists) and could not render the hero (nothing measured), so it sat on a skeleton with no exit -- reproduced live on preview right after the seeded valuation returned. Flagged as a theoretical risk when chat#1895 shipped; row 8 makes it routine, so it is fixed here rather than left. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Preview verification — the seeding works end to end, and it surfaced one thing worth fixing hereVerified on The call sequence now includes the seedingAdded Del Water Gap through Toast confirmed: "Valuing Del Water Gap's catalog…"
The catalog materialises, and the dead end is gone
That is the Done-when for row 8: the CTA that #1895 added now lands somewhere real. 🔴 What this surfaced, and fixed in
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hooks/useAddSpotifyArtist.ts (1)
52-65: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAwait catalog resolution before skipping valuation seeding.
catalogsQuery.datacan be undefined while the["catalogs"]query is loading. That path falls through theif (catalogs && catalogs.length === 0)branch permanently, sorunValuation()never runs for the newly added artist. Await a successful catalog lookup, or queue the valuation oncecatalogsQuery.isSuccess && catalogs.length === 0.🤖 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 52 - 65, Update the catalog check in the artist-add flow around catalogsQuery and runValuation so valuation seeding waits for a completed successful catalog lookup. Only call runValuation when catalogsQuery.isSuccess and catalogs.length is zero; preserve the existing toast.promise and query invalidation behavior.
🤖 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 `@components/Onboarding/SetupValuation.tsx`:
- Around line 48-72: Update the !valuation.show branch in SetupValuation so the
measuring state renders only when hasCatalog is true and isError is false. For
missing-catalog or error states, preserve a neutral loading/redirect fallback
until the existing useEffect navigation completes, avoiding the measuring
message and weekly-report CTA.
---
Outside diff comments:
In `@hooks/useAddSpotifyArtist.ts`:
- Around line 52-65: Update the catalog check in the artist-add flow around
catalogsQuery and runValuation so valuation seeding waits for a completed
successful catalog lookup. Only call runValuation when catalogsQuery.isSuccess
and catalogs.length is zero; preserve the existing toast.promise and query
invalidation behavior.
🪄 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: f16366a6-6fb5-4b35-a2e4-3a2788c0c7b6
⛔ Files ignored due to path filters (4)
components/Onboarding/__tests__/AddArtistForm.test.tsxis excluded by!**/*.test.*and included bycomponents/**components/Onboarding/__tests__/SetupValuation.test.tsxis excluded by!**/*.test.*and included bycomponents/**hooks/onboarding/__tests__/useAddRosterArtist.test.tsxis excluded by!**/*.test.*and included byhooks/**lib/artists/__tests__/addSpotifyArtist.test.tsis excluded by!**/*.test.*and included bylib/**
📒 Files selected for processing (5)
components/Onboarding/AddArtistForm.tsxcomponents/Onboarding/SetupValuation.tsxhooks/onboarding/useAddRosterArtist.tshooks/useAddSpotifyArtist.tslib/artists/addSpotifyArtist.ts
💤 Files with no reviewable changes (1)
- hooks/onboarding/useAddRosterArtist.ts
| // A catalog exists but has no valuation yet. Seeding (chat#1889 row 8) | ||
| // creates the catalog seconds after the first artist is added and its | ||
| // measurements land later, so this window is routine. The redirect above | ||
| // cannot fire (there IS a catalog) and the hero cannot render, so without a | ||
| // terminal state here the route is a skeleton with no exit. | ||
| if (!valuation.show) { | ||
| return ( | ||
| <section className="mx-auto flex w-full max-w-xl flex-col items-center gap-4 px-6 py-8 text-center"> | ||
| <h1 className="text-2xl font-semibold text-foreground"> | ||
| Measuring your catalog | ||
| </h1> | ||
| <p className="text-sm text-muted-foreground"> | ||
| We are pulling play counts for every track. This usually takes a | ||
| minute. Your baseline value appears here as soon as it is ready. | ||
| </p> | ||
| <Link | ||
| href="/setup/tasks" | ||
| className={cn(buttonVariants(), "min-w-[200px]")} | ||
| > | ||
| Set up your weekly report | ||
| </Link> | ||
| </section> | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard the measuring state with the catalog state.
useEffect redirects only after paint. When hasCatalog is false or isError is true, this new branch can briefly show a misleading measuring screen and weekly-report CTA. Render the terminal state only after confirming a catalog exists; keep a neutral redirect/loading fallback otherwise.
🤖 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 `@components/Onboarding/SetupValuation.tsx` around lines 48 - 72, Update the
!valuation.show branch in SetupValuation so the measuring state renders only
when hasCatalog is true and isError is false. For missing-catalog or error
states, preserve a neutral loading/redirect fallback until the existing
useEffect navigation completes, avoiding the measuring message and weekly-report
CTA.
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Confidence score: 3/5
- In
components/Onboarding/SetupValuation.tsx, the new “Measuring your catalog” end state can become stale because the valuation data is not actively refreshed, so users may wait indefinitely even after processing completes — add polling/refetchInterval (or another explicit refresh trigger) to the valuation-related queries. - In
components/Onboarding/SetupValuation.tsx, rendering the!valuation.showbranch beforehasCatalog/isErrorchecks can briefly show the wrong terminal onboarding state before the redirect effect runs, which can confuse users and mask real error/no-catalog paths — gate that branch behind catalog/error guards or reorder the conditional flow.
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="components/Onboarding/SetupValuation.tsx">
<violation number="1" location="components/Onboarding/SetupValuation.tsx:53">
P2: The new `!valuation.show` branch renders the "Measuring your catalog" terminal state without checking `hasCatalog`/`isError` first. Since the redirect to `/catalogs` happens in a `useEffect` (which only fires after paint), a user with no catalog or an error state can briefly flash this measuring screen and weekly-report CTA before being redirected. Consider guarding this branch with `hasCatalog && !isError` (or keeping a neutral loading fallback) so the terminal state only renders once a catalog is confirmed.</violation>
<violation number="2" location="components/Onboarding/SetupValuation.tsx:60">
P2: The new 'Measuring your catalog' state promises the valuation will 'appear here as soon as it is ready,' but there's no polling/refetchInterval on the underlying useCatalogs/useCatalogMeasurements queries, so the user has to manually reload (and even then may hit the 5-minute staleTime cache) to see it update. Consider adding a refetchInterval while valuation.show is false so the page actually updates without a manual refresh.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| Measuring your catalog | ||
| </h1> | ||
| <p className="text-sm text-muted-foreground"> | ||
| We are pulling play counts for every track. This usually takes a |
There was a problem hiding this comment.
P2: The new 'Measuring your catalog' state promises the valuation will 'appear here as soon as it is ready,' but there's no polling/refetchInterval on the underlying useCatalogs/useCatalogMeasurements queries, so the user has to manually reload (and even then may hit the 5-minute staleTime cache) to see it update. Consider adding a refetchInterval while valuation.show is false so the page actually updates without a manual refresh.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/SetupValuation.tsx, line 60:
<comment>The new 'Measuring your catalog' state promises the valuation will 'appear here as soon as it is ready,' but there's no polling/refetchInterval on the underlying useCatalogs/useCatalogMeasurements queries, so the user has to manually reload (and even then may hit the 5-minute staleTime cache) to see it update. Consider adding a refetchInterval while valuation.show is false so the page actually updates without a manual refresh.</comment>
<file context>
@@ -45,6 +45,31 @@ const SetupValuation = () => {
+ Measuring your catalog
+ </h1>
+ <p className="text-sm text-muted-foreground">
+ We are pulling play counts for every track. This usually takes a
+ minute. Your baseline value appears here as soon as it is ready.
+ </p>
</file context>
| // measurements land later, so this window is routine. The redirect above | ||
| // cannot fire (there IS a catalog) and the hero cannot render, so without a | ||
| // terminal state here the route is a skeleton with no exit. | ||
| if (!valuation.show) { |
There was a problem hiding this comment.
P2: The new !valuation.show branch renders the "Measuring your catalog" terminal state without checking hasCatalog/isError first. Since the redirect to /catalogs happens in a useEffect (which only fires after paint), a user with no catalog or an error state can briefly flash this measuring screen and weekly-report CTA before being redirected. Consider guarding this branch with hasCatalog && !isError (or keeping a neutral loading fallback) so the terminal state only renders once a catalog is confirmed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/SetupValuation.tsx, line 53:
<comment>The new `!valuation.show` branch renders the "Measuring your catalog" terminal state without checking `hasCatalog`/`isError` first. Since the redirect to `/catalogs` happens in a `useEffect` (which only fires after paint), a user with no catalog or an error state can briefly flash this measuring screen and weekly-report CTA before being redirected. Consider guarding this branch with `hasCatalog && !isError` (or keeping a neutral loading fallback) so the terminal state only renders once a catalog is confirmed.</comment>
<file context>
@@ -45,6 +45,31 @@ const SetupValuation = () => {
+ // measurements land later, so this window is routine. The redirect above
+ // cannot fire (there IS a catalog) and the hero cannot render, so without a
+ // terminal state here the route is a skeleton with no exit.
+ if (!valuation.show) {
+ return (
+ <section className="mx-auto flex w-full max-w-xl flex-col items-center gap-4 px-6 py-8 text-center">
</file context>
| <section className="mx-auto flex w-full max-w-xl flex-col items-center gap-4 px-6 py-8 text-center"> | ||
| <h1 className="text-2xl font-semibold text-foreground"> | ||
| Measuring your catalog | ||
| </h1> | ||
| <p className="text-sm text-muted-foreground"> | ||
| We are pulling play counts for every track. This usually takes a | ||
| minute. Your baseline value appears here as soon as it is ready. | ||
| </p> | ||
| <Link | ||
| href="/setup/tasks" | ||
| className={cn(buttonVariants(), "min-w-[200px]")} | ||
| > | ||
| Set up your weekly report | ||
| </Link> | ||
| </section> |
There was a problem hiding this comment.
OCP
- actual: net new code defined inline of existing component file
- required: new component file for the new component code definition.
There was a problem hiding this comment.
Done in 2f6aee00 — extracted to components/Onboarding/MeasuringCatalogPanel.tsx (one exported component per file). SetupValuation now composes it (if (!valuation.show) return <MeasuringCatalogPanel />;) instead of carrying the markup inline, so a future state is a new file rather than another branch edited into this one. 89 tests pass; tsc at baseline; eslint clean.
…OCP) Review feedback on chat#1900: the measuring state was net-new component code defined inline in an existing component file. It now lives in components/Onboarding/MeasuringCatalogPanel.tsx, so SetupValuation is extended by composition rather than modified for each new state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🔴 Blocker found on re-test — seeding creates a duplicate roster artist. Moving this to draft.Caught by @sweetmantech spotting the same artist twice in the sidebar of my own screenshot. It is real, and it is caused by this PR. Re-tested from a genuinely empty account ( [
{ "name": "Del Water Gap", "account_id": "6e4d3f5f-87d5-47f3-a9b6-b7058d24bb99",
"socials": ["open.spotify.com/artist/0xPoVNPnxIIUS1vrxAYV00"] },
{ "name": "Del Water Gap", "account_id": "7647f901-a640-434c-b9c9-a18682435092",
"socials": ["open.spotify.com/artist/0xPoVNPnxIIUS1vrxAYV00"] }
]Same name, same avatar, same Spotify id, two roster entries. MechanismThe instrumented call log shows the client issued exactly one So the second artist was created server-side, by the valuation.
The api deliberately moved away from creating an artist per signup because it duplicated. This PR pairs a client-side create with a server-side canonical attach and reintroduces exactly that. ScopePre-existing on the Options
I'd take (1). It is the only one that also fixes the existing dialog path, and it belongs in What still stands from this PREverything else verified on the new account: seeding fires on the first add, the catalog materialises, and the "Measuring your catalog" terminal state renders correctly in the real window with a working exit — the case that was a blank skeleton before. Screenshots to follow. The duplicate is the one blocker. |
There was a problem hiding this comment.
0 issues found across 2 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 3 unresolved issues from previous reviews.
Re-trigger cubic
|
Blocker fix is up: api#791 — Per @sweetmantech's call the lookup is global, not per-account: artists are canonical and shared and Merge order is api#791 → this PR. Once #791 is on |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f6aee0069
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| */ | ||
| const AddArtistForm = () => { | ||
| const { addArtist, isAdding } = useAddRosterArtist(); | ||
| const { add, isAdding } = useAddSpotifyArtist(); |
There was a problem hiding this comment.
When a valuation-funnel account already has a catalog and uses this setup form to add another artist, this swapped-in hook also selects and persists the newly created artist via getArtists(created.account_id). The setup payoff renders through useHomeValuation, which scopes measurements to the selected artist and hides whole-catalog results when the echoed scope differs; because no new valuation is kicked when a catalog already exists, the newly added artist has no measured catalog and the user loses the existing baseline valuation in the done panel or /setup/valuation. Use an onboarding path that refreshes without selecting, or restore the previous selection after the add.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 3/5
- In
components/Onboarding/AddArtistForm.tsx, moving touseAddSpotifyArtistappears to value only the first selected artist while later artists are still selected/persisted viagetArtists, which can leave onboarding with partially initialized artist data and inconsistent downstream behavior — ensure each added artist triggersPOST /api/valuation(or block persistence until valuation completes).
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="components/Onboarding/AddArtistForm.tsx">
<violation number="1" location="components/Onboarding/AddArtistForm.tsx:26">
P2: Switching AddArtistForm to useAddSpotifyArtist means adding a second artist during setup will select and persist that new artist via getArtists, but useAddSpotifyArtist only triggers POST /api/valuation for the first artist when no catalog exists yet. If an account already has a catalog and baseline valuation, adding another artist here selects it without any valuation, and since useHomeValuation scopes results to the selected artist, the previously-computed baseline valuation would disappear from the done panel / /setup/valuation. Consider refreshing without changing the selection, or restoring the prior selection after the add when a catalog already exists.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| */ | ||
| const AddArtistForm = () => { | ||
| const { addArtist, isAdding } = useAddRosterArtist(); | ||
| const { add, isAdding } = useAddSpotifyArtist(); |
There was a problem hiding this comment.
P2: Switching AddArtistForm to useAddSpotifyArtist means adding a second artist during setup will select and persist that new artist via getArtists, but useAddSpotifyArtist only triggers POST /api/valuation for the first artist when no catalog exists yet. If an account already has a catalog and baseline valuation, adding another artist here selects it without any valuation, and since useHomeValuation scopes results to the selected artist, the previously-computed baseline valuation would disappear from the done panel / /setup/valuation. Consider refreshing without changing the selection, or restoring the prior selection after the add when a catalog already exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/AddArtistForm.tsx, line 26:
<comment>Switching AddArtistForm to useAddSpotifyArtist means adding a second artist during setup will select and persist that new artist via getArtists, but useAddSpotifyArtist only triggers POST /api/valuation for the first artist when no catalog exists yet. If an account already has a catalog and baseline valuation, adding another artist here selects it without any valuation, and since useHomeValuation scopes results to the selected artist, the previously-computed baseline valuation would disappear from the done panel / /setup/valuation. Consider refreshing without changing the selection, or restoring the prior selection after the add when a catalog already exists.</comment>
<file context>
@@ -15,16 +15,19 @@ import type { SpotifyArtistSearchResult } from "@/types/spotify";
*/
const AddArtistForm = () => {
- const { addArtist, isAdding } = useAddRosterArtist();
+ const { add, isAdding } = useAddSpotifyArtist();
const [isOpen, setIsOpen] = useState(false);
</file context>
1 participant
Matrix row 8 in chat#1889 — the fix for the dead end #1895 surfaced.
Why
/setup/artistsadded artists throughuseAddRosterArtist, which creates and enriches but never values.useAddSpotifyArtist— already used by the/artistsdialog — also fire-and-forget kicksPOST /api/valuationviarunValuationwhen the account has no catalog yet. Its docstring has said so since chat#1867:So the same artist added at
/artistsgot a catalog and at/setup/artistsdid not. Verified live on a fresh account during #1895's review: two artists added through setup, both carrying real Spotify ids,GET /api/accounts/{id}/catalogsstill[]. With #1895 on prod that means every cold-start account now finishes setup on the "Claim your catalog" fallback, whose CTA lands on/catalogs→ "No catalogs found." with no create affordance.What changed
AddArtistFormadds throughuseAddSpotifyArtist, passing the whole Spotify result.useAddRosterArtistdeleted (and its test). Its enrichment duplicatedaddSpotifyArtist— identicalcreateRosterArtist→saveArtist({ image, profileUrls: { SPOTIFY } })— which predates chat#1892. One caller, so this is a swap that removes code.addSpotifyArtistenrichment is now non-fatal, carrying chat#1892's guard across. It awaitedsaveArtistbare, so a failedPATCHthrew past the create that had already succeeded: the caller reported failure over an existing artist and a retry duplicated it. This had to land with the swap or the swap would have reintroduced the defect chat#1892 just closed.No api or docs change: this reuses
POST /api/valuation(api#776) exactly as the/artistsdialog already does, so there is no contract to write first.Verification
TDD, red → green, two units:
pnpm exec tsc --noEmit→ 13 errors, identical tomain's baseline.eslintclean on all touched files.Preview verification to follow on this PR.
Tracked in chat#1889 (matrix row 8).
Summary by cubic
Seeds the catalog when the first artist is added during setup by routing AddArtistForm through
useAddSpotifyArtist, and adds a measuring state in /setup/valuation so onboarding no longer dead-ends (chat#1889 row 8). This matches the /artists dialog and kicks POST /api/valuation on first add.Bug Fixes
useAddSpotifyArtistnow runs valuation on the first artist added in setup, preventing empty-catalog onboarding paths (fixes the fallback seen after feat(onboarding): end setup on the catalog valuation (chat#1889 row 10) #1895)./setup/valuationshows a “Measuring your catalog” state when a catalog exists but has no valuation yet, avoiding an endless skeleton and guiding users to/setup/tasks.addSpotifyArtistnon-fatal to avoid false failures and duplicate creates.Refactors
useAddRosterArtistand switchedAddArtistFormtouseAddSpotifyArtist(passes the full Spotify result). Tests updated.MeasuringCatalogPanelinto its own component and used it inSetupValuationfor cleaner composition.Written for commit 2f6aee0. Summary will update on new commits.
Summary by CodeRabbit