feat: optional catalog_id on POST /api/emails leads the email with the value delta - #799
Open
sweetmantech wants to merge 2 commits into
Open
feat: optional catalog_id on POST /api/emails leads the email with the value delta#799sweetmantech wants to merge 2 commits into
sweetmantech wants to merge 2 commits into
Conversation
…ations Every valuation run and each first whole-catalog measurements read of the day now writes a row to catalog_valuations, and the new GET returns the series latest-first (limit=1 = current value). chat#1889 row 15. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e value delta Subject gets prefixed with the catalog's current value and signed change since the previous measurement; a valuation hero lands above the body. Ownership-gated in the handler, best-effort end to end - a delta failure never costs a delivery. chat#1911 row 5. 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.
|
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (12)
📒 Files selected for processing (14)
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 |
19 tasks
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found across 26 files
Confidence score: 3/5
lib/catalog/getCatalogMeasurementsHandler.tsnow performspersistCatalogValuationon a GET path, so read requests can cause writes and side effects through retries/caching patterns, which raises regression risk for API consumers and infrastructure assumptions — move persistence behind an explicit write path or background workflow.lib/catalog/persistCatalogValuation.tsuses a non-atomic check-then-insert for daily dedupe, so concurrent measurement requests can create duplicate same-day valuation rows and inconsistent history — enforce a catalog/day uniqueness constraint and switch to atomic upsert/RPC logic.lib/supabase/catalog_valuations/selectCatalogValuations.tsallows valuation-history read failures to look like empty history ingetCatalogValuationDelta, which can silently produce fallback-based deltas instead of surfacing data-access errors — distinguish error/null from true empty results and propagate a failure state.lib/catalog/__tests__/getCatalogValuationDelta.test.tsandlib/catalog/__tests__/getCatalogValuationsHandler.test.tscurrently miss key assertions (fallback wiring calls and sanitized 500 body), so regressions in error handling and data-source selection can slip through — tighten those assertions before relying on these paths.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/catalog/getCatalogMeasurementsHandler.ts">
<violation number="1" location="lib/catalog/getCatalogMeasurementsHandler.ts:65">
P2: Custom agent: **API Design Consistency and Maintainability**
The GET endpoint for `/api/catalogs/{catalogId}/measurements` now writes to the database via `persistCatalogValuation`, which violates the API design rule that GET should be for data retrieval only and POST for mutations. Every GET request now queries (and may insert into) `catalog_valuations`, creating a side effect in what should be a safe, idempotent read endpoint. Consider moving the persistence to a POST endpoint or a background task so the GET handler remains read-only.</violation>
<violation number="2" location="lib/catalog/getCatalogMeasurementsHandler.ts:65">
P3: Whole-catalog measurement reads now wait for valuation-history I/O before returning. Every such request performs the dedupe query, and the first request of a day also waits for the insert, even though persistence is explicitly best-effort and does not affect the response payload. Moving this write behind the response lifecycle (for example with the existing `after` mechanism) or to a background job would keep page refresh latency independent of history persistence.</violation>
</file>
<file name="lib/catalog/validateGetCatalogValuationsQuery.ts">
<violation number="1" location="lib/catalog/validateGetCatalogValuationsQuery.ts:43">
P3: Catalog query validation behavior now lives in two near-identical implementations, so future auth or error-envelope fixes can diverge. Consider extracting the shared authenticated-query validation flow and retaining endpoint-specific schemas.</violation>
</file>
<file name="lib/catalog/__tests__/getCatalogValuationDelta.test.ts">
<violation number="1" location="lib/catalog/__tests__/getCatalogValuationDelta.test.ts:32">
P2: Fallback test sets up aggregate/earliest-release mocks but never asserts they were called, so the test can pass even if the internal fallback wiring is broken (e.g., the function starts using a different data source instead of `selectCatalogMeasurementsAggregate`).</violation>
</file>
<file name="lib/catalog/persistCatalogValuation.ts">
<violation number="1" location="lib/catalog/persistCatalogValuation.ts:35">
P2: Concurrent measurement page loads can create multiple same-day history rows despite `dedupeDaily`; the check-then-insert sequence is not atomic. Enforce catalog/day uniqueness and use an atomic insert/upsert (or an RPC) so duplicate rows do not skew the valuation history/delta.</violation>
</file>
<file name="lib/catalog/__tests__/getCatalogValuationsHandler.test.ts">
<violation number="1" location="lib/catalog/__tests__/getCatalogValuationsHandler.test.ts:113">
P3: The 500-error test only checks `response.status` but not the response body. Add an assertion that the body contains the safe `'Internal server error'` message (not raw exception text), per the project convention for 500 responses.</violation>
</file>
<file name="lib/emails/valuationDelta/buildValuationDeltaSubjectPrefix.ts">
<violation number="1" location="lib/emails/valuationDelta/buildValuationDeltaSubjectPrefix.ts:16">
P2: The percent-change formula `((current - previous) / previous) * 100` and the signed formatting (`+10.0%` / `-10.0%`) are duplicated between `buildValuationDeltaSubjectPrefix.ts` and `renderValuationDeltaHero.ts`. Each contains the exact same arithmetic and string-building logic, which means any future adjustment to how percentages are rounded, signed, or displayed must be updated in both places. Consider extracting a shared helper such as `computeCatalogPercentChange(delta)` that returns the signed formatted string, so the subject prefix and the hero block each call it in one line.</violation>
</file>
<file name="lib/supabase/catalog_valuations/selectCatalogValuations.ts">
<violation number="1" location="lib/supabase/catalog_valuations/selectCatalogValuations.ts:28">
P2: A valuation-history read failure is being represented as an empty-history result to callers. `getCatalogValuationDelta` treats `null` exactly like an empty array (`if (rows && rows.length > 0)`), then falls back to the live measurements and can add a baseline subject/hero to the email even though the history lookup failed. That contradicts the documented “any failure -> null” behavior and can present a stale or misleading valuation during a database outage; the selector/consumer contract should distinguish an empty result from a failed read.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant EmailHandler as sendEmailHandler
participant BodyValidator as validateSendEmailBody
participant CatalogOwner as selectAccountCatalog
participant EmailProcessor as processAndSendEmail
participant DeltaResolver as getCatalogValuationDelta
participant ValuationsDB as selectCatalogValuations
participant LiveMeas as "Live Measurements"
participant PrefixBuilder as buildValuationDeltaSubjectPrefix
participant HeroRenderer as renderValuationDeltaHero
participant Resend as "Email Service (Resend)"
participant ValuationPersister as persistCatalogValuation
note over Client,ValuationPersister: NEW: Optional catalog_id on POST /api/emails (chat#1911 row 5)
Client->>EmailHandler: POST /api/emails { catalog_id? }
EmailHandler->>BodyValidator: Validate request body + auth
BodyValidator-->>EmailHandler: { accountId, catalog_id, ... }
alt catalog_id present
EmailHandler->>CatalogOwner: selectAccountCatalog(accountId, catalogId)
alt Catalog owned by caller
CatalogOwner-->>EmailHandler: { account, catalog }
EmailHandler->>EmailProcessor: processAndSendEmail({ catalog_id })
else Catalog not owned or missing
CatalogOwner-->>EmailHandler: null
note over EmailHandler,EmailProcessor: Drop catalog_id (email sends unchanged)
EmailHandler->>EmailProcessor: processAndSendEmail({ catalog_id: undefined })
end
else no catalog_id
EmailHandler->>EmailProcessor: processAndSendEmail({ subject, html, ... })
end
note over EmailProcessor: Enrichment is best-effort (try/catch)
EmailProcessor->>DeltaResolver: getCatalogValuationDelta(catalogId)
DeltaResolver->>ValuationsDB: selectCatalogValuations(limit=2)
alt Two or more rows
ValuationsDB-->>DeltaResolver: current + previous
else One row (baseline)
ValuationsDB-->>DeltaResolver: current (previous null)
else No history rows
DeltaResolver->>LiveMeas: Fallback: aggregate + earliestReleaseDate + computeValuationBand
alt Measured songs exist
LiveMeas-->>DeltaResolver: current (previous null)
else Empty catalog
LiveMeas-->>DeltaResolver: null
end
end
alt Delta resolved
DeltaResolver-->>EmailProcessor: { current, previous }
EmailProcessor->>PrefixBuilder: buildValuationDeltaSubjectPrefix(delta)
PrefixBuilder-->>EmailProcessor: "$1.1M (+10.0%) · "
EmailProcessor->>HeroRenderer: renderValuationDeltaHero(delta)
HeroRenderer-->>EmailProcessor: hero HTML block
EmailProcessor->>EmailProcessor: Prepend hero to bodyHtml, set finalSubject
else Delta null (failure or empty)
DeltaResolver-->>EmailProcessor: null
note over EmailProcessor: Email unchanged (subject and body stay original)
end
EmailProcessor->>Resend: sendEmailWithResend({ subject: finalSubject, html: bodyHtml + layout })
Resend-->>EmailProcessor: email id
EmailProcessor-->>EmailHandler: success
EmailHandler-->>Client: 200 OK
note over ValuationPersister: Persistence paths (chat#1889 row 15)
%% Called from getCatalogMeasurementsHandler (whole-catalog read)
note over ValuationPersister: GET /api/catalogs/{id}/measurements (no artist-scope) → persistCatalogValuation(dedupeDaily: true)
%% Called from runValuationHandler (every valuation run)
note over ValuationPersister: POST /api/valuations/run → persistCatalogValuation(dedupeDaily: false)
ValuationPersister->>ValuationsDB: insertCatalogValuation(...)
alt Dedupe daily enabled
ValuationPersister->>ValuationsDB: selectCatalogValuations(limit=1) first
opt Latest row is from a different day (or none)
ValuationPersister->>ValuationsDB: insertCatalogValuation(...)
end
else Unconditional
ValuationPersister->>ValuationsDB: insertCatalogValuation(...)
end
note over ValuationPersister: Best‑effort (errors logged, never thrown)
note over Client,ValuationPersister: Also NEW: GET /api/catalogs/{catalogId}/valuations returns persisted history (auth + ownership gate)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // row 15) — daily-deduped so page refreshes don't mint rows. Artist-scoped | ||
| // reads value a slice of the catalog, not the catalog, so they never write. | ||
| if (!artistAccountId) { | ||
| await persistCatalogValuation({ |
There was a problem hiding this comment.
P2: Custom agent: API Design Consistency and Maintainability
The GET endpoint for /api/catalogs/{catalogId}/measurements now writes to the database via persistCatalogValuation, which violates the API design rule that GET should be for data retrieval only and POST for mutations. Every GET request now queries (and may insert into) catalog_valuations, creating a side effect in what should be a safe, idempotent read endpoint. Consider moving the persistence to a POST endpoint or a background task so the GET handler remains read-only.
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 65:
<comment>The GET endpoint for `/api/catalogs/{catalogId}/measurements` now writes to the database via `persistCatalogValuation`, which violates the API design rule that GET should be for data retrieval only and POST for mutations. Every GET request now queries (and may insert into) `catalog_valuations`, creating a side effect in what should be a safe, idempotent read endpoint. Consider moving the persistence to a POST endpoint or a background task so the GET handler remains read-only.</comment>
<file context>
@@ -57,6 +58,19 @@ export async function getCatalogMeasurementsHandler(
+ // row 15) — daily-deduped so page refreshes don't mint rows. Artist-scoped
+ // reads value a slice of the catalog, not the catalog, so they never write.
+ if (!artistAccountId) {
+ await persistCatalogValuation({
+ catalogId,
+ valuation,
</file context>
| row(1_000_000, "2026-07-22T00:00:00Z"), | ||
| ] as never); | ||
|
|
||
| const delta = await getCatalogValuationDelta({ catalogId }); |
There was a problem hiding this comment.
P2: Fallback test sets up aggregate/earliest-release mocks but never asserts they were called, so the test can pass even if the internal fallback wiring is broken (e.g., the function starts using a different data source instead of selectCatalogMeasurementsAggregate).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/__tests__/getCatalogValuationDelta.test.ts, line 32:
<comment>Fallback test sets up aggregate/earliest-release mocks but never asserts they were called, so the test can pass even if the internal fallback wiring is broken (e.g., the function starts using a different data source instead of `selectCatalogMeasurementsAggregate`).</comment>
<file context>
@@ -0,0 +1,85 @@
+ row(1_000_000, "2026-07-22T00:00:00Z"),
+ ] as never);
+
+ const delta = await getCatalogValuationDelta({ catalogId });
+
+ expect(selectCatalogValuations).toHaveBeenCalledWith({ catalogId, limit: 2 });
</file context>
| }): Promise<void> { | ||
| try { | ||
| if (dedupeDaily) { | ||
| const latest = await selectCatalogValuations({ catalogId, limit: 1 }); |
There was a problem hiding this comment.
P2: Concurrent measurement page loads can create multiple same-day history rows despite dedupeDaily; the check-then-insert sequence is not atomic. Enforce catalog/day uniqueness and use an atomic insert/upsert (or an RPC) so duplicate rows do not skew the valuation history/delta.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/persistCatalogValuation.ts, line 35:
<comment>Concurrent measurement page loads can create multiple same-day history rows despite `dedupeDaily`; the check-then-insert sequence is not atomic. Enforce catalog/day uniqueness and use an atomic insert/upsert (or an RPC) so duplicate rows do not skew the valuation history/delta.</comment>
<file context>
@@ -0,0 +1,52 @@
+}): Promise<void> {
+ try {
+ if (dedupeDaily) {
+ const latest = await selectCatalogValuations({ catalogId, limit: 1 });
+ const newestDay = latest?.[0]?.measured_at?.slice(0, 10);
+ const today = new Date().toISOString().slice(0, 10);
</file context>
| if (!delta.previous) return `${value} baseline · `; | ||
| if (delta.previous.mid <= 0) return `${value} · `; | ||
|
|
||
| const pct = ((delta.current.mid - delta.previous.mid) / delta.previous.mid) * 100; |
There was a problem hiding this comment.
P2: The percent-change formula ((current - previous) / previous) * 100 and the signed formatting (+10.0% / -10.0%) are duplicated between buildValuationDeltaSubjectPrefix.ts and renderValuationDeltaHero.ts. Each contains the exact same arithmetic and string-building logic, which means any future adjustment to how percentages are rounded, signed, or displayed must be updated in both places. Consider extracting a shared helper such as computeCatalogPercentChange(delta) that returns the signed formatted string, so the subject prefix and the hero block each call it in one line.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/emails/valuationDelta/buildValuationDeltaSubjectPrefix.ts, line 16:
<comment>The percent-change formula `((current - previous) / previous) * 100` and the signed formatting (`+10.0%` / `-10.0%`) are duplicated between `buildValuationDeltaSubjectPrefix.ts` and `renderValuationDeltaHero.ts`. Each contains the exact same arithmetic and string-building logic, which means any future adjustment to how percentages are rounded, signed, or displayed must be updated in both places. Consider extracting a shared helper such as `computeCatalogPercentChange(delta)` that returns the signed formatted string, so the subject prefix and the hero block each call it in one line.</comment>
<file context>
@@ -0,0 +1,19 @@
+ if (!delta.previous) return `${value} baseline · `;
+ if (delta.previous.mid <= 0) return `${value} · `;
+
+ const pct = ((delta.current.mid - delta.previous.mid) / delta.previous.mid) * 100;
+ const signed = `${pct >= 0 ? "+" : "-"}${Math.abs(pct).toFixed(1)}%`;
+ return `${value} (${signed}) · `;
</file context>
|
|
||
| if (error) { | ||
| console.error("Error fetching catalog_valuations:", error); | ||
| return null; |
There was a problem hiding this comment.
P2: A valuation-history read failure is being represented as an empty-history result to callers. getCatalogValuationDelta treats null exactly like an empty array (if (rows && rows.length > 0)), then falls back to the live measurements and can add a baseline subject/hero to the email even though the history lookup failed. That contradicts the documented “any failure -> null” behavior and can present a stale or misleading valuation during a database outage; the selector/consumer contract should distinguish an empty result from a failed read.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/supabase/catalog_valuations/selectCatalogValuations.ts, line 28:
<comment>A valuation-history read failure is being represented as an empty-history result to callers. `getCatalogValuationDelta` treats `null` exactly like an empty array (`if (rows && rows.length > 0)`), then falls back to the live measurements and can add a baseline subject/hero to the email even though the history lookup failed. That contradicts the documented “any failure -> null” behavior and can present a stale or misleading valuation during a database outage; the selector/consumer contract should distinguish an empty result from a failed read.</comment>
<file context>
@@ -0,0 +1,32 @@
+
+ if (error) {
+ console.error("Error fetching catalog_valuations:", error);
+ return null;
+ }
+
</file context>
| request: NextRequest, | ||
| catalogId: string, | ||
| ): Promise<NextResponse | GetCatalogValuationsQuery> { | ||
| const authResult = await validateAuthContext(request); |
There was a problem hiding this comment.
P3: Catalog query validation behavior now lives in two near-identical implementations, so future auth or error-envelope fixes can diverge. Consider extracting the shared authenticated-query validation flow and retaining endpoint-specific schemas.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/validateGetCatalogValuationsQuery.ts, line 43:
<comment>Catalog query validation behavior now lives in two near-identical implementations, so future auth or error-envelope fixes can diverge. Consider extracting the shared authenticated-query validation flow and retaining endpoint-specific schemas.</comment>
<file context>
@@ -0,0 +1,65 @@
+ request: NextRequest,
+ catalogId: string,
+): Promise<NextResponse | GetCatalogValuationsQuery> {
+ const authResult = await validateAuthContext(request);
+ if (authResult instanceof NextResponse) return authResult;
+
</file context>
|
|
||
| const response = await getCatalogValuationsHandler(makeRequest(), catalogId); | ||
|
|
||
| expect(response.status).toBe(500); |
There was a problem hiding this comment.
P3: The 500-error test only checks response.status but not the response body. Add an assertion that the body contains the safe 'Internal server error' message (not raw exception text), per the project convention for 500 responses.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/__tests__/getCatalogValuationsHandler.test.ts, line 113:
<comment>The 500-error test only checks `response.status` but not the response body. Add an assertion that the body contains the safe `'Internal server error'` message (not raw exception text), per the project convention for 500 responses.</comment>
<file context>
@@ -0,0 +1,115 @@
+
+ const response = await getCatalogValuationsHandler(makeRequest(), catalogId);
+
+ expect(response.status).toBe(500);
+ });
+});
</file context>
| // row 15) — daily-deduped so page refreshes don't mint rows. Artist-scoped | ||
| // reads value a slice of the catalog, not the catalog, so they never write. | ||
| if (!artistAccountId) { | ||
| await persistCatalogValuation({ |
There was a problem hiding this comment.
P3: Whole-catalog measurement reads now wait for valuation-history I/O before returning. Every such request performs the dedupe query, and the first request of a day also waits for the insert, even though persistence is explicitly best-effort and does not affect the response payload. Moving this write behind the response lifecycle (for example with the existing after mechanism) or to a background job would keep page refresh latency independent of history persistence.
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 65:
<comment>Whole-catalog measurement reads now wait for valuation-history I/O before returning. Every such request performs the dedupe query, and the first request of a day also waits for the insert, even though persistence is explicitly best-effort and does not affect the response payload. Moving this write behind the response lifecycle (for example with the existing `after` mechanism) or to a background job would keep page refresh latency independent of history persistence.</comment>
<file context>
@@ -57,6 +58,19 @@ export async function getCatalogMeasurementsHandler(
+ // row 15) — daily-deduped so page refreshes don't mint rows. Artist-scoped
+ // reads value a slice of the catalog, not the catalog, so they never write.
+ if (!artistAccountId) {
+ await persistCatalogValuation({
+ catalogId,
+ valuation,
</file context>
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.
Summary
Row 5 of chat#1911: the weekly report leads with the value delta. Implements the contract in docs#281 - an optional
catalog_idonPOST /api/emailsthat turns any send into a delta-led report: subject prefixed$1.1M (+10.0%) · <subject>, valuation hero block above the body.Stacked on api#795 (needs
selectCatalogValuations+ thecatalog_valuationshistory), so this branch contains #795's commit until it merges. Merge order: docs#281 → database#52 → api#795 → this.What it does
lib/catalog/getCatalogValuationDelta.ts: the two most recent history rows →{current, previous}; one row → baseline (previous: null); empty history but measured songs → read-time band fallback (so the first report after the table ships still leads with a number); empty catalog or any failure →null, never throws.lib/emails/valuationDelta/buildValuationDeltaSubjectPrefix.ts:$1.1M (+10.0%) ·/$900K (-10.0%) ·/$1.1M baseline ·/ percent omitted when the previous mid is zero. Money via the sharedformatCompactUsd(DRY with the valuation report).lib/emails/valuationDelta/renderValuationDeltaHero.ts: previous → current + signed % (green/red), or the baseline note "Your baseline is set. The next report shows the change." Table-based inline-style markup like the valuation report blocks. No em dashes (asserted by test).processAndSendEmail: opt-in viacatalog_id; enrichment is try/caught so a delta failure never costs a delivery.sendEmailHandler+validateSendEmailBody:catalog_id(uuid) accepted on the REST body and passed through only afterselectAccountCatalogconfirms ownership - an unowned id is dropped and the email sends unchanged (documented behavior; a caller can never lead their email with someone else's valuation).send_emaildeliberately untouched (YAGNI): the weekly-report task sends flow through the REST path, and the MCP handler has no account resolution today.Tests (TDD, red→green)
19 new tests across 3 new suites + 7 wiring cases in
processAndSendEmail.test.ts/sendEmailHandler.test.ts(subject+hero on delta, unchanged on null delta, still-sends on throw, no delta path without catalog_id, ownership pass-through/drop/skip). All confirmed RED first.tsc --noEmit: exactly the 236-error baseline. Note: the one hit insideprocessAndSendEmail.test.ts(result.errornarrowing) pre-exists on the base branch at line 129 (verified by stashing); my insertions only shifted its line number.Tracked in chat#1911 row 5.
🤖 Generated with Claude Code
Summary by cubic
POST
/api/emailsaccepts an optionalcatalog_idto lead emails with the catalog’s valuation delta (subject prefix + hero), gated by ownership. Added persisted valuation history and a new GET/api/catalogs/{catalogId}/valuationsto power the delta and reports, fulfilling chat#1911 (weekly report leads with value delta).New Features
/api/emails: optionalcatalog_id; only applied if the caller owns the catalog; best‑effort (email sends unchanged on unowned/empty/failure).catalog_valuations: on valuation runs (always) and on whole‑catalog measurements reads once per UTC day (deduped)./api/catalogs/{catalogId}/valuations: auth + ownership; 404 if unowned;limit1–100 (default 30); latest‑first rows with{ low, mid, high, measured_song_count, total_streams, measured_at }.processAndSendEmail.Migration
catalog_valuationstable before enabling this feature.send_emailpath is unchanged.Written for commit 8adecb2. Summary will update on new commits.