feat(catalogs): GET /api/catalogs/measurements — per-ISRC latest playcounts + derived valuation band (chat#1850)#757
Merged
Conversation
…counts + derived valuation band (chat#1850)
Implements the docs#265 contract: catalog ownership from auth (no IDOR),
catalog_songs → latest spotify platform_displayed_play_count per ISRC,
and a valuation band { low, mid, high } derived at read time with the
marketing card's exact model (lifetime-average run-rate over catalog age
from the source run's Spotify release dates, NLS, 10/13/16x multiples).
TDD red→green per unit; full suite 3923 pass; tsc adds no new errors;
lint + 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. |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (10)
✨ 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 |
Contributor
Author
Preview verification —
|
| # | Case | Request | Expected | Got | Body |
|---|---|---|---|---|---|
| 1 | Missing catalogId |
GET /api/catalogs/measurements (no auth) |
400 | 400 ✅ | {"status":"error","missing_fields":["catalogId"],"error":"catalogId parameter is required"} |
| 2 | Invalid catalogId |
GET …?catalogId=not-a-uuid (no auth) |
400 | 400 ✅ | {"status":"error","missing_fields":["catalogId"],"error":"catalogId must be a valid UUID"} |
| 3 | Unauthed, real catalog | GET …?catalogId=740d5050-40ec-4892-a040-b78bb50fef2f (the Del Water Gap catalog from chat#1850) |
401 | 401 ✅ | {"status":"error","error":"Exactly one of x-api-key or Authorization must be provided"} |
| 4 | Invalid x-api-key, real catalog |
same URL + bogus key | 401 | 401 ✅ | {"status":"error","message":"Unauthorized"} |
| 5 | CORS preflight | OPTIONS /api/catalogs/measurements |
200 | 200 ✅ | — |
Done-when status
| Check | Status |
|---|---|
| 400 / 401 paths live on preview | ✅ verified above |
| 404 unknown/foreign catalog | ✅ unit-tested (handler returns an indistinguishable 404 via the account_catalogs ownership check); ⏳ not exercised on preview — needs any authenticated credential |
| 200 happy path: per-track play counts + band for the Del Water Gap catalog, consistent with the marketing card | ⏳ pending an authenticated test — I have no Privy bearer / API key for the owning account (sweetmantech+jul620261135am@…) in this session and did not fake one. Covered by unit tests (latest-per-ISRC reduction, band math asserted against the marketing constants); needs one authenticated GET …?catalogId=740d5050-40ec-4892-a040-b78bb50fef2f to confirm live numbers |
One caveat for the live check: the band matches a marketing run only when the catalog's snapshot has album_ids (album-scoped valuation runs do); otherwise catalog_age_years falls back to the same default_5y the marketing card documents, and the band shifts accordingly.
🤖 Generated with Claude Code
20 tasks
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
lib/catalog/getCatalogMeasurementsHandler.ts (1)
24-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHandler exceeds the 20-line function guideline; consider extracting the fetch/compute stage.
The function is correct and each step delegates to a well-named helper, but it composes five distinct concerns (query validation, auth, ownership check, parallel data fetch, response shaping) in one ~44-line body.
♻️ Optional extraction of the fetch+compute stage
+async function buildMeasurementsPayload(catalogId: string) { + const songs = await selectCatalogSongTitles(catalogId); + const titles = new Map(songs.map(s => [s.isrc, s.title])); + const [rows, earliestReleaseDate] = await Promise.all([ + songs.length > 0 + ? selectSongMeasurements({ + songs: songs.map(s => s.isrc), + platform: "spotify", + metric: "platform_displayed_play_count", + }) + : Promise.resolve([]), + getCatalogEarliestReleaseDate(catalogId), + ]); + const { measurements, totalStreams } = latestMeasurementsPerIsrc(rows, titles); + const { valuation, catalogAgeYears } = computeValuationBand({ totalStreams, earliestReleaseDate }); + return { measurements, valuation, totalStreams, catalogAgeYears }; +}As per path instructions, "Keep functions under 50 lines" and "Single responsibility per function." This is a nice-to-have given the current step-by-step readability is still reasonable.
🤖 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/getCatalogMeasurementsHandler.ts` around lines 24 - 73, The getCatalogMeasurementsHandler function is still doing too many steps in one body, even though it delegates to helpers. Extract the parallel fetch and valuation/response shaping into a separate helper (for example around selectCatalogSongTitles, selectSongMeasurements, getCatalogEarliestReleaseDate, latestMeasurementsPerIsrc, and computeValuationBand) so the handler stays under the line limit and follows single-responsibility.Source: Path instructions
lib/catalog/validateGetCatalogMeasurementsQuery.ts (1)
5-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMigrate this validator to Zod 4's error API.
z.string({ message })and chained.uuid("...")are deprecated; replace them withz.uuid({ error: ... })(or anerrorcallback if you need separate required vs format messages).🤖 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/validateGetCatalogMeasurementsQuery.ts` around lines 5 - 9, Update getCatalogMeasurementsQuerySchema to use Zod 4’s error API instead of the deprecated z.string({ message }) and chained .uuid("...") calls. In validateGetCatalogMeasurementsQuery.ts, switch the catalogId validator to z.uuid with an error handler or callback so it can preserve the missing-value message and the invalid-UUID message separately, keeping the schema behavior in getCatalogMeasurementsQuerySchema the same.lib/spotify/getAlbums.ts (1)
25-43: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider parallelizing batch fetches for large catalogs.
Batches are fetched sequentially; for catalogs with many albums this adds latency proportional to batch count.
Promise.allacross batches would reduce total wait time.🤖 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/getAlbums.ts` around lines 25 - 43, The batch fetching in getAlbums is currently sequential, which increases latency for large id lists. Refactor the loop in getAlbums to build per-batch fetch promises and resolve them in parallel with Promise.all, while preserving the existing response parsing and error handling. Keep the batching logic and the SpotifyAlbum accumulation intact, but make sure the final albums array still includes all successful batch results in the same shape.
🤖 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 `@lib/catalog/computeValuationBand.ts`:
- Around line 31-35: `computeValuationBand` currently lets an invalid
`params.earliestReleaseDate` produce `NaN`, which then propagates into the
valuation band. In the date-handling block inside `computeValuationBand`,
validate the parsed date before computing `ageMs` and only overwrite
`catalogAgeYears` when the date is valid; otherwise leave the fallback
`DEFAULT_AGE_YEARS` in place. Use the existing `now`, `YEAR_MS`, and
`catalogAgeYears` logic, but add a guard around the `new
Date(params.earliestReleaseDate)` result so unparseable inputs do not affect the
response.
In `@lib/catalog/validateGetCatalogMeasurementsQuery.ts`:
- Around line 26-36: The error payload in validateGetCatalogMeasurementsQuery
uses missing_fields for every validation failure, which is misleading for
format/invalid UUID cases. Update the response shape in
validateGetCatalogMeasurementsQuery to distinguish absent fields from invalid
values by using a clearer key or conditional mapping based on firstError.path
and firstError.message, so catalogId validation failures are reported as format
errors rather than missing fields.
In `@lib/spotify/getAlbums.ts`:
- Around line 29-39: The Spotify request in getAlbums should be protected by a
timeout so an unresponsive call does not stall the best-effort measurements
path. Update the fetch in getAlbums to use AbortSignal.timeout(...) or an
AbortController-based fallback for older Node versions, and make sure the
timeout is applied to the existing fetch call before handling the response.ok
error path.
---
Nitpick comments:
In `@lib/catalog/getCatalogMeasurementsHandler.ts`:
- Around line 24-73: The getCatalogMeasurementsHandler function is still doing
too many steps in one body, even though it delegates to helpers. Extract the
parallel fetch and valuation/response shaping into a separate helper (for
example around selectCatalogSongTitles, selectSongMeasurements,
getCatalogEarliestReleaseDate, latestMeasurementsPerIsrc, and
computeValuationBand) so the handler stays under the line limit and follows
single-responsibility.
In `@lib/catalog/validateGetCatalogMeasurementsQuery.ts`:
- Around line 5-9: Update getCatalogMeasurementsQuerySchema to use Zod 4’s error
API instead of the deprecated z.string({ message }) and chained .uuid("...")
calls. In validateGetCatalogMeasurementsQuery.ts, switch the catalogId validator
to z.uuid with an error handler or callback so it can preserve the missing-value
message and the invalid-UUID message separately, keeping the schema behavior in
getCatalogMeasurementsQuerySchema the same.
In `@lib/spotify/getAlbums.ts`:
- Around line 25-43: The batch fetching in getAlbums is currently sequential,
which increases latency for large id lists. Refactor the loop in getAlbums to
build per-batch fetch promises and resolve them in parallel with Promise.all,
while preserving the existing response parsing and error handling. Keep the
batching logic and the SpotifyAlbum accumulation intact, but make sure the final
albums array still includes all successful batch results in the same shape.
🪄 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: 5003c0d0-28a5-4c14-86fe-cd90fef63e5e
⛔ Files ignored due to path filters (9)
lib/catalog/__tests__/computeValuationBand.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/catalog/__tests__/getCatalogEarliestReleaseDate.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/catalog/__tests__/getCatalogMeasurementsHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/catalog/__tests__/latestMeasurementsPerIsrc.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/spotify/__tests__/getAlbums.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/supabase/account_catalogs/__tests__/selectAccountCatalog.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/supabase/playcount_snapshots/__tests__/selectPlaycountSnapshots.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (10)
app/api/catalogs/measurements/route.tslib/catalog/computeValuationBand.tslib/catalog/getCatalogEarliestReleaseDate.tslib/catalog/getCatalogMeasurementsHandler.tslib/catalog/latestMeasurementsPerIsrc.tslib/catalog/validateGetCatalogMeasurementsQuery.tslib/spotify/getAlbums.tslib/supabase/account_catalogs/selectAccountCatalog.tslib/supabase/catalog_songs/selectCatalogSongTitles.tslib/supabase/playcount_snapshots/selectPlaycountSnapshots.ts
Comment on lines
+31
to
+35
| let catalogAgeYears = DEFAULT_AGE_YEARS; | ||
| if (params.earliestReleaseDate) { | ||
| const ageMs = now.getTime() - new Date(params.earliestReleaseDate).getTime(); | ||
| catalogAgeYears = Math.max(1, Math.round(ageMs / YEAR_MS)); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Unparseable earliestReleaseDate silently propagates NaN into the valuation band.
Math.max(1, Math.round(ageMs / YEAR_MS)) returns NaN (not 1) if ageMs is NaN — i.e., if earliestReleaseDate fails to parse as a Date. This would silently corrupt the response with NaN valuations instead of falling back to the documented default age.
🛡️ Proposed guard against invalid date strings
let catalogAgeYears = DEFAULT_AGE_YEARS;
if (params.earliestReleaseDate) {
- const ageMs = now.getTime() - new Date(params.earliestReleaseDate).getTime();
- catalogAgeYears = Math.max(1, Math.round(ageMs / YEAR_MS));
+ const releaseTime = new Date(params.earliestReleaseDate).getTime();
+ if (!Number.isNaN(releaseTime)) {
+ const ageMs = now.getTime() - releaseTime;
+ catalogAgeYears = Math.max(1, Math.round(ageMs / YEAR_MS));
+ }
}📝 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
| let catalogAgeYears = DEFAULT_AGE_YEARS; | |
| if (params.earliestReleaseDate) { | |
| const ageMs = now.getTime() - new Date(params.earliestReleaseDate).getTime(); | |
| catalogAgeYears = Math.max(1, Math.round(ageMs / YEAR_MS)); | |
| } | |
| let catalogAgeYears = DEFAULT_AGE_YEARS; | |
| if (params.earliestReleaseDate) { | |
| const releaseTime = new Date(params.earliestReleaseDate).getTime(); | |
| if (!Number.isNaN(releaseTime)) { | |
| const ageMs = now.getTime() - releaseTime; | |
| catalogAgeYears = Math.max(1, Math.round(ageMs / YEAR_MS)); | |
| } | |
| } |
🤖 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/computeValuationBand.ts` around lines 31 - 35,
`computeValuationBand` currently lets an invalid `params.earliestReleaseDate`
produce `NaN`, which then propagates into the valuation band. In the
date-handling block inside `computeValuationBand`, validate the parsed date
before computing `ageMs` and only overwrite `catalogAgeYears` when the date is
valid; otherwise leave the fallback `DEFAULT_AGE_YEARS` in place. Use the
existing `now`, `YEAR_MS`, and `catalogAgeYears` logic, but add a guard around
the `new Date(params.earliestReleaseDate)` result so unparseable inputs do not
affect the response.
Comment on lines
+26
to
+36
| if (!result.success) { | ||
| const firstError = result.error.issues[0]; | ||
| return NextResponse.json( | ||
| { | ||
| status: "error", | ||
| missing_fields: firstError.path, | ||
| error: firstError.message, | ||
| }, | ||
| { status: 400, headers: getCorsHeaders() }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
missing_fields is misleading for format errors.
When catalogId is present but not a valid UUID, the response still reports it under missing_fields, which is inaccurate and could confuse API consumers debugging a format issue vs. an absent parameter.
💡 Proposed rename for clarity
return NextResponse.json(
{
status: "error",
- missing_fields: firstError.path,
+ invalid_fields: firstError.path,
error: firstError.message,
},
{ status: 400, headers: getCorsHeaders() },
);📝 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
| if (!result.success) { | |
| const firstError = result.error.issues[0]; | |
| return NextResponse.json( | |
| { | |
| status: "error", | |
| missing_fields: firstError.path, | |
| error: firstError.message, | |
| }, | |
| { status: 400, headers: getCorsHeaders() }, | |
| ); | |
| } | |
| if (!result.success) { | |
| const firstError = result.error.issues[0]; | |
| return NextResponse.json( | |
| { | |
| status: "error", | |
| invalid_fields: firstError.path, | |
| error: firstError.message, | |
| }, | |
| { status: 400, headers: getCorsHeaders() }, | |
| ); | |
| } |
🤖 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/validateGetCatalogMeasurementsQuery.ts` around lines 26 - 36, The
error payload in validateGetCatalogMeasurementsQuery uses missing_fields for
every validation failure, which is misleading for format/invalid UUID cases.
Update the response shape in validateGetCatalogMeasurementsQuery to distinguish
absent fields from invalid values by using a clearer key or conditional mapping
based on firstError.path and firstError.message, so catalogId validation
failures are reported as format errors rather than missing fields.
Comment on lines
+29
to
+39
| const response = await fetch(url, { | ||
| method: "GET", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${accessToken}`, | ||
| }, | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| return { albums: null, error: new Error("Spotify API request failed") }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant files and inspect the surrounding code.
git ls-files 'lib/spotify/getAlbums.ts' 'lib/**/getCatalogEarliestReleaseDate.ts' 'package.json' 'tsconfig.json' 'README.md'
echo
echo "=== lib/spotify/getAlbums.ts ==="
cat -n lib/spotify/getAlbums.ts
echo
echo "=== getCatalogEarliestReleaseDate.ts matches ==="
rg -n "getCatalogEarliestReleaseDate|getAlbums" lib -A 8 -B 8
echo
echo "=== package.json ==="
cat -n package.json
echo
echo "=== tsconfig.json (if present) ==="
if [ -f tsconfig.json ]; then cat -n tsconfig.json; fi
echo
echo "=== Node/runtime hints ==="
rg -n '"node"|engines|AbortSignal\.timeout|fetch\(' -S package.json README.md lib tsconfig.jsonRepository: recoupable/api
Length of output: 38319
Add a timeout to the Spotify fetch in lib/spotify/getAlbums.ts:29-35. An unresponsive Spotify request can stall the best-effort measurements endpoint; use AbortSignal.timeout(...), or an AbortController fallback if older Node support is required.
🤖 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/getAlbums.ts` around lines 29 - 39, The Spotify request in
getAlbums should be protected by a timeout so an unresponsive call does not
stall the best-effort measurements path. Update the fetch in getAlbums to use
AbortSignal.timeout(...) or an AbortController-based fallback for older Node
versions, and make sure the timeout is applied to the existing fetch call before
handling the response.ok error path.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
10 issues found across 19 files
Confidence score: 3/5
- The highest-risk item is in
lib/catalog/getCatalogEarliestReleaseDate.ts: importinggenerateAccessTokenat module load can throw when Spotify credentials are missing, which can fail the route before the default-age fallback runs and break catalog measurements entirely for those environments — defer token setup/import until runtime inside the Spotify path so fallback logic still executes. lib/supabase/account_catalogs/selectAccountCatalog.tsandlib/catalog/getCatalogMeasurementsHandler.tscurrently blur backend failures into normal outcomes (null) and use an unpaginated measurements query, so users may see incorrect 404s or silently incompletemeasurements/total_streamson larger catalogs — propagate Supabase errors as 500s and page or aggregate server-side before reducing latest-per-ISRC.lib/catalog/computeValuationBand.tscan produceNaNvaluation bands whenearliestReleaseDateis a truthy but invalid date string, andlib/catalog/__tests__/getCatalogMeasurementsHandler.test.tshas time-dependent expectations that can hide or intermittently expose this behavior — validate/parsing-guard the date input and freeze/mock time in the valuation-band test before merging.lib/spotify/getAlbums.tshas no request timeout, so an unresponsive Spotify call can hang the measurements flow; related tests inlib/spotify/__tests__/getAlbums.test.tsandlib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.tsalso miss key assertions (Authorization header, second batch URL, and!innerselect semantics), reducing confidence against regressions — add a fetch timeout and tighten these assertions to de-risk integration behavior.
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/spotify/__tests__/getAlbums.test.ts">
<violation number="1" location="lib/spotify/__tests__/getAlbums.test.ts:26">
P2: The request Authorization header is never verified. The test provides `accessToken: "tok"` but only inspects the URL — the request headers (`mock.calls[0][1]`) are never asserted. Since the Spotify API requires Bearer token auth, add an assertion that the first call includes `Authorization: Bearer tok` to catch regressions that drop or malform the header.</violation>
<violation number="2" location="lib/spotify/__tests__/getAlbums.test.ts:31">
P3: Only the first batch URL is verified. The second `fetch` call's URL goes unchecked, so batching errors in subsequent requests (wrong IDs, wrong size) would not be caught. Add an assertion on the second URL for symmetry.</violation>
</file>
<file name="lib/catalog/getCatalogEarliestReleaseDate.ts">
<violation number="1" location="lib/catalog/getCatalogEarliestReleaseDate.ts:2">
P2: Catalog measurements cannot fall back to the default age when Spotify credentials are absent: importing `generateAccessToken` throws during module load, so the route can fail before `getCatalogEarliestReleaseDate` returns `null`. Consider moving Spotify credential failure into the best-effort path, e.g. lazy-load/wrap token generation so missing config returns `null`.</violation>
</file>
<file name="lib/supabase/account_catalogs/selectAccountCatalog.ts">
<violation number="1" location="lib/supabase/account_catalogs/selectAccountCatalog.ts:27">
P2: A backend failure during the ownership check is reported to clients as a missing catalog because `selectAccountCatalog` converts Supabase errors to `null`. Throwing here lets `getCatalogMeasurementsHandler` return the intended 500 while preserving `null` for true no-row results.</violation>
</file>
<file name="lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts">
<violation number="1" location="lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts:32">
P2: Test omits assertion on `.select()` call parameters. The function uses `select("songs!inner (isrc, name)")` where the `!inner` join modifier is semantically meaningful — dropping it would let orphaned `catalog_songs` rows leak through instead of being filtered. Add an assertion like `expect(builder.select).toHaveBeenCalledWith("songs!inner (isrc, name)")` to pin the join type.</violation>
</file>
<file name="lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts">
<violation number="1" location="lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts:99">
P2: The test "returns latest per-ISRC measurements + the derived valuation band" freezes `earliestReleaseDate` to `2016-07-01` but relies on real system time to determine the catalog age — `computeValuationBand` is called without a `now` override, so every day `new Date()` shifts. The expected valuation constants are hardcoded for a 10-year age (2016 → 2026), meaning the test will produce wrong assertions starting ~mid-2027 when the age naturally ticks to 11 years.
`computeValuationBand` explicitly exposes a `now?: Date` parameter documented as "Clock override for tests" — the test should freeze time with `vi.setSystemTime` so the valuation output is deterministic regardless of when the suite runs.</violation>
<violation number="2" location="lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts:151">
P2: The 500-error test verifies status but not the response body content. Per team convention, 500 responses should return a hardcoded "Internal server error" without leaking the exception message. The handler is implemented correctly (errorResponse("Internal server error", 500)), so the risk is in regression — a future change to the catch block could accidentally include the error message without a body assertion catching it. Add a check like `const body = await res.json(); expect(body.error).toBe("Internal server error");` to lock in the expected safe envelope.</violation>
</file>
<file name="lib/catalog/getCatalogMeasurementsHandler.ts">
<violation number="1" location="lib/catalog/getCatalogMeasurementsHandler.ts:48">
P2: Large catalogs or catalogs with measurement history can return incomplete `measurements`/`total_streams` because this fetch relies on an unpaginated Supabase select before reducing latest per ISRC. Consider moving the latest-per-song selection into a paginated/SQL-side query scoped to the catalog instead of fetching an implicitly capped series.
(Based on your team's feedback about preferring DB-side pagination over raising Supabase query limits.) [FEEDBACK_USED]</violation>
</file>
<file name="lib/catalog/computeValuationBand.ts">
<violation number="1" location="lib/catalog/computeValuationBand.ts:33">
P2: If `earliestReleaseDate` is a truthy but unparseable date string (e.g. `"unknown"` or a malformed ISO value), `new Date(params.earliestReleaseDate).getTime()` returns `NaN`, which propagates through the subtraction, `Math.round`, and `Math.max(1, NaN)` — all yielding `NaN` rather than the intended `DEFAULT_AGE_YEARS` fallback. Consider validating the parsed timestamp before using it:
```ts
const releaseTime = new Date(params.earliestReleaseDate).getTime();
if (!Number.isNaN(releaseTime)) {
const ageMs = now.getTime() - releaseTime;
catalogAgeYears = Math.max(1, Math.round(ageMs / YEAR_MS));
}
```</violation>
</file>
<file name="lib/spotify/getAlbums.ts">
<violation number="1" location="lib/spotify/getAlbums.ts:29">
P2: This `fetch` call has no timeout. If Spotify is unresponsive, the request will hang indefinitely, blocking the measurements endpoint response. Consider adding `signal: AbortSignal.timeout(10_000)` (or similar) to bound the wait time and allow the best-effort fallback (null release date → default age) to kick in.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as External Client (Chat Frontend)
participant Next as Next.js API Route
participant Handler as getCatalogMeasurementsHandler
participant Auth as validateAuthContext
participant Validator as validateGetCatalogMeasurementsQuery
participant DB as Supabase Database
participant Spotify as Spotify API
participant Valuation as computeValuationBand
Client->>Next: GET /api/catalogs/measurements?catalogId={uuid}
Next->>Handler: Forward request
Handler->>Validator: validate query params
alt 400: Invalid/missing catalogId
Validator-->>Handler: Return NextResponse 400
Handler-->>Next: Forward 400 response
Next-->>Client: 400 Invalid catalogId
else Valid query
Validator-->>Handler: { catalogId }
end
Handler->>Auth: Validate authentication (Privy bearer or x-api-key)
alt 401: Unauthenticated
Auth-->>Handler: Return NextResponse 401
Handler-->>Next: Forward 401 response
Next-->>Client: 401 Unauthorized
else Authenticated
Auth-->>Handler: { accountId, orgId }
end
Handler->>DB: selectAccountCatalog({ accountId, catalogId })
alt 404: Not found or not owned
DB-->>Handler: null
Handler-->>Next: Return 404
Next-->>Client: 404 Catalog not found
else Owned catalog found
DB-->>Handler: { account, catalog } link row
end
Handler->>DB: selectCatalogSongTitles(catalogId)
DB-->>Handler: [{ isrc, title }, ...]
Note over Handler: Parallel fetch: measurements + earliest release date
alt Songs exist in catalog
Handler->>DB: selectSongMeasurements({ songs, platform: "spotify", metric: "platform_displayed_play_count" })
DB-->>Handler: [{ song, value, captured_at }, ...] (newest-first)
else Empty catalog
Handler-->>Handler: Use empty array
end
Handler->>DB: getCatalogEarliestReleaseDate(catalogId)
DB->>DB: selectPlaycountSnapshots({ catalog: catalogId })
DB-->>Handler: Snapshot with album_ids
opt Album IDs found on snapshot
Handler->>Spotify: generateAccessToken()
Spotify-->>Handler: { access_token }
Handler->>Spotify: getAlbums({ ids, accessToken })
loop Batch per 20 album IDs
Spotify-->>Handler: Album metadata with release dates
end
alt Spotify fetch succeeds
Handler-->>Handler: Find earliest release date
else Fetch fails
Handler-->>Handler: Use null (fallback)
end
end
Handler->>Handler: latestMeasurementsPerIsrc(rows, titles)
Note over Handler: Reduce to newest capture per ISRC, sort by playcount desc
Handler->>Valuation: computeValuationBand({ totalStreams, earliestReleaseDate })
alt Age known from release date
Valuation-->>Handler: { valuation: { low, mid, high }, catalogAgeYears }
else No release date (fallback)
Valuation-->>Handler: Default 5 years age
else Zero streams
Valuation-->>Handler: { low: 0, mid: 0, high: 0 }
end
Handler->>Next: successResponse({ measurements, valuation, total_streams, catalog_age_years })
Next-->>Client: 200 JSON response
Note over Handler: Error path (any async failure)
Handler->>Handler: Catch block → console.error
Handler-->>Next: 500 Internal server error
Next-->>Client: 500 Error response
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| new Response(JSON.stringify(albumsPage(ids.slice(20))), { status: 200 }), | ||
| ); | ||
|
|
||
| const { albums, error } = await getAlbums({ ids, accessToken: "tok" }); |
There was a problem hiding this comment.
P2: The request Authorization header is never verified. The test provides accessToken: "tok" but only inspects the URL — the request headers (mock.calls[0][1]) are never asserted. Since the Spotify API requires Bearer token auth, add an assertion that the first call includes Authorization: Bearer tok to catch regressions that drop or malform the header.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/spotify/__tests__/getAlbums.test.ts, line 26:
<comment>The request Authorization header is never verified. The test provides `accessToken: "tok"` but only inspects the URL — the request headers (`mock.calls[0][1]`) are never asserted. Since the Spotify API requires Bearer token auth, add an assertion that the first call includes `Authorization: Bearer tok` to catch regressions that drop or malform the header.</comment>
<file context>
@@ -0,0 +1,52 @@
+ new Response(JSON.stringify(albumsPage(ids.slice(20))), { status: 200 }),
+ );
+
+ const { albums, error } = await getAlbums({ ids, accessToken: "tok" });
+
+ expect(error).toBeNull();
</file context>
| @@ -0,0 +1,31 @@ | |||
| import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots"; | |||
| import generateAccessToken from "@/lib/spotify/generateAccessToken"; | |||
There was a problem hiding this comment.
P2: Catalog measurements cannot fall back to the default age when Spotify credentials are absent: importing generateAccessToken throws during module load, so the route can fail before getCatalogEarliestReleaseDate returns null. Consider moving Spotify credential failure into the best-effort path, e.g. lazy-load/wrap token generation so missing config returns null.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/getCatalogEarliestReleaseDate.ts, line 2:
<comment>Catalog measurements cannot fall back to the default age when Spotify credentials are absent: importing `generateAccessToken` throws during module load, so the route can fail before `getCatalogEarliestReleaseDate` returns `null`. Consider moving Spotify credential failure into the best-effort path, e.g. lazy-load/wrap token generation so missing config returns `null`.</comment>
<file context>
@@ -0,0 +1,31 @@
+import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots";
+import generateAccessToken from "@/lib/spotify/generateAccessToken";
+import getAlbums from "@/lib/spotify/getAlbums";
+
</file context>
| .eq("catalog", catalogId) | ||
| .maybeSingle(); | ||
|
|
||
| if (error) { |
There was a problem hiding this comment.
P2: A backend failure during the ownership check is reported to clients as a missing catalog because selectAccountCatalog converts Supabase errors to null. Throwing here lets getCatalogMeasurementsHandler return the intended 500 while preserving null for true no-row results.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/supabase/account_catalogs/selectAccountCatalog.ts, line 27:
<comment>A backend failure during the ownership check is reported to clients as a missing catalog because `selectAccountCatalog` converts Supabase errors to `null`. Throwing here lets `getCatalogMeasurementsHandler` return the intended 500 while preserving `null` for true no-row results.</comment>
<file context>
@@ -0,0 +1,33 @@
+ .eq("catalog", catalogId)
+ .maybeSingle();
+
+ if (error) {
+ console.error("Error fetching account_catalogs:", error);
+ return null;
</file context>
| error: null, | ||
| }); | ||
|
|
||
| const result = await selectCatalogSongTitles("cat_1"); |
There was a problem hiding this comment.
P2: Test omits assertion on .select() call parameters. The function uses select("songs!inner (isrc, name)") where the !inner join modifier is semantically meaningful — dropping it would let orphaned catalog_songs rows leak through instead of being filtered. Add an assertion like expect(builder.select).toHaveBeenCalledWith("songs!inner (isrc, name)") to pin the join type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts, line 32:
<comment>Test omits assertion on `.select()` call parameters. The function uses `select("songs!inner (isrc, name)")` where the `!inner` join modifier is semantically meaningful — dropping it would let orphaned `catalog_songs` rows leak through instead of being filtered. Add an assertion like `expect(builder.select).toHaveBeenCalledWith("songs!inner (isrc, name)")` to pin the join type.</comment>
<file context>
@@ -0,0 +1,49 @@
+ error: null,
+ });
+
+ const result = await selectCatalogSongTitles("cat_1");
+
+ expect(supabase.from).toHaveBeenCalledWith("catalog_songs");
</file context>
|
|
||
| const res = await getCatalogMeasurementsHandler(makeRequest()); | ||
|
|
||
| expect(res.status).toBe(500); |
There was a problem hiding this comment.
P2: The 500-error test verifies status but not the response body content. Per team convention, 500 responses should return a hardcoded "Internal server error" without leaking the exception message. The handler is implemented correctly (errorResponse("Internal server error", 500)), so the risk is in regression — a future change to the catch block could accidentally include the error message without a body assertion catching it. Add a check like const body = await res.json(); expect(body.error).toBe("Internal server error"); to lock in the expected safe envelope.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts, line 151:
<comment>The 500-error test verifies status but not the response body content. Per team convention, 500 responses should return a hardcoded "Internal server error" without leaking the exception message. The handler is implemented correctly (errorResponse("Internal server error", 500)), so the risk is in regression — a future change to the catch block could accidentally include the error message without a body assertion catching it. Add a check like `const body = await res.json(); expect(body.error).toBe("Internal server error");` to lock in the expected safe envelope.</comment>
<file context>
@@ -0,0 +1,154 @@
+
+ const res = await getCatalogMeasurementsHandler(makeRequest());
+
+ expect(res.status).toBe(500);
+ consoleError.mockRestore();
+ });
</file context>
| const titles = new Map(songs.map(s => [s.isrc, s.title])); | ||
| const [rows, earliestReleaseDate] = await Promise.all([ | ||
| songs.length > 0 | ||
| ? selectSongMeasurements({ |
There was a problem hiding this comment.
P2: Large catalogs or catalogs with measurement history can return incomplete measurements/total_streams because this fetch relies on an unpaginated Supabase select before reducing latest per ISRC. Consider moving the latest-per-song selection into a paginated/SQL-side query scoped to the catalog instead of fetching an implicitly capped series.
(Based on your team's feedback about preferring DB-side pagination over raising Supabase query limits.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/getCatalogMeasurementsHandler.ts, line 48:
<comment>Large catalogs or catalogs with measurement history can return incomplete `measurements`/`total_streams` because this fetch relies on an unpaginated Supabase select before reducing latest per ISRC. Consider moving the latest-per-song selection into a paginated/SQL-side query scoped to the catalog instead of fetching an implicitly capped series.
(Based on your team's feedback about preferring DB-side pagination over raising Supabase query limits.) </comment>
<file context>
@@ -0,0 +1,73 @@
+ const titles = new Map(songs.map(s => [s.isrc, s.title]));
+ const [rows, earliestReleaseDate] = await Promise.all([
+ songs.length > 0
+ ? selectSongMeasurements({
+ songs: songs.map(s => s.isrc),
+ platform: "spotify",
</file context>
| { song: "ISRC1", value: 100, captured_at: "2026-06-01T00:00:00Z" }, | ||
| ] as never); | ||
| // 10 years of catalog age | ||
| vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue("2016-07-01"); |
There was a problem hiding this comment.
P2: The test "returns latest per-ISRC measurements + the derived valuation band" freezes earliestReleaseDate to 2016-07-01 but relies on real system time to determine the catalog age — computeValuationBand is called without a now override, so every day new Date() shifts. The expected valuation constants are hardcoded for a 10-year age (2016 → 2026), meaning the test will produce wrong assertions starting ~mid-2027 when the age naturally ticks to 11 years.
computeValuationBand explicitly exposes a now?: Date parameter documented as "Clock override for tests" — the test should freeze time with vi.setSystemTime so the valuation output is deterministic regardless of when the suite runs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts, line 99:
<comment>The test "returns latest per-ISRC measurements + the derived valuation band" freezes `earliestReleaseDate` to `2016-07-01` but relies on real system time to determine the catalog age — `computeValuationBand` is called without a `now` override, so every day `new Date()` shifts. The expected valuation constants are hardcoded for a 10-year age (2016 → 2026), meaning the test will produce wrong assertions starting ~mid-2027 when the age naturally ticks to 11 years.
`computeValuationBand` explicitly exposes a `now?: Date` parameter documented as "Clock override for tests" — the test should freeze time with `vi.setSystemTime` so the valuation output is deterministic regardless of when the suite runs.</comment>
<file context>
@@ -0,0 +1,154 @@
+ { song: "ISRC1", value: 100, captured_at: "2026-06-01T00:00:00Z" },
+ ] as never);
+ // 10 years of catalog age
+ vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue("2016-07-01");
+
+ const res = await getCatalogMeasurementsHandler(makeRequest());
</file context>
|
|
||
| let catalogAgeYears = DEFAULT_AGE_YEARS; | ||
| if (params.earliestReleaseDate) { | ||
| const ageMs = now.getTime() - new Date(params.earliestReleaseDate).getTime(); |
There was a problem hiding this comment.
P2: If earliestReleaseDate is a truthy but unparseable date string (e.g. "unknown" or a malformed ISO value), new Date(params.earliestReleaseDate).getTime() returns NaN, which propagates through the subtraction, Math.round, and Math.max(1, NaN) — all yielding NaN rather than the intended DEFAULT_AGE_YEARS fallback. Consider validating the parsed timestamp before using it:
const releaseTime = new Date(params.earliestReleaseDate).getTime();
if (!Number.isNaN(releaseTime)) {
const ageMs = now.getTime() - releaseTime;
catalogAgeYears = Math.max(1, Math.round(ageMs / YEAR_MS));
}Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/computeValuationBand.ts, line 33:
<comment>If `earliestReleaseDate` is a truthy but unparseable date string (e.g. `"unknown"` or a malformed ISO value), `new Date(params.earliestReleaseDate).getTime()` returns `NaN`, which propagates through the subtraction, `Math.round`, and `Math.max(1, NaN)` — all yielding `NaN` rather than the intended `DEFAULT_AGE_YEARS` fallback. Consider validating the parsed timestamp before using it:
```ts
const releaseTime = new Date(params.earliestReleaseDate).getTime();
if (!Number.isNaN(releaseTime)) {
const ageMs = now.getTime() - releaseTime;
catalogAgeYears = Math.max(1, Math.round(ageMs / YEAR_MS));
}
```</comment>
<file context>
@@ -0,0 +1,48 @@
+
+ let catalogAgeYears = DEFAULT_AGE_YEARS;
+ if (params.earliestReleaseDate) {
+ const ageMs = now.getTime() - new Date(params.earliestReleaseDate).getTime();
+ catalogAgeYears = Math.max(1, Math.round(ageMs / YEAR_MS));
+ }
</file context>
| for (let i = 0; i < ids.length; i += BATCH_SIZE) { | ||
| const batch = ids.slice(i, i + BATCH_SIZE); | ||
| const url = `https://api.spotify.com/v1/albums?ids=${encodeURIComponent(batch.join(","))}`; | ||
| const response = await fetch(url, { |
There was a problem hiding this comment.
P2: This fetch call has no timeout. If Spotify is unresponsive, the request will hang indefinitely, blocking the measurements endpoint response. Consider adding signal: AbortSignal.timeout(10_000) (or similar) to bound the wait time and allow the best-effort fallback (null release date → default age) to kick in.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/spotify/getAlbums.ts, line 29:
<comment>This `fetch` call has no timeout. If Spotify is unresponsive, the request will hang indefinitely, blocking the measurements endpoint response. Consider adding `signal: AbortSignal.timeout(10_000)` (or similar) to bound the wait time and allow the best-effort fallback (null release date → default age) to kick in.</comment>
<file context>
@@ -0,0 +1,54 @@
+ for (let i = 0; i < ids.length; i += BATCH_SIZE) {
+ const batch = ids.slice(i, i + BATCH_SIZE);
+ const url = `https://api.spotify.com/v1/albums?ids=${encodeURIComponent(batch.join(","))}`;
+ const response = await fetch(url, {
+ method: "GET",
+ headers: {
</file context>
| expect(error).toBeNull(); | ||
| expect(albums).toHaveLength(25); | ||
| expect(fetch).toHaveBeenCalledTimes(2); | ||
| const firstUrl = vi.mocked(fetch).mock.calls[0][0] as string; |
There was a problem hiding this comment.
P3: Only the first batch URL is verified. The second fetch call's URL goes unchecked, so batching errors in subsequent requests (wrong IDs, wrong size) would not be caught. Add an assertion on the second URL for symmetry.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/spotify/__tests__/getAlbums.test.ts, line 31:
<comment>Only the first batch URL is verified. The second `fetch` call's URL goes unchecked, so batching errors in subsequent requests (wrong IDs, wrong size) would not be caught. Add an assertion on the second URL for symmetry.</comment>
<file context>
@@ -0,0 +1,52 @@
+ expect(error).toBeNull();
+ expect(albums).toHaveLength(25);
+ expect(fetch).toHaveBeenCalledTimes(2);
+ const firstUrl = vi.mocked(fetch).mock.calls[0][0] as string;
+ expect(firstUrl).toContain("https://api.spotify.com/v1/albums?ids=");
+ expect(decodeURIComponent(firstUrl)).toContain(ids.slice(0, 20).join(","));
</file context>
|
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
Author
Preview verification — 2026-07-06 (head
|
| Check | Documented | Actual | Result |
|---|---|---|---|
GET /api/catalogs/measurements (no param) |
400 | 400 catalogId parameter is required |
✅ |
?catalogId=not-a-uuid |
400 | 400 catalogId must be a valid UUID |
✅ |
| No auth | 401 | 401 Exactly one of x-api-key or Authorization must be provided |
✅ |
| Unknown catalog (authed, random v4 uuid) | 404 | 404 Catalog not found |
✅ |
Foreign catalog (authed, another account's 740d5050…) |
404 | 404 Catalog not found — no IDOR |
✅ |
OPTIONS |
200 | 200 | ✅ |
| Happy path (authed, own catalog) | 200 envelope | 200 — see below | ✅ |
Happy path detail — created [TEST] chat#1850 api#757 measurements verification (catalog 7d3e5c35-1fac-4b88-aa90-e2e4a52dfe78) under the test account and seeded 5 Del Water Gap songs by ISRC:
measurements: 5 rows, per-ISRC latest, sorted by playcount desc — e.g.To Philly5,953,607,measured_at 2026-07-06T16:40:34Z(today's snapshot run — confirms globalsong_measurementsreuse, no new measurement needed).valuation:{ low: 103227.64, mid: 150299.44, high: 211410.20 }total_streams: 18,505,795 ·catalog_age_years: 5- Formula spot-check: 18,505,795 ÷ 5y × $0.0035 × 1.25 (low gross-up) × 0.6375 (NLS 0.85×0.75) × 10 (low multiple) = $103,230 ≈ returned low $103,227 — matches
marketing/lib/valuation/computeCatalogValuation.tsexactly. ✅
Response ↔ docs reconciliation: top-level keys status, measurements, valuation, total_streams, catalog_age_years — matches the merged docs#265 contract field-for-field; no undocumented fields observed, no documented-but-absent fields.
Two observations for reviewers:
- Ownership is stricter than the sibling. This endpoint 404s a foreign catalog even for the admin bearer, while sibling
GET /api/catalogs/songsserves foreign catalogs to the same token. Strict-by-default seems right for a new endpoint, but if internal admin tooling (account-health reports) is expected to read any account's measurements, an explicit admin path will be needed later. Flagging intentionality, not requesting a change. - The
[TEST]-prefixed catalog above was left in the shared DB per the test-data convention — safe to delete anytime.
🤖 Generated with Claude Code
Contributor
Author
Addendum — docs ↔ implementation reconciliation (2026-07-06)Field-by-field diff of the merged docs#265 contract vs the live preview responses. Correction to my earlier comment: "no undocumented fields observed" was true of the 200 body but not of the 400s. Exact matches:
Differences found:
None of these block merge (the error schema doesn't forbid extra properties). Suggested follow-up: a one-line docs patch adding optional 🤖 Generated with Claude Code |
Contributor
Author
🐛 P1 found post-merge: results silently capped at 1,000 rows — valuation undercounts large catalogsFound 2026-07-06 while building a real 15-artist roster catalog (2,679 measured songs) for chat#1850 homepage testing.
The 1,000 is the Supabase/PostgREST default row limit — the latest-measurements read doesn't paginate, and Suggested fix: page through the read ( Tracked as an Open item on recoupable/chat#1850. 🤖 Generated with Claude Code |
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.
What
Implements
GET /api/catalogs/measurements?catalogId=per the merged-first docs contract recoupable/docs#265 — the data plumbing for the Catalog HQ homepage hero in recoupable/chat#1850 (deferred from chat#1801 / chat#1803).Merge order: docs#265 (contract) → this PR. Base is
mainper api#750 (thetest-branch PR flow is retired; repo CLAUDE.md says never targettest).How it works
validateAuthContext(Privy bearer orx-api-key); ownership checked throughaccount_catalogs— a catalog that doesn't exist or belongs to another account is an indistinguishable 404.catalog_songs→ songs (isrc + title) →song_measurementsfiltered tospotify/platform_displayed_play_count, reduced to the newest capture per ISRC (latestMeasurementsPerIsrc), sorted playcount-desc. Never-measured songs are omitted (per contract).{ low, mid, high }: derived at read time (not persisted, per the chat#1850 architecture decision) inlib/catalog/computeValuationBand.ts, mirroring the marketing card's exact model (marketing/lib/valuation/computeCatalogValuation.ts+nlsBandFromSpotifyGross.ts): annual run-rate = lifetime streams ÷ catalog age (lifetime-average proxy) → NLS ($0.0035/stream × 1.25/1.4/1.6 gross-up × 0.85 × 0.75) → 10×/13×/16× multiples.getCatalogEarliestReleaseDatereadsalbum_idsoff the catalog's newestplaycount_snapshotsrow and fetches release dates from Spotify (new batchedlib/spotify/getAlbums.ts, 20 ids/request, ~2 requests for a typical catalog). Best-effort: any gap falls back to the marketing card's documenteddefault_5yage, surfaced ascatalog_age_yearsin the response. This is the one input that can differ from a given marketing run (e.g. ISRC-scoped snapshots have noalbum_ids) — documented rather than faked.Files
Route
app/api/catalogs/measurements/route.ts→ handlerlib/catalog/getCatalogMeasurementsHandler.ts→ validate (validateGetCatalogMeasurementsQuery, zod uuid) → data fns (lib/supabase/account_catalogs/selectAccountCatalog.ts,lib/supabase/catalog_songs/selectCatalogSongTitles.ts, existingselectSongMeasurements+selectPlaycountSnapshotsextended with acatalogfilter) → pure shaping (latestMeasurementsPerIsrc,computeValuationBand). One exported function per file; all files < 100 LOC.Testing (TDD red → green per unit)
pnpm exec tsc --noEmit: no new errors (200 pre-existing onmain, all in test files excluded fromnext build; identical count with this branch).eslint --fixclean;prettier --checkclean.Error paths (as documented in the contract)
400 missing/invalid
catalogId· 401 unauthed · 404 unknown/foreign catalog · 500 unexpected.🤖 Generated with Claude Code
Summary by cubic
Adds
GET /api/catalogs/measurements?catalogId=to return latest per-ISRC Spotify playcounts plus a read-time valuation band for the catalog. Secures access by account and matches the marketing card’s valuation model.platform_displayed_play_countper ISRC fromsong_measurements(sorted desc); unmeasured songs omitted.validateAuthContextandaccount_catalogs; foreign/unknown catalogs return 404.{ measurements, valuation: { low, mid, high }, total_streams, catalog_age_years }.getAlbums, 20 ids/request) with a 5y fallback.catalogId, 401 unauthenticated, 404 not found, 500 on unexpected errors.Written for commit 621c169. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes