feat: homepage valuation hero (Catalog HQ 1/3, chat#1850)#1852
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: 8 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 (2)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds a homepage valuation feature: new API function and types for catalog measurements, ChangesHomepage Valuation Hero Feature
Catalog Utility Formatting Cleanups
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant HomePage
participant useHomeValuation
participant useCatalogMeasurements
participant getCatalogMeasurements
participant getValuationHeroState
participant ValuationHero
HomePage->>useHomeValuation: request valuation state
useHomeValuation->>useCatalogMeasurements: query(catalogId, artistAccountId)
useCatalogMeasurements->>getCatalogMeasurements: fetch(catalogId, accessToken)
getCatalogMeasurements-->>useCatalogMeasurements: CatalogMeasurementsResponse
useHomeValuation->>getValuationHeroState: derive show/hide + fields
getValuationHeroState-->>useHomeValuation: ValuationHeroState
useHomeValuation-->>HomePage: HomeValuationState
alt show is true
HomePage->>ValuationHero: render(artistName, valuation, measuredTrackCount)
HomePage->>HomePage: hideGreeting=true for NewChatBootstrap
else show is false
HomePage->>HomePage: render NewChatBootstrap only
end
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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="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>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
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
… delete the client-side matcher The hero now passes the selected artist's account_id as artist_account_id to GET /api/catalogs/measurements (docs#267 contract, api#763 impl): the api scopes measurements + valuation server-side via catalog_songs ∩ song_artists over the account's single catalog. The response echoes the applied filter and the resolver requires the echo to match before rendering an artist-labeled value — a pre-v2 deployment ignores the unknown param and the hero hides instead of showing whole-catalog money under an artist's name (deploy-skew safe). Deletes findArtistCatalog + isArtistCatalogMatch and their tests: the name-convention matching and 50-song sampling (which missed artists deeper in large catalogs) are subsumed by the server-side filter. Supersedes the per-artist-catalog prototype (rejected 2026-07-06, see #1850 decision callout). 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. |
Reworked to the v2 endpoint filter (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
hooks/useHomeValuation.ts (1)
7-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
artistNamefield is misleadingly named-it can hold the catalog name.
HomeValuationState.artistNameis documented/typed as if it's always an artist name, but whenshowArtistis false (lines 52-54) it's populated withstate.catalogNameinstead. A consumer reading the type won't realize this field can represent two different concepts. Renaming to something likedisplayName(withshowArtistalready indicating what kind of label it is) would make the contract self-documenting.As per coding guidelines, "Ensure code is self-documenting through precise naming with verbs for functions and nouns for variables."♻️ Proposed rename
export type HomeValuationState = | { show: false } | { show: true; - artistName: string; + displayName: string; artistImage: string; valuation: CatalogValuationBand; measuredTrackCount: number; }; ... return { show: true, - artistName: state.showArtist + displayName: state.showArtist ? selectedArtistName || state.catalogName : state.catalogName,Also applies to: 50-58
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hooks/useHomeValuation.ts` around lines 7 - 15, The HomeValuationState shape uses artistName for a value that can be either an artist name or a catalog name, which is misleading. Rename the field to a neutral, self-documenting name like displayName in HomeValuationState and in the logic inside useHomeValuation where state.artistName is assigned from either artistName or catalogName, then update all consumers to use the new property consistently.Source: Coding guidelines
lib/catalog/getCatalogMeasurements.ts (1)
60-64: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUnvalidated cast of API response to typed interface.
await response.json()is cast directly toCatalogMeasurementsResponsewith no runtime validation. If the API returns a shape that doesn't match (missingvaluation, wrong field types), TypeScript won't catch it and the bad data flows intogetValuationHeroState/useHomeValuationsilently until something downstream throws or renders garbage. Given this is a "trust boundary" for an actively-rolling-out endpoint (per the JSDoc above), a lightweight zod schema/safeParsewould catch contract drift at the source rather than relying on ad-hoc optional chaining downstream.🛡️ Proposed fix using zod
+import { z } from "zod"; + +const catalogMeasurementsSchema = z.object({ + status: z.string(), + measurements: z.array( + z.object({ + isrc: z.string(), + playcount: z.number().nullable().optional(), + measured_at: z.string().nullable().optional(), + }), + ), + valuation: z.object({ low: z.number(), mid: z.number(), high: z.number() }), + artist_account_id: z.string().nullable().optional(), + error: z.string().optional(), +}); + const data: CatalogMeasurementsResponse = await response.json(); + const parsed = catalogMeasurementsSchema.parse(data);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/catalog/getCatalogMeasurements.ts` around lines 60 - 64, The response from response.json() in getCatalogMeasurements is being trusted without runtime validation, so add a lightweight schema check before treating it as CatalogMeasurementsResponse. Use a zod schema or equivalent safeParse near getCatalogMeasurements to validate the expected shape (including valuation and status/error fields) and throw a clear error when the API contract drifts, instead of passing potentially invalid data into getValuationHeroState and useHomeValuation.Source: Coding guidelines
components/Home/ValuationHero.tsx (3)
5-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExport the props interface.
ValuationHeroPropsisn't exported, deviating from the project's naming/export convention for component prop types.As per coding guidelines, "Export component prop types with explicit
ComponentNamePropsnaming convention."🔧 Proposed fix
-interface ValuationHeroProps { +export interface ValuationHeroProps {🤖 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/Home/ValuationHero.tsx` around lines 5 - 10, ValuationHeroProps is defined but not exported, which breaks the component prop-type export convention. Update the ValuationHeroProps interface in ValuationHero to be exported so other modules can reference the component props type consistently. Keep the existing ComponentNameProps naming pattern intact and only adjust the interface declaration.Source: Coding guidelines
28-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
alttext on artist avatar.
ImageWithFallbackis rendered without analtattribute, so screen readers get no description of the image. SinceartistNameis already available in scope, pass it through.As per coding guidelines, "Provide proper ARIA roles/states and test with screen readers" and "Start with semantic HTML first, then augment with ARIA if needed."
♿ Proposed fix
<span className="inline-block size-10 shrink-0 overflow-hidden rounded-full ring-1 ring-foreground"> - <ImageWithFallback src={artistImage} /> + <ImageWithFallback src={artistImage} alt={artistName} /> </span>🤖 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/Home/ValuationHero.tsx` around lines 28 - 32, The artist avatar image is missing accessible alternative text in ValuationHero. Update the ImageWithFallback usage inside the artistImage conditional to pass the available artistName as the alt value so screen readers can describe the avatar. Keep the fix localized to the ImageWithFallback component call and preserve the existing wrapper markup.Source: Coding guidelines
33-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a semantic heading for the artist name.
The artist name (the section's primary label) is a
<span>rather than a heading element, which reduces navigability for screen-reader users who jump between headings.As per coding guidelines, "Use semantic HTML elements appropriate to the component's role."
🤖 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/Home/ValuationHero.tsx` around lines 33 - 40, The artist name in ValuationHero is rendered as a non-semantic span, which should be a heading for better accessibility and navigation. Update the artistName label in the ValuationHero component to use an appropriate heading element while preserving its styling, and keep the surrounding text as the supporting subtitle so screen readers can jump by section heading.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.
Inline comments:
In `@hooks/useHomeValuation.ts`:
- Around line 33-37: Resolve the catalog selection in useHomeValuation before
calling useCatalogMeasurements, since catalogs[0] is arbitrary for the
many-to-many account_catalogs relationship. Update the catalog lookup logic to
choose the intended catalog explicitly based on the current account or artist
context instead of array order, and keep the selected catalog value flowing into
useCatalogMeasurements so the correct catalog name/value and artist-scoped
measurement are used.
---
Nitpick comments:
In `@components/Home/ValuationHero.tsx`:
- Around line 5-10: ValuationHeroProps is defined but not exported, which breaks
the component prop-type export convention. Update the ValuationHeroProps
interface in ValuationHero to be exported so other modules can reference the
component props type consistently. Keep the existing ComponentNameProps naming
pattern intact and only adjust the interface declaration.
- Around line 28-32: The artist avatar image is missing accessible alternative
text in ValuationHero. Update the ImageWithFallback usage inside the artistImage
conditional to pass the available artistName as the alt value so screen readers
can describe the avatar. Keep the fix localized to the ImageWithFallback
component call and preserve the existing wrapper markup.
- Around line 33-40: The artist name in ValuationHero is rendered as a
non-semantic span, which should be a heading for better accessibility and
navigation. Update the artistName label in the ValuationHero component to use an
appropriate heading element while preserving its styling, and keep the
surrounding text as the supporting subtitle so screen readers can jump by
section heading.
In `@hooks/useHomeValuation.ts`:
- Around line 7-15: The HomeValuationState shape uses artistName for a value
that can be either an artist name or a catalog name, which is misleading. Rename
the field to a neutral, self-documenting name like displayName in
HomeValuationState and in the logic inside useHomeValuation where
state.artistName is assigned from either artistName or catalogName, then update
all consumers to use the new property consistently.
In `@lib/catalog/getCatalogMeasurements.ts`:
- Around line 60-64: The response from response.json() in getCatalogMeasurements
is being trusted without runtime validation, so add a lightweight schema check
before treating it as CatalogMeasurementsResponse. Use a zod schema or
equivalent safeParse near getCatalogMeasurements to validate the expected shape
(including valuation and status/error fields) and throw a clear error when the
API contract drifts, instead of passing potentially invalid data into
getValuationHeroState and useHomeValuation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be40aaba-5e06-4eec-ad15-6a2d1d21b790
⛔ Files ignored due to path filters (3)
lib/catalog/__tests__/formatValuationAmount.test.tsis excluded by!**/*.test.*and included bylib/**lib/catalog/__tests__/getCatalogMeasurements.test.tsis excluded by!**/*.test.*and included bylib/**lib/home/__tests__/getValuationHeroState.test.tsis excluded by!**/*.test.*and included bylib/**
📒 Files selected for processing (16)
components/Home/HomePage.tsxcomponents/Home/ValuationHero.tsxcomponents/VercelChat/NewChatBootstrap.tsxcomponents/VercelChat/chat.tsxhooks/useCatalogMeasurements.tshooks/useHomeValuation.tslib/catalog/createCatalogResult.tslib/catalog/createSearchResult.tslib/catalog/formatArtists.tslib/catalog/formatValuationAmount.tslib/catalog/getCatalogMeasurements.tslib/catalog/getCatalogSongs.tslib/catalog/parseCsvFile.tslib/catalog/postCatalogSongs.tslib/catalog/uploadBatchSongs.tslib/home/getValuationHeroState.ts
| const { data: catalogsData, isError: catalogsFailed } = useCatalogs(); | ||
| const catalog = catalogsData?.catalogs?.[0]; | ||
|
|
||
| const { data: measurements, isError: measurementsFailed } = | ||
| useCatalogMeasurements(catalog?.id, selectedArtistAccountId ?? undefined); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether accounts can have multiple catalogs and how catalog/artist ownership is modeled.
ast-grep outline hooks/useCatalogs.ts
rg -n -A5 -B5 'catalogs' hooks/useCatalogs.ts
rg -n 'interface Catalog' -A10 types/Catalog.tsRepository: recoupable/chat
Length of output: 778
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the home valuation hook and measurement hook signatures.
ast-grep outline hooks/useHomeValuation.ts
ast-grep outline hooks/useCatalogMeasurements.ts
printf '\n--- useHomeValuation.ts ---\n'
cat -n hooks/useHomeValuation.ts | sed -n '1,220p'
printf '\n--- useCatalogMeasurements.ts ---\n'
cat -n hooks/useCatalogMeasurements.ts | sed -n '1,220p'Repository: recoupable/chat
Length of output: 4414
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find catalog-related data model and any code that treats the first catalog specially.
rg -n "catalogs\\?\\[0\\]|catalogsData\\.catalogs|selectedArtistAccountId|artist_account_id|catalog_id" hooks src . --glob '!node_modules' --glob '!dist' --glob '!build'Repository: recoupable/chat
Length of output: 8901
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the API layer for catalogs and measurements to understand ownership/filtering.
rg -n -A4 -B4 "getCatalogs|getCatalogMeasurements|CatalogsResponse|MeasurementsResponse" . --glob '!node_modules' --glob '!dist' --glob '!build'Repository: recoupable/chat
Length of output: 16726
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the valuation state logic and its tests for any contract about catalog selection.
ast-grep outline lib/home/getValuationHeroState.ts
printf '\n--- getValuationHeroState.ts ---\n'
cat -n lib/home/getValuationHeroState.ts | sed -n '1,220p'
printf '\n--- getValuationHeroState.test.ts ---\n'
cat -n lib/home/__tests__/getValuationHeroState.test.ts | sed -n '1,260p'
printf '\n--- catalog selection references ---\n'
rg -n "catalogs\\[0\\]|selected catalog|first catalog|exactly one catalog|multiple catalogs|catalog.*order" lib hooks components types --glob '!node_modules' --glob '!dist' --glob '!build'Repository: recoupable/chat
Length of output: 11202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any selected-catalog state or artist-to-catalog ownership mapping.
rg -n "selectedCatalog|currentCatalog|catalog.*selected|setSelectedCatalog|catalog_id.*artist|artist.*catalog_id|catalogs\\[0\\]" hooks components providers lib types --glob '!node_modules' --glob '!dist' --glob '!build'Repository: recoupable/chat
Length of output: 894
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the catalog schema and any relations to accounts/artists.
rg -n -A12 -B4 "catalogs:|catalog_id|account_id.*catalog|catalog.*account|foreignKeyName.*catalog" types/database.types.ts lib hooks components --glob '!node_modules' --glob '!dist' --glob '!build'
printf '\n--- catalog table slices ---\n'
sed -n '1,220p' types/Catalog.ts
printf '\n--- getCatalogs.ts ---\n'
sed -n '1,220p' lib/catalog/getCatalogs.tsRepository: recoupable/chat
Length of output: 20397
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the catalog table definition and its foreign keys in the generated database types.
rg -n -A20 -B6 "catalogs: \\{" types/database.types.ts
rg -n -A12 -B4 "catalogs_.*fkey|catalog_id.*fkey|account_id.*fkey" types/database.types.tsRepository: recoupable/chat
Length of output: 10995
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the catalog table definition and its foreign keys in the generated database types.
rg -n -A20 -B6 "catalogs: \\{" types/database.types.ts
rg -n -A12 -B4 "catalogs_.*fkey|catalog_id.*fkey|account_id.*fkey" types/database.types.ts
printf '\n--- getCatalogs.ts ---\n'
sed -n '1,220p' lib/catalog/getCatalogs.tsRepository: recoupable/chat
Length of output: 12323
Resolve the catalog before calling measurements
account_catalogs is many-to-many, so catalogs[0] is arbitrary here. For accounts with more than one catalog, this can show the wrong catalog name/value or hide a valid artist-scoped measurement. Pick the intended catalog explicitly instead of relying on array order.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hooks/useHomeValuation.ts` around lines 33 - 37, Resolve the catalog
selection in useHomeValuation before calling useCatalogMeasurements, since
catalogs[0] is arbitrary for the many-to-many account_catalogs relationship.
Update the catalog lookup logic to choose the intended catalog explicitly based
on the current account or artist context instead of array order, and keep the
selected catalog value flowing into useCatalogMeasurements so the correct
catalog name/value and artist-scoped measurement are used.
There was a problem hiding this comment.
1 issue found across 17 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>
<file name="hooks/useHomeValuation.ts">
<violation number="1" location="hooks/useHomeValuation.ts:34">
P2: Artist-scoped valuation can disappear for multi-catalog accounts because this always queries the first catalog before applying `artist_account_id`. Consider selecting the catalog that contains the selected artist, or changing the data path to query the artist scope across all account catalogs instead of anchoring to `catalogs[0]`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…_song_count aggregate Amendment per the 2026-07-07 decision on chat#1850 (docs#267 respec, api#763 amendment, database#42 RPCs): the v2 measurements response now carries whole-scope SQL aggregates and a caller-paginated rows array. - getCatalogMeasurements/useCatalogMeasurements accept a limit param (react-query key includes it); the hero requests limit=1 since it only needs the aggregates, not the rows - getValuationHeroState reads measured_song_count instead of measurements.length: zero -> hide (nothing measured in scope), absent -> hide (pre-v2 shape whose numbers come from a capped read) - echo-field gating unchanged: an artist-selected hero renders only when the response echoes that exact artist_account_id TDD: tests adjusted RED first; full suite 106 tests, tsc 8 pre-existing errors (0 new), prettier clean. 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.
0 issues found across 6 files (changes from recent commits).
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
…gId}/measurements REST amendment on chat#1850: path for identity, query for modifiers. getCatalogMeasurements builds the dynamic-segment URL; the query string carries only artist_account_id/limit. No behavior change in the hero. 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. |
Amended to the final v2 contract (
|
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
| * hero above the chat area instead (recoupable/chat#1850). Defaults to | ||
| * false so every existing mount keeps the greeting. | ||
| */ | ||
| hideGreeting?: boolean; |
There was a problem hiding this comment.
YAGNI - this hideGreeting is a code smell. Why is it required? Why are we supporting 2 distinct, UI flows instead of unifying to one, consistent flow? If catalog info is missing, we should add a CTA rather than a generic greeting. right? Why does Vercel Chat need knowledge of this? What would happen if we deleted any greeting from this chat component to follow SRP keeping the responsibility in a standalone greeting / catalog component?
There was a problem hiding this comment.
You were right on all four counts — fixed in 72262809:
Why did it exist? It was scar tissue: PR 1 needed the greeting suppressed when the hero rendered, and the stack 'fixed' it two PRs later (1854 replaced it with a modules prop) — which was the same smell relocated. Neither should have put UI-flow knowledge in the chat component.
What happened when we deleted greeting knowledge from chat.tsx: it got simpler and more general. Chat now exposes one generic emptyStateHeader?: ReactNode slot and renders whatever it's given — zero greeting/hero imports, no branching. There's also now exactly ONE empty-state layout for every mount (flex spacers inside a scrollable container: short content centers, tall content scrolls with the input docked beneath) — the two distinct flows are gone at the layout level too.
Where the responsibility went: a standalone components/Home/HomeHeader.tsx owns the homepage decision — valuation hero when measured data exists for the scope, otherwise ChatGreeting plus a 'Get a free catalog valuation →' CTA into the funnel (your point about the dead-end greeting). The session chat route passes <ChatGreeting /> explicitly; / and /chat pass <HomeHeader />.
hideGreeting is deleted repo-wide (0 refs). Stack re-rebased with 1854's modules prop unified into the same slot (suggestedPrompts survives as its real addition): 1853 1aec69d4, 1854 94d4a48f — 123/123 at the top, tsc 0 new, prettier clean.
There was a problem hiding this comment.
Superseding my previous reply — you then asked the sharper question (why touch chat.tsx at all / emptyStateHeader is prop drilling), and you were right on both: the slot was still the same smell, relocated. Reworked in 540a53c4:
chat.tsx, NewChatBootstrap and HomePage are now byte-identical to main in this PR (verified with git diff origin/main — empty). The extension point already existed: chat.tsx has always delegated its empty-state header to ChatGreeting. That component now IS the standalone greeting/catalog component from your first comment — it renders the artist-scoped valuation hero when measured data exists for the current scope, otherwise the greeting plus the 'Get a free catalog valuation →' CTA. All context arrives through provider-backed hooks (useHomeValuation → ArtistProvider + react-query), zero new props — context management, not prop threads.
Bonus from doing it this way: every empty-chat surface (/, /chat, /sessions/{id}/chats/{id}) gets the same single flow for free, because they all already render ChatGreeting.
Stack restacked from the pre-slot commits: 1853 e2af7d41 (tasks module, HomePage-only), 1854 962e621b (its modules/chips design restored — chat.tsx changes now live only in that PR, where they can be debated on their own merits). 123/123 at the top, tsc 0 new, prettier clean.
…ader slot + unified HomeHeader (SRP/YAGNI) Review feedback on #1852 (chat.tsx hideGreeting): the chat component was carrying greeting knowledge and two UI flows. Chat now exposes a generic emptyStateHeader slot and renders whatever it's given; hideGreeting is deleted. Each mount composes its own header: the homepage and /chat pass HomeHeader — ONE flow that shows the artist-scoped valuation hero when measured data exists, otherwise the greeting plus a 'Get a free catalog valuation' CTA into the funnel (no more bare greeting dead-end) — and the session chat route passes ChatGreeting explicitly. 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.
0 issues found across 8 files (changes from recent commits).
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
…tsx changes, no prop drilling (KISS/OCP) Review feedback on #1852: chat.tsx should not be modified in this PR, and header composition via props is drilling context that providers already own. ChatGreeting — the component chat.tsx already delegates its empty-state header to — becomes the artist header: the artist-scoped valuation hero when measured data exists for the current scope (via useHomeValuation, provider-backed), otherwise the greeting plus a 'Get a free catalog valuation' CTA into the funnel. chat.tsx, NewChatBootstrap and HomePage are byte-identical to main in this PR; every empty-chat surface gets the same single flow for free. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
sweetmantech
force-pushed
the
feat/home-valuation-hero
branch
from
7226280 to
540a53c
Compare
|
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. |
Final live verification —
|
| Selected artist | Hero showed | SQL ground truth (mid / measured) | Match |
|---|---|---|---|
| Ana Bárbara | $8.1M · $5.6M–$11.4M · 154 tracks | $8.1M / 154 | ✅ |
| Apache (switched) | $6M · $4.1M–$8.5M · 208 tracks | $6,030,019 / 208 | ✅ |
Network proof: GET /api/catalogs/7d3e5c35…/measurements?artist_account_id=ebae4bb9…&limit=1 — the v2 RESTful path, artist filter applied server-side, limit=1 (the hero reads measured_song_count, fetches no rows), echo gate satisfied (hero rendered ⇒ response echoed the requested scope). The pre-switch hydration fetch without the artist param correctly did not render under an artist label (echo null ≠ selected artist).
Unified surface check: /chat renders the same header ($6M/208 for Apache) — one flow everywhere ChatGreeting mounts, as designed.
Not exercised live: the greeting + "Get a free catalog valuation →" CTA branch — every artist in this account's roster now has measured songs (the fixture is thorough to a fault), so no zero-data artist was available; the branch is unit-covered and renders for any account/artist without measurements.
Merge order reminder: docs#267 ✅ · database#42 ✅ · api#763 ✅ (all live) → this PR is ready to merge → re-target chat#1853 (e2af7d41) → chat#1854 (962e621b).
🤖 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 estimated value, range, and measured track count above chat. It renders inside
ChatGreetingand falls back to the greeting with a “Get a free catalog valuation” CTA when data isn’t available, aligned with #1850.New Features
ChatGreetingnow showsValuationHerowhen valuation data exists; otherwise it shows the greeting plus a CTA toVALUATION_URL.useHomeValuationcomposesuseCatalogs+useCatalogMeasurementsand gates visibility viagetValuationHeroState; data comes fromGET /api/catalogs/{catalogId}/measurements[?artist_account_id][&limit=1]with compact USD viaformatValuationAmount. Any error or missingmeasured_song_counthides the hero.Refactors
ChatGreeting; noChatchanges or prop drilling.artist_account_idfilter and only renders when the response echoes the same scope; pre‑v2 responses or zeromeasured_song_countfall back to the greeting. Removed the oldfindArtistCatalog/isArtistCatalogMatchpath.Written for commit 540a53c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes