feat: post-valuation catalog report on /catalogs/[catalogId] (chat#1867 sequence step 1) - #1873
Merged
Merged
Conversation
…67 sequence step 1)
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>
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. |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds a tabbed catalog report replacing the songs-only page, with valuation calculations, measurement rollups, release artwork, insights, loading and error states, and a weekly-report CTA. ChangesCatalog report
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
25 tasks
![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: b06ec65705
ℹ️ 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".
Comment on lines
+52
to
+56
| const totalStreams = | ||
| measurements.total_streams ?? | ||
| measurements.measurements.reduce((sum, m) => sum + (m?.playcount ?? 0), 0); | ||
| const measuredSongCount = | ||
| measurements.measured_song_count ?? measurements.measurements.length; |
There was a problem hiding this comment.
When the measurements API omits total_streams or measured_song_count (the pre-v2 response shape this component claims to support), this fallback treats the limit=100 page requested above as the whole catalog. Any catalog with more than 100 measured tracks will show an understated stream total/valuation and misleading coverage; use the API's authoritative valuation/aggregates or keep the report unavailable until whole-scope totals are present.
Useful? React with 👍 / 👎.
| ): UseQueryResult<CatalogSongsResponse> => { | ||
| return useQuery({ | ||
| queryKey: ["catalogReportSongs", catalogId], | ||
| queryFn: () => getCatalogSongs(catalogId, REPORT_SONGS_PAGE_SIZE, 1), |
There was a problem hiding this comment.
For catalogs with more than 100 songs, this only loads page 1, but CatalogReportContent uses songsQuery.data?.songs to build the release table while separately using whole-catalog counts/stream aggregates. The report can therefore omit later releases and undercount release streams even though the stats indicate a larger catalog; fetch all pages or move the release rollup to the backend.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/catalog/buildReleaseRollups.ts (1)
50-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptimize Map lookup.
You can avoid looking up the ISRC in the Map twice by retrieving the value once and checking if it exists. This is a common practice for slightly cleaner and faster Map interactions. As per path instructions, prefer simple, readable implementations over clever optimizations, which in this case means utilizing the return value of
.get()directly.💡 Proposed refactor
- if (song.isrc && playcountByIsrc.has(song.isrc)) { - rollup.measuredTrackCount += 1; - rollup.streams += playcountByIsrc.get(song.isrc) ?? 0; - } + if (song.isrc) { + const playcount = playcountByIsrc.get(song.isrc); + if (playcount !== undefined) { + rollup.measuredTrackCount += 1; + rollup.streams += playcount; + } + }🤖 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/buildReleaseRollups.ts` around lines 50 - 53, Update the rollup logic around the song ISRC lookup to retrieve the playcount from playcountByIsrc once, then use that retrieved value to determine whether to increment measuredTrackCount and streams. Preserve the existing behavior for missing ISRCs and zero-valued playcounts.Source: Path instructions
components/Catalog/report/CatalogReportPage.tsx (1)
24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
<Link>for navigation over imperative routing.Using an anchor element (
<Link>) for routing provides better accessibility and allows Next.js to prefetch the route. As per coding guidelines, use semantic HTML elements appropriate to the component's role. A navigation link should be an<a>tag rather than a<button>.♻️ Proposed refactor
Update the element:
- <button - type="button" - onClick={() => router.push("/catalogs")} - className="flex items-center gap-1 rounded px-2 py-1 text-sm text-muted-foreground transition-colors hover:text-foreground focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2" - > + <Link + href="/catalogs" + className="flex items-center gap-1 rounded px-2 py-1 text-sm text-muted-foreground transition-colors hover:text-foreground focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2" + > <ArrowLeft className="h-4 w-4" /> Back to Catalogs - </button> + </Link>And update your imports at the top of the file:
import Link from "next/link"; // Remove: import { useRouter } from "next/navigation";🤖 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/Catalog/report/CatalogReportPage.tsx` around lines 24 - 31, Replace the imperative navigation button in CatalogReportPage with a Next.js Link targeting "/catalogs", preserving the existing styling and arrow label. Remove the now-unused useRouter usage and import, and add the Link import.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 `@components/Catalog/report/CatalogReleasesTable.tsx`:
- Around line 26-30: Update the table header <th> elements in
CatalogReleasesTable to include scope="col", preserving their existing classes,
text, and column order.
In `@lib/spotify/getAlbumArtwork.ts`:
- Around line 21-24: Sanitize embedded double quotes in album and artistName
before constructing q in getAlbumArtwork, escaping or removing them so quoted
Spotify search terms remain valid. Preserve the existing conditional query
structure and URLSearchParams configuration.
---
Nitpick comments:
In `@components/Catalog/report/CatalogReportPage.tsx`:
- Around line 24-31: Replace the imperative navigation button in
CatalogReportPage with a Next.js Link targeting "/catalogs", preserving the
existing styling and arrow label. Remove the now-unused useRouter usage and
import, and add the Link import.
In `@lib/catalog/buildReleaseRollups.ts`:
- Around line 50-53: Update the rollup logic around the song ISRC lookup to
retrieve the playcount from playcountByIsrc once, then use that retrieved value
to determine whether to increment measuredTrackCount and streams. Preserve the
existing behavior for missing ISRCs and zero-valued playcounts.
🪄 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: 9421822d-322b-4dc7-8a2a-d5ff92fdfc1b
⛔ Files ignored due to path filters (4)
lib/catalog/__tests__/buildReleaseRollups.test.tsis excluded by!**/*.test.*and included bylib/**lib/valuation/__tests__/buildReportInsights.test.tsis excluded by!**/*.test.*and included bylib/**lib/valuation/__tests__/computeCatalogValuation.test.tsis excluded by!**/*.test.*and included bylib/**lib/valuation/__tests__/nlsBandFromSpotifyGross.test.tsis excluded by!**/*.test.*and included bylib/**
📒 Files selected for processing (22)
app/catalogs/[catalogId]/page.tsxcomponents/Catalog/CatalogSongsPage.tsxcomponents/Catalog/report/CatalogReleaseRow.tsxcomponents/Catalog/report/CatalogReleasesTable.tsxcomponents/Catalog/report/CatalogReportContent.tsxcomponents/Catalog/report/CatalogReportCta.tsxcomponents/Catalog/report/CatalogReportError.tsxcomponents/Catalog/report/CatalogReportInsights.tsxcomponents/Catalog/report/CatalogReportPage.tsxcomponents/Catalog/report/CatalogReportSkeleton.tsxcomponents/Catalog/report/CatalogReportStats.tsxcomponents/Catalog/report/CatalogValuationCard.tsxcomponents/Catalog/report/ReleaseArtTile.tsxhooks/useAlbumArtwork.tshooks/useCatalogReportSongs.tslib/catalog/buildReleaseRollups.tslib/catalog/getCatalogMeasurements.tslib/spotify/getAlbumArtwork.tslib/valuation/buildReportInsights.tslib/valuation/computeCatalogValuation.tslib/valuation/formatCompact.tslib/valuation/nlsBandFromSpotifyGross.ts
💤 Files with no reviewable changes (1)
- components/Catalog/CatalogSongsPage.tsx
Comment on lines
+26
to
+30
| <th className="py-2 pr-3 font-medium">Release</th> | ||
| <th className="py-2 px-3 font-medium text-right"> | ||
| Tracks measured | ||
| </th> | ||
| <th className="py-2 pl-3 font-medium text-right">Streams</th> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add scope="col" to table headers for accessibility.
To improve screen reader support and explicitly associate these headers with their respective columns, consider adding the scope="col" attribute to the <th> elements. As per path instructions, React components must always include accessibility support and semantic HTML.
♿ Proposed accessibility fix
- <th className="py-2 pr-3 font-medium">Release</th>
- <th className="py-2 px-3 font-medium text-right">
- Tracks measured
- </th>
- <th className="py-2 pl-3 font-medium text-right">Streams</th>
+ <th scope="col" className="py-2 pr-3 font-medium">Release</th>
+ <th scope="col" className="py-2 px-3 font-medium text-right">
+ Tracks measured
+ </th>
+ <th scope="col" className="py-2 pl-3 font-medium text-right">Streams</th>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <th className="py-2 pr-3 font-medium">Release</th> | |
| <th className="py-2 px-3 font-medium text-right"> | |
| Tracks measured | |
| </th> | |
| <th className="py-2 pl-3 font-medium text-right">Streams</th> | |
| <th scope="col" className="py-2 pr-3 font-medium">Release</th> | |
| <th scope="col" className="py-2 px-3 font-medium text-right"> | |
| Tracks measured | |
| </th> | |
| <th scope="col" className="py-2 pl-3 font-medium text-right">Streams</th> |
🤖 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/Catalog/report/CatalogReleasesTable.tsx` around lines 26 - 30,
Update the table header <th> elements in CatalogReleasesTable to include
scope="col", preserving their existing classes, text, and column order.
Source: Path instructions
Comment on lines
+21
to
+24
| const q = artistName | ||
| ? `album:"${album}" artist:"${artistName}"` | ||
| : `album:"${album}"`; | ||
| const params = new URLSearchParams({ q, type: "album", limit: "1" }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Sanitize double quotes to prevent search query failures.
Interpolating album and artistName directly into the query string wrapped in double quotes can break the Spotify search syntax if either string itself contains double quotes (e.g., Director's "Cut"). Consider removing or escaping them to ensure the query remains valid.
🛠️ Proposed fix to sanitize inputs
- const q = artistName
- ? `album:"${album}" artist:"${artistName}"`
- : `album:"${album}"`;
+ const safeAlbum = album.replace(/"/g, '');
+ const safeArtist = artistName?.replace(/"/g, '');
+ const q = safeArtist
+ ? `album:"${safeAlbum}" artist:"${safeArtist}"`
+ : `album:"${safeAlbum}"`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const q = artistName | |
| ? `album:"${album}" artist:"${artistName}"` | |
| : `album:"${album}"`; | |
| const params = new URLSearchParams({ q, type: "album", limit: "1" }); | |
| const safeAlbum = album.replace(/"/g, ''); | |
| const safeArtist = artistName?.replace(/"/g, ''); | |
| const q = safeArtist | |
| ? `album:"${safeAlbum}" artist:"${safeArtist}"` | |
| : `album:"${safeAlbum}"`; | |
| const params = new URLSearchParams({ q, type: "album", limit: "1" }); |
🤖 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/spotify/getAlbumArtwork.ts` around lines 21 - 24, Sanitize embedded
double quotes in album and artistName before constructing q in getAlbumArtwork,
escaping or removing them so quoted Spotify search terms remain valid. Preserve
the existing conditional query structure and URLSearchParams configuration.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found across 26 files
Confidence score: 2/5
components/Catalog/report/CatalogReportContent.tsxandhooks/useCatalogReportSongs.tscurrently mix whole-catalog headline stats with first-page-only song/release data (including the pre-v2total_streamsfallback), so larger catalogs can show incomplete rows and miss concentration signals while appearing complete — fetch all needed pages (or consistently scope every metric to the same page limit) before merging.lib/valuation/computeCatalogValuation.tstreats catalogs younger than one year as a 5-year fallback, which can materially understate annual run-rate and valuation in generated reports — accept any finite positive age and reserve fallback only for zero/negative values.components/Catalog/report/CatalogReportContent.tsxcan render a “successful” report whensongsQueryfails by falling back to empty songs, leading to silent data loss in the releases table and misleading totals context — surface an explicit error/loading state tied tosongsQuerysuccess before allowing this path.lib/catalog/buildReleaseRollups.tscan merge distinct releases that share an album title, inflating per-release track/stream totals and distorting breakdowns — key rollups by a stable release identifier (or title+artist discriminator) before merge to avoid incorrect aggregation.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="components/Catalog/report/CatalogReportError.tsx">
<violation number="1" location="components/Catalog/report/CatalogReportError.tsx:13">
P2: The error card renders dynamically after a failed API call but has no ARIA live region, so screen reader users won't hear the error state change. Add `role="alert"` to the outer container so assistive technology announces the message when it appears.</violation>
</file>
<file name="components/Catalog/report/CatalogReportContent.tsx">
<violation number="1" location="components/Catalog/report/CatalogReportContent.tsx:33">
P1: Catalogs over 100 tracks produce incomplete release rows and can miss the concentration diagnosis, while the headline streams/counts cover the full catalog. Fetch all required pages (or clearly constrain every report metric to the same page) before building rollups and insights.</violation>
<violation number="2" location="components/Catalog/report/CatalogReportContent.tsx:38">
P2: When `songsQuery` fails but `measurementsQuery` succeeds, the report renders with empty song data and no error indication. `songsQuery.data` is undefined, so `songs` falls back to `[]` (empty releases table), and `totalSongs` falls back to `measuredSongCount`, causing the stats to display "X of X" (e.g. "100 of 100") — misleadingly suggesting all songs were measured when they weren't. Consider checking `songsQuery.error` and either showing a banner or falling through to an error state when the songs fetch fails, to avoid presenting data that implies complete measurement coverage.</violation>
<violation number="3" location="components/Catalog/report/CatalogReportContent.tsx:54">
P2: When `total_streams` is absent (pre-v2 response), the fallback sums only the capped 100-measurement page — any catalog with more than 100 measured tracks will show an understated stream total and therefore an understated valuation. Consider either surfacing this as a known limitation in the UI (e.g., "based on first 100 tracks") or keeping the report unavailable until whole-scope aggregates are present from the API.</violation>
</file>
<file name="components/Catalog/report/ReleaseArtTile.tsx">
<violation number="1" location="components/Catalog/report/ReleaseArtTile.tsx:17">
P2: Opening a catalog with many releases dispatches one Spotify-search request per row at once, potentially up to the report's 100-song fetch scope. This can make initial rendering slow and rate-limit the shared search endpoint; consider returning artwork with the report data, batching lookups, or only resolving artwork for a bounded/visible set of rows.</violation>
</file>
<file name="lib/catalog/buildReleaseRollups.ts">
<violation number="1" location="lib/catalog/buildReleaseRollups.ts:35">
P2: Catalogs containing distinct releases with the same album title will show one merged release and inflated track/stream totals. Use a release identifier when available, or include a stable artist/release discriminator in the grouping key.</violation>
</file>
<file name="lib/valuation/computeCatalogValuation.ts">
<violation number="1" location="lib/valuation/computeCatalogValuation.ts:57">
P2: Catalogs younger than one year are valued using the 5-year fallback rather than their reported age, materially understating their annual run-rate and report value. Accept any finite positive age; only zero or negative ages need the fallback.</violation>
</file>
<file name="components/Catalog/report/CatalogReleasesTable.tsx">
<violation number="1" location="components/Catalog/report/CatalogReleasesTable.tsx:22">
P2: The overflow-x-auto container isn't keyboard-accessible — keyboard-only users can't horizontally scroll the table when it overflows</violation>
</file>
<file name="lib/spotify/getAlbumArtwork.ts">
<violation number="1" location="lib/spotify/getAlbumArtwork.ts:22">
P3: Albums or artists containing `"` can generate a malformed or altered Spotify query, causing artwork lookup misses or wrong matches. Escape filter delimiters in both values before constructing `q`.</violation>
</file>
<file name="components/Catalog/report/CatalogReportSkeleton.tsx">
<violation number="1" location="components/Catalog/report/CatalogReportSkeleton.tsx:13">
P3: The skeleton's outermost card div is missing `bg-card` that all sibling report components include alongside this shadow pattern. When the skeleton transitions to the real content with `bg-card`, the background will visibly flash, making the load feel less polished.</violation>
</file>
<file name="hooks/useCatalogReportSongs.ts">
<violation number="1" location="hooks/useCatalogReportSongs.ts:19">
P2: For catalogs with more than 100 songs, only the first page is fetched here, but `CatalogReportContent` uses the result to build the per-release table while the stat row shows whole-catalog counts. This means the release table can omit later releases entirely, creating an inconsistency between the stats ("X releases") and the visible table. Either paginate through all songs, move the rollup to the backend, or clearly label the release table as showing a subset.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const measurementsQuery = useCatalogMeasurements( | ||
| catalogId, | ||
| undefined, | ||
| MEASUREMENTS_PAGE_LIMIT, |
There was a problem hiding this comment.
P1: Catalogs over 100 tracks produce incomplete release rows and can miss the concentration diagnosis, while the headline streams/counts cover the full catalog. Fetch all required pages (or clearly constrain every report metric to the same page) before building rollups and insights.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Catalog/report/CatalogReportContent.tsx, line 33:
<comment>Catalogs over 100 tracks produce incomplete release rows and can miss the concentration diagnosis, while the headline streams/counts cover the full catalog. Fetch all required pages (or clearly constrain every report metric to the same page) before building rollups and insights.</comment>
<file context>
@@ -0,0 +1,87 @@
+ const measurementsQuery = useCatalogMeasurements(
+ catalogId,
+ undefined,
+ MEASUREMENTS_PAGE_LIMIT,
+ );
+ const songsQuery = useCatalogReportSongs(catalogId);
</file context>
| const notFound = error?.message?.includes("404"); | ||
|
|
||
| return ( | ||
| <div className="max-w-3xl rounded-2xl bg-card p-6 sm:p-8 shadow-[0_0_0_1px_var(--border)]"> |
There was a problem hiding this comment.
P2: The error card renders dynamically after a failed API call but has no ARIA live region, so screen reader users won't hear the error state change. Add role="alert" to the outer container so assistive technology announces the message when it appears.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Catalog/report/CatalogReportError.tsx, line 13:
<comment>The error card renders dynamically after a failed API call but has no ARIA live region, so screen reader users won't hear the error state change. Add `role="alert"` to the outer container so assistive technology announces the message when it appears.</comment>
<file context>
@@ -0,0 +1,28 @@
+ const notFound = error?.message?.includes("404");
+
+ return (
+ <div className="max-w-3xl rounded-2xl bg-card p-6 sm:p-8 shadow-[0_0_0_1px_var(--border)]">
+ <h2 className="font-heading text-sm font-bold text-foreground">
+ {notFound
</file context>
Suggested change
| <div className="max-w-3xl rounded-2xl bg-card p-6 sm:p-8 shadow-[0_0_0_1px_var(--border)]"> | |
| <div className="max-w-3xl rounded-2xl bg-card p-6 sm:p-8 shadow-[0_0_0_1px_var(--border)]" role="alert"> |
| * never a broken image, never a crash. | ||
| */ | ||
| const ReleaseArtTile = ({ album, artistName }: ReleaseArtTileProps) => { | ||
| const { data: artworkUrl } = useAlbumArtwork(album, artistName); |
There was a problem hiding this comment.
P2: Opening a catalog with many releases dispatches one Spotify-search request per row at once, potentially up to the report's 100-song fetch scope. This can make initial rendering slow and rate-limit the shared search endpoint; consider returning artwork with the report data, batching lookups, or only resolving artwork for a bounded/visible set of rows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Catalog/report/ReleaseArtTile.tsx, line 17:
<comment>Opening a catalog with many releases dispatches one Spotify-search request per row at once, potentially up to the report's 100-song fetch scope. This can make initial rendering slow and rate-limit the shared search endpoint; consider returning artwork with the report data, batching lookups, or only resolving artwork for a bounded/visible set of rows.</comment>
<file context>
@@ -0,0 +1,42 @@
+ * never a broken image, never a crash.
+ */
+const ReleaseArtTile = ({ album, artistName }: ReleaseArtTileProps) => {
+ const { data: artworkUrl } = useAlbumArtwork(album, artistName);
+
+ if (artworkUrl) {
</file context>
| for (const song of songs ?? []) { | ||
| if (!song) continue; | ||
| const album = song.album?.trim() || null; | ||
| const key = album ?? UNTITLED_KEY; |
There was a problem hiding this comment.
P2: Catalogs containing distinct releases with the same album title will show one merged release and inflated track/stream totals. Use a release identifier when available, or include a stable artist/release discriminator in the grouping key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/buildReleaseRollups.ts, line 35:
<comment>Catalogs containing distinct releases with the same album title will show one merged release and inflated track/stream totals. Use a release identifier when available, or include a stable artist/release discriminator in the grouping key.</comment>
<file context>
@@ -0,0 +1,65 @@
+ for (const song of songs ?? []) {
+ if (!song) continue;
+ const album = song.album?.trim() || null;
+ const key = album ?? UNTITLED_KEY;
+
+ let rollup = rollups.get(key);
</file context>
| const hasValidAge = | ||
| typeof reportedAge === "number" && | ||
| Number.isFinite(reportedAge) && | ||
| reportedAge >= 1; |
There was a problem hiding this comment.
P2: Catalogs younger than one year are valued using the 5-year fallback rather than their reported age, materially understating their annual run-rate and report value. Accept any finite positive age; only zero or negative ages need the fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/valuation/computeCatalogValuation.ts, line 57:
<comment>Catalogs younger than one year are valued using the 5-year fallback rather than their reported age, materially understating their annual run-rate and report value. Accept any finite positive age; only zero or negative ages need the fallback.</comment>
<file context>
@@ -0,0 +1,92 @@
+ const hasValidAge =
+ typeof reportedAge === "number" &&
+ Number.isFinite(reportedAge) &&
+ reportedAge >= 1;
+ const catalogAgeYears = hasValidAge ? reportedAge : DEFAULT_AGE_YEARS;
+ const ageSource: CatalogValuation["assumptions"]["ageSource"] = hasValidAge
</file context>
| const songsQuery = useCatalogReportSongs(catalogId); | ||
|
|
||
| const measurements = measurementsQuery.data; | ||
| const songs = songsQuery.data?.songs; |
There was a problem hiding this comment.
P2: When songsQuery fails but measurementsQuery succeeds, the report renders with empty song data and no error indication. songsQuery.data is undefined, so songs falls back to [] (empty releases table), and totalSongs falls back to measuredSongCount, causing the stats to display "X of X" (e.g. "100 of 100") — misleadingly suggesting all songs were measured when they weren't. Consider checking songsQuery.error and either showing a banner or falling through to an error state when the songs fetch fails, to avoid presenting data that implies complete measurement coverage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Catalog/report/CatalogReportContent.tsx, line 38:
<comment>When `songsQuery` fails but `measurementsQuery` succeeds, the report renders with empty song data and no error indication. `songsQuery.data` is undefined, so `songs` falls back to `[]` (empty releases table), and `totalSongs` falls back to `measuredSongCount`, causing the stats to display "X of X" (e.g. "100 of 100") — misleadingly suggesting all songs were measured when they weren't. Consider checking `songsQuery.error` and either showing a banner or falling through to an error state when the songs fetch fails, to avoid presenting data that implies complete measurement coverage.</comment>
<file context>
@@ -0,0 +1,87 @@
+ const songsQuery = useCatalogReportSongs(catalogId);
+
+ const measurements = measurementsQuery.data;
+ const songs = songsQuery.data?.songs;
+
+ const releases = useMemo(
</file context>
|
|
||
| const totalStreams = | ||
| measurements.total_streams ?? | ||
| measurements.measurements.reduce((sum, m) => sum + (m?.playcount ?? 0), 0); |
There was a problem hiding this comment.
P2: When total_streams is absent (pre-v2 response), the fallback sums only the capped 100-measurement page — any catalog with more than 100 measured tracks will show an understated stream total and therefore an understated valuation. Consider either surfacing this as a known limitation in the UI (e.g., "based on first 100 tracks") or keeping the report unavailable until whole-scope aggregates are present from the API.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Catalog/report/CatalogReportContent.tsx, line 54:
<comment>When `total_streams` is absent (pre-v2 response), the fallback sums only the capped 100-measurement page — any catalog with more than 100 measured tracks will show an understated stream total and therefore an understated valuation. Consider either surfacing this as a known limitation in the UI (e.g., "based on first 100 tracks") or keeping the report unavailable until whole-scope aggregates are present from the API.</comment>
<file context>
@@ -0,0 +1,87 @@
+
+ const totalStreams =
+ measurements.total_streams ??
+ measurements.measurements.reduce((sum, m) => sum + (m?.playcount ?? 0), 0);
+ const measuredSongCount =
+ measurements.measured_song_count ?? measurements.measurements.length;
</file context>
| ): UseQueryResult<CatalogSongsResponse> => { | ||
| return useQuery({ | ||
| queryKey: ["catalogReportSongs", catalogId], | ||
| queryFn: () => getCatalogSongs(catalogId, REPORT_SONGS_PAGE_SIZE, 1), |
There was a problem hiding this comment.
P2: For catalogs with more than 100 songs, only the first page is fetched here, but CatalogReportContent uses the result to build the per-release table while the stat row shows whole-catalog counts. This means the release table can omit later releases entirely, creating an inconsistency between the stats ("X releases") and the visible table. Either paginate through all songs, move the rollup to the backend, or clearly label the release table as showing a subset.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useCatalogReportSongs.ts, line 19:
<comment>For catalogs with more than 100 songs, only the first page is fetched here, but `CatalogReportContent` uses the result to build the per-release table while the stat row shows whole-catalog counts. This means the release table can omit later releases entirely, creating an inconsistency between the stats ("X releases") and the visible table. Either paginate through all songs, move the rollup to the backend, or clearly label the release table as showing a subset.</comment>
<file context>
@@ -0,0 +1,26 @@
+): UseQueryResult<CatalogSongsResponse> => {
+ return useQuery({
+ queryKey: ["catalogReportSongs", catalogId],
+ queryFn: () => getCatalogSongs(catalogId, REPORT_SONGS_PAGE_SIZE, 1),
+ enabled: !!catalogId,
+ staleTime: 5 * 60 * 1000,
</file context>
| ): Promise<string | null> { | ||
| try { | ||
| const q = artistName | ||
| ? `album:"${album}" artist:"${artistName}"` |
There was a problem hiding this comment.
P3: Albums or artists containing " can generate a malformed or altered Spotify query, causing artwork lookup misses or wrong matches. Escape filter delimiters in both values before constructing q.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/spotify/getAlbumArtwork.ts, line 22:
<comment>Albums or artists containing `"` can generate a malformed or altered Spotify query, causing artwork lookup misses or wrong matches. Escape filter delimiters in both values before constructing `q`.</comment>
<file context>
@@ -0,0 +1,35 @@
+): Promise<string | null> {
+ try {
+ const q = artistName
+ ? `album:"${album}" artist:"${artistName}"`
+ : `album:"${album}"`;
+ const params = new URLSearchParams({ q, type: "album", limit: "1" });
</file context>
| aria-busy="true" | ||
| aria-label="Loading catalog report" | ||
| > | ||
| <div className="rounded-2xl p-6 sm:p-8 shadow-[0_0_0_1px_var(--border)]"> |
There was a problem hiding this comment.
P3: The skeleton's outermost card div is missing bg-card that all sibling report components include alongside this shadow pattern. When the skeleton transitions to the real content with bg-card, the background will visibly flash, making the load feel less polished.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Catalog/report/CatalogReportSkeleton.tsx, line 13:
<comment>The skeleton's outermost card div is missing `bg-card` that all sibling report components include alongside this shadow pattern. When the skeleton transitions to the real content with `bg-card`, the background will visibly flash, making the load feel less polished.</comment>
<file context>
@@ -0,0 +1,28 @@
+ aria-busy="true"
+ aria-label="Loading catalog report"
+ >
+ <div className="rounded-2xl p-6 sm:p-8 shadow-[0_0_0_1px_var(--border)]">
+ <Skeleton className="h-3 w-40" />
+ <Skeleton className="mt-4 h-14 w-48" />
</file context>
Suggested change
| <div className="rounded-2xl p-6 sm:p-8 shadow-[0_0_0_1px_var(--border)]"> | |
| <div className="rounded-2xl bg-card p-6 sm:p-8 shadow-[0_0_0_1px_var(--border)]"> |
Collaborator
Author
|
Preview verification — happy path · preview Tested the way that matches the real onboarding flow: a brand-new single account does the whole thing — signup → funnel valuation → claim → report — so the account that claims is the account that reads. (My earlier walkthrough hit "No valuation found" only because I read the report on a different account than the one that claimed — a cross-account test artifact, now documented and dismissed on #1867; the api-side "fix" for it, api#775, was closed as not-needed.) Setup (all as one fresh account
7/7 (+ clean console). The report renders the real $ figure + release table on first paint for a genuine fresh signup. Report (full): Report hero — valuation echo + stat row + release table with art: Manage songs tab (existing UI kept): Note: production chat still shows the old bare "Catalog Songs" screen for the same claim (it doesn't have this PR yet) — this is the exact stall #1873 removes. |
This was referenced Jul 21, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ships the "Post-valuation catalog report page (sequence step 1)" item from #1867.
Problem
The marketing valuation CTA ("Get the full report with Recoup") lands on
/catalogs/{id}, which rendered a bare "Catalog Songs" ISRC-search/CSV-upload admin screen: no valuation echo, no report, no next action (verified on prod 2026-07-20; the exact stall point for every valuation-path signup in the 07-13 to 07-20 cohort).What changed
/catalogs/[catalogId]now opens as a catalog report, with the existing songs/ISRC management UI kept reachable as a secondary Manage songs tab (nothing removed except the now-unusedCatalogSongsPagewrapper;CatalogSongsPageContentis reused as-is).Report tab (default):
buildReportInsights): lifetime-average proxy caveat (always), unmeasured-tracks suppression (when coverage < 100%), single-release concentration (when top release >= 60% of streams). Each pairs what's suppressing the value with the Recoup behavior that addresses it./tasks(the dedicated first-task step ships in a sibling PR per the Onboarding sequence: state-derived resumable flow (valuation aha → roster/socials/catalog verify → first task) + activation fixes #1867 sequence).Data plumbing:
GET /api/catalogs/{catalogId}/measurementsvia the existinguseCatalogMeasurementshook (shipped with the homepage hero, feat: homepage valuation hero (Catalog HQ 1/3, chat#1850) #1852), extending its response type with the v2 fields (total_streams,catalog_age_years,title) as optionals. Pre-v2 responses degrade gracefully (totals fall back to summing the fetched page).GET /api/catalogs/songs, first 100, one-shot query — the report aggregates rather than lists).Valuation math (
lib/valuation/): portsmarketing/lib/valuation/computeCatalogValuation.ts+nlsBandFromSpotifyGross.tsinto chat with identical constants (annual = lifetime/age; central = annual x $0.0035 x 1.4 gross-up x 0.6375 net x 13; low 1.25/10x, high 1.6/16x; 5y default age). Age uses the api'scatalog_age_years(derived server-side from the earliest Spotify release date) with the marketing lib's 5-year default as fallback. Same formula as the api'scomputeValuationBand, so the number matches the endpoint's own band and the marketing card.Null safety: the report path is null-safe independently of the sibling crash-fix PR —
buildReleaseRollupstolerates null album/name/artists entries (unit-tested), and rows render placeholders ("Untitled release", "Unknown artist", initial tile) instead of throwing. The Manage songs tab keeps today's behavior; the toggle crash fix is its own #1867 item.TDD
Computation libs written test-first (RED confirmed before implementation):
nlsBandFromSpotifyGross,computeCatalogValuation,buildReleaseRollups,buildReportInsights. One exported function per file, all files < 100 LOC.How to test
/catalogs/{id}-> the report is the first paint: $ central value + range, stat row, release table with art, diagnosis cards, and exactly one primary CTA ("Set up your weekly report" ->/tasks).Verification
Local (this branch, rebased on
main@ 3dccdae):pnpm test— 148/148 pass (includes 15 new unit tests).pnpm exec tsc --noEmit— clean apart from two pre-existing errors onmain(SpotifyDeepResearchResult.tsxpng module,extractSendEmailResults.test.tsUIMessage shape), reproduced on a clean tree.pnpm build— webpack compile passes; the build then fails at page-data collection for the unrelated/api/agent-creatorroute, and a baseline build of untouchedmainfails at the exact same point (env-dependent, no local.env— CI runspnpm testonly). Vercel's preview build is the real gate here.eslint+prettieron all touched files — clean (next lintitself is broken repo-wide on Next 16).Preview verification pending — needs a signed-in session against a claimed catalog; note previews hit the test api base, which must be current with api
mainfor the measurements v2 fields.Part of #1867 (does not close it).
🤖 Generated with Claude Code
Summary by cubic
Turns
/catalogs/[catalogId]into a post-valuation catalog report with valuation, key stats, per-release breakdown, actionable insights, and one clear next step. Keeps the existing songs/ISRC management UI as a secondary “Manage songs” tab. Part of #1867 (sequence step 1).catalog_age_yearswith a 5y fallback./tasks.useCatalogMeasurements(v2 fields optional, graceful fallback) and fetches the first 100 catalog songs for aggregation; replaces theCatalogSongsPagewrapper withCatalogReportPagewhile reusingCatalogSongsPageContent.Written for commit b06ec65. Summary will update on new commits.
Summary by CodeRabbit