Skip to content

feat(onboarding): state-derived onboarding router with resume-on-landing + skip-for-now - #1872

Merged
sweetmantech merged 5 commits into
testfrom
feat/onboarding-router
Jul 21, 2026
Merged

feat(onboarding): state-derived onboarding router with resume-on-landing + skip-for-now#1872
sweetmantech merged 5 commits into
testfrom
feat/onboarding-router

Conversation

@sweetmantech

@sweetmantech sweetmantech commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Part of #1867 ("State-derived onboarding router: resume-on-landing + skip-for-now").

What

On every authenticated landing on chat home, the app derives the account's onboarding step from the activation checkpoint predicates and routes accordingly:

  • Incomplete account -> renders the onboarding sequence at the first unmet step, with an always-visible "Skip for now".
  • Skip -> drops to the normal app with a dismissible checklist pinned bottom-right; "Resume setup" re-opens the sequence.
  • Fully activated account -> normal home, never sees the sequence.

Checkpoint predicates (in sequence order), all over existing data:

Step Predicate Source
artists account has >= 1 rostered artist roster via existing fetchArtists (account_artist_ids)
socials every rostered artist has >= 1 linked social account_socials on the roster response
catalog account has >= 1 claimed catalog existing getCatalogs (account_catalogs)
task account has >= 1 enabled scheduled task existing getTasks (scheduled_actions.enabled)

Derived, never stored

Per the issue's architecture decision: there is no onboarding_step column and no wizard cursor. The step is a pure function of account state recomputed on every landing, so:

  • drop-off/return and multi-device resume are correct for free (state lives in the DB, not the browser);
  • out-of-band completion (valuation auto-claim, chat, API) auto-advances the step and can never desync;
  • "skip" and "checklist dismissed" are session-scoped escape hatches (sessionStorage), not persisted state: the next visit re-derives and resumes by default.

The gate also fails open: while any checkpoint source is loading or errored, the normal app renders, so the sequence never flashes for activated accounts and a failed fetch never walls anyone off.

