feat(onboarding): end setup on the catalog valuation (chat#1889 row 10)#1895
feat(onboarding): end setup on the catalog valuation (chat#1889 row 10)#1895sweetmantech wants to merge 1 commit into
Conversation
chat#1889 matrix row 10 — the payoff this issue is named for. Finishing roster + socials ended on a generic green CheckCircle2, and /setup/valuation was a bare redirect to /catalogs, so the welcome email's "See your baseline valuation" link had no real destination. A signup converted on a number and never saw it again. - RosterVerifiedPanel renders the account's valuation with the shared homepage hero (useHomeValuation + ValuationHero), then points at the weekly report. - New SetupValuation mounts at /setup/valuation with the same hero, falling back to /catalogs only when there is no catalog to value. - The no-valuation fallback now points at the step that unlocks one (/setup/catalog) instead of dead-ending on "Back to Recoup". Reuses the existing hero rather than building a second valuation surface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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. |
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✨ Finishing Touches🧪 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: 3047b82c60
ℹ️ 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".
| useEffect(() => { | ||
| if (isLoading) return; | ||
| if (!hasCatalog || isError) router.replace("/catalogs"); |
There was a problem hiding this comment.
On a direct visit from the welcome email, useCatalogs() is initially disabled because UserProvider has not asynchronously populated userData.account_id yet. In React Query v5 that disabled query has isLoading === false, so this effect sees hasCatalog === false and immediately replaces the new valuation route with /catalogs, including for accounts that do have a catalog. Gate the redirect on the catalog query actually reaching a terminal fetched state (and on account/auth readiness) rather than isLoading alone.
Useful? React with 👍 / 👎.
| if (!hasCatalog || isError) router.replace("/catalogs"); | ||
| }, [isLoading, hasCatalog, isError, router]); | ||
|
|
||
| if (isLoading || !valuation.show) { |
There was a problem hiding this comment.
When the catalog query succeeds but useHomeValuation() cannot produce a hero—for example, the catalog has no measured songs, the measurements request fails, or the selected artist has no scoped measurements—hasCatalog prevents the redirect while this branch renders skeletons forever. These are terminal show: false states in getValuationHeroState, not loading states, so affected users never get either the valuation or an actionable fallback.
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.
4 issues found across 4 files
Confidence score: 2/5
- In
components/Onboarding/SetupValuation.tsx, the redirect can fire before auth/catalog hydration completes, sending authenticated users to/catalogsbefore the welcome-email valuation route can load; this is a concrete flow break for onboarding users — gate redirect logic on a successful catalog response. - In
components/Onboarding/SetupValuation.tsx, the same skeleton is used for both “still loading” and “valuation unavailable,” so accounts withvaluation.show = falsecan appear to load forever instead of seeing a stable fallback; this can cause users to wait or abandon the flow — split loading vs empty/error rendering states. - In
components/Onboarding/RosterVerifiedPanel.tsx, users with existing catalogs can briefly see contradictory CTAs (“Roster verified” and “Claim your catalog”) while valuation queries settle, which can undermine trust in onboarding state accuracy — hold the panel in a loading state until catalog/measurement checks resolve. components/Onboarding/SetupValuation.tsxcurrently mixes redirect lifecycle and presentation in an oversized component, increasing the chance of future regressions in this already state-sensitive path — extract redirect/query-state handling or hero rendering into focused hooks/components.
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:23">
P3: This component exceeds the repository's 20-line function limit and mixes redirect lifecycle with two presentation states. Extract the redirect/query-state handling or the hero content into focused components/hooks to keep the onboarding UI maintainable.</violation>
<violation number="2" location="components/Onboarding/SetupValuation.tsx:31">
P1: Authenticated visitors can be redirected to `/catalogs` before user/auth hydration enables `useCatalogs`, so the welcome-email payoff route never loads its valuation. Redirect only after a successful catalog response confirms the array is empty; preserve the error fallback separately.</violation>
<violation number="3" location="components/Onboarding/SetupValuation.tsx:34">
P1: The loading skeleton is shown for both "catalogs still loading" and "valuation not available" states without distinguishing between them. When the account has a catalog but `valuation.show` is permanently false (measurements API error, missing valuation data, or zero measured tracks), the user sees an infinite skeleton with no fallback or way to proceed.
The `useHomeValuation` hook can return `{ show: false }` permanently even when `hasCatalog` is true (e.g., `measurementsFailed`, `!measurements?.valuation`, `!measurements.measured_song_count`). At that point, the useEffect won't redirect (because `hasCatalog` is true and `isError` from `useCatalogs()` is false), and the skeleton renders forever.
`RosterVerifiedPanel` handles this exact scenario gracefully by falling back to the confirmation state with a pointer to `/setup/catalog`. `SetupValuation` should do the same — either show a fallback UI or redirect to a safe destination when the valuation data is genuinely unavailable despite having a catalog.</violation>
</file>
<file name="components/Onboarding/RosterVerifiedPanel.tsx">
<violation number="1" location="components/Onboarding/RosterVerifiedPanel.tsx:23">
P3: Completing socials can briefly show “Roster verified” and “Claim your catalog” for accounts that already have a catalog while valuation queries load. Render a loading state until the catalog/measurement lookup settles, then show this fallback only for a settled no-valuation result.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| useEffect(() => { | ||
| if (isLoading) return; | ||
| if (!hasCatalog || isError) router.replace("/catalogs"); |
There was a problem hiding this comment.
P1: Authenticated visitors can be redirected to /catalogs before user/auth hydration enables useCatalogs, so the welcome-email payoff route never loads its valuation. Redirect only after a successful catalog response confirms the array is empty; preserve the error fallback separately.
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 31:
<comment>Authenticated visitors can be redirected to `/catalogs` before user/auth hydration enables `useCatalogs`, so the welcome-email payoff route never loads its valuation. Redirect only after a successful catalog response confirms the array is empty; preserve the error fallback separately.</comment>
<file context>
@@ -0,0 +1,65 @@
+
+ useEffect(() => {
+ if (isLoading) return;
+ if (!hasCatalog || isError) router.replace("/catalogs");
+ }, [isLoading, hasCatalog, isError, router]);
+
</file context>
| if (!hasCatalog || isError) router.replace("/catalogs"); | ||
| }, [isLoading, hasCatalog, isError, router]); | ||
|
|
||
| if (isLoading || !valuation.show) { |
There was a problem hiding this comment.
P1: The loading skeleton is shown for both "catalogs still loading" and "valuation not available" states without distinguishing between them. When the account has a catalog but valuation.show is permanently false (measurements API error, missing valuation data, or zero measured tracks), the user sees an infinite skeleton with no fallback or way to proceed.
The useHomeValuation hook can return { show: false } permanently even when hasCatalog is true (e.g., measurementsFailed, !measurements?.valuation, !measurements.measured_song_count). At that point, the useEffect won't redirect (because hasCatalog is true and isError from useCatalogs() is false), and the skeleton renders forever.
RosterVerifiedPanel handles this exact scenario gracefully by falling back to the confirmation state with a pointer to /setup/catalog. SetupValuation should do the same — either show a fallback UI or redirect to a safe destination when the valuation data is genuinely unavailable despite having a catalog.
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 34:
<comment>The loading skeleton is shown for both "catalogs still loading" and "valuation not available" states without distinguishing between them. When the account has a catalog but `valuation.show` is permanently false (measurements API error, missing valuation data, or zero measured tracks), the user sees an infinite skeleton with no fallback or way to proceed.
The `useHomeValuation` hook can return `{ show: false }` permanently even when `hasCatalog` is true (e.g., `measurementsFailed`, `!measurements?.valuation`, `!measurements.measured_song_count`). At that point, the useEffect won't redirect (because `hasCatalog` is true and `isError` from `useCatalogs()` is false), and the skeleton renders forever.
`RosterVerifiedPanel` handles this exact scenario gracefully by falling back to the confirmation state with a pointer to `/setup/catalog`. `SetupValuation` should do the same — either show a fallback UI or redirect to a safe destination when the valuation data is genuinely unavailable despite having a catalog.</comment>
<file context>
@@ -0,0 +1,65 @@
+ if (!hasCatalog || isError) router.replace("/catalogs");
+ }, [isLoading, hasCatalog, isError, router]);
+
+ if (isLoading || !valuation.show) {
+ return (
+ <div className="mx-auto flex w-full max-w-xl flex-col gap-4 px-6 py-8">
</file context>
| * cold-start signup still lands somewhere actionable rather than on an empty | ||
| * hero. | ||
| */ | ||
| const SetupValuation = () => { |
There was a problem hiding this comment.
P3: This component exceeds the repository's 20-line function limit and mixes redirect lifecycle with two presentation states. Extract the redirect/query-state handling or the hero content into focused components/hooks to keep the onboarding UI maintainable.
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 23:
<comment>This component exceeds the repository's 20-line function limit and mixes redirect lifecycle with two presentation states. Extract the redirect/query-state handling or the hero content into focused components/hooks to keep the onboarding UI maintainable.</comment>
<file context>
@@ -0,0 +1,65 @@
+ * cold-start signup still lands somewhere actionable rather than on an empty
+ * hero.
+ */
+const SetupValuation = () => {
+ const router = useRouter();
+ const valuation = useHomeValuation();
</file context>
| const RosterVerifiedPanel = () => { | ||
| const valuation = useHomeValuation(); | ||
|
|
||
| if (valuation.show) { |
There was a problem hiding this comment.
P3: Completing socials can briefly show “Roster verified” and “Claim your catalog” for accounts that already have a catalog while valuation queries load. Render a loading state until the catalog/measurement lookup settles, then show this fallback only for a settled no-valuation result.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/RosterVerifiedPanel.tsx, line 23:
<comment>Completing socials can briefly show “Roster verified” and “Claim your catalog” for accounts that already have a catalog while valuation queries load. Render a loading state until the catalog/measurement lookup settles, then show this fallback only for a settled no-valuation result.</comment>
<file context>
@@ -3,25 +3,66 @@
+const RosterVerifiedPanel = () => {
+ const valuation = useHomeValuation();
+
+ if (valuation.show) {
+ return (
+ <section className="flex flex-col items-center gap-4 py-8">
</file context>
1 participant
Matrix row 10 in chat#1889 — the payoff the issue is named for. Independent of the convergence chain.
Why
Two dead ends on the same number:
CheckCircle2and a "Back to Recoup" button./setup/valuationwas a bareredirect("/catalogs")— so the welcome email's "See your baseline valuation" link had no real destination.A signup converts on a valuation and then never sees it again inside the product.
What changed
RosterVerifiedPanelrenders the account's valuation using the existing homepage hero (useHomeValuation+ValuationHero) — no second valuation surface — then points at the weekly report that keeps it measured.SetupValuationmounts at/setup/valuationwith the same hero, and falls back to/catalogsonly when the account genuinely has no catalog to value./setup/catalog) instead of dead-ending on "Back to Recoup".Verification
TDD, red → green:
```
RED — panel still renders the generic check, no valuation
× ends setup on the catalog valuation, not a generic green check
Tests 1 failed | 1 passed (2)
GREEN
pnpm exec vitest run components/Onboarding
Test Files 1 passed (1)
Tests 2 passed (2)
```
Tests assert both directions: the valuation figure and measured-track count render when a valuation exists (
$1.4M,42 tracks measured), and the confirmation state still appears when it does not — so a cold-start account can't hit an empty hero.pnpm exec tsc --noEmitclean for the touched files. Preview click-through pending (needs an authed account with a measured catalog).Tracked in chat#1889 (matrix row 10).
Summary by cubic
Finish onboarding on the catalog valuation and make /setup/valuation show the same valuation instead of redirecting. Delivers the payoff from chat#1889 row 10 and guides users to set up weekly reports with clear fallbacks.
RosterVerifiedPanelreusesValuationHeroviauseHomeValuation; if no valuation, show confirmation with a CTA to /setup/catalog.SetupValuationat /setup/valuation. Renders the same hero; only redirects to /catalogs when the account has no catalog.RosterVerifiedPanelcovering both the valuation view and the no-valuation fallback.Written for commit 3047b82. Summary will update on new commits.