feat: homepage valuation hero (Catalog HQ 1/3, chat#1850)#1852
feat: homepage valuation hero (Catalog HQ 1/3, chat#1850)#1852sweetmantech wants to merge 3 commits into
Conversation
For accounts with >=1 catalog, the homepage renders the estimated catalog value (artist, mid figure with display treatment, low-to-high range, measured-track count) above the chat area, fed by the new GET /api/catalogs/measurements endpoint via composed hooks (useCatalogs -> useCatalogMeasurements -> useHomeValuation). The endpoint is still rolling out: any failure (404, error envelope, no catalogs) hides the hero and keeps the existing greeting untouched. Co-Authored-By: Claude Fable 5 <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.
|
|
Warning Review limit reached
Next review available in: 6 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 Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (11)
✨ 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: 528523bada
ℹ️ 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 { data: catalogsData, isError: catalogsFailed } = useCatalogs(); | ||
| const catalogs = catalogsData?.catalogs; | ||
| const { data: measurements, isError: measurementsFailed } = | ||
| useCatalogMeasurements(catalogs?.[0]?.id); |
There was a problem hiding this comment.
When an account has multiple catalogs and the user has selected an artist that is not represented by catalogs[0], this fetches measurements for the first catalog but the returned hero is labeled with selectedArtist?.name and selectedArtist?.image. That makes the homepage show one catalog's valuation under another artist's name/avatar; choose the catalog that matches the selected artist or label the card with the catalog whose measurements were fetched.
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.
2 issues found across 12 files
Confidence score: 3/5
- In
hooks/useHomeValuation.ts, valuation data appears to be fetched from a source that can drift from the currentselectedArtist, so the hero may show the wrong artist identity for the catalog being valued; merging as-is risks user-facing trust issues and incorrect context in valuation flows — align the fetch key/source with the active artist/catalog before merging and verify with a multi-artist account case. - In
lib/catalog/getCatalogMeasurements.ts, missing try/catch logging breaks the API error-reporting consistency used elsewhere, which can make production failures harder to diagnose even if behavior otherwise matches; add the sameconsole.error+ re-throw pattern before merging to preserve observability.
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="lib/catalog/getCatalogMeasurements.ts">
<violation number="1" location="lib/catalog/getCatalogMeasurements.ts:29">
P2: Add a try/catch wrapper with console.error logging before re-throwing, matching the established error-logging pattern used by every other catalog API fetcher (getCatalogs, getCatalogSongs, getSongsByIsrc, postCatalogSongs). Without it, HTTP and network errors from this endpoint will be thrown without the diagnostic console.error that the rest of the codebase emits, making production debugging harder.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * The endpoint is being rolled out; callers must treat any thrown error as | ||
| * "no valuation available" and fall back gracefully. | ||
| */ | ||
| export async function getCatalogMeasurements( |
There was a problem hiding this comment.
P2: Add a try/catch wrapper with console.error logging before re-throwing, matching the established error-logging pattern used by every other catalog API fetcher (getCatalogs, getCatalogSongs, getSongsByIsrc, postCatalogSongs). Without it, HTTP and network errors from this endpoint will be thrown without the diagnostic console.error that the rest of the codebase emits, making production debugging harder.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/getCatalogMeasurements.ts, line 29:
<comment>Add a try/catch wrapper with console.error logging before re-throwing, matching the established error-logging pattern used by every other catalog API fetcher (getCatalogs, getCatalogSongs, getSongsByIsrc, postCatalogSongs). Without it, HTTP and network errors from this endpoint will be thrown without the diagnostic console.error that the rest of the codebase emits, making production debugging harder.</comment>
<file context>
@@ -0,0 +1,55 @@
+ * The endpoint is being rolled out; callers must treat any thrown error as
+ * "no valuation available" and fall back gracefully.
+ */
+export async function getCatalogMeasurements(
+ catalogId: string,
+ accessToken: string,
</file context>
|
Preview verification (sha 528523b, https://chat-git-feat-home-valuation-hero-recoup.vercel.app/):
|
Live preview verification — 2026-07-06 (authenticated pass)Setup: api
(screenshot hosted on the 🐛 One real finding — artist/catalog pairingThe hero shows the selected artist (Ana Bárbara, this account's active workspace artist) next to the valuation of a Del Water Gap catalog. The artist label comes from the workspace context while the catalog is just the account's first/only catalog — the two aren't linked. Correct for the target funnel case (fresh valuation lead: one artist, one catalog) but wrong pairing on multi-artist accounts like this one. Suggested fix (fine as a fast-follow, doesn't block the stack): derive the hero's label from the catalog ( Notes for the record
🤖 Generated with Claude Code |
…ngs in the catalog before pairing The hero paired the workspace's selected artist with the account's first catalog unconditionally, so switching artists kept showing another catalog's value under the new artist's name (trust-killer on multi-artist accounts, caught in live preview verification on #1852). Now: with no artist selected the hero shows the whole catalog's value under the catalog name; with an artist selected it resolves the catalog by name match (findArtistCatalog), verifies the pairing via the existing GET /api/catalogs/songs artistName filter (total_count > 0, pageSize 1), and hides while resolving, on zero match, or on check failure — falling back to the greeting. Switching artists re-resolves. Co-Authored-By: Claude Fable 5 <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. |
Fix pushed: artist-scoped hero (
|
There was a problem hiding this comment.
1 issue found across 5 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="lib/catalog/getCatalogMeasurements.ts">
<violation number="1" location="lib/catalog/getCatalogMeasurements.ts:29">
P2: Add a try/catch wrapper with console.error logging before re-throwing, matching the established error-logging pattern used by every other catalog API fetcher (getCatalogs, getCatalogSongs, getSongsByIsrc, postCatalogSongs). Without it, HTTP and network errors from this endpoint will be thrown without the diagnostic console.error that the rest of the codebase emits, making production debugging harder.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…tistName filter no-ops on unlinked songs Live preview verification caught the api-side hole: GET /api/catalogs/songs with artistName=Ana+Bárbara returned all 6 songs (total_count 6, artists:[null]) of a catalog she has no songs in — the filter rides the artists join that api#681 relaxed to a LEFT-join, so unlinked songs pass any artist filter. The hero therefore now fetches the catalog's songs unfiltered and matches client-side (isArtistCatalogMatch): catalog named for the artist, or the artist present in a song's artists array. Null- or unlinked songs match nobody. api-side filter fix tracked on #1850. Co-Authored-By: Claude Fable 5 <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. |
There was a problem hiding this comment.
1 issue found across 5 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/useHomeValuation.ts">
<violation number="1" location="hooks/useHomeValuation.ts:47">
P1: The artist verification only checks the first page of songs (up to 50), so a legitimate artist match can be missed when the catalog has more than 50 songs and the artist's songs appear on later pages. When that happens, the valuation hero will silently fall back to the greeting even though the artist belongs to the catalog — a false negative that breaks the hero for larger catalogs.
The root cause is in the shift from API-side filtering (previous `pageSize: 1, artistName: selectedArtistName` → read `total_count`) to client-side matching (fetch 50 songs, check locally). The new approach intentionally avoids the API's `artistName` filter because it no-ops on unlinked songs, but it needs to check all songs — not just the first page — to be correct.
Suggestions (not all in scope for this PR):
- Fetch multiple pages until a match is found or songs are exhausted (use `hasNextPage`/`fetchNextPage` from the existing infinite query)
- Or, bump `pageSize` to a value that covers the full catalog (tradeoff: larger payload)
- Or, fetch all pages into a flat array before checking</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| pageSize: 50, | ||
| enabled: !!catalog?.id && !!selectedArtistName, | ||
| }); | ||
| const songsPage = catalogSongs?.pages?.[0]?.songs; |
There was a problem hiding this comment.
P1: The artist verification only checks the first page of songs (up to 50), so a legitimate artist match can be missed when the catalog has more than 50 songs and the artist's songs appear on later pages. When that happens, the valuation hero will silently fall back to the greeting even though the artist belongs to the catalog — a false negative that breaks the hero for larger catalogs.
The root cause is in the shift from API-side filtering (previous pageSize: 1, artistName: selectedArtistName → read total_count) to client-side matching (fetch 50 songs, check locally). The new approach intentionally avoids the API's artistName filter because it no-ops on unlinked songs, but it needs to check all songs — not just the first page — to be correct.
Suggestions (not all in scope for this PR):
- Fetch multiple pages until a match is found or songs are exhausted (use
hasNextPage/fetchNextPagefrom the existing infinite query) - Or, bump
pageSizeto a value that covers the full catalog (tradeoff: larger payload) - Or, fetch all pages into a flat array before checking
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useHomeValuation.ts, line 47:
<comment>The artist verification only checks the first page of songs (up to 50), so a legitimate artist match can be missed when the catalog has more than 50 songs and the artist's songs appear on later pages. When that happens, the valuation hero will silently fall back to the greeting even though the artist belongs to the catalog — a false negative that breaks the hero for larger catalogs.
The root cause is in the shift from API-side filtering (previous `pageSize: 1, artistName: selectedArtistName` → read `total_count`) to client-side matching (fetch 50 songs, check locally). The new approach intentionally avoids the API's `artistName` filter because it no-ops on unlinked songs, but it needs to check all songs — not just the first page — to be correct.
Suggestions (not all in scope for this PR):
- Fetch multiple pages until a match is found or songs are exhausted (use `hasNextPage`/`fetchNextPage` from the existing infinite query)
- Or, bump `pageSize` to a value that covers the full catalog (tradeoff: larger payload)
- Or, fetch all pages into a flat array before checking</comment>
<file context>
@@ -36,21 +39,28 @@ const useHomeValuation = (): HomeValuationState => {
enabled: !!catalog?.id && !!selectedArtistName,
});
- const artistSongCount = artistSongs?.pages?.[0]?.pagination.total_count;
+ const songsPage = catalogSongs?.pages?.[0]?.songs;
+ const artistMatched =
+ catalog && selectedArtistName && songsPage
</file context>
Live verification of the artist-scoping fix — 2026-07-06 (head
|
| Check | Before (528523ba) |
After (7355a775) |
|---|---|---|
| Ana Bárbara selected, account owns a Del Water Gap catalog | ❌ Hero showed "Ana Bárbara — $150.3K" (another catalog's money) | ✅ No hero — greeting "Ask me about Ana Bárbara" |
| Switch artist (→ Apache) | ❌ Value stayed put under the new artist | ✅ Re-resolves: greeting follows the artist, hero stays hidden |
| Network path | catalogs/songs?…&artistName=… (filter no-ops, total_count: 6 for an artist with 0 songs) |
catalogs/songs?catalog_id=…&page=1&limit=50 (no filter param) + client-side match — null-linked songs match nobody |
| Console | — | clean (one pre-existing form-field a11y notice, unrelated) |
Root cause of the first attempt's failure (for the record): the api's artistName filter on GET /api/catalogs/songs doesn't filter unlinked songs — artistName=Ana+Bárbara returned all 6 songs (artists:[null]) of a catalog she has no songs in. Latent since api#681 relaxed the artists join to a LEFT-join; my first commit (537fb3f6) trusted that filter. The shipped fix (7355a775) matches client-side against catalog name / the songs' artists arrays instead. api-side filter fix logged as an Open item on chat#1850.
Positive case caveat, stated honestly: this account's roster has no artist with songs in its catalog, so the hero-visible path wasn't re-exercised live after the rework — it's covered by the unit matrix (21 lib/home tests incl. object/string artist shapes and the name-convention match) and its rendering path (unchanged ValuationHero) was live-verified earlier today ($150.3K screenshot above). The real funnel case (claimed catalog named "{Artist} Catalog" + artist-linked songs via api#684) satisfies the matcher two ways.
Stack state: 1853 → 2874fedd, 1854 → efd97b87 (rebased on this fix), 128/128 at the top, tsc 0 new errors.
🤖 Generated with Claude Code
1 participant
What this does
First PR of the Catalog HQ homepage stack (#1850, "Open — chat homepage: Catalog HQ").
For accounts with at least one catalog, the homepage now renders a valuation hero above the chat area: artist name + avatar, the estimated catalog value (display treatment on the mid figure), the low-to-high range, and the measured-track count — the same number the marketing valuation card showed the lead.
GET /api/accounts/{accountId}/catalogs(what the Catalogs page uses) → newGET /api/catalogs/measurements?catalogId=(per-ISRC latest measurements +valuation: { low, mid, high }; contract per Valuation-first homepage (Catalog HQ): surface catalog value + task runs on chat home; fix the .dev cross-domain auth regression #1850, implemented in a parallel api PR).retry: falseon the measurements query so a 404 falls back immediately.useHomeValuationcomposesuseCatalogs+ newuseCatalogMeasurements+ArtistProvider; no transport changes. The hero-visibility decision is a pure function (getValuationHeroState) with full unit coverage.hideGreetingis an optional, default-false prop threadedHomePage → NewChatBootstrap → Chat, so every otherChatmount is untouched.app/globals.css), so the figure uses the app's existing heading font at display scale rather than adding a new font in this PR.Tests (TDD — confirmed RED before implementing)
14 new unit tests, full suite 97 passed (28 files):
lib/home/__tests__/getValuationHeroState.test.ts— hero hidden while loading / no catalogs / catalogs failed / measurements failed / no valuation band; shown with band + measured-track count when data is complete (the Done-when).lib/catalog/__tests__/getCatalogMeasurements.test.ts— GET URL + bearer auth, 404 throws, error envelope throws.lib/catalog/__tests__/formatValuationAmount.test.ts—$1.4M/$959K/$2Mcompact formatting.pnpm exec tsc --noEmit: no new errors (the two pre-existing failures inlib/emails/__tests__/extractSendEmailResults.test.tsandcomponents/VercelChat/tools/SpotifyDeepResearchResult.tsxare onmainand untouched).eslintclean on all changed files (next lintitself is broken repo-wide under Next 16).Screenshots
N/A — the measurements endpoint is not live on prod/preview yet, so the hero cannot render there; the verifiable behavior on preview is the fallback (unchanged greeting), which I verified (see PR comment).
Stack
Part of a 3-PR stack for chat#1850 (merge in order, each PR only after its base):
feat/home-valuation-hero→ basemainfeat/home-tasks-module→ basefeat/home-valuation-herofeat/home-command-bar→ basefeat/home-tasks-moduleAfter this merges, PR 2 gets re-targeted to
main. Note: the tracking issue's PR table says basetest, but the repo retiredtestas a PR target (#1843) — the stack bases onmainper current CLAUDE.md.Closes nothing on its own — tracked in #1850.
🤖 Generated with Claude Code
Summary by cubic
Adds a valuation hero to the homepage for catalog accounts, showing the estimated value, range, and measured tracks; it hides the default greeting when visible. Artist-scoped and now verified client-side to avoid mislabeling, meeting #1850.
New Features
ValuationHeroabove chat when valuation data exists; otherwise keep the greeting. Shows the catalog label when no artist is selected; shows the artist label only when matched.useHomeValuationhook composinguseCatalogs,useCatalogMeasurements,useCatalogSongswith helpersfindArtistCatalog,getValuationHeroState, andisArtistCatalogMatch.getCatalogMeasurements(GET/api/catalogs/measurements) andformatValuationAmountfor compact USD; 404/error/missing data hides the hero (retry: false).Bug Fixes
isArtistCatalogMatch(catalog name match or artist in song artists; null/unlinked songs match nobody), and hide the hero while resolving or on no match.Written for commit 7355a77. Summary will update on new commits.