Structure

  • lib/onboarding/getOnboardingCheckpoints.ts - the four predicates -> checkpoint flags (pure)
  • lib/onboarding/getOnboardingStep.ts - first unmet checkpoint -> step enum (pure, core logic)
  • lib/onboarding/getOnboardingView.ts - {isReady, step, skipped, dismissed} -> none | sequence | checklist (pure)
  • lib/onboarding/getOnboardingStepContent.ts - placeholder card copy + link per step
  • hooks/useOnboardingState.ts - composes existing data hooks (useArtistProvider, useCatalogs, useScheduledActions) into derived state
  • hooks/useOnboardingGate.ts - view decision + session-scoped skip/resume/dismiss
  • components/Onboarding/* - sequence container, step placeholder card, checkpoint list, pinned checklist
  • components/Home/HomePage.tsx - chat-home integration point

Step cards are titled placeholders (purpose + link to the existing page where the step can be completed today); each step's real in-sequence experience ships in the sibling PRs on #1867, slotting into this frame.

Tests

TDD (failing tests first, then implementation). 18 new unit tests in lib/onboarding/__tests__/:

  • getOnboardingStep: all 16 predicate combinations against the spec's priority order, plus null/undefined/empty socials, per-artist socials requirement, disabled and paused (null-enabled) tasks, out-of-band advancement (valuation auto-claim lands on task), fully-activated -> complete, determinism.
  • getOnboardingCheckpoints: order, independence of flags, socials incomplete while roster empty.
  • getOnboardingView: loading -> none, activated -> never, resume-by-default, skip -> checklist, dismiss -> none.
  • getOnboardingStepContent: every step has title/description/link to an existing route.

Verification

  • pnpm exec vitest run: 40 files / 148 tests, all green (130 pre-existing + 18 new).
  • pnpm exec tsc --noEmit: 8 errors, all pre-existing on main (verified identical with this change stashed); 0 in new files.
  • pnpm build (local): webpack compile passed and the build's TypeScript phase passed (no type errors); the build then fails at "Collecting page data" for /api/agent-creator with Missing Supabase credentials: { hasUrl: false, hasKey: false } - environmental (this checkout has no .env, only .env.example), unrelated to this client-side change and identical without it. The Vercel preview build (which has env) is the authoritative build check.
  • Prettier clean on all touched files (next lint is broken on main: Invalid project directory ... /lint).
  • Preview verification: pending (see below).

How to test on preview

  1. Fresh/incomplete account: sign in with an account that has a valuation-claimed catalog but no enabled task -> landing on / shows the sequence at its derived step (e.g. "Schedule your first report" with earlier checkpoints ticked). Complete a step out-of-band (e.g. create a task via API/chat), reload -> the sequence auto-advances or disappears.
  2. Skip: click "Skip for now" -> normal chat home with the "Finish setting up" checklist pinned bottom-right; "Resume setup" returns to the sequence; X dismisses the checklist for the session; a fresh session resumes the sequence.
  3. Activated account: sign in with an account that has artists + socials + catalog + an enabled task -> normal home, no sequence, no checklist.

🤖 Generated with Claude Code


Summary by cubic

Adds a state-derived onboarding router that resumes on landing and supports “skip for now.” After skip, a pinned checklist stays visible until setup is complete; a “Continue” button re-opens onboarding. Activated accounts go straight to chat.

  • New Features

    • Derives the current step from account data (artists → socials → catalog → task). No stored cursor.
    • Soft gate via useOnboardingGate: incomplete accounts see the sequence; after skip, a persistent pinned checklist appears with a clear “Continue” button to resume.
    • One fresh catalogs read per landing on the onboarding path to avoid stale steps after out‑of‑band claims.
  • Bug Fixes

    • Checklist positioning avoids right‑rail clipping.
    • Session skip flag is account‑scoped and storage‑safe (fail‑open on errors).
    • Roster fetch errors keep onboarding unresolved (no flash) via isError and deriveOnboardingState.
    • Corrected step card heading to h3 and stabilized first‑landing verification.

Written for commit ea068a8. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added a guided onboarding sequence showing setup progress and the next recommended step.
    • Added a dismissible onboarding checklist with options to resume setup or skip it temporarily.
    • Onboarding progress now reflects completed artists, social links, catalogs, and scheduled tasks.
    • Added session-based controls for skipping or dismissing onboarding.
  • Bug Fixes
    • Improved handling of artist roster errors so failed loading is not treated as an empty roster.
    • Catalog information is refreshed when returning to the landing page.

…ing + skip-for-now

On every authenticated landing, chat home derives the onboarding step
from the activation checkpoint predicates (has artists -> artists have
socials -> has claimed catalog -> has enabled task) over existing data
sources, never a stored wizard cursor (#1867). Incomplete
accounts resume the sequence at the first unmet step; an always-visible
skip drops to the app with a dismissible session-scoped checklist;
activated accounts never see it. Step cards are titled placeholders
linking to the existing pages until the step PRs land.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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.

@vercel

vercel Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
chat Ready Ready Preview Jul 21, 2026 11:32am

Request Review

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@sweetmantech, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 57 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bfd34762-9488-472f-a362-c95e03bc74c4

📥 Commits

Reviewing files that changed from the base of the PR and between 13ed5d6 and ea068a8.

⛔ Files ignored due to path filters (5)
  • components/Home/__tests__/HomePage.onboarding.test.tsx is excluded by !**/*.test.* and included by components/**
  • hooks/__tests__/useOnboardingSessionFlags.test.tsx is excluded by !**/*.test.* and included by hooks/**
  • lib/onboarding/__tests__/getOnboardingFlagKey.test.ts is excluded by !**/*.test.* and included by lib/**
  • lib/onboarding/__tests__/getOnboardingView.test.ts is excluded by !**/*.test.* and included by lib/**
  • lib/onboarding/__tests__/writeOnboardingFlag.test.ts is excluded by !**/*.test.* and included by lib/**
📒 Files selected for processing (7)
  • components/Home/HomePage.tsx
  • components/Onboarding/OnboardingChecklist.tsx
  • hooks/useOnboardingGate.ts
  • hooks/useOnboardingSessionFlags.ts
  • lib/onboarding/getOnboardingFlagKey.ts
  • lib/onboarding/getOnboardingView.ts
  • lib/onboarding/types.ts
📝 Walkthrough

Walkthrough

The PR adds a state-derived onboarding flow with activation checkpoints, session-scoped skip and dismissal controls, readiness-aware data composition, and conditional sequence or checklist rendering on the home page.

Changes

Onboarding state model and derivation

Layer / File(s) Summary
Onboarding state model and derivation
lib/onboarding/types.ts, lib/onboarding/getOnboardingCheckpoints.ts, lib/onboarding/getOnboardingStep.ts, lib/onboarding/deriveOnboardingState.ts
Defines onboarding types and derives checkpoint completion, the current step, and readiness from account data.

Landing data readiness wiring

Layer / File(s) Summary
Landing data readiness wiring
hooks/artists/useArtistsRoster.ts, hooks/useArtists.tsx, hooks/useRefreshCatalogsOnLanding.ts, hooks/useOnboardingState.ts
Propagates roster errors, refreshes catalogs on landing, and composes source data for onboarding state derivation.

Session gate and view selection

Layer / File(s) Summary
Session gate and view selection
lib/onboarding/getOnboardingView.ts, lib/onboarding/*OnboardingFlag.ts, hooks/useOnboardingSessionFlags.ts, hooks/useOnboardingGate.ts
Stores account-scoped session flags and exposes sequence, checklist, or no-onboarding views with control callbacks.

Landing-page onboarding UI

Layer / File(s) Summary
Landing-page onboarding UI
components/Onboarding/*, components/Home/HomePage.tsx
Renders the current onboarding step, checkpoint list, skip control, dismissible checklist, and home-page integration.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HomePage
  participant useOnboardingGate
  participant useOnboardingState
  participant getOnboardingView
  participant OnboardingSequence
  participant OnboardingChecklist

  HomePage->>useOnboardingGate: request onboarding view and callbacks
  useOnboardingGate->>useOnboardingState: derive readiness, step, and checkpoints
  useOnboardingGate->>getOnboardingView: evaluate readiness and session flags
  getOnboardingView-->>useOnboardingGate: return none, sequence, or checklist
  useOnboardingGate-->>HomePage: return onboarding state and actions
  HomePage->>OnboardingSequence: render incomplete step when view is sequence
  HomePage->>OnboardingChecklist: render checklist when view is checklist
Loading

Possibly related issues

Poem

Checkpoints bloom in ordered rows,
A guided path now clearly shows.
Skip the trail or start anew,
Session flags remember you.
Home wakes the setup flow—
One small step, then onward go.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Solid & Clean Code ✅ Passed PASS — the onboarding flow is decomposed into small pure helpers and thin hooks/components, with shared state and storage logic centralized.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/onboarding-router

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 773ee81c82

ℹ️ 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".

export function useOnboardingState(): OnboardingState {
const { authenticated } = usePrivy();
const { artists, isLoading: artistsLoading } = useArtistProvider();
const catalogsQuery = useCatalogs();

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 Badge Force catalog refetches for onboarding derivation

When a user has already loaded / with no catalogs and then claims a catalog through valuation/chat/API or another tab, this reuses useCatalogs(), whose query is treated as fresh for 5 minutes (hooks/useCatalogs.ts sets staleTime: 5 * 60 * 1000 and disables focus refetch). Returning to / within that window keeps catalogsQuery.isSuccess true with cached [], so getOnboardingStep still shows the catalog step instead of advancing from the DB, which breaks the intended state-derived resume behavior.

Useful? React with 👍 / 👎.

Comment thread hooks/useOnboardingState.ts Outdated
// app instead of mis-deriving a step from partial state.
const isReady =
authenticated &&
!artistsLoading &&

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 Badge Fail open when the artist roster query errors

If /api/artists errors after auth, useArtistsRoster supplies artists = [] and exposes only isPending as isLoading; once the errored query is no longer pending, this condition marks onboarding ready as soon as catalogs/tasks succeed. The gate then derives the first step as artists from partial state and replaces home with the sequence, even though failed checkpoint fetches are supposed to fail open to the normal app.

Useful? React with 👍 / 👎.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 17 files

Confidence score: 5/5

  • Safe to merge after the addressed issues were fixed.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread hooks/useOnboardingGate.ts Outdated
Comment thread hooks/useOnboardingGate.ts Outdated
Comment thread hooks/useOnboardingState.ts
Comment thread hooks/useOnboardingState.ts Outdated
Comment thread components/Onboarding/OnboardingStepCard.tsx Outdated
Comment thread hooks/useOnboardingState.ts

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Why are both these changes added explicitly to the home page instead of to the global layout? Is there a reason we're applying this to one specific page route instead of globally across the app pages?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Deliberate for slice 1, but it's a fair challenge. Three reasons it's on HomePage rather than the global layout: (1) the sequence must not intercept its own step pages/catalogs/{id} (the report the valuation email deep-links to), /onboarding/*, /artists, /tasks are all pages the sequence sends users to; a global gate would need an exclusion list to avoid trapping users in a loop, and that list doesn't stabilize until the step PRs land. (2) Home is where the documented failure lives — the issue's repro is 'returning user lands on a blank chat box', and / is the app's default landing. (3) Smallest blast radius while the router's derivation logic gets its first prod exposure.

Agreed on the end state though: once the step pages are merged, promoting the checklist to the global authenticated layout is cheap and safe (it's non-intercepting), and the sequence gate can go global behind an exclusion set of sequence-owned + public routes. Proposing that as a follow-up item on #1867 rather than expanding this PR.

@sweetmantech

Copy link
Copy Markdown
Collaborator Author

Preview verification — 2026-07-20 · preview chat-p8yftyrp0-recoup.vercel.app (sha-verified against PR head 773ee81c), Chrome DevTools, brand-new signup sweetmantech+july2020261738@gmail.com (zero artists/socials/catalogs/tasks — the true fresh-account fixture; enabled by clearing the preview Privy test app, see infra note).

# Scenario Expected Actual Result
1 First landing immediately after signup Sequence at step 1 Normal app home — no sequence (fail-open engaged; see root cause below) ⚠️ pre-existing blocker, not this PR
2 Reload (same account) Sequence at derived step "SET UP RECOUP · STEP 1 OF 4 — Confirm your artists", checklist shows all 4 incomplete
3 "Skip for now" Normal app + pinned checklist App home + "Finish setting up" checklist w/ Resume + Dismiss
4 "Resume setup" Back to sequence at derived step Sequence at step 1
5 Reload while unskipped Resume-by-default Sequence renders again at step 1
6 "Dismiss onboarding checklist" Checklist hides Checklist persists after click (re-snapshotted to confirm; no console error) bug in this PR — fixing
7 Console after healthy load + all interactions Clean Zero errors (post-reload)

Screenshots (assets branch assets/pr1872-preview-verification @ c98795a5):

Scenario 2 — fresh account lands on the sequence at the derived step:
sequence step 1

Scenario 3 — skip drops to the app with the pinned checklist:
skip + checklist

Scenario 6 — checklist still present after Dismiss (bug):
dismiss bug

Root cause for scenario 1 (pre-existing, NOT this PR's code): during first-ever sign-in the app fired POST /api/accounts twice — creating an email-less orphan account (401bdd34…) and the real one (d2f2308d…, carries the email; both confirmed in Supabase). UserProvider.userData.account_id held the orphan, so useCatalogs (main code, untouched by this PR) looped on 403 Access denied to specified account_id. The router's fail-open then correctly showed the normal app. Self-heals on reload once UserProvider resolves the right account. This needs its own bug item — it damages the exact first-impression moment the sequence exists for, and it likely orphans an account row on every fresh web signup.

Infra note: preview sign-ins were blocked for ~6 weeks by the preview Privy test app (cmc52us2g…) sitting at its 150-user cap (last new user 2026-06-04). Cleaned to 3 users today, which is what made this fresh-signup test possible.

Next on this PR: fixing the Dismiss bug plus the cubic review findings (account-scoped skip state, storage-safe wrapper, catalogs staleness, roster-error fail-open, heading hierarchy) — will push and re-verify.

…unt-scoped session flags, storage safety, fresh catalogs per landing, roster-error fail-open, h3 card heading

Fixes the preview-verified dismiss bug and all cubic findings on #1872:

- Dismiss live bug: the checklist was position:fixed to the viewport, so
  the z-[65] ArtistsSidebar rail overlapped its right edge and clipped
  the dismiss button off-screen. Now absolute within the (relative) home
  content area. Gate state was already single-owner (HomePage passes
  callbacks down) — locked in with a HomePage-level dismiss/skip/resume
  test.
- P1 cross-account leak: skip/dismiss sessionStorage keys are scoped by
  account id and re-derived when the account in the tab changes
  (useOnboardingSessionFlags).
- P2 storage safety: read/write go through safe helpers that no-op on
  throwing storage so the gate fails open instead of crashing.
- P2 catalogs staleness: useRefreshCatalogsOnLanding invalidates the
  catalogs cache on onboarding mount for one fresh read per landing;
  useCatalogs behavior unchanged for other consumers.
- P2 roster-error fail-open: useArtistsRoster now exposes isError;
  deriveOnboardingState keeps isReady false on roster error so the view
  resolves to none.
- P3 hook length: projection extracted to pure deriveOnboardingState.
- P3 heading hierarchy: step card h1 -> h3 under the container h2.

Tests: TDD red-first; +26 tests (jsdom + @testing-library/react added as
devDeps; vitest include extended to .test.tsx). 174 passing; tsc clean
apart from the 7 pre-existing main errors in lib/emails tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sweetmantech

Copy link
Copy Markdown
Collaborator Author

Review fixes — all findings addressed in 13ed5d69

# Finding Disposition
1 Live bug: Dismiss does nothing (preview verification) Fixed — root cause was layout, not state-sync. The checklist was position: fixed to the viewport, but the layout places the ArtistsSidebar rail (z-[65]) on the right edge, overlapping the card's right side and clipping the Dismiss button off-screen (visible in the verification screenshot: card overflowing the right edge). Now absolute inside the (relative) home content container, so it always sits in the visible content area, clear of the rail at any width. The hypothesized dual-hook-instance split was checked and does not exist: useOnboardingGate has exactly one owner (HomePage) and the checklist receives onDismiss/onResume via props — the pre-fix behavior test confirmed dismiss logic already worked at the HomePage level. Both are now locked in by components/Home/__tests__/HomePage.onboarding.test.tsx: full skip → checklist → dismiss-hides flow, skip/resume still working, plus a regression guard that the checklist is never fixed again. Single-source-of-truth is documented in the gate's docstring.
2 Cubic P1: cross-account sessionStorage leak Fixed. Flags are keyed recoup-onboarding-<flag>:<accountId> (getOnboardingFlagKey) and the new useOnboardingSessionFlags(accountId) re-derives state whenever the account in the tab changes. Test proves Account B never inherits Account A's skip/dismiss, and A's choices survive switching back.
3 Cubic P2: sessionStorage can throw Fixed. All reads/writes go through readOnboardingFlag / writeOnboardingFlag, which try/catch and fail open (read → false, write → no-op; in-memory state still applies). Tests cover throwing getItem/setItem.
4 Cubic P2: catalogs staleness on landing Fixed. useRefreshCatalogsOnLanding (mounted only by useOnboardingState) invalidates the ["catalogs"] cache once on mount, so every landing gets one fresh read and out-of-band claims advance the step immediately. useCatalogs itself is untouched — other consumers keep the 5-min staleTime; test asserts unrelated caches are left alone.
5 Cubic P2: roster error treated as resolved Fixed. useArtistsRoster now exposes isError (threaded through useArtists/ArtistProvider), and the projection keeps isReady false on roster error, so the view resolves to none (fail open) instead of walling an activated account into step 1. Test: deriveOnboardingState with artistsError: true → not ready.
6 Cubic P3: hook length / concern mixing Fixed. The projection now lives in pure lib/onboarding/deriveOnboardingState.ts; useOnboardingState only composes sources and memoizes the call.
7 Cubic P3: h1 under h2 Fixed. Step card heading is now h3 under the container's h2; test asserts the outline (h2h3, zero h1s).

Verification: TDD red-first for every change (7 new test files failing before implementation). pnpm exec vitest run: 47 files / 174 tests green (148 pre-existing + 26 new). pnpm exec tsc --noEmit: only the 7 pre-existing lib/emails test errors from main, 0 in touched files. Prettier clean. Test infra: added jsdom + @testing-library/react devDeps and extended the vitest glob to **/__tests__/**/*.test.{ts,tsx} to make the required HomePage-level component test possible.

Still to re-verify on preview: the checklist's new in-content position (Dismiss reachable, card fully visible next to the rail) — the layout aspect can't be proven in jsdom.

🤖 Generated with Claude Code

@coderabbitai coderabbitai Bot left a comment

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.

🧹 Nitpick comments (2)
components/Onboarding/OnboardingChecklist.tsx (1)

3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use Radix UI icons for UI component indicators.

These UI components currently import structural indicators from Lucide React. As per coding guidelines: Use @radix-ui/react-icons for UI component icons (not Lucide React).

  • components/Onboarding/OnboardingChecklist.tsx#L3-L3: Replace the X icon import from lucide-react with the equivalent from @radix-ui/react-icons (e.g., Cross2Icon).
  • components/Onboarding/OnboardingCheckpointList.tsx#L3-L3: Replace the Check icon import from lucide-react with the equivalent from @radix-ui/react-icons (e.g., CheckIcon).
🤖 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/OnboardingChecklist.tsx` at line 3, Replace the Lucide
React X import in components/Onboarding/OnboardingChecklist.tsx at lines 3-3
with the equivalent Cross2Icon from `@radix-ui/react-icons`, and replace the Check
import in components/Onboarding/OnboardingCheckpointList.tsx at lines 3-3 with
CheckIcon from the same package; update usages as needed while preserving the
existing indicator behavior.

Source: Coding guidelines

components/Onboarding/OnboardingStepCard.tsx (1)

13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Export explicit props interfaces extending React.ComponentProps.

The new components do not follow the strict architectural pattern for component properties. As per coding guidelines: Extend native HTML attributes using React.ComponentProps<"element"> pattern and Export component prop types with explicit ComponentNameProps naming convention.

  • components/Onboarding/OnboardingStepCard.tsx#L13-L13: Export OnboardingStepCardProps extending React.ComponentProps<"section"> and spread ...props onto the section element.
  • components/Onboarding/OnboardingCheckpointList.tsx#L12-L16: Export OnboardingCheckpointListProps extending React.ComponentProps<"ul"> and spread ...props onto the ul element.
  • components/Onboarding/OnboardingChecklist.tsx#L7-L11: Export OnboardingChecklistProps extending React.ComponentProps<"aside"> and spread ...props onto the aside element.
  • components/Home/HomePage.tsx#L11-L11: Export HomePageProps extending React.ComponentProps<"div"> and spread ...props onto the root div element.
🤖 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/OnboardingStepCard.tsx` at line 13, Update
components/Onboarding/OnboardingStepCard.tsx:13 to export
OnboardingStepCardProps extending React.ComponentProps<"section">, accept those
props alongside step, and spread ...props onto the root section. Apply the same
pattern to components/Onboarding/OnboardingCheckpointList.tsx:12-16 with
exported OnboardingCheckpointListProps extending React.ComponentProps<"ul"> and
spread props on the ul; components/Onboarding/OnboardingChecklist.tsx:7-11 with
exported OnboardingChecklistProps extending React.ComponentProps<"aside"> and
spread props on the aside; and components/Home/HomePage.tsx:11 with exported
HomePageProps extending React.ComponentProps<"div"> and spread props on the root
div.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@components/Onboarding/OnboardingChecklist.tsx`:
- Line 3: Replace the Lucide React X import in
components/Onboarding/OnboardingChecklist.tsx at lines 3-3 with the equivalent
Cross2Icon from `@radix-ui/react-icons`, and replace the Check import in
components/Onboarding/OnboardingCheckpointList.tsx at lines 3-3 with CheckIcon
from the same package; update usages as needed while preserving the existing
indicator behavior.

In `@components/Onboarding/OnboardingStepCard.tsx`:
- Line 13: Update components/Onboarding/OnboardingStepCard.tsx:13 to export
OnboardingStepCardProps extending React.ComponentProps<"section">, accept those
props alongside step, and spread ...props onto the root section. Apply the same
pattern to components/Onboarding/OnboardingCheckpointList.tsx:12-16 with
exported OnboardingCheckpointListProps extending React.ComponentProps<"ul"> and
spread props on the ul; components/Onboarding/OnboardingChecklist.tsx:7-11 with
exported OnboardingChecklistProps extending React.ComponentProps<"aside"> and
spread props on the aside; and components/Home/HomePage.tsx:11 with exported
HomePageProps extending React.ComponentProps<"div"> and spread props on the root
div.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e777091e-aadf-4c45-a576-c4fe77216bd8

📥 Commits

Reviewing files that changed from the base of the PR and between 3dccdae and 13ed5d6.

⛔ Files ignored due to path filters (15)
  • components/Home/__tests__/HomePage.onboarding.test.tsx is excluded by !**/*.test.* and included by components/**
  • hooks/__tests__/useOnboardingSessionFlags.test.tsx is excluded by !**/*.test.* and included by hooks/**
  • hooks/__tests__/useRefreshCatalogsOnLanding.test.tsx is excluded by !**/*.test.* and included by hooks/**
  • lib/onboarding/__tests__/deriveOnboardingState.test.ts is excluded by !**/*.test.* and included by lib/**
  • lib/onboarding/__tests__/getOnboardingCheckpoints.test.ts is excluded by !**/*.test.* and included by lib/**
  • lib/onboarding/__tests__/getOnboardingFlagKey.test.ts is excluded by !**/*.test.* and included by lib/**
  • lib/onboarding/__tests__/getOnboardingStep.edge.test.ts is excluded by !**/*.test.* and included by lib/**
  • lib/onboarding/__tests__/getOnboardingStep.test.ts is excluded by !**/*.test.* and included by lib/**
  • lib/onboarding/__tests__/getOnboardingStepContent.test.ts is excluded by !**/*.test.* and included by lib/**
  • lib/onboarding/__tests__/getOnboardingView.test.ts is excluded by !**/*.test.* and included by lib/**
  • lib/onboarding/__tests__/readOnboardingFlag.test.ts is excluded by !**/*.test.* and included by lib/**
  • lib/onboarding/__tests__/writeOnboardingFlag.test.ts is excluded by !**/*.test.* and included by lib/**
  • package.json is excluded by none and included by none
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml and included by none
  • vitest.config.ts is excluded by none and included by none
📒 Files selected for processing (20)
  • components/Home/HomePage.tsx
  • components/Onboarding/OnboardingChecklist.tsx
  • components/Onboarding/OnboardingCheckpointList.tsx
  • components/Onboarding/OnboardingSequence.tsx
  • components/Onboarding/OnboardingStepCard.tsx
  • hooks/artists/useArtistsRoster.ts
  • hooks/useArtists.tsx
  • hooks/useOnboardingGate.ts
  • hooks/useOnboardingSessionFlags.ts
  • hooks/useOnboardingState.ts
  • hooks/useRefreshCatalogsOnLanding.ts
  • lib/onboarding/deriveOnboardingState.ts
  • lib/onboarding/getOnboardingCheckpoints.ts
  • lib/onboarding/getOnboardingFlagKey.ts
  • lib/onboarding/getOnboardingStep.ts
  • lib/onboarding/getOnboardingStepContent.ts
  • lib/onboarding/getOnboardingView.ts
  • lib/onboarding/readOnboardingFlag.ts
  • lib/onboarding/types.ts
  • lib/onboarding/writeOnboardingFlag.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

3 issues found across 24 files (changes from recent commits).

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="hooks/useOnboardingSessionFlags.ts">

<violation number="1" location="hooks/useOnboardingSessionFlags.ts:24">
P2: An account switch can briefly render the previous account's skip/dismiss state because these values remain in React state until the `[accountId]` effect runs. Deriving or invalidating the flags synchronously for the active account would prevent the wrong onboarding view from flashing when the new account is already ready.</violation>
</file>

<file name="hooks/useRefreshCatalogsOnLanding.ts">

<violation number="1" location="hooks/useRefreshCatalogsOnLanding.ts:17">
P3: Every landing invalidates the entire `catalogs` query-key family rather than the current account's entry, so previously cached accounts are unnecessarily made stale and can incur extra network requests later. Scoping the invalidation to the active `accountId` would preserve the intended fresh read without invalidating unrelated account caches.</violation>
</file>

<file name="lib/onboarding/deriveOnboardingState.ts">

<violation number="1" location="lib/onboarding/deriveOnboardingState.ts:55">
P2: On a landing with cached data, the gate can briefly derive onboarding from stale catalogs/tasks: `useOnboardingState` supplies `catalogsReady`/`tasksReady` from React Query `isSuccess`, which stays true during an `isFetching` background refetch (and catalogs are explicitly invalidated on landing). Including the query fetch state in readiness would keep the gate failed open until refreshed checkpoint data is settled.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

export function useOnboardingSessionFlags(
accountId: string,
): OnboardingSessionFlags {
const [skipped, setSkipped] = useState(() =>

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: An account switch can briefly render the previous account's skip/dismiss state because these values remain in React state until the [accountId] effect runs. Deriving or invalidating the flags synchronously for the active account would prevent the wrong onboarding view from flashing when the new account is already ready.

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

<comment>An account switch can briefly render the previous account's skip/dismiss state because these values remain in React state until the `[accountId]` effect runs. Deriving or invalidating the flags synchronously for the active account would prevent the wrong onboarding view from flashing when the new account is already ready.</comment>

<file context>
@@ -0,0 +1,54 @@
+export function useOnboardingSessionFlags(
+  accountId: string,
+): OnboardingSessionFlags {
+  const [skipped, setSkipped] = useState(() =>
+    readOnboardingFlag("skipped", accountId),
+  );
</file context>

authenticated &&
!artistsLoading &&
!artistsError &&
catalogsReady &&

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: On a landing with cached data, the gate can briefly derive onboarding from stale catalogs/tasks: useOnboardingState supplies catalogsReady/tasksReady from React Query isSuccess, which stays true during an isFetching background refetch (and catalogs are explicitly invalidated on landing). Including the query fetch state in readiness would keep the gate failed open until refreshed checkpoint data is settled.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/onboarding/deriveOnboardingState.ts, line 55:

<comment>On a landing with cached data, the gate can briefly derive onboarding from stale catalogs/tasks: `useOnboardingState` supplies `catalogsReady`/`tasksReady` from React Query `isSuccess`, which stays true during an `isFetching` background refetch (and catalogs are explicitly invalidated on landing). Including the query fetch state in readiness would keep the gate failed open until refreshed checkpoint data is settled.</comment>

<file context>
@@ -0,0 +1,60 @@
+      authenticated &&
+      !artistsLoading &&
+      !artistsError &&
+      catalogsReady &&
+      tasksReady,
+    step: getOnboardingStep(state),
</file context>

const queryClient = useQueryClient();

useEffect(() => {
queryClient.invalidateQueries({ queryKey: ["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.

P3: Every landing invalidates the entire catalogs query-key family rather than the current account's entry, so previously cached accounts are unnecessarily made stale and can incur extra network requests later. Scoping the invalidation to the active accountId would preserve the intended fresh read without invalidating unrelated account caches.

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

<comment>Every landing invalidates the entire `catalogs` query-key family rather than the current account's entry, so previously cached accounts are unnecessarily made stale and can incur extra network requests later. Scoping the invalidation to the active `accountId` would preserve the intended fresh read without invalidating unrelated account caches.</comment>

<file context>
@@ -0,0 +1,19 @@
+  const queryClient = useQueryClient();
+
+  useEffect(() => {
+    queryClient.invalidateQueries({ queryKey: ["catalogs"] });
+  }, [queryClient]);
+}
</file context>

…verification

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sweetmantech

Copy link
Copy Markdown
Collaborator Author

Final combined-build verification — 2026-07-20 · preview chat-k2gxdn08o-recoup.vercel.app (sha-verified against branch head 16a182d5 = this PR's fixes + main incl. #1876/#1868), brand-new signup sweetmantech+july2020261856@gmail.com.

# Gate Actual Result
1 Sequence renders on the very first landing (no reload) — the merge-blocking criterion "SET UP RECOUP · STEP 1 OF 4 — Confirm your artists" rendered on first landing after OTP; ~40s provisioning, then straight into the sequence
2 One account per signup (#1875/#1876 in effect) Supabase: single row 834a293d-… with the signup email; no orphan
3 Console on first landing Only the benign pre-auth /api/sandboxes 401; zero 403s, zero catalog errors
4 Checklist layout after skip Card fully inside the content area, clear of the right rail; full text + Resume + visible Dismiss ✕
5 Skip state account-scoping Live sessionStorage inspection: recoup-onboarding-skipped:834a293d-… — keyed to this account only
6 Dismiss Checklist removed on click
7 Heading hierarchy h2 → h3 outline confirmed in the a11y tree

Screenshots (assets branch assets/pr1872-final-verification @ 675de022):

Gate 1 — brand-new signup's very first landing:
first landing sequence

Gates 4-6 — repositioned checklist with reachable Dismiss:
fixed checklist

Earlier rounds for comparison: initial matrix (5/7 + two bugs), per-finding fixes, #1876's standalone verification.

All merge gates from #1867's quality bar are now met. Ready for merge review.

@sweetmantech
sweetmantech changed the base branch from main to test July 21, 2026 11:09
…ssible reminder

Remove the checklist-dismissed escape hatch entirely. After "skip for now"
the bottom-right "Finish setting up" card is now always present while
onboarding is incomplete (survives refresh via the session skip flag), and
clicking it re-opens the sequence — there is no way to permanently hide it.

Net-removal: drops the second session flag, its storage read/write usage,
the dismissChecklist callback threading, the X/Dismiss button, and the
separate "Resume setup" button (the card header is now the resume trigger).
getOnboardingView collapses to two views (sequence | checklist) and
OnboardingFlag to a single member.

+47 / -134. Full suite 181 pass, tsc clean on touched files, prettier clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

0 issues found across 12 files (changes from recent commits).

Requires human review: Auto-approval blocked by 3 unresolved issues from previous reviews.

Re-trigger cubic

…utton

The checklist header "Finish setting up" is a plain title again (it read as
non-clickable). Re-opening the sequence is now a clearly-styled primary
"Continue" button at the bottom of the card, replacing the ambiguous
click-the-header affordance.

Suite 181 pass, tsc clean on touched files, prettier clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

0 issues found across 2 files (changes from recent commits).

Requires human review: Auto-approval blocked by 3 unresolved issues from previous reviews.

Re-trigger cubic

sweetmantech added a commit that referenced this pull request Jul 22, 2026
…t-task emails, add-artist, Kimi K3 (#1880)

* feat(onboarding): state-derived onboarding router with resume-on-landing + skip-for-now (#1872)

* feat(onboarding): state-derived onboarding router with resume-on-landing + skip-for-now

On every authenticated landing, chat home derives the onboarding step
from the activation checkpoint predicates (has artists -> artists have
socials -> has claimed catalog -> has enabled task) over existing data
sources, never a stored wizard cursor (#1867). Incomplete
accounts resume the sequence at the first unmet step; an always-visible
skip drops to the app with a dismissible session-scoped checklist;
activated accounts never see it. Step cards are titled placeholders
linking to the existing pages until the step PRs land.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(onboarding): review fixes — checklist clipped by right rail, account-scoped session flags, storage safety, fresh catalogs per landing, roster-error fail-open, h3 card heading

Fixes the preview-verified dismiss bug and all cubic findings on #1872:

- Dismiss live bug: the checklist was position:fixed to the viewport, so
  the z-[65] ArtistsSidebar rail overlapped its right edge and clipped
  the dismiss button off-screen. Now absolute within the (relative) home
  content area. Gate state was already single-owner (HomePage passes
  callbacks down) — locked in with a HomePage-level dismiss/skip/resume
  test.
- P1 cross-account leak: skip/dismiss sessionStorage keys are scoped by
  account id and re-derived when the account in the tab changes
  (useOnboardingSessionFlags).
- P2 storage safety: read/write go through safe helpers that no-op on
  throwing storage so the gate fails open instead of crashing.
- P2 catalogs staleness: useRefreshCatalogsOnLanding invalidates the
  catalogs cache on onboarding mount for one fresh read per landing;
  useCatalogs behavior unchanged for other consumers.
- P2 roster-error fail-open: useArtistsRoster now exposes isError;
  deriveOnboardingState keeps isReady false on roster error so the view
  resolves to none.
- P3 hook length: projection extracted to pure deriveOnboardingState.
- P3 heading hierarchy: step card h1 -> h3 under the container h2.

Tests: TDD red-first; +26 tests (jsdom + @testing-library/react added as
devDeps; vitest include extended to .test.tsx). 174 passing; tsc clean
apart from the 7 pre-existing main errors in lib/emails tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(onboarding): make the skip checklist a persistent, non-dismissible reminder

Remove the checklist-dismissed escape hatch entirely. After "skip for now"
the bottom-right "Finish setting up" card is now always present while
onboarding is incomplete (survives refresh via the session skip flag), and
clicking it re-opens the sequence — there is no way to permanently hide it.

Net-removal: drops the second session flag, its storage read/write usage,
the dismissChecklist callback threading, the X/Dismiss button, and the
separate "Resume setup" button (the card header is now the resume trigger).
getOnboardingView collapses to two views (sequence | checklist) and
OnboardingFlag to a single member.

+47 / -134. Full suite 181 pass, tsc clean on touched files, prettier clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(onboarding): restore plain title + add explicit "Continue" button

The checklist header "Finish setting up" is a plain title again (it read as
non-clickable). Re-opening the sequence is now a clearly-styled primary
"Continue" button at the bottom of the card, replacing the ambiguous
click-the-header affordance.

Suite 181 pass, tsc clean on touched files, prettier clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* feat: post-valuation catalog report on /catalogs/[catalogId] (chat#1867 sequence step 1) (#1873)

The marketing valuation CTA landed on a bare Catalog Songs admin screen.
/catalogs/{id} now opens as a real catalog report: valuation echo
(central value + range via the ported marketing formula), lifetime
streams, tracks/releases measured, per-release table with album art,
per-section diagnosis + prescription copy, and one primary CTA
(set up your weekly report -> /tasks). The existing songs/ISRC
management UI stays reachable as a secondary Manage songs tab.

Valuation math mirrors marketing/lib/valuation (identical constants;
age from the api's catalog_age_years, 5y default). Release rollups are
null-safe on album/name/artist metadata independently of the sibling
crash-fix PR. Reuses the chat#1852 useCatalogMeasurements hook,
extending its response type with the v2 aggregate fields as optionals.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* feat: roster + socials verification onboarding steps (chat#1867) (#1870)

* feat: roster + socials verification onboarding steps (chat#1867)

Two self-contained onboarding step components for the chat#1867
sequence, mounted standalone at /onboarding/roster so the slice is
user-testable before the onboarding router lands:

- ConfirmRosterStep: shows the auto-created artist(s) from the
  valuation flow, inline "add another artist" for multi-artist
  managers (existing POST /api/artists endpoint), confirm to advance.
- VerifySocialsStep: per rostered artist, lists the auto-matched
  socials with handle + follower count; each match is confirmed or
  rejected, wrong matches are fixed by pasting the correct profile
  link (existing PATCH /api/artists/{id} profileUrls path), and
  artists with no socials record an explicit "none".

Verification state is pure client-side logic (lib/onboarding/*,
TDD'd): verdict reducers, per-artist and step-level resolution
predicates, PATCH payload building with APPPLE->APPLE platform-key
normalization, and a follower-count accessor tolerant of the API's
snake_case rows behind chat's camelCase SOCIAL type.

No new endpoints, tables, or context providers - hooks compose the
existing ArtistProvider / OrganizationProvider / Privy auth.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(onboarding): simplify verify-socials to accept-by-default (chat#1867)

Per design review: drop the per-social Correct/Wrong verdicts, the
resolution/gating machinery, and the explicit "This artist has no socials"
button. Matches are accepted by default; the only actions are Edit (fix a
wrong link, existing PATCH profileUrls path) and — for an artist with no
matches — a soft nudge + "Add a profile". Continue always proceeds (empty
= implicit none).

Deletes the verdict code that would otherwise be merged and immediately
removed: socialVerificationTypes, applySocialVerdict, isArtistSocialsResolved,
areAllArtistsResolved, markArtistHasNoSocials, findSocialIdByPlatform,
useSocialsVerification, SocialVerifyRow (+ their tests). Net −13 files.

Note: "Remove/delete a social" (the other half of the design) needs a new
api endpoint — the api's PATCH profileUrls is per-platform merge with no
delete-social path — tracked as a fast-follow on chat#1867.

Full suite 144 pass, tsc clean on touched files, lint + prettier clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(onboarding): always offer Add-a-profile per artist, not just empty ones

A matched-social list can still be missing platforms (e.g. Spotify found
but no Instagram). Surface 'Add a profile' below every artist's socials,
not only when zero were auto-matched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* feat(onboarding): first-task step — pre-run weekly report + confirm schedule (#1871)

Sequence final step for chat#1867 (respec 2026-07-20, pre-run shape):
generate the first weekly catalog report immediately through the normal
chat pipeline (same POST /api/chat transport a send uses), show it
finished, then ask one question — "get this every Monday?". Confirm
creates the enabled weekly task via the existing POST /api/tasks path
(which also mints the schedule) and shows the next-run time; decline
creates nothing.

Mounted standalone at /onboarding/first-task so the step is
user-testable before the OnboardingSequence container exists. The
pre-run prompt and the scheduled task prompt come from one builder, so
the preview the user confirms is byte-for-byte what Monday's run uses.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* feat(onboarding): first-task weekly report actually emails — LA EQUIS template + Kimi K3 (chat#1867) (#1877)

* feat(onboarding): first-task weekly report actually emails (LA EQUIS template + Kimi K3)

The first-task prompt generated a report but never emailed it (email is a
prompt-driven send_email call, and the prompt had no recipient or send
instruction) — proven by firing the created task: the agent ran and persisted
a report, but email_send_log stayed empty.

Generalize buildFirstTaskPrompt from the proven LA EQUIS weekly report
(scheduled_action 39fb5f68, running week over week): resolve the artist's
Spotify id from its connected profile -> capture this week's play counts ->
compute real week-over-week per-track deltas -> build a fully hex-specced
inline-CSS HTML email -> send via the recoup-platform-email-helper skill ->
confirm the Resend id. Carries the sandbox no-python and anti-fabrication
guardrails verbatim (what makes the send reliable). Thread the account email
(recipientEmail) + artistAccountId through both callers so the pre-run preview
and the Monday scheduled task both send to the account holder.

Bump both surfaces from DEFAULT_MODEL (openai/gpt-5.4-mini — completes the data
calls but gives up before composing the long HTML email, the LA EQUIS lesson)
to a new REPORT_MODEL (moonshotai/kimi-k3), which reliably composes and sends.

chat#1867 (first-task email-prompt gap).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(onboarding): default model -> Kimi K3 (KISS); harden email-less + shell paths

Address review feedback on #1877:

- Sweets (KISS): collapse the new REPORT_MODEL constant into DEFAULT_MODEL —
  set DEFAULT_MODEL = moonshotai/kimi-k3 app-wide (the fast gpt-5.4-mini default
  gave up before composing the long HTML report email). Revert both onboarding
  hooks to DEFAULT_MODEL; relabel the featured picker entry (id: DEFAULT_MODEL)
  from "GPT-5.4 Mini" to "Kimi K3" so it isn't mislabeled.
- CodeRabbit (email-less login): a wallet/phone/social-only Privy account has no
  email — FirstTaskStep now shows a distinct "add an email" alert (not an
  infinite "Preparing…"), and useConfirmFirstTask's onError distinguishes
  EMAIL_REQUIRED from a generic retry.
- Codex (shell-safety): artist/track/album names are user-entered; add a prompt
  guardrail to pass them as single quoted args so a name with quotes/;/$() can't
  alter the email-helper command.

Codex "non-Pro users get rejected" P2 is a false positive: the prior default
(gpt-5.4-mini) is also non-free by isFreeModel yet ships as the default, so the
API does not hard-block non-free default models.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Update lib/consts.ts

* chore(models): refresh featured model list to latest per series

Sweets: while touching the featured list, bring every entry to its series'
latest (verified present in the AI Gateway catalog):
- GPT-5.2 -> GPT-5.5
- Claude Opus 4.5 -> 4.8
- Claude Sonnet 4.5 -> Sonnet 5
- Gemini 2.5 Flash Lite -> Gemini 3.5 Flash Lite
- Gemini 3 Pro -> Gemini 3.1 Pro
- Grok 4 -> Grok 4.5
(Kimi K3 already latest.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(chat): reuse the real Message renderer in the first-task pre-run (DRY)

The onboarding pre-run rendered only the last assistant message's TEXT
(getReportTextFromMessages -> <Response>), dropping tool-call and reasoning
components — a downgraded, diverging copy of the chat UI. #1877's tool-heavy
email workflow made the gap obvious (raw narration, no tool components).

Reuse the normal chat's <Message>/<MessageParts> so tool calls/results get their
real components. MessageParts was coupled to useVercelChatContext (status +
reload); decouple it: both are now optional props with a context fallback, so
the normal chat is behaviorally unchanged (props omitted -> context used) while
the pre-run supplies them without mounting a provider (VercelChatProvider owns
its own useChat instance, so wrapping the pre-run would double the chat).

- VercelChatProvider: export VercelChatContext for the optional read.
- MessageParts/Message: optional status/reload props, fall back to context.
- useFirstTaskReport: expose messages + status.
- FirstTaskReportRun: render assistant messages via <Message> (filtering out the
  auto-sent prompt user message), not a text dump.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(artists): Add New Artist opens a shared Spotify search — kills the /artists loop (chat#1867) (#1878)

* fix(artists): Add New Artist opens a shared Spotify search (kills the /artists loop)

"Add New Artist" (and the sidebar/header equivalents) called toggleCreation,
which pushed `/?q=create a new artist` and relied on the home CHAT to interpret
it. The onboarding router (chat#1872) now intercepts the home for incomplete
accounts and renders "Finish setting up" instead, so the query never ran — the
"Open your roster" CTA bounced back to /artists → infinite loop.

Replace the redirect with a shared Spotify artist-search dialog:
- lib/spotify/parseSpotifyArtistResults.ts — pure parser for the
  GET /api/spotify/search?type=artist envelope (id/name/imageUrl/profileUrl/followers).
- lib/artists/addSpotifyArtist.ts — create by name (POST /api/artists) then link
  the Spotify image + profile URL (PATCH), so the roster gets real data.
- useSpotifyArtistSearch (debounced, abortable) + useAddSpotifyArtist hooks.
- components/Artists/SpotifyArtistSearch.tsx — the shared typeahead (reused next
  for social enrichment + verify-socials).
- components/Artists/AddArtistDialog.tsx — global dialog, mounted in layout.
- useArtistMode.toggleCreation now opens the dialog (isCreationOpen/closeCreation)
  instead of the redirect — fixes /artists, sidebar, and header at once.

TDD: parseSpotifyArtistResults 5/5, addSpotifyArtist 2/2, red→green. tsc clean on
touched files, eslint/prettier clean.

chat#1867 (/artists add-artist loop).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(artists): reuse canonical Spotify types; drop redundant parser (−106 LOC)

Simplification pass on the add-artist slice:
- Delete lib/spotify/parseSpotifyArtistResults.ts (+ its 5 tests) and the custom
  SpotifyArtistResult type — types/spotify.ts already exports SpotifySearchResponse
  ({ artists?: { items: SpotifyArtistSearchResult[] } }) and SpotifyArtistSearchResult
  (id/name/external_urls/images/followers). useSpotifyArtistSearch now reads
  `data.artists?.items` directly and returns the canonical type; the component and
  addSpotifyArtist read images[0].url / followers.total / external_urls.spotify inline.

Kept (checked, "no simpler"): addSpotifyArtist's two-step create→link, because
POST /api/artists only accepts a name; and useSpotifyArtistSearch, since there is
no existing debounced-search hook to reuse.

Net -106 LOC; behavior unchanged; addSpotifyArtist 2/2 green, tsc 0 new.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(onboarding): seed the catalog by kicking /api/valuation on first artist add (chat#1867) (#1879)

* feat(onboarding): seed the catalog by kicking /api/valuation on first artist add

Closes the onboarding catalog dead-end for direct signups. When the first artist
is added via the Spotify search (#1878 already links their Spotify profile),
useAddSpotifyArtist now fire-and-forget kicks POST /api/valuation
{ spotify_artist_id } (api#776) — only when the account has no catalog yet —
which materializes the catalog + value band in ~20s. So the "Claim your catalog"
step is already complete by the time the user reaches it, and they flow through
to the first-task step (weekly report emails).

- lib/valuation/runValuation.ts — client POST /api/valuation (TDD 2/2).
- hooks/useAddSpotifyArtist.ts — background kick on first add (gated on an empty,
  loaded catalogs query), surfaced via a toast.promise ("Valuing … → Your catalog
  is ready"), invalidating the catalogs query on success so the sequence advances.

chat#1867 (seed onboarding catalog on first artist add).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(models): fix organizeModels fixture — featured id gpt-5.2 → gpt-5.5 (post-#1877)

The featured-models refresh in #1877 replaced openai/gpt-5.2 with openai/gpt-5.5,
but organizeModels.test.ts still asserted gpt-5.2 was featured — failing CI on
test. Update the fixture + assertion to a currently-featured id.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant