From f99277e0bcd6dff79f1f67e0f933b8d2dfbd8597 Mon Sep 17 00:00:00 2001 From: "sweetman.eth" Date: Tue, 16 Jun 2026 11:40:47 -0500 Subject: [PATCH 1/7] feat(measurement-jobs): free-tier card gate (setup mode) + instant backfill drain (#671) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two chat#1796 refinements on the historical (Songstats) path: 1. Free-tier card-on-file link. The gate was issuing the paid subscription checkout ($99/mo after a 30-day trial). New createCardOnFileSession uses Stripe Checkout `mode: "setup"` — collects a card for $0, no subscription, no Stripe product. The account then pays only for metered usage via credits. 2. Instant drain. After enqueuing a historical job, fire-and-forget start(songstatsBackfillWorkflow) so the backfill begins immediately instead of waiting up to 24h for the cron. Safe by reuse: the workflow's budget gate (limit − reserve − rolling-30d ledger) caps it to the Songstats quota and SKIP LOCKED prevents double-claiming with the daily cron, which stays as the backstop. Only kicks when something was actually enqueued. 26 new/updated unit tests; research+stripe+workflows suite 453 green; tsc/lint/format clean. --- .../enqueueHistoricalBackfill.test.ts | 19 +++++++++ .../ensureSongstatsPaymentMethod.test.ts | 12 +++--- .../enqueueHistoricalBackfill.ts | 15 +++++++ .../ensureSongstatsPaymentMethod.ts | 7 +++- .../__tests__/createCardOnFileSession.test.ts | 39 +++++++++++++++++++ lib/stripe/createCardOnFileSession.ts | 31 +++++++++++++++ 6 files changed, 115 insertions(+), 8 deletions(-) create mode 100644 lib/stripe/__tests__/createCardOnFileSession.test.ts create mode 100644 lib/stripe/createCardOnFileSession.ts diff --git a/lib/research/measurement_jobs/__tests__/enqueueHistoricalBackfill.test.ts b/lib/research/measurement_jobs/__tests__/enqueueHistoricalBackfill.test.ts index 20cde74a7..03010fa41 100644 --- a/lib/research/measurement_jobs/__tests__/enqueueHistoricalBackfill.test.ts +++ b/lib/research/measurement_jobs/__tests__/enqueueHistoricalBackfill.test.ts @@ -3,6 +3,7 @@ import { enqueueHistoricalBackfill } from "../enqueueHistoricalBackfill"; import { resolveScopeSongs } from "../resolveScopeSongs"; import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements"; import { upsertSongstatsBackfillQueue } from "@/lib/supabase/songstats_backfill_queue/upsertSongstatsBackfillQueue"; +import { start } from "workflow/api"; vi.mock("../resolveScopeSongs", () => ({ resolveScopeSongs: vi.fn() })); vi.mock("@/lib/supabase/song_measurements/selectSongMeasurements", () => ({ @@ -11,6 +12,10 @@ vi.mock("@/lib/supabase/song_measurements/selectSongMeasurements", () => ({ vi.mock("@/lib/supabase/songstats_backfill_queue/upsertSongstatsBackfillQueue", () => ({ upsertSongstatsBackfillQueue: vi.fn(), })); +vi.mock("workflow/api", () => ({ start: vi.fn() })); +vi.mock("@/app/workflows/songstatsBackfillWorkflow", () => ({ + songstatsBackfillWorkflow: vi.fn(), +})); describe("enqueueHistoricalBackfill", () => { beforeEach(() => { @@ -47,6 +52,8 @@ describe("enqueueHistoricalBackfill", () => { expect(r).toEqual({ data: { status: "success", source: "historical", id: null, enqueued: 2, skipped: 1 }, }); + // kick the drain immediately so the user doesn't wait for the daily cron + expect(start).toHaveBeenCalledTimes(1); }); it("ranks a song with no prior measurement at 0", async () => { @@ -57,4 +64,16 @@ describe("enqueueHistoricalBackfill", () => { expect(upsertSongstatsBackfillQueue).toHaveBeenCalledWith({ song: "I9", rank_score: 0 }); }); + + it("does NOT kick the drain when everything was already backfilled (nothing enqueued)", async () => { + vi.mocked(resolveScopeSongs).mockResolvedValue(["I1"]); + vi.mocked(selectSongMeasurements).mockResolvedValue([ + { song: "I1", value: 500, data_source: "songstats" }, + ] as never); + + const r = await enqueueHistoricalBackfill({ isrcs: ["I1"] }); + + expect((r as { data: { enqueued: number } }).data.enqueued).toBe(0); + expect(start).not.toHaveBeenCalled(); + }); }); diff --git a/lib/research/measurement_jobs/__tests__/ensureSongstatsPaymentMethod.test.ts b/lib/research/measurement_jobs/__tests__/ensureSongstatsPaymentMethod.test.ts index 1e2c1f072..d75629a53 100644 --- a/lib/research/measurement_jobs/__tests__/ensureSongstatsPaymentMethod.test.ts +++ b/lib/research/measurement_jobs/__tests__/ensureSongstatsPaymentMethod.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { ensureSongstatsPaymentMethod } from "../ensureSongstatsPaymentMethod"; import { findStripeCustomerForAccount } from "@/lib/stripe/findStripeCustomerForAccount"; import { findDefaultPaymentMethodForCustomer } from "@/lib/stripe/findDefaultPaymentMethodForCustomer"; -import { createStripeSession } from "@/lib/stripe/createStripeSession"; +import { createCardOnFileSession } from "@/lib/stripe/createCardOnFileSession"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({})) })); vi.mock("@/lib/stripe/findStripeCustomerForAccount", () => ({ @@ -11,7 +11,7 @@ vi.mock("@/lib/stripe/findStripeCustomerForAccount", () => ({ vi.mock("@/lib/stripe/findDefaultPaymentMethodForCustomer", () => ({ findDefaultPaymentMethodForCustomer: vi.fn(), })); -vi.mock("@/lib/stripe/createStripeSession", () => ({ createStripeSession: vi.fn() })); +vi.mock("@/lib/stripe/createCardOnFileSession", () => ({ createCardOnFileSession: vi.fn() })); describe("ensureSongstatsPaymentMethod", () => { beforeEach(() => vi.clearAllMocks()); @@ -23,17 +23,17 @@ describe("ensureSongstatsPaymentMethod", () => { const r = await ensureSongstatsPaymentMethod("acc_1"); expect(r).toBeNull(); - expect(createStripeSession).not.toHaveBeenCalled(); + expect(createCardOnFileSession).not.toHaveBeenCalled(); }); it("402s with a free-tier checkout link when there is no Stripe customer", async () => { vi.mocked(findStripeCustomerForAccount).mockResolvedValue(null); - vi.mocked(createStripeSession).mockResolvedValue({ url: "https://checkout/free" } as never); + vi.mocked(createCardOnFileSession).mockResolvedValue({ url: "https://checkout/free" } as never); const r = await ensureSongstatsPaymentMethod("acc_1"); expect(findDefaultPaymentMethodForCustomer).not.toHaveBeenCalled(); - expect(createStripeSession).toHaveBeenCalledWith("acc_1", expect.any(String)); + expect(createCardOnFileSession).toHaveBeenCalledWith("acc_1", expect.any(String)); expect((r as Response).status).toBe(402); expect(await (r as Response).json()).toMatchObject({ status: "error", @@ -44,7 +44,7 @@ describe("ensureSongstatsPaymentMethod", () => { it("402s with a checkout link when the customer exists but has no card", async () => { vi.mocked(findStripeCustomerForAccount).mockResolvedValue("cus_1"); vi.mocked(findDefaultPaymentMethodForCustomer).mockResolvedValue(null); - vi.mocked(createStripeSession).mockResolvedValue({ url: "https://checkout/free" } as never); + vi.mocked(createCardOnFileSession).mockResolvedValue({ url: "https://checkout/free" } as never); const r = await ensureSongstatsPaymentMethod("acc_1"); diff --git a/lib/research/measurement_jobs/enqueueHistoricalBackfill.ts b/lib/research/measurement_jobs/enqueueHistoricalBackfill.ts index 3c0cc7af5..ffd6f3f20 100644 --- a/lib/research/measurement_jobs/enqueueHistoricalBackfill.ts +++ b/lib/research/measurement_jobs/enqueueHistoricalBackfill.ts @@ -1,6 +1,8 @@ +import { start } from "workflow/api"; import { resolveScopeSongs } from "./resolveScopeSongs"; import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements"; import { upsertSongstatsBackfillQueue } from "@/lib/supabase/songstats_backfill_queue/upsertSongstatsBackfillQueue"; +import { songstatsBackfillWorkflow } from "@/app/workflows/songstatsBackfillWorkflow"; import type { CreateMeasurementJobBody } from "./validateCreateMeasurementJobRequest"; const METRIC = "platform_displayed_play_count"; @@ -61,5 +63,18 @@ export async function enqueueHistoricalBackfill( enqueued += batch.length; } + // Kick the drain now instead of waiting for the daily cron. The workflow's own + // budget gate (limit − reserve − rolling-30d ledger) means it only drains what + // the Songstats quota allows and then stops; SKIP LOCKED claims keep it from + // double-processing alongside the cron, which stays as the backstop. Fire-and- + // forget — a scheduling hiccup must not fail the (already-enqueued) job. + if (enqueued > 0) { + try { + await start(songstatsBackfillWorkflow); + } catch (error) { + console.error("[measurement-jobs] failed to kick backfill drain:", error); + } + } + return { data: { status: "success", source: "historical", id: null, enqueued, skipped } }; } diff --git a/lib/research/measurement_jobs/ensureSongstatsPaymentMethod.ts b/lib/research/measurement_jobs/ensureSongstatsPaymentMethod.ts index 71e3dd37c..8654dd19e 100644 --- a/lib/research/measurement_jobs/ensureSongstatsPaymentMethod.ts +++ b/lib/research/measurement_jobs/ensureSongstatsPaymentMethod.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { findStripeCustomerForAccount } from "@/lib/stripe/findStripeCustomerForAccount"; import { findDefaultPaymentMethodForCustomer } from "@/lib/stripe/findDefaultPaymentMethodForCustomer"; -import { createStripeSession } from "@/lib/stripe/createStripeSession"; +import { createCardOnFileSession } from "@/lib/stripe/createCardOnFileSession"; import { CREDIT_AUTO_RECHARGE_FALLBACK_SUCCESS_URL } from "@/lib/credits/const"; /** @@ -22,7 +22,10 @@ export async function ensureSongstatsPaymentMethod( const paymentMethod = customerId ? await findDefaultPaymentMethodForCustomer(customerId) : null; if (paymentMethod) return null; - const session = await createStripeSession(accountId, CREDIT_AUTO_RECHARGE_FALLBACK_SUCCESS_URL); + const session = await createCardOnFileSession( + accountId, + CREDIT_AUTO_RECHARGE_FALLBACK_SUCCESS_URL, + ); return NextResponse.json( { status: "error", diff --git a/lib/stripe/__tests__/createCardOnFileSession.test.ts b/lib/stripe/__tests__/createCardOnFileSession.test.ts new file mode 100644 index 000000000..14d1e0016 --- /dev/null +++ b/lib/stripe/__tests__/createCardOnFileSession.test.ts @@ -0,0 +1,39 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const { checkoutSessionsCreate, resolveStripeCustomerForAccountMock } = vi.hoisted(() => ({ + checkoutSessionsCreate: vi.fn(), + resolveStripeCustomerForAccountMock: vi.fn(), +})); + +vi.mock("@/lib/stripe/client", () => ({ + default: { checkout: { sessions: { create: checkoutSessionsCreate } } }, +})); +vi.mock("@/lib/stripe/resolveStripeCustomerForAccount", () => ({ + resolveStripeCustomerForAccount: resolveStripeCustomerForAccountMock, +})); + +const { createCardOnFileSession } = await import("@/lib/stripe/createCardOnFileSession"); + +describe("createCardOnFileSession", () => { + beforeEach(() => { + vi.clearAllMocks(); + checkoutSessionsCreate.mockResolvedValue({ id: "cs_x", url: "https://checkout/setup" }); + resolveStripeCustomerForAccountMock.mockResolvedValue("cus_acc1"); + }); + + it("creates a setup-mode session (collect card only, no subscription/price)", async () => { + await createCardOnFileSession("acc-1", "https://example.com/success"); + + expect(resolveStripeCustomerForAccountMock).toHaveBeenCalledWith("acc-1"); + const params = checkoutSessionsCreate.mock.calls[0][0]; + expect(params).toMatchObject({ + customer: "cus_acc1", + mode: "setup", + client_reference_id: "acc-1", + success_url: "https://example.com/success", + }); + // free tier: no subscription, no line_items/price + expect(params.mode).not.toBe("subscription"); + expect(params.line_items).toBeUndefined(); + }); +}); diff --git a/lib/stripe/createCardOnFileSession.ts b/lib/stripe/createCardOnFileSession.ts new file mode 100644 index 000000000..f94959542 --- /dev/null +++ b/lib/stripe/createCardOnFileSession.ts @@ -0,0 +1,31 @@ +import type Stripe from "stripe"; +import stripeClient from "@/lib/stripe/client"; +import { resolveStripeCustomerForAccount } from "@/lib/stripe/resolveStripeCustomerForAccount"; + +/** + * A Stripe Checkout session that **only collects a card on file** — `mode: + * "setup"`, $0, no subscription or price. This is the "free tier": the account + * saves a payment method (so metered Songstats usage can be charged later via + * the credits system) without committing to any recurring plan. Needs no Stripe + * product. Contrast with {@link createStripeSession}, which is the paid + * subscription flow. + * + * @param accountId - The account to attach the card to. + * @param successUrl - Where Stripe redirects after the card is saved. + */ +export async function createCardOnFileSession( + accountId: string, + successUrl: string, +): Promise { + const metadata = { accountId }; + const customer = await resolveStripeCustomerForAccount(accountId); + + return stripeClient.checkout.sessions.create({ + customer, + mode: "setup", + currency: "usd", + client_reference_id: accountId, + metadata, + success_url: successUrl, + }); +} From 5fa5e3ac7c580ba52c5c53db7a9bdc2f7216035b Mon Sep 17 00:00:00 2001 From: "sweetman.eth" Date: Tue, 16 Jun 2026 16:41:22 -0500 Subject: [PATCH 2/7] fix(songstats-backfill): backoff on 429 + defer instead of churn (chat#1797) (#673) Pacing/backoff + per-step logging for the Songstats backfill drain (chat#1797 bullets 1 & 3). Bounded exponential backoff (fetchSongstatsWithBackoff, 502/503/504/408/429), defer-to-pending past the bound with claimed-batch release, per-step + per-batch logging. --- .../__tests__/backfillTrackStep.test.ts | 92 +++++++++---------- .../songstatsBackfillWorkflow.test.ts | 61 ++++++++++++ app/workflows/backfillTrackStep.ts | 59 +++++++----- app/workflows/releaseClaimedRowsStep.ts | 15 +++ app/workflows/songstatsBackfillWorkflow.ts | 38 ++++++-- .../fetchSongstatsWithBackoff.test.ts | 91 ++++++++++++++++++ .../__tests__/isRetryableStatus.test.ts | 26 ++++++ lib/songstats/fetchSongstatsWithBackoff.ts | 61 ++++++++++++ lib/songstats/isRetryableStatus.ts | 14 +++ .../updateSongstatsBackfillQueue.test.ts | 32 +++++-- .../updateSongstatsBackfillQueue.ts | 15 ++- 11 files changed, 406 insertions(+), 98 deletions(-) create mode 100644 app/workflows/__tests__/songstatsBackfillWorkflow.test.ts create mode 100644 app/workflows/releaseClaimedRowsStep.ts create mode 100644 lib/songstats/__tests__/fetchSongstatsWithBackoff.test.ts create mode 100644 lib/songstats/__tests__/isRetryableStatus.test.ts create mode 100644 lib/songstats/fetchSongstatsWithBackoff.ts create mode 100644 lib/songstats/isRetryableStatus.ts diff --git a/app/workflows/__tests__/backfillTrackStep.test.ts b/app/workflows/__tests__/backfillTrackStep.test.ts index ebe773704..6c211ed61 100644 --- a/app/workflows/__tests__/backfillTrackStep.test.ts +++ b/app/workflows/__tests__/backfillTrackStep.test.ts @@ -1,12 +1,14 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { backfillTrackStep } from "../backfillTrackStep"; -import { fetchSongstats } from "@/lib/songstats/fetchSongstats"; +import { fetchSongstatsWithBackoff } from "@/lib/songstats/fetchSongstatsWithBackoff"; import { upsertSongMeasurements } from "@/lib/supabase/song_measurements/upsertSongMeasurements"; import { insertSongstatsQuotaLedger } from "@/lib/supabase/songstats_quota_ledger/insertSongstatsQuotaLedger"; import { updateSongstatsBackfillQueue } from "@/lib/supabase/songstats_backfill_queue/updateSongstatsBackfillQueue"; -vi.mock("@/lib/songstats/fetchSongstats", () => ({ fetchSongstats: vi.fn() })); +vi.mock("@/lib/songstats/fetchSongstatsWithBackoff", () => ({ + fetchSongstatsWithBackoff: vi.fn(), +})); vi.mock("@/lib/supabase/song_measurements/upsertSongMeasurements", () => ({ upsertSongMeasurements: vi.fn(), })); @@ -22,22 +24,20 @@ const ROW = { id: "q1", song: "USA2P2015959" } as never; describe("backfillTrackStep", () => { beforeEach(() => { vi.clearAllMocks(); + vi.spyOn(console, "log").mockImplementation(() => {}); vi.mocked(upsertSongMeasurements).mockResolvedValue([] as never); }); - it("writes the historic series as songstats measurements, records spend, marks done", async () => { - vi.mocked(fetchSongstats).mockResolvedValue({ + it("writes the historic series, records the spend, marks done on 200", async () => { + vi.mocked(fetchSongstatsWithBackoff).mockResolvedValue({ status: 200, + attempts: 1, + retriesExhausted: false, data: { stats: [ { source: "spotify", - data: { - history: [ - { date: "2025-01-01", streams_total: 1008736324 }, - { date: "2026-01-01", streams_total: 1330251464 }, - ], - }, + data: { history: [{ date: "2025-01-01", streams_total: 1008736324 }] }, }, ], }, @@ -45,10 +45,11 @@ describe("backfillTrackStep", () => { const result = await backfillTrackStep(ROW); - expect(fetchSongstats).toHaveBeenCalledWith("tracks/historic_stats", { + expect(fetchSongstatsWithBackoff).toHaveBeenCalledWith("tracks/historic_stats", { isrc: "USA2P2015959", source: "spotify", }); + // exact transformation: each history point → a permanent songstats measurement expect(upsertSongMeasurements).toHaveBeenCalledWith([ { song: "USA2P2015959", @@ -59,74 +60,65 @@ describe("backfillTrackStep", () => { data_source: "songstats", raw_ref: "songstats-backfill", }, - { - song: "USA2P2015959", - platform: "spotify", - metric: "platform_displayed_play_count", - value: 1330251464, - captured_at: "2026-01-01T00:00:00.000Z", - data_source: "songstats", - raw_ref: "songstats-backfill", - }, ]); expect(insertSongstatsQuotaLedger).toHaveBeenCalledWith({ hits: 1, purpose: "backfill USA2P2015959", }); - expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith("q1", { status: "done" }); + expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith(["q1"], { status: "done" }); expect(result).toEqual({ ok: true, hitsSpent: 1 }); }); - it("marks a transient upstream error (429) as failed (reclaimable) and records the spend", async () => { - vi.mocked(fetchSongstats).mockResolvedValue({ status: 429, data: {} }); - - const result = await backfillTrackStep(ROW); - - // transient -> 'failed' so the daily reclaim sweep returns it to 'pending' - expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith("q1", { status: "failed" }); - expect(insertSongstatsQuotaLedger).toHaveBeenCalledWith({ - hits: 1, - purpose: "backfill USA2P2015959 (failed 429)", + it("DEFERS (pending, no quota hit, signals stop) when backoff is exhausted on 429", async () => { + vi.mocked(fetchSongstatsWithBackoff).mockResolvedValue({ + status: 429, + attempts: 6, + retriesExhausted: true, + data: {}, }); - expect(upsertSongMeasurements).not.toHaveBeenCalled(); - expect(result).toEqual({ ok: false, hitsSpent: 1 }); - }); - - it("marks a transient 5xx as failed (reclaimable)", async () => { - vi.mocked(fetchSongstats).mockResolvedValue({ status: 504, data: {} }); const result = await backfillTrackStep(ROW); - expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith("q1", { status: "failed" }); - expect(result).toEqual({ ok: false, hitsSpent: 1 }); + // left pending for the next drain; NO ledger hit (Songstats consumed nothing) + expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith(["q1"], { status: "pending" }); + expect(insertSongstatsQuotaLedger).not.toHaveBeenCalled(); + expect(upsertSongMeasurements).not.toHaveBeenCalled(); + expect(result).toEqual({ ok: false, hitsSpent: 0, deferred: true }); }); - it("marks a permanent client error (403) as done so reclaim never recycles it", async () => { - vi.mocked(fetchSongstats).mockResolvedValue({ status: 403, data: {} }); + it("marks a definitive 404 (no history) as done and records the spend", async () => { + vi.mocked(fetchSongstatsWithBackoff).mockResolvedValue({ + status: 404, + attempts: 1, + retriesExhausted: false, + data: {}, + }); const result = await backfillTrackStep(ROW); - // non-retryable 4xx (not 408/429) is terminal -> 'done', not 'failed' - expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith("q1", { status: "done" }); + expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith(["q1"], { status: "done" }); expect(insertSongstatsQuotaLedger).toHaveBeenCalledWith({ hits: 1, - purpose: "backfill USA2P2015959 (terminal 403)", + purpose: "backfill USA2P2015959 (no data 404)", }); expect(result).toEqual({ ok: false, hitsSpent: 1 }); }); - it("marks a definitive 404 (no history exists) as done so it is never retried", async () => { - vi.mocked(fetchSongstats).mockResolvedValue({ status: 404, data: {} }); + it("marks a permanent 4xx (403) as done (terminal) and records the spend", async () => { + vi.mocked(fetchSongstatsWithBackoff).mockResolvedValue({ + status: 403, + attempts: 1, + retriesExhausted: false, + data: {}, + }); const result = await backfillTrackStep(ROW); - // terminal no-data -> 'done', not 'failed' — the reclaim sweep must not resurrect it - expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith("q1", { status: "done" }); + expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith(["q1"], { status: "done" }); expect(insertSongstatsQuotaLedger).toHaveBeenCalledWith({ hits: 1, - purpose: "backfill USA2P2015959 (no data 404)", + purpose: "backfill USA2P2015959 (terminal 403)", }); - expect(upsertSongMeasurements).not.toHaveBeenCalled(); expect(result).toEqual({ ok: false, hitsSpent: 1 }); }); }); diff --git a/app/workflows/__tests__/songstatsBackfillWorkflow.test.ts b/app/workflows/__tests__/songstatsBackfillWorkflow.test.ts new file mode 100644 index 000000000..cc435558a --- /dev/null +++ b/app/workflows/__tests__/songstatsBackfillWorkflow.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { songstatsBackfillWorkflow } from "../songstatsBackfillWorkflow"; + +import { getBackfillBudgetStep } from "../getBackfillBudgetStep"; +import { claimBackfillRowsStep } from "../claimBackfillRowsStep"; +import { backfillTrackStep } from "../backfillTrackStep"; +import { releaseClaimedRowsStep } from "../releaseClaimedRowsStep"; + +vi.mock("../getBackfillBudgetStep", () => ({ getBackfillBudgetStep: vi.fn() })); +vi.mock("../claimBackfillRowsStep", () => ({ claimBackfillRowsStep: vi.fn() })); +vi.mock("../backfillTrackStep", () => ({ backfillTrackStep: vi.fn() })); +vi.mock("../releaseClaimedRowsStep", () => ({ releaseClaimedRowsStep: vi.fn() })); + +const row = (id: string) => ({ id, song: id }) as never; + +describe("songstatsBackfillWorkflow", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.mocked(releaseClaimedRowsStep).mockResolvedValue(undefined); + }); + + it("releases the rest of the claimed batch to pending when a track defers", async () => { + vi.mocked(getBackfillBudgetStep).mockResolvedValue(100); + vi.mocked(claimBackfillRowsStep).mockResolvedValue([row("r1"), row("r2"), row("r3")]); + vi.mocked(backfillTrackStep) + .mockResolvedValueOnce({ ok: true, hitsSpent: 1 }) // r1 + .mockResolvedValueOnce({ ok: false, hitsSpent: 0, deferred: true }); // r2 defers + + const result = await songstatsBackfillWorkflow(); + + // r2 is set pending by the step itself; the unprocessed remainder (r3) is released here + expect(releaseClaimedRowsStep).toHaveBeenCalledWith(["r3"]); + expect(backfillTrackStep).toHaveBeenCalledTimes(2); // stopped at the defer, never reached r3 + expect(result).toEqual({ backfilled: 1, failed: 0, deferred: true }); + }); + + it("drains until the queue is empty and never releases when nothing defers", async () => { + vi.mocked(getBackfillBudgetStep).mockResolvedValue(100); + vi.mocked(claimBackfillRowsStep) + .mockResolvedValueOnce([row("a"), row("b")]) + .mockResolvedValueOnce([]); // queue drained + vi.mocked(backfillTrackStep) + .mockResolvedValueOnce({ ok: true, hitsSpent: 1 }) + .mockResolvedValueOnce({ ok: false, hitsSpent: 1 }); // terminal (e.g. 404) + + const result = await songstatsBackfillWorkflow(); + + expect(releaseClaimedRowsStep).not.toHaveBeenCalled(); + expect(result).toEqual({ backfilled: 1, failed: 1, deferred: false }); + }); + + it("does not drain when there is no budget", async () => { + vi.mocked(getBackfillBudgetStep).mockResolvedValue(0); + + const result = await songstatsBackfillWorkflow(); + + expect(claimBackfillRowsStep).not.toHaveBeenCalled(); + expect(result).toEqual({ backfilled: 0, failed: 0, deferred: false }); + }); +}); diff --git a/app/workflows/backfillTrackStep.ts b/app/workflows/backfillTrackStep.ts index 3247ff4cf..fd3947432 100644 --- a/app/workflows/backfillTrackStep.ts +++ b/app/workflows/backfillTrackStep.ts @@ -1,4 +1,4 @@ -import { fetchSongstats } from "@/lib/songstats/fetchSongstats"; +import { fetchSongstatsWithBackoff } from "@/lib/songstats/fetchSongstatsWithBackoff"; import { upsertSongMeasurements } from "@/lib/supabase/song_measurements/upsertSongMeasurements"; import { insertSongstatsQuotaLedger } from "@/lib/supabase/songstats_quota_ledger/insertSongstatsQuotaLedger"; import { updateSongstatsBackfillQueue } from "@/lib/supabase/songstats_backfill_queue/updateSongstatsBackfillQueue"; @@ -8,40 +8,48 @@ import { Tables } from "@/types/database.types"; const METRIC = "platform_displayed_play_count"; /** - * Backfill one claimed queue row: fetch the track's Songstats historic series - * (one quota hit — recorded win or lose), write each point as a permanent - * `songstats`-labeled measurement, and close the row. Failures mark the row - * failed without failing the run — the next row may still succeed. + * Backfill one claimed queue row, with bounded exponential backoff on Songstats' + * rate limit (Songstats is the rate authority — see chat#1797): + * - **200** → write each history point as a permanent `songstats` measurement, + * record the spend, mark `done`. + * - **404 / other 4xx** → a real request with a definitive answer; terminal, so + * mark `done` (404 = no history) and record the spend. + * - **backoff exhausted** (still 429/5xx after retries) → **defer**: leave the row + * `pending` for the next drain, consume no quota, and signal the workflow to + * stop (`deferred`) — Songstats is saturated right now. * * @param row - The claimed queue row (already in_progress) - * @returns ok + hits spent (always 1; the hit is consumed even on failure) + * @returns ok + hitsSpent (0 when deferred) + `deferred` when Songstats is saturated */ export async function backfillTrackStep( row: Tables<"songstats_backfill_queue">, -): Promise<{ ok: boolean; hitsSpent: number }> { +): Promise<{ ok: boolean; hitsSpent: number; deferred?: boolean }> { "use step"; - const result = await fetchSongstats("tracks/historic_stats", { + const result = await fetchSongstatsWithBackoff("tracks/historic_stats", { isrc: row.song, source: "spotify", }); - if (result.status !== 200) { - const status = result.status; - const isNoData = status === 404; - // Only transient errors are retryable: 408 (timeout), 429 (quota), any 5xx. - const isRetryable = status === 408 || status === 429 || status >= 500; - - // `failed` is reclaimable (the daily sweep returns it to `pending`, bounded - // by the rolling-window budget). 404 no-data and other permanent 4xx are - // terminal → `done`, so reclaim never recycles a track that can't succeed. - const nextStatus = isRetryable ? "failed" : "done"; - - let outcome = `terminal ${status}`; - if (isNoData) outcome = "no data 404"; - else if (isRetryable) outcome = `failed ${status}`; + if (result.retriesExhausted) { + // Still retryable (429 throttle / 408 / gateway 5xx) past the backoff bound — + // leave it for the next run, spend nothing. + console.log( + `[backfill] ${row.song} deferred (retryable ${result.status} after ${result.attempts} tries)`, + ); + await updateSongstatsBackfillQueue([row.id], { status: "pending" }); + return { ok: false, hitsSpent: 0, deferred: true }; + } - await insertSongstatsQuotaLedger({ hits: 1, purpose: `backfill ${row.song} (${outcome})` }); - await updateSongstatsBackfillQueue(row.id, { status: nextStatus }); + if (result.status !== 200) { + const noData = result.status === 404; + console.log( + `[backfill] ${row.song} done (${noData ? "no data 404" : `terminal ${result.status}`})`, + ); + await insertSongstatsQuotaLedger({ + hits: 1, + purpose: `backfill ${row.song} (${noData ? "no data 404" : `terminal ${result.status}`})`, + }); + await updateSongstatsBackfillQueue([row.id], { status: "done" }); return { ok: false, hitsSpent: 1 }; } @@ -67,7 +75,8 @@ export async function backfillTrackStep( }); await upsertSongMeasurements(rows); + console.log(`[backfill] ${row.song} done (${rows.length} points written)`); await insertSongstatsQuotaLedger({ hits: 1, purpose: `backfill ${row.song}` }); - await updateSongstatsBackfillQueue(row.id, { status: "done" }); + await updateSongstatsBackfillQueue([row.id], { status: "done" }); return { ok: true, hitsSpent: 1 }; } diff --git a/app/workflows/releaseClaimedRowsStep.ts b/app/workflows/releaseClaimedRowsStep.ts new file mode 100644 index 000000000..788d752a0 --- /dev/null +++ b/app/workflows/releaseClaimedRowsStep.ts @@ -0,0 +1,15 @@ +import { updateSongstatsBackfillQueue } from "@/lib/supabase/songstats_backfill_queue/updateSongstatsBackfillQueue"; + +/** + * Durable step: return unprocessed claimed rows to `pending` when the drain + * stops early on a defer, so the next run retries them immediately instead of + * waiting on the stale-reclaim sweep. + * + * @param ids - Queue row ids still `in_progress` from the aborted batch. + */ +export async function releaseClaimedRowsStep(ids: string[]): Promise { + "use step"; + if (ids.length === 0) return; + await updateSongstatsBackfillQueue(ids, { status: "pending" }); + console.log(`[songstats-backfill] released ${ids.length} claimed rows back to pending`); +} diff --git a/app/workflows/songstatsBackfillWorkflow.ts b/app/workflows/songstatsBackfillWorkflow.ts index 0a1960bf9..f94c0223d 100644 --- a/app/workflows/songstatsBackfillWorkflow.ts +++ b/app/workflows/songstatsBackfillWorkflow.ts @@ -1,15 +1,20 @@ import { getBackfillBudgetStep } from "@/app/workflows/getBackfillBudgetStep"; import { claimBackfillRowsStep } from "@/app/workflows/claimBackfillRowsStep"; import { backfillTrackStep } from "@/app/workflows/backfillTrackStep"; +import { releaseClaimedRowsStep } from "@/app/workflows/releaseClaimedRowsStep"; const BATCH_SIZE = 25; /** - * Durable Songstats backfill drain (recoupable/chat#1791 write path): check - * the rolling-window budget, claim value-ranked rows via the SKIP LOCKED RPC, - * backfill each track's historic series into the measurement store, and stop - * when the queue or the budget is dry. Every quota hit converts into - * permanent owned data (fetch-once: captured history is never refetched). + * Durable Songstats backfill drain (recoupable/chat#1791 write path): claim + * value-ranked rows via the SKIP LOCKED RPC and backfill each track's historic + * series, with per-track exponential backoff handling Songstats' rate limit + * (chat#1797). **Stops as soon as a track defers** — Songstats still + * rate-limiting it past the backoff bound — releasing the rest of the claimed + * batch back to `pending` (so the next drain retries them immediately rather + * than waiting on stale-reclaim) instead of hammering a saturated API. Every + * successful hit converts into permanent owned data (fetch-once: captured + * history is never refetched). */ export async function songstatsBackfillWorkflow() { "use workflow"; @@ -17,13 +22,23 @@ export async function songstatsBackfillWorkflow() { let budget = await getBackfillBudgetStep(); let backfilled = 0; let failed = 0; + let deferred = false; - while (budget > 0) { + drain: while (budget > 0) { const rows = await claimBackfillRowsStep(Math.min(budget, BATCH_SIZE)); if (rows.length === 0) break; + console.log(`[songstats-backfill] claimed ${rows.length} rows`); - for (const row of rows) { - const result = await backfillTrackStep(row); + for (let i = 0; i < rows.length; i += 1) { + const result = await backfillTrackStep(rows[i]); + if (result.deferred) { + // Songstats is saturated — stop now. The deferred row is already back to + // `pending`; release the rest of this claimed batch too so they don't sit + // `in_progress` until stale-reclaim. + deferred = true; + await releaseClaimedRowsStep(rows.slice(i + 1).map(r => r.id)); + break drain; + } budget -= result.hitsSpent; if (result.ok) backfilled += 1; else failed += 1; @@ -31,6 +46,9 @@ export async function songstatsBackfillWorkflow() { } } - console.log(`[songstats-backfill] done: ${backfilled} backfilled, ${failed} failed`); - return { backfilled, failed }; + console.log( + `[songstats-backfill] done: ${backfilled} backfilled, ${failed} terminal` + + (deferred ? ", deferred (rate-limited)" : ""), + ); + return { backfilled, failed, deferred }; } diff --git a/lib/songstats/__tests__/fetchSongstatsWithBackoff.test.ts b/lib/songstats/__tests__/fetchSongstatsWithBackoff.test.ts new file mode 100644 index 000000000..21015ef14 --- /dev/null +++ b/lib/songstats/__tests__/fetchSongstatsWithBackoff.test.ts @@ -0,0 +1,91 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { fetchSongstatsWithBackoff } from "../fetchSongstatsWithBackoff"; +import { fetchSongstats } from "../fetchSongstats"; + +vi.mock("../fetchSongstats", () => ({ fetchSongstats: vi.fn() })); + +const noSleep = vi.fn().mockResolvedValue(undefined); + +describe("fetchSongstatsWithBackoff", () => { + beforeEach(() => vi.clearAllMocks()); + + it("returns immediately on 200 with no retries or sleeps", async () => { + vi.mocked(fetchSongstats).mockResolvedValue({ status: 200, data: { ok: true } }); + + const r = await fetchSongstatsWithBackoff( + "tracks/historic_stats", + { isrc: "I" }, + { sleep: noSleep }, + ); + + expect(fetchSongstats).toHaveBeenCalledTimes(1); + expect(noSleep).not.toHaveBeenCalled(); + expect(r).toMatchObject({ status: 200, attempts: 1, retriesExhausted: false }); + }); + + it("does NOT retry a non-retryable status (404)", async () => { + vi.mocked(fetchSongstats).mockResolvedValue({ status: 404, data: {} }); + + const r = await fetchSongstatsWithBackoff("p", undefined, { sleep: noSleep }); + + expect(fetchSongstats).toHaveBeenCalledTimes(1); + expect(r).toMatchObject({ status: 404, retriesExhausted: false }); + }); + + it("backs off and retries on 429, succeeding on a later attempt", async () => { + vi.mocked(fetchSongstats) + .mockResolvedValueOnce({ status: 429, data: {} }) + .mockResolvedValueOnce({ status: 429, data: {} }) + .mockResolvedValueOnce({ status: 200, data: { ok: true } }); + + const r = await fetchSongstatsWithBackoff("p", undefined, { sleep: noSleep, baseMs: 100 }); + + expect(fetchSongstats).toHaveBeenCalledTimes(3); + expect(noSleep).toHaveBeenCalledTimes(2); + // exponential: 100 then 200 + expect(noSleep).toHaveBeenNthCalledWith(1, 100); + expect(noSleep).toHaveBeenNthCalledWith(2, 200); + expect(r).toMatchObject({ status: 200, attempts: 3, retriesExhausted: false }); + }); + + it("gives up after maxRetries on persistent 429 and flags retriesExhausted", async () => { + vi.mocked(fetchSongstats).mockResolvedValue({ status: 429, data: {} }); + + const r = await fetchSongstatsWithBackoff("p", undefined, { sleep: noSleep, maxRetries: 3 }); + + expect(fetchSongstats).toHaveBeenCalledTimes(4); // 1 initial + 3 retries + expect(noSleep).toHaveBeenCalledTimes(3); + expect(r).toMatchObject({ status: 429, retriesExhausted: true }); + }); + + it("treats transient gateway 5xx (503) and 408 as retryable", async () => { + vi.mocked(fetchSongstats) + .mockResolvedValueOnce({ status: 503, data: {} }) + .mockResolvedValueOnce({ status: 200, data: {} }); + const r = await fetchSongstatsWithBackoff("p", undefined, { sleep: noSleep, maxRetries: 2 }); + expect(fetchSongstats).toHaveBeenCalledTimes(2); + expect(r).toMatchObject({ status: 200, retriesExhausted: false }); + }); + + it("does NOT retry a 500 (fetchSongstats maps missing key / fetch failure to 500)", async () => { + vi.mocked(fetchSongstats).mockResolvedValue({ status: 500, data: {} }); + + const r = await fetchSongstatsWithBackoff("p", undefined, { sleep: noSleep }); + + expect(fetchSongstats).toHaveBeenCalledTimes(1); + expect(noSleep).not.toHaveBeenCalled(); + expect(r).toMatchObject({ status: 500, retriesExhausted: false }); + }); + + it("caps the backoff at maxMs", async () => { + vi.mocked(fetchSongstats).mockResolvedValue({ status: 429, data: {} }); + await fetchSongstatsWithBackoff("p", undefined, { + sleep: noSleep, + baseMs: 1000, + maxMs: 1500, + maxRetries: 3, + }); + // 1000, 2000->capped 1500, 4000->capped 1500 + expect(noSleep.mock.calls.map(c => c[0])).toEqual([1000, 1500, 1500]); + }); +}); diff --git a/lib/songstats/__tests__/isRetryableStatus.test.ts b/lib/songstats/__tests__/isRetryableStatus.test.ts new file mode 100644 index 000000000..a1b707f09 --- /dev/null +++ b/lib/songstats/__tests__/isRetryableStatus.test.ts @@ -0,0 +1,26 @@ +import { describe, it, expect } from "vitest"; +import { isRetryableStatus } from "../isRetryableStatus"; + +describe("isRetryableStatus", () => { + it("retries transient throttling/timeouts: 408, 429", () => { + expect(isRetryableStatus(408)).toBe(true); + expect(isRetryableStatus(429)).toBe(true); + }); + + it("retries transient gateway 5xx: 502, 503, 504", () => { + expect(isRetryableStatus(502)).toBe(true); + expect(isRetryableStatus(503)).toBe(true); + expect(isRetryableStatus(504)).toBe(true); + }); + + it("does NOT retry 500/501 (fetchSongstats maps missing key + fetch failures to 500)", () => { + expect(isRetryableStatus(500)).toBe(false); + expect(isRetryableStatus(501)).toBe(false); + }); + + it("does NOT retry definitive responses: 200, 404, 403", () => { + expect(isRetryableStatus(200)).toBe(false); + expect(isRetryableStatus(404)).toBe(false); + expect(isRetryableStatus(403)).toBe(false); + }); +}); diff --git a/lib/songstats/fetchSongstatsWithBackoff.ts b/lib/songstats/fetchSongstatsWithBackoff.ts new file mode 100644 index 000000000..8b3a4b097 --- /dev/null +++ b/lib/songstats/fetchSongstatsWithBackoff.ts @@ -0,0 +1,61 @@ +import { fetchSongstats } from "@/lib/songstats/fetchSongstats"; +import { isRetryableStatus } from "@/lib/songstats/isRetryableStatus"; +import { delay } from "@/lib/time/delay"; +import type { ProxyResult } from "@/lib/research/ProxyResult"; + +// Short, in-step backoff for Songstats' per-second rate limit. The default +// budget is 1+2+4+8 = 15s of waits (base 1s, doubling, capped at 8s, 4 retries), +// well within a workflow step's duration. Persistent rejection defers the row to +// the next drain run rather than sleeping for minutes inside one invocation. +const DEFAULT_MAX_RETRIES = 4; +const DEFAULT_BASE_MS = 1000; +const DEFAULT_MAX_MS = 8_000; + +export type FetchSongstatsBackoffOptions = { + maxRetries?: number; + baseMs?: number; + maxMs?: number; + /** Injectable for tests; defaults to a real timer. */ + sleep?: (ms: number) => Promise; +}; + +export type BackoffResult = ProxyResult & { + /** Total attempts made (1 + retries). */ + attempts: number; + /** True when the final result is still retryable after exhausting retries. */ + retriesExhausted: boolean; +}; + +/** + * Call Songstats with bounded exponential backoff on transient rejections + * (429 rate limit, 408, 5xx). Retries the **same** request up to `maxRetries` + * with `min(maxMs, baseMs * 2^attempt)` waits; a 200 or any non-retryable status + * (e.g. 404) returns immediately. When backoff is exhausted and the call is + * still being rejected, `retriesExhausted` is true so the caller can defer the + * work (leave it `pending`) rather than burn it — Songstats is the rate + * authority, not a local quota ledger. + * + * @param path - Songstats enterprise path (e.g. `tracks/historic_stats`). + * @param queryParams - Query params forwarded to Songstats. + * @param options - Backoff tuning + an injectable `sleep`. + */ +export async function fetchSongstatsWithBackoff( + path: string, + queryParams?: Record, + options: FetchSongstatsBackoffOptions = {}, +): Promise { + const maxRetries = options.maxRetries ?? DEFAULT_MAX_RETRIES; + const baseMs = options.baseMs ?? DEFAULT_BASE_MS; + const maxMs = options.maxMs ?? DEFAULT_MAX_MS; + const sleep = options.sleep ?? delay; + + let result = await fetchSongstats(path, queryParams); + let retries = 0; + while (isRetryableStatus(result.status) && retries < maxRetries) { + await sleep(Math.min(maxMs, baseMs * 2 ** retries)); + result = await fetchSongstats(path, queryParams); + retries += 1; + } + + return { ...result, attempts: retries + 1, retriesExhausted: isRetryableStatus(result.status) }; +} diff --git a/lib/songstats/isRetryableStatus.ts b/lib/songstats/isRetryableStatus.ts new file mode 100644 index 000000000..eb0004ee3 --- /dev/null +++ b/lib/songstats/isRetryableStatus.ts @@ -0,0 +1,14 @@ +/** + * Whether a Songstats response status is worth retrying with backoff. + * + * Transient: 408 (request timeout), 429 (rate limit), and the gateway 5xx + * 502/503/504 (bad gateway / unavailable / gateway timeout). Deliberately + * **excludes** 500/501 — `fetchSongstats` maps a missing API key and any + * non-HTTP fetch failure to 500, which are permanent for that request, so + * retrying them just burns the backoff budget before the row defers. + * + * @param status - HTTP status from `fetchSongstats`. + */ +export function isRetryableStatus(status: number): boolean { + return status === 408 || status === 429 || status === 502 || status === 503 || status === 504; +} diff --git a/lib/supabase/songstats_backfill_queue/__tests__/updateSongstatsBackfillQueue.test.ts b/lib/supabase/songstats_backfill_queue/__tests__/updateSongstatsBackfillQueue.test.ts index eb8d02300..73be73472 100644 --- a/lib/supabase/songstats_backfill_queue/__tests__/updateSongstatsBackfillQueue.test.ts +++ b/lib/supabase/songstats_backfill_queue/__tests__/updateSongstatsBackfillQueue.test.ts @@ -14,24 +14,40 @@ vi.mock("../../serverClient", () => { describe("updateSongstatsBackfillQueue", () => { beforeEach(() => vi.clearAllMocks()); - it("updates a queue row's status by id", async () => { - const eq = vi.fn().mockResolvedValue({ error: null }); - const update = vi.fn().mockReturnValue({ eq }); + it("updates a single row by id (one-element array)", async () => { + const inFn = vi.fn().mockResolvedValue({ error: null }); + const update = vi.fn().mockReturnValue({ in: inFn }); vi.mocked(supabase.from).mockReturnValue({ update } as never); - await updateSongstatsBackfillQueue("q1", { status: "done" }); + await updateSongstatsBackfillQueue(["q1"], { status: "done" }); expect(supabase.from).toHaveBeenCalledWith("songstats_backfill_queue"); expect(update).toHaveBeenCalledWith({ status: "done" }); - expect(eq).toHaveBeenCalledWith("id", "q1"); + expect(inFn).toHaveBeenCalledWith("id", ["q1"]); + }); + + it("bulk-updates many rows in one call (e.g. releasing a claimed batch)", async () => { + const inFn = vi.fn().mockResolvedValue({ error: null }); + const update = vi.fn().mockReturnValue({ in: inFn }); + vi.mocked(supabase.from).mockReturnValue({ update } as never); + + await updateSongstatsBackfillQueue(["q1", "q2"], { status: "pending" }); + + expect(update).toHaveBeenCalledWith({ status: "pending" }); + expect(inFn).toHaveBeenCalledWith("id", ["q1", "q2"]); + }); + + it("is a no-op on an empty id list (no DB call)", async () => { + await updateSongstatsBackfillQueue([], { status: "pending" }); + expect(supabase.from).not.toHaveBeenCalled(); }); it("throws on update error", async () => { - const eq = vi.fn().mockResolvedValue({ error: { message: "boom" } }); - const update = vi.fn().mockReturnValue({ eq }); + const inFn = vi.fn().mockResolvedValue({ error: { message: "boom" } }); + const update = vi.fn().mockReturnValue({ in: inFn }); vi.mocked(supabase.from).mockReturnValue({ update } as never); - await expect(updateSongstatsBackfillQueue("q1", { status: "failed" })).rejects.toThrow( + await expect(updateSongstatsBackfillQueue(["q1"], { status: "failed" })).rejects.toThrow( "Failed to update songstats backfill queue: boom", ); }); diff --git a/lib/supabase/songstats_backfill_queue/updateSongstatsBackfillQueue.ts b/lib/supabase/songstats_backfill_queue/updateSongstatsBackfillQueue.ts index b462381ab..d2469412d 100644 --- a/lib/supabase/songstats_backfill_queue/updateSongstatsBackfillQueue.ts +++ b/lib/supabase/songstats_backfill_queue/updateSongstatsBackfillQueue.ts @@ -2,17 +2,22 @@ import supabase from "../serverClient"; import { TablesUpdate } from "@/types/database.types"; /** - * Update a backfill queue row (mark done/failed after a claim). + * Update one or more backfill queue rows by id in a single round trip. Handles + * both the per-row status flip after a claim (`[row.id]`) and the bulk release + * of a claimed batch back to `pending` when the drain stops early (`.in` works + * for one id or many). No-op on an empty id list. * - * @param id - The queue row id - * @param fields - Fields to update + * @param ids - Queue row ids to update + * @param fields - Fields to set (e.g. `{ status: "done" }`) * @throws Error if the update fails */ export async function updateSongstatsBackfillQueue( - id: string, + ids: string[], fields: TablesUpdate<"songstats_backfill_queue">, ): Promise { - const { error } = await supabase.from("songstats_backfill_queue").update(fields).eq("id", id); + if (ids.length === 0) return; + + const { error } = await supabase.from("songstats_backfill_queue").update(fields).in("id", ids); if (error) { throw new Error(`Failed to update songstats backfill queue: ${error.message}`); From 5f459463996e1139a40743eaf5fd83c54f4ed04a Mon Sep 17 00:00:00 2001 From: "sweetman.eth" Date: Tue, 16 Jun 2026 17:14:31 -0500 Subject: [PATCH 3/7] refactor(songstats): remove local quota ledger + budget gate (chat#1797) (#674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bullet 2 of chat#1797 (code half). Songstats is the rate authority — removes getBackfillBudgetStep, the budget gate, and insertSongstatsQuotaLedger/selectSongstatsQuotaSpent. The drain now claims+processes regardless of the ledger (un-stalls the backfill); the songstats_quota_ledger table is dropped in recoupable/database#35 (apply AFTER this deploys). --- .../__tests__/backfillTrackStep.test.ts | 34 +++++------------- .../__tests__/getBackfillBudgetStep.test.ts | 32 ----------------- .../songstatsBackfillWorkflow.test.ts | 24 ++++++------- app/workflows/backfillTrackStep.ts | 29 +++++++-------- app/workflows/getBackfillBudgetStep.ts | 21 ----------- app/workflows/songstatsBackfillWorkflow.ts | 29 +++++++-------- .../insertSongstatsQuotaLedger.test.ts | 32 ----------------- .../selectSongstatsQuotaSpent.test.ts | 35 ------------------- .../insertSongstatsQuotaLedger.ts | 19 ---------- .../selectSongstatsQuotaSpent.ts | 23 ------------ 10 files changed, 42 insertions(+), 236 deletions(-) delete mode 100644 app/workflows/__tests__/getBackfillBudgetStep.test.ts delete mode 100644 app/workflows/getBackfillBudgetStep.ts delete mode 100644 lib/supabase/songstats_quota_ledger/__tests__/insertSongstatsQuotaLedger.test.ts delete mode 100644 lib/supabase/songstats_quota_ledger/__tests__/selectSongstatsQuotaSpent.test.ts delete mode 100644 lib/supabase/songstats_quota_ledger/insertSongstatsQuotaLedger.ts delete mode 100644 lib/supabase/songstats_quota_ledger/selectSongstatsQuotaSpent.ts diff --git a/app/workflows/__tests__/backfillTrackStep.test.ts b/app/workflows/__tests__/backfillTrackStep.test.ts index 6c211ed61..c923cb030 100644 --- a/app/workflows/__tests__/backfillTrackStep.test.ts +++ b/app/workflows/__tests__/backfillTrackStep.test.ts @@ -3,7 +3,6 @@ import { backfillTrackStep } from "../backfillTrackStep"; import { fetchSongstatsWithBackoff } from "@/lib/songstats/fetchSongstatsWithBackoff"; import { upsertSongMeasurements } from "@/lib/supabase/song_measurements/upsertSongMeasurements"; -import { insertSongstatsQuotaLedger } from "@/lib/supabase/songstats_quota_ledger/insertSongstatsQuotaLedger"; import { updateSongstatsBackfillQueue } from "@/lib/supabase/songstats_backfill_queue/updateSongstatsBackfillQueue"; vi.mock("@/lib/songstats/fetchSongstatsWithBackoff", () => ({ @@ -12,9 +11,6 @@ vi.mock("@/lib/songstats/fetchSongstatsWithBackoff", () => ({ vi.mock("@/lib/supabase/song_measurements/upsertSongMeasurements", () => ({ upsertSongMeasurements: vi.fn(), })); -vi.mock("@/lib/supabase/songstats_quota_ledger/insertSongstatsQuotaLedger", () => ({ - insertSongstatsQuotaLedger: vi.fn(), -})); vi.mock("@/lib/supabase/songstats_backfill_queue/updateSongstatsBackfillQueue", () => ({ updateSongstatsBackfillQueue: vi.fn(), })); @@ -28,7 +24,7 @@ describe("backfillTrackStep", () => { vi.mocked(upsertSongMeasurements).mockResolvedValue([] as never); }); - it("writes the historic series, records the spend, marks done on 200", async () => { + it("writes the historic series and marks done on 200", async () => { vi.mocked(fetchSongstatsWithBackoff).mockResolvedValue({ status: 200, attempts: 1, @@ -61,15 +57,11 @@ describe("backfillTrackStep", () => { raw_ref: "songstats-backfill", }, ]); - expect(insertSongstatsQuotaLedger).toHaveBeenCalledWith({ - hits: 1, - purpose: "backfill USA2P2015959", - }); expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith(["q1"], { status: "done" }); - expect(result).toEqual({ ok: true, hitsSpent: 1 }); + expect(result).toEqual({ ok: true }); }); - it("DEFERS (pending, no quota hit, signals stop) when backoff is exhausted on 429", async () => { + it("DEFERS (pending, signals stop) when backoff is exhausted on 429", async () => { vi.mocked(fetchSongstatsWithBackoff).mockResolvedValue({ status: 429, attempts: 6, @@ -79,14 +71,12 @@ describe("backfillTrackStep", () => { const result = await backfillTrackStep(ROW); - // left pending for the next drain; NO ledger hit (Songstats consumed nothing) expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith(["q1"], { status: "pending" }); - expect(insertSongstatsQuotaLedger).not.toHaveBeenCalled(); expect(upsertSongMeasurements).not.toHaveBeenCalled(); - expect(result).toEqual({ ok: false, hitsSpent: 0, deferred: true }); + expect(result).toEqual({ ok: false, deferred: true }); }); - it("marks a definitive 404 (no history) as done and records the spend", async () => { + it("marks a definitive 404 (no history) as done", async () => { vi.mocked(fetchSongstatsWithBackoff).mockResolvedValue({ status: 404, attempts: 1, @@ -97,14 +87,10 @@ describe("backfillTrackStep", () => { const result = await backfillTrackStep(ROW); expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith(["q1"], { status: "done" }); - expect(insertSongstatsQuotaLedger).toHaveBeenCalledWith({ - hits: 1, - purpose: "backfill USA2P2015959 (no data 404)", - }); - expect(result).toEqual({ ok: false, hitsSpent: 1 }); + expect(result).toEqual({ ok: false }); }); - it("marks a permanent 4xx (403) as done (terminal) and records the spend", async () => { + it("marks a permanent 4xx (403) as done (terminal)", async () => { vi.mocked(fetchSongstatsWithBackoff).mockResolvedValue({ status: 403, attempts: 1, @@ -115,10 +101,6 @@ describe("backfillTrackStep", () => { const result = await backfillTrackStep(ROW); expect(updateSongstatsBackfillQueue).toHaveBeenCalledWith(["q1"], { status: "done" }); - expect(insertSongstatsQuotaLedger).toHaveBeenCalledWith({ - hits: 1, - purpose: "backfill USA2P2015959 (terminal 403)", - }); - expect(result).toEqual({ ok: false, hitsSpent: 1 }); + expect(result).toEqual({ ok: false }); }); }); diff --git a/app/workflows/__tests__/getBackfillBudgetStep.test.ts b/app/workflows/__tests__/getBackfillBudgetStep.test.ts deleted file mode 100644 index 6c3fb8159..000000000 --- a/app/workflows/__tests__/getBackfillBudgetStep.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { getBackfillBudgetStep } from "../getBackfillBudgetStep"; - -import { selectSongstatsQuotaSpent } from "@/lib/supabase/songstats_quota_ledger/selectSongstatsQuotaSpent"; - -vi.mock("@/lib/supabase/songstats_quota_ledger/selectSongstatsQuotaSpent", () => ({ - selectSongstatsQuotaSpent: vi.fn(), -})); - -describe("getBackfillBudgetStep", () => { - beforeEach(() => { - vi.clearAllMocks(); - delete process.env.SONGSTATS_QUOTA_LIMIT; - delete process.env.SONGSTATS_QUOTA_RESERVE; - }); - - it("computes limit - reserve - spent over the rolling 30d window", async () => { - vi.mocked(selectSongstatsQuotaSpent).mockResolvedValue(300); - - const budget = await getBackfillBudgetStep(); - - expect(budget).toBe(1000 - 100 - 300); - const since = vi.mocked(selectSongstatsQuotaSpent).mock.calls[0][0]; - const ageDays = (Date.now() - new Date(since).getTime()) / 86400000; - expect(Math.round(ageDays)).toBe(30); - }); - - it("never returns negative", async () => { - vi.mocked(selectSongstatsQuotaSpent).mockResolvedValue(5000); - expect(await getBackfillBudgetStep()).toBe(0); - }); -}); diff --git a/app/workflows/__tests__/songstatsBackfillWorkflow.test.ts b/app/workflows/__tests__/songstatsBackfillWorkflow.test.ts index cc435558a..b18c5aeb7 100644 --- a/app/workflows/__tests__/songstatsBackfillWorkflow.test.ts +++ b/app/workflows/__tests__/songstatsBackfillWorkflow.test.ts @@ -1,12 +1,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { songstatsBackfillWorkflow } from "../songstatsBackfillWorkflow"; -import { getBackfillBudgetStep } from "../getBackfillBudgetStep"; import { claimBackfillRowsStep } from "../claimBackfillRowsStep"; import { backfillTrackStep } from "../backfillTrackStep"; import { releaseClaimedRowsStep } from "../releaseClaimedRowsStep"; -vi.mock("../getBackfillBudgetStep", () => ({ getBackfillBudgetStep: vi.fn() })); vi.mock("../claimBackfillRowsStep", () => ({ claimBackfillRowsStep: vi.fn() })); vi.mock("../backfillTrackStep", () => ({ backfillTrackStep: vi.fn() })); vi.mock("../releaseClaimedRowsStep", () => ({ releaseClaimedRowsStep: vi.fn() })); @@ -21,41 +19,39 @@ describe("songstatsBackfillWorkflow", () => { }); it("releases the rest of the claimed batch to pending when a track defers", async () => { - vi.mocked(getBackfillBudgetStep).mockResolvedValue(100); vi.mocked(claimBackfillRowsStep).mockResolvedValue([row("r1"), row("r2"), row("r3")]); vi.mocked(backfillTrackStep) - .mockResolvedValueOnce({ ok: true, hitsSpent: 1 }) // r1 - .mockResolvedValueOnce({ ok: false, hitsSpent: 0, deferred: true }); // r2 defers + .mockResolvedValueOnce({ ok: true }) // r1 + .mockResolvedValueOnce({ ok: false, deferred: true }); // r2 defers const result = await songstatsBackfillWorkflow(); // r2 is set pending by the step itself; the unprocessed remainder (r3) is released here expect(releaseClaimedRowsStep).toHaveBeenCalledWith(["r3"]); expect(backfillTrackStep).toHaveBeenCalledTimes(2); // stopped at the defer, never reached r3 - expect(result).toEqual({ backfilled: 1, failed: 0, deferred: true }); + expect(result).toEqual({ backfilled: 1, terminal: 0, deferred: true }); }); it("drains until the queue is empty and never releases when nothing defers", async () => { - vi.mocked(getBackfillBudgetStep).mockResolvedValue(100); vi.mocked(claimBackfillRowsStep) .mockResolvedValueOnce([row("a"), row("b")]) .mockResolvedValueOnce([]); // queue drained vi.mocked(backfillTrackStep) - .mockResolvedValueOnce({ ok: true, hitsSpent: 1 }) - .mockResolvedValueOnce({ ok: false, hitsSpent: 1 }); // terminal (e.g. 404) + .mockResolvedValueOnce({ ok: true }) + .mockResolvedValueOnce({ ok: false }); // terminal (e.g. 404) const result = await songstatsBackfillWorkflow(); expect(releaseClaimedRowsStep).not.toHaveBeenCalled(); - expect(result).toEqual({ backfilled: 1, failed: 1, deferred: false }); + expect(result).toEqual({ backfilled: 1, terminal: 1, deferred: false }); }); - it("does not drain when there is no budget", async () => { - vi.mocked(getBackfillBudgetStep).mockResolvedValue(0); + it("stops immediately when the first claim is empty", async () => { + vi.mocked(claimBackfillRowsStep).mockResolvedValue([]); const result = await songstatsBackfillWorkflow(); - expect(claimBackfillRowsStep).not.toHaveBeenCalled(); - expect(result).toEqual({ backfilled: 0, failed: 0, deferred: false }); + expect(backfillTrackStep).not.toHaveBeenCalled(); + expect(result).toEqual({ backfilled: 0, terminal: 0, deferred: false }); }); }); diff --git a/app/workflows/backfillTrackStep.ts b/app/workflows/backfillTrackStep.ts index fd3947432..4a627ee88 100644 --- a/app/workflows/backfillTrackStep.ts +++ b/app/workflows/backfillTrackStep.ts @@ -1,6 +1,5 @@ import { fetchSongstatsWithBackoff } from "@/lib/songstats/fetchSongstatsWithBackoff"; import { upsertSongMeasurements } from "@/lib/supabase/song_measurements/upsertSongMeasurements"; -import { insertSongstatsQuotaLedger } from "@/lib/supabase/songstats_quota_ledger/insertSongstatsQuotaLedger"; import { updateSongstatsBackfillQueue } from "@/lib/supabase/songstats_backfill_queue/updateSongstatsBackfillQueue"; import { historicStatsPayloadSchema } from "@/lib/research/playcounts/historicStatsPayloadSchema"; import { Tables } from "@/types/database.types"; @@ -9,21 +8,22 @@ const METRIC = "platform_displayed_play_count"; /** * Backfill one claimed queue row, with bounded exponential backoff on Songstats' - * rate limit (Songstats is the rate authority — see chat#1797): + * rate limit (Songstats is the rate authority — see chat#1797; there is no local + * quota ledger): * - **200** → write each history point as a permanent `songstats` measurement, - * record the spend, mark `done`. + * mark `done`. * - **404 / other 4xx** → a real request with a definitive answer; terminal, so - * mark `done` (404 = no history) and record the spend. + * mark `done` (404 = no history). * - **backoff exhausted** (still 429/5xx after retries) → **defer**: leave the row - * `pending` for the next drain, consume no quota, and signal the workflow to - * stop (`deferred`) — Songstats is saturated right now. + * `pending` for the next drain and signal the workflow to stop (`deferred`) — + * Songstats is saturated right now. * * @param row - The claimed queue row (already in_progress) - * @returns ok + hitsSpent (0 when deferred) + `deferred` when Songstats is saturated + * @returns `ok` (true on a written backfill) + `deferred` when Songstats is saturated */ export async function backfillTrackStep( row: Tables<"songstats_backfill_queue">, -): Promise<{ ok: boolean; hitsSpent: number; deferred?: boolean }> { +): Promise<{ ok: boolean; deferred?: boolean }> { "use step"; const result = await fetchSongstatsWithBackoff("tracks/historic_stats", { isrc: row.song, @@ -32,12 +32,12 @@ export async function backfillTrackStep( if (result.retriesExhausted) { // Still retryable (429 throttle / 408 / gateway 5xx) past the backoff bound — - // leave it for the next run, spend nothing. + // leave it for the next run. console.log( `[backfill] ${row.song} deferred (retryable ${result.status} after ${result.attempts} tries)`, ); await updateSongstatsBackfillQueue([row.id], { status: "pending" }); - return { ok: false, hitsSpent: 0, deferred: true }; + return { ok: false, deferred: true }; } if (result.status !== 200) { @@ -45,12 +45,8 @@ export async function backfillTrackStep( console.log( `[backfill] ${row.song} done (${noData ? "no data 404" : `terminal ${result.status}`})`, ); - await insertSongstatsQuotaLedger({ - hits: 1, - purpose: `backfill ${row.song} (${noData ? "no data 404" : `terminal ${result.status}`})`, - }); await updateSongstatsBackfillQueue([row.id], { status: "done" }); - return { ok: false, hitsSpent: 1 }; + return { ok: false }; } const parsed = historicStatsPayloadSchema.safeParse(result.data); @@ -76,7 +72,6 @@ export async function backfillTrackStep( await upsertSongMeasurements(rows); console.log(`[backfill] ${row.song} done (${rows.length} points written)`); - await insertSongstatsQuotaLedger({ hits: 1, purpose: `backfill ${row.song}` }); await updateSongstatsBackfillQueue([row.id], { status: "done" }); - return { ok: true, hitsSpent: 1 }; + return { ok: true }; } diff --git a/app/workflows/getBackfillBudgetStep.ts b/app/workflows/getBackfillBudgetStep.ts deleted file mode 100644 index 866ee7605..000000000 --- a/app/workflows/getBackfillBudgetStep.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { selectSongstatsQuotaSpent } from "@/lib/supabase/songstats_quota_ledger/selectSongstatsQuotaSpent"; - -const DEFAULT_QUOTA_LIMIT = 1000; -const DEFAULT_QUOTA_RESERVE = 100; -const WINDOW_DAYS = 30; - -/** - * Remaining Songstats budget for this run: plan limit minus a reserve (kept - * for request-path fallbacks and manual research) minus hits spent in the - * rolling 30-day window. Never negative. - * - * @returns Hits the backfill worker may spend now - */ -export async function getBackfillBudgetStep(): Promise { - "use step"; - const limit = Number(process.env.SONGSTATS_QUOTA_LIMIT) || DEFAULT_QUOTA_LIMIT; - const reserve = Number(process.env.SONGSTATS_QUOTA_RESERVE) || DEFAULT_QUOTA_RESERVE; - const since = new Date(Date.now() - WINDOW_DAYS * 24 * 60 * 60 * 1000).toISOString(); - const spent = await selectSongstatsQuotaSpent(since); - return Math.max(0, limit - reserve - spent); -} diff --git a/app/workflows/songstatsBackfillWorkflow.ts b/app/workflows/songstatsBackfillWorkflow.ts index f94c0223d..459d18870 100644 --- a/app/workflows/songstatsBackfillWorkflow.ts +++ b/app/workflows/songstatsBackfillWorkflow.ts @@ -1,4 +1,3 @@ -import { getBackfillBudgetStep } from "@/app/workflows/getBackfillBudgetStep"; import { claimBackfillRowsStep } from "@/app/workflows/claimBackfillRowsStep"; import { backfillTrackStep } from "@/app/workflows/backfillTrackStep"; import { releaseClaimedRowsStep } from "@/app/workflows/releaseClaimedRowsStep"; @@ -8,24 +7,22 @@ const BATCH_SIZE = 25; /** * Durable Songstats backfill drain (recoupable/chat#1791 write path): claim * value-ranked rows via the SKIP LOCKED RPC and backfill each track's historic - * series, with per-track exponential backoff handling Songstats' rate limit - * (chat#1797). **Stops as soon as a track defers** — Songstats still - * rate-limiting it past the backoff bound — releasing the rest of the claimed - * batch back to `pending` (so the next drain retries them immediately rather - * than waiting on stale-reclaim) instead of hammering a saturated API. Every - * successful hit converts into permanent owned data (fetch-once: captured - * history is never refetched). + * series. Per-track exponential backoff absorbs Songstats' rate limit; the run + * **stops as soon as a track defers** (still retryable past the backoff bound), + * releasing the rest of the claimed batch back to `pending` so the next drain + * retries them immediately rather than waiting on stale-reclaim. Otherwise it + * drains until the queue has no claimable `pending` rows. Every backfill + * converts into permanent owned data (fetch-once). */ export async function songstatsBackfillWorkflow() { "use workflow"; - let budget = await getBackfillBudgetStep(); let backfilled = 0; - let failed = 0; + let terminal = 0; let deferred = false; - drain: while (budget > 0) { - const rows = await claimBackfillRowsStep(Math.min(budget, BATCH_SIZE)); + drain: while (true) { + const rows = await claimBackfillRowsStep(BATCH_SIZE); if (rows.length === 0) break; console.log(`[songstats-backfill] claimed ${rows.length} rows`); @@ -39,16 +36,14 @@ export async function songstatsBackfillWorkflow() { await releaseClaimedRowsStep(rows.slice(i + 1).map(r => r.id)); break drain; } - budget -= result.hitsSpent; if (result.ok) backfilled += 1; - else failed += 1; - if (budget <= 0) break; + else terminal += 1; } } console.log( - `[songstats-backfill] done: ${backfilled} backfilled, ${failed} terminal` + + `[songstats-backfill] done: ${backfilled} backfilled, ${terminal} terminal` + (deferred ? ", deferred (rate-limited)" : ""), ); - return { backfilled, failed, deferred }; + return { backfilled, terminal, deferred }; } diff --git a/lib/supabase/songstats_quota_ledger/__tests__/insertSongstatsQuotaLedger.test.ts b/lib/supabase/songstats_quota_ledger/__tests__/insertSongstatsQuotaLedger.test.ts deleted file mode 100644 index 4f7d5c2a8..000000000 --- a/lib/supabase/songstats_quota_ledger/__tests__/insertSongstatsQuotaLedger.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { insertSongstatsQuotaLedger } from "../insertSongstatsQuotaLedger"; -import supabase from "../../serverClient"; - -vi.mock("../../serverClient", () => { - const mockFrom = vi.fn(); - const mockRpc = vi.fn(); - return { default: { from: mockFrom, rpc: mockRpc } }; -}); - -describe("insertSongstatsQuotaLedger", () => { - beforeEach(() => vi.clearAllMocks()); - - it("records spent hits", async () => { - const insert = vi.fn().mockResolvedValue({ error: null }); - vi.mocked(supabase.from).mockReturnValue({ insert } as never); - - await insertSongstatsQuotaLedger({ hits: 1, purpose: "backfill USA2P2015959" }); - - expect(supabase.from).toHaveBeenCalledWith("songstats_quota_ledger"); - expect(insert).toHaveBeenCalledWith([{ hits: 1, purpose: "backfill USA2P2015959" }]); - }); - - it("throws on insert error (spend must be recorded)", async () => { - const insert = vi.fn().mockResolvedValue({ error: { message: "boom" } }); - vi.mocked(supabase.from).mockReturnValue({ insert } as never); - - await expect(insertSongstatsQuotaLedger({ hits: 1 })).rejects.toThrow( - "Failed to record songstats quota spend: boom", - ); - }); -}); diff --git a/lib/supabase/songstats_quota_ledger/__tests__/selectSongstatsQuotaSpent.test.ts b/lib/supabase/songstats_quota_ledger/__tests__/selectSongstatsQuotaSpent.test.ts deleted file mode 100644 index 6d59c6267..000000000 --- a/lib/supabase/songstats_quota_ledger/__tests__/selectSongstatsQuotaSpent.test.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { selectSongstatsQuotaSpent } from "../selectSongstatsQuotaSpent"; -import supabase from "../../serverClient"; - -vi.mock("../../serverClient", () => { - const mockFrom = vi.fn(); - const mockRpc = vi.fn(); - return { default: { from: mockFrom, rpc: mockRpc } }; -}); - -describe("selectSongstatsQuotaSpent", () => { - beforeEach(() => vi.clearAllMocks()); - - it("sums hits spent since the window start", async () => { - const gte = vi.fn().mockResolvedValue({ data: [{ hits: 3 }, { hits: 7 }], error: null }); - const select = vi.fn().mockReturnValue({ gte }); - vi.mocked(supabase.from).mockReturnValue({ select } as never); - - const result = await selectSongstatsQuotaSpent("2026-05-12T00:00:00Z"); - - expect(supabase.from).toHaveBeenCalledWith("songstats_quota_ledger"); - expect(gte).toHaveBeenCalledWith("spent_at", "2026-05-12T00:00:00Z"); - expect(result).toBe(10); - }); - - it("treats errors as full quota spent (fail safe — do not overspend)", async () => { - const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); - const gte = vi.fn().mockResolvedValue({ data: null, error: { message: "boom" } }); - const select = vi.fn().mockReturnValue({ gte }); - vi.mocked(supabase.from).mockReturnValue({ select } as never); - - expect(await selectSongstatsQuotaSpent("2026-05-12T00:00:00Z")).toBe(Number.MAX_SAFE_INTEGER); - consoleError.mockRestore(); - }); -}); diff --git a/lib/supabase/songstats_quota_ledger/insertSongstatsQuotaLedger.ts b/lib/supabase/songstats_quota_ledger/insertSongstatsQuotaLedger.ts deleted file mode 100644 index 777c42a79..000000000 --- a/lib/supabase/songstats_quota_ledger/insertSongstatsQuotaLedger.ts +++ /dev/null @@ -1,19 +0,0 @@ -import supabase from "../serverClient"; -import { TablesInsert } from "@/types/database.types"; - -/** - * Record Songstats quota spend. Throws on failure — unrecorded spend would - * let the worker silently blow the rolling-window budget. - * - * @param entry - hits spent, optional purpose/account attribution - * @throws Error if the insert fails - */ -export async function insertSongstatsQuotaLedger( - entry: TablesInsert<"songstats_quota_ledger">, -): Promise { - const { error } = await supabase.from("songstats_quota_ledger").insert([entry]); - - if (error) { - throw new Error(`Failed to record songstats quota spend: ${error.message}`); - } -} diff --git a/lib/supabase/songstats_quota_ledger/selectSongstatsQuotaSpent.ts b/lib/supabase/songstats_quota_ledger/selectSongstatsQuotaSpent.ts deleted file mode 100644 index 0610a4b27..000000000 --- a/lib/supabase/songstats_quota_ledger/selectSongstatsQuotaSpent.ts +++ /dev/null @@ -1,23 +0,0 @@ -import supabase from "../serverClient"; - -/** - * Sum Songstats hits spent since a window start (the rolling 30-day quota - * check). Errors are treated as the full quota being spent — failing safe: - * the worker must never overspend because the ledger was unreadable. - * - * @param since - Inclusive ISO lower bound of the window - * @returns Total hits spent in the window - */ -export async function selectSongstatsQuotaSpent(since: string): Promise { - const { data, error } = await supabase - .from("songstats_quota_ledger") - .select("hits") - .gte("spent_at", since); - - if (error) { - console.error("Error reading songstats quota ledger:", error); - return Number.MAX_SAFE_INTEGER; - } - - return (data || []).reduce((sum, row) => sum + row.hits, 0); -} From f24d4c7ecaf4469c234378916dafffddc1425416 Mon Sep 17 00:00:00 2001 From: "sweetman.eth" Date: Thu, 18 Jun 2026 15:06:39 -0500 Subject: [PATCH 4/7] feat: POST /api/catalogs (create + materialize from valuation snapshot) (#677) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: POST /api/catalogs create + materialize from valuation snapshot Creates a catalog owned by the authenticated account (account derived from credentials via validateAuthContext, never the body). With from.snapshot_id, materializes the catalog from a completed valuation snapshot: creates the catalogs row, links account_catalogs, adds the snapshot's measured ISRCs as catalog_songs, and records the catalog on the snapshot. Re-claiming the same snapshot is idempotent. TDD: validateCreateCatalogBody (6 tests) + createCatalogHandler (8 tests), red->green. New supabase wrappers: insertCatalog, selectCatalogById, insertAccountCatalog, updateSnapshotCatalog. Implements recoupable/chat#1801 Phase 2. Matches docs contract recoupable/docs#243. Co-Authored-By: Claude Opus 4.8 (1M context) * refactor: re-anchor POST /api/catalogs to merged contract + review fixes - Flatten request to the merged docs#243 contract: from:{snapshot_id} -> a root snapshot field (validator + handler + tests). Error copy follows. - DRY/SRP: drop the inline success() helper; use the shared successResponse(). - KISS rename: materializeSnapshotCatalog.ts -> createSnapshotCatalog.ts. - DRY: delete the redundant updateSnapshotCatalog helper; reuse the existing updatePlaycountSnapshot(id, fields). Validator change done red->green. lib/catalog: 24 tests pass; tsc + eslint clean. Addresses review on PR #677. * fix: materialize catalog songs from song_measurements, not snapshot.isrcs Testing the full materialize path surfaced a real bug: a valuation snapshot is album_ids-scoped, so its own isrcs column is null — createSnapshotCatalog read snapshot.isrcs and would link an EMPTY catalog. The measured ISRCs live in song_measurements (snapshot lineage), so source them there. New selectSnapshotIsrcs(snapshotId) helper (distinct song_measurements.song for the snapshot). createSnapshotCatalog now uses it. TDD: new createSnapshotCatalog.test.ts (3 tests) red->green; lib/catalog 27 pass. Addresses PR #677 verification. * refactor: reuse selectSongMeasurements (snapshot filter) instead of a new helper KISS/DRY per review: drop selectSnapshotIsrcs; add an optional snapshot filter to the existing selectSongMeasurements, and derive distinct ISRCs in createSnapshotCatalog. lib/catalog + song_measurements: 36 tests pass. Addresses review on PR #677. --------- Co-authored-by: Claude Opus 4.8 (1M context) --- app/api/catalogs/route.ts | 26 +++ .../__tests__/createCatalogHandler.test.ts | 160 ++++++++++++++++++ .../__tests__/createSnapshotCatalog.test.ts | 92 ++++++++++ .../validateCreateCatalogBody.test.ts | 50 ++++++ lib/catalog/createCatalogHandler.ts | 74 ++++++++ lib/catalog/createSnapshotCatalog.ts | 45 +++++ lib/catalog/validateCreateCatalogBody.ts | 47 +++++ .../account_catalogs/insertAccountCatalog.ts | 21 +++ lib/supabase/catalogs/insertCatalog.ts | 19 +++ lib/supabase/catalogs/selectCatalogById.ts | 19 +++ .../selectSongMeasurements.ts | 14 +- 11 files changed, 562 insertions(+), 5 deletions(-) create mode 100644 app/api/catalogs/route.ts create mode 100644 lib/catalog/__tests__/createCatalogHandler.test.ts create mode 100644 lib/catalog/__tests__/createSnapshotCatalog.test.ts create mode 100644 lib/catalog/__tests__/validateCreateCatalogBody.test.ts create mode 100644 lib/catalog/createCatalogHandler.ts create mode 100644 lib/catalog/createSnapshotCatalog.ts create mode 100644 lib/catalog/validateCreateCatalogBody.ts create mode 100644 lib/supabase/account_catalogs/insertAccountCatalog.ts create mode 100644 lib/supabase/catalogs/insertCatalog.ts create mode 100644 lib/supabase/catalogs/selectCatalogById.ts diff --git a/app/api/catalogs/route.ts b/app/api/catalogs/route.ts new file mode 100644 index 000000000..5ab6c6692 --- /dev/null +++ b/app/api/catalogs/route.ts @@ -0,0 +1,26 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { createCatalogHandler } from "@/lib/catalog/createCatalogHandler"; + +/** + * OPTIONS handler for CORS preflight requests. + * + * @returns A NextResponse with CORS headers. + */ +export async function OPTIONS() { + return new NextResponse(null, { + status: 200, + headers: getCorsHeaders(), + }); +} + +/** + * POST handler for creating a catalog (optionally materialized from a + * valuation snapshot). + * + * @param request - The request object containing the catalog body. + * @returns A NextResponse with the created catalog. + */ +export async function POST(request: NextRequest) { + return createCatalogHandler(request); +} diff --git a/lib/catalog/__tests__/createCatalogHandler.test.ts b/lib/catalog/__tests__/createCatalogHandler.test.ts new file mode 100644 index 000000000..5e2b1ebf0 --- /dev/null +++ b/lib/catalog/__tests__/createCatalogHandler.test.ts @@ -0,0 +1,160 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +import { createCatalogHandler } from "../createCatalogHandler"; +import { validateCreateCatalogBody } from "../validateCreateCatalogBody"; +import { createSnapshotCatalog } from "../createSnapshotCatalog"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots"; +import { selectCatalogById } from "@/lib/supabase/catalogs/selectCatalogById"; +import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; +import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); +vi.mock("../validateCreateCatalogBody", () => ({ validateCreateCatalogBody: vi.fn() })); +vi.mock("../createSnapshotCatalog", () => ({ createSnapshotCatalog: vi.fn() })); +vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); +vi.mock("@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots", () => ({ + selectPlaycountSnapshots: vi.fn(), +})); +vi.mock("@/lib/supabase/catalogs/selectCatalogById", () => ({ selectCatalogById: vi.fn() })); +vi.mock("@/lib/supabase/catalogs/insertCatalog", () => ({ insertCatalog: vi.fn() })); +vi.mock("@/lib/supabase/account_catalogs/insertAccountCatalog", () => ({ + insertAccountCatalog: vi.fn(), +})); + +const accountId = "550e8400-e29b-41d4-a716-446655440000"; +const otherAccountId = "550e8400-e29b-41d4-a716-446655440999"; +const snapshotId = "11111111-2222-3333-4444-555555555555"; +const catalogId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; + +const catalog = { + id: catalogId, + name: "Bad Bunny — Catalog", + created_at: "2026-06-18T00:00:00Z", + updated_at: "2026-06-18T00:00:00Z", +}; + +const makeRequest = () => new NextRequest("http://localhost/api/catalogs", { method: "POST" }); + +const okAuth = () => + vi.mocked(validateAuthContext).mockResolvedValue({ accountId, orgId: null, authToken: "t" }); + +describe("createCatalogHandler", () => { + beforeEach(() => vi.clearAllMocks()); + + it("short-circuits with the validator error and never authenticates", async () => { + const err = NextResponse.json({ status: "error" }, { status: 400 }); + vi.mocked(validateCreateCatalogBody).mockReturnValue(err); + + const res = await createCatalogHandler(makeRequest()); + + expect(res).toBe(err); + expect(validateAuthContext).not.toHaveBeenCalled(); + }); + + it("short-circuits with the auth error when unauthenticated", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ name: "X" }); + const authErr = NextResponse.json({ status: "error" }, { status: 401 }); + vi.mocked(validateAuthContext).mockResolvedValue(authErr); + + const res = await createCatalogHandler(makeRequest()); + + expect(res).toBe(authErr); + expect(insertCatalog).not.toHaveBeenCalled(); + }); + + it("creates an empty catalog from a name-only body and links the account", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ name: "Bad Bunny — Catalog" }); + okAuth(); + vi.mocked(insertCatalog).mockResolvedValue(catalog); + + const res = await createCatalogHandler(makeRequest()); + const body = await res.json(); + + expect(res.status).toBe(200); + expect(insertCatalog).toHaveBeenCalledWith("Bad Bunny — Catalog"); + expect(insertAccountCatalog).toHaveBeenCalledWith({ account: accountId, catalog: catalogId }); + expect(body).toEqual({ status: "success", catalog, songs_added: 0 }); + expect(selectPlaycountSnapshots).not.toHaveBeenCalled(); + }); + + it("materializes a catalog from an owned, not-yet-claimed snapshot", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ + name: "Bad Bunny — Catalog", + snapshot: snapshotId, + }); + okAuth(); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ + { id: snapshotId, account: accountId, catalog: null, isrcs: ["A", "B"] } as never, + ]); + vi.mocked(createSnapshotCatalog).mockResolvedValue({ catalog, songsAdded: 2 }); + + const res = await createCatalogHandler(makeRequest()); + const body = await res.json(); + + expect(res.status).toBe(200); + expect(createSnapshotCatalog).toHaveBeenCalledWith({ + accountId, + snapshot: expect.objectContaining({ id: snapshotId }), + name: "Bad Bunny — Catalog", + }); + expect(body).toEqual({ status: "success", catalog, songs_added: 2 }); + }); + + it("returns 404 when the snapshot does not exist", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ snapshot: snapshotId }); + okAuth(); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([]); + + const res = await createCatalogHandler(makeRequest()); + + expect(res.status).toBe(404); + expect(createSnapshotCatalog).not.toHaveBeenCalled(); + }); + + it("returns 403 when the snapshot belongs to a different account", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ snapshot: snapshotId }); + okAuth(); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ + { id: snapshotId, account: otherAccountId, catalog: null, isrcs: ["A"] } as never, + ]); + + const res = await createCatalogHandler(makeRequest()); + + expect(res.status).toBe(403); + expect(createSnapshotCatalog).not.toHaveBeenCalled(); + }); + + it("is idempotent: returns the existing catalog when the snapshot is already claimed", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ snapshot: snapshotId }); + okAuth(); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ + { id: snapshotId, account: accountId, catalog: catalogId, isrcs: ["A", "B"] } as never, + ]); + vi.mocked(selectCatalogById).mockResolvedValue(catalog); + + const res = await createCatalogHandler(makeRequest()); + const body = await res.json(); + + expect(res.status).toBe(200); + expect(selectCatalogById).toHaveBeenCalledWith(catalogId); + expect(createSnapshotCatalog).not.toHaveBeenCalled(); + expect(body).toEqual({ status: "success", catalog, songs_added: 0 }); + }); + + it("returns a generic 500 without leaking the underlying error", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ name: "X" }); + okAuth(); + vi.mocked(insertCatalog).mockRejectedValue(new Error("db down at 10.0.0.1:5432")); + + const res = await createCatalogHandler(makeRequest()); + const body = await res.json(); + + expect(res.status).toBe(500); + expect(body.status).toBe("error"); + expect(JSON.stringify(body)).not.toContain("10.0.0.1"); + }); +}); diff --git a/lib/catalog/__tests__/createSnapshotCatalog.test.ts b/lib/catalog/__tests__/createSnapshotCatalog.test.ts new file mode 100644 index 000000000..9ee7a3ede --- /dev/null +++ b/lib/catalog/__tests__/createSnapshotCatalog.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import { createSnapshotCatalog } from "../createSnapshotCatalog"; +import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; +import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; +import { insertCatalogSongs } from "@/lib/supabase/catalog_songs/insertCatalogSongs"; +import { updatePlaycountSnapshot } from "@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot"; +import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements"; + +vi.mock("@/lib/supabase/catalogs/insertCatalog", () => ({ insertCatalog: vi.fn() })); +vi.mock("@/lib/supabase/account_catalogs/insertAccountCatalog", () => ({ + insertAccountCatalog: vi.fn(), +})); +vi.mock("@/lib/supabase/catalog_songs/insertCatalogSongs", () => ({ insertCatalogSongs: vi.fn() })); +vi.mock("@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot", () => ({ + updatePlaycountSnapshot: vi.fn(), +})); +vi.mock("@/lib/supabase/song_measurements/selectSongMeasurements", () => ({ + selectSongMeasurements: vi.fn(), +})); + +const accountId = "550e8400-e29b-41d4-a716-446655440000"; +const snapshotId = "11111111-2222-3333-4444-555555555555"; +const catalogId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; +const catalog = { id: catalogId, name: "Bad Bunny Catalog", created_at: "t", updated_at: "t" }; +// A real valuation snapshot is scoped by album_ids, so its own `isrcs` column is null — +// the measured ISRCs live in song_measurements (sourced via selectSongMeasurements). +const snapshot = { id: snapshotId, account: accountId, catalog: null, isrcs: null } as never; +const measurement = (song: string) => ({ song }) as never; + +describe("createSnapshotCatalog", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(insertCatalog).mockResolvedValue(catalog); + }); + + it("sources measured ISRCs from song_measurements (by snapshot) and adds them as catalog songs", async () => { + vi.mocked(selectSongMeasurements).mockResolvedValue([ + measurement("ISRC_A"), + measurement("ISRC_B"), + measurement("ISRC_C"), + ]); + + const result = await createSnapshotCatalog({ accountId, snapshot, name: "Bad Bunny Catalog" }); + + expect(insertCatalog).toHaveBeenCalledWith("Bad Bunny Catalog"); + expect(insertAccountCatalog).toHaveBeenCalledWith({ account: accountId, catalog: catalogId }); + // ISRCs come from measurements by snapshot, NOT snapshot.isrcs (null here) + expect(selectSongMeasurements).toHaveBeenCalledWith({ snapshot: snapshotId }); + expect(insertCatalogSongs).toHaveBeenCalledWith([ + { catalog: catalogId, song: "ISRC_A" }, + { catalog: catalogId, song: "ISRC_B" }, + { catalog: catalogId, song: "ISRC_C" }, + ]); + expect(updatePlaycountSnapshot).toHaveBeenCalledWith(snapshotId, { catalog: catalogId }); + expect(result).toEqual({ catalog, songsAdded: 3 }); + }); + + it("dedupes ISRCs across multiple measurement rows per track", async () => { + vi.mocked(selectSongMeasurements).mockResolvedValue([ + measurement("ISRC_A"), + measurement("ISRC_A"), + measurement("ISRC_B"), + ]); + + const result = await createSnapshotCatalog({ accountId, snapshot }); + + expect(insertCatalogSongs).toHaveBeenCalledWith([ + { catalog: catalogId, song: "ISRC_A" }, + { catalog: catalogId, song: "ISRC_B" }, + ]); + expect(result).toEqual({ catalog, songsAdded: 2 }); + }); + + it("adds no songs when the snapshot has no measurements", async () => { + vi.mocked(selectSongMeasurements).mockResolvedValue([]); + + const result = await createSnapshotCatalog({ accountId, snapshot }); + + expect(insertCatalogSongs).not.toHaveBeenCalled(); + expect(updatePlaycountSnapshot).toHaveBeenCalledWith(snapshotId, { catalog: catalogId }); + expect(result).toEqual({ catalog, songsAdded: 0 }); + }); + + it("falls back to a default name when none is supplied", async () => { + vi.mocked(selectSongMeasurements).mockResolvedValue([]); + + await createSnapshotCatalog({ accountId, snapshot }); + + expect(insertCatalog).toHaveBeenCalledWith("Valuation Catalog"); + }); +}); diff --git a/lib/catalog/__tests__/validateCreateCatalogBody.test.ts b/lib/catalog/__tests__/validateCreateCatalogBody.test.ts new file mode 100644 index 000000000..670895f6a --- /dev/null +++ b/lib/catalog/__tests__/validateCreateCatalogBody.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from "vitest"; +import { NextResponse } from "next/server"; + +import { validateCreateCatalogBody } from "../validateCreateCatalogBody"; + +const snapshotId = "550e8400-e29b-41d4-a716-446655440000"; + +describe("validateCreateCatalogBody", () => { + it("accepts a name-only body", () => { + const result = validateCreateCatalogBody({ name: "My Catalog" }); + expect(result).toEqual({ name: "My Catalog" }); + }); + + it("accepts a snapshot-only body", () => { + const result = validateCreateCatalogBody({ snapshot: snapshotId }); + expect(result).toEqual({ snapshot: snapshotId }); + }); + + it("accepts name and snapshot together", () => { + const result = validateCreateCatalogBody({ + name: "Bad Bunny — Catalog", + snapshot: snapshotId, + }); + expect(result).toEqual({ + name: "Bad Bunny — Catalog", + snapshot: snapshotId, + }); + }); + + it("rejects an empty body (neither name nor snapshot) with 400", async () => { + const result = validateCreateCatalogBody({}); + expect(result).toBeInstanceOf(NextResponse); + const res = result as NextResponse; + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.status).toBe("error"); + }); + + it("rejects a non-uuid snapshot with 400", async () => { + const result = validateCreateCatalogBody({ snapshot: "not-a-uuid" }); + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + }); + + it("rejects a null/non-object body with 400", async () => { + const result = validateCreateCatalogBody(null); + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + }); +}); diff --git a/lib/catalog/createCatalogHandler.ts b/lib/catalog/createCatalogHandler.ts new file mode 100644 index 000000000..583746971 --- /dev/null +++ b/lib/catalog/createCatalogHandler.ts @@ -0,0 +1,74 @@ +import { NextRequest, NextResponse } from "next/server"; +import { errorResponse } from "@/lib/networking/errorResponse"; +import { successResponse } from "@/lib/networking/successResponse"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { validateCreateCatalogBody } from "./validateCreateCatalogBody"; +import { createSnapshotCatalog } from "./createSnapshotCatalog"; +import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots"; +import { selectCatalogById } from "@/lib/supabase/catalogs/selectCatalogById"; +import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; +import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; + +const DEFAULT_CATALOG_NAME = "Valuation Catalog"; + +/** + * POST /api/catalogs + * + * Creates a catalog owned by the authenticated account. The owning account is + * resolved from credentials (Privy bearer or x-api-key), never from the body. + * + * With `snapshot`, materializes the catalog from a completed valuation snapshot: + * the snapshot must be owned by the caller, and re-claiming the same snapshot is + * idempotent (returns the catalog already created for that run). + * + * @param request - The request object + * @returns A NextResponse with `{ status, catalog, songs_added }` + */ +export async function createCatalogHandler(request: NextRequest): Promise { + try { + const body = await request.json().catch(() => null); + + const validated = validateCreateCatalogBody(body); + if (validated instanceof NextResponse) { + return validated; + } + + const authResult = await validateAuthContext(request); + if (authResult instanceof NextResponse) { + return authResult; + } + const { accountId } = authResult; + + if (!validated.snapshot) { + const catalog = await insertCatalog(validated.name ?? DEFAULT_CATALOG_NAME); + await insertAccountCatalog({ account: accountId, catalog: catalog.id }); + return successResponse({ catalog, songs_added: 0 }); + } + + const [snapshot] = await selectPlaycountSnapshots({ id: validated.snapshot }); + if (!snapshot) { + return errorResponse("Snapshot not found", 404); + } + if (snapshot.account !== accountId) { + return errorResponse("Snapshot belongs to a different account", 403); + } + + // Idempotent re-claim: the run already produced a catalog. + if (snapshot.catalog) { + const existing = await selectCatalogById(snapshot.catalog); + if (existing) { + return successResponse({ catalog: existing, songs_added: 0 }); + } + } + + const { catalog, songsAdded } = await createSnapshotCatalog({ + accountId, + snapshot, + name: validated.name, + }); + return successResponse({ catalog, songs_added: songsAdded }); + } catch (error) { + console.error("Error creating catalog:", error); + return errorResponse("Internal server error", 500); + } +} diff --git a/lib/catalog/createSnapshotCatalog.ts b/lib/catalog/createSnapshotCatalog.ts new file mode 100644 index 000000000..ab1ba96c2 --- /dev/null +++ b/lib/catalog/createSnapshotCatalog.ts @@ -0,0 +1,45 @@ +import { Tables } from "@/types/database.types"; +import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; +import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; +import { insertCatalogSongs } from "@/lib/supabase/catalog_songs/insertCatalogSongs"; +import { updatePlaycountSnapshot } from "@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot"; +import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements"; + +const DEFAULT_CATALOG_NAME = "Valuation Catalog"; + +/** + * Creates an account-linked catalog from a valuation snapshot: creates the + * `catalogs` row, links it to the account via `account_catalogs`, adds the + * snapshot's **measured** ISRCs (from `song_measurements`, not the snapshot's + * own `isrcs` column — that's null for album-scoped valuation runs) as + * `catalog_songs`, and records the new catalog on the snapshot (the idempotency + * key for re-claims). + * + * Callers must first confirm the snapshot is owned by `accountId` and not yet + * claimed (`snapshot.catalog` is null). + * + * @param params.accountId - Owning account (already authorized) + * @param params.snapshot - The owned, unclaimed snapshot row + * @param params.name - Optional catalog name; falls back to a default + * @returns The created catalog and the number of songs added + */ +export async function createSnapshotCatalog(params: { + accountId: string; + snapshot: Tables<"playcount_snapshots">; + name?: string; +}): Promise<{ catalog: Tables<"catalogs">; songsAdded: number }> { + const { accountId, snapshot, name } = params; + + const catalog = await insertCatalog(name ?? DEFAULT_CATALOG_NAME); + await insertAccountCatalog({ account: accountId, catalog: catalog.id }); + + const measurements = await selectSongMeasurements({ snapshot: snapshot.id }); + const isrcs = [...new Set(measurements.map(m => m.song))]; + if (isrcs.length > 0) { + await insertCatalogSongs(isrcs.map(isrc => ({ catalog: catalog.id, song: isrc }))); + } + + await updatePlaycountSnapshot(snapshot.id, { catalog: catalog.id }); + + return { catalog, songsAdded: isrcs.length }; +} diff --git a/lib/catalog/validateCreateCatalogBody.ts b/lib/catalog/validateCreateCatalogBody.ts new file mode 100644 index 000000000..b41c058c1 --- /dev/null +++ b/lib/catalog/validateCreateCatalogBody.ts @@ -0,0 +1,47 @@ +import { NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { z } from "zod"; + +export const createCatalogBodySchema = z + .object({ + name: z.string().min(1, "name must not be empty").optional(), + snapshot: z.string().uuid("snapshot must be a valid UUID").optional(), + }) + .refine(data => data.name !== undefined || data.snapshot !== undefined, { + message: "Provide at least one of name or snapshot", + }); + +export type CreateCatalogBody = z.infer; + +/** + * Validates a create-catalog request body. + * + * Accepts `{ name?, snapshot? }`; at least one is required. `snapshot` is a + * completed playcount snapshot id (valuation run) to materialize from. The + * owning account is never taken from the body - it is resolved from the + * request credentials by the handler. + * + * @param body - The parsed request body to validate. + * @returns A NextResponse with a 400 error if validation fails, or the + * validated body if it passes. + */ +export function validateCreateCatalogBody(body: unknown): NextResponse | CreateCatalogBody { + const result = createCatalogBodySchema.safeParse(body); + + 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(), + }, + ); + } + + return result.data; +} diff --git a/lib/supabase/account_catalogs/insertAccountCatalog.ts b/lib/supabase/account_catalogs/insertAccountCatalog.ts new file mode 100644 index 000000000..1facd5b63 --- /dev/null +++ b/lib/supabase/account_catalogs/insertAccountCatalog.ts @@ -0,0 +1,21 @@ +import supabase from "../serverClient"; + +/** + * Links a catalog to an account by inserting an `account_catalogs` row. + * + * @param params.account - Account id (owner) + * @param params.catalog - Catalog id to link + * @throws Error if the insert fails + */ +export async function insertAccountCatalog(params: { + account: string; + catalog: string; +}): Promise { + const { error } = await supabase + .from("account_catalogs") + .insert({ account: params.account, catalog: params.catalog }); + + if (error) { + throw new Error(`Failed to link account_catalogs: ${error.message}`); + } +} diff --git a/lib/supabase/catalogs/insertCatalog.ts b/lib/supabase/catalogs/insertCatalog.ts new file mode 100644 index 000000000..ecd0c667b --- /dev/null +++ b/lib/supabase/catalogs/insertCatalog.ts @@ -0,0 +1,19 @@ +import supabase from "../serverClient"; +import { Tables } from "@/types/database.types"; + +/** + * Inserts a new `catalogs` row and returns it. + * + * @param name - Display name for the catalog + * @returns The inserted catalog row + * @throws Error if the insert fails + */ +export async function insertCatalog(name: string): Promise> { + const { data, error } = await supabase.from("catalogs").insert({ name }).select().single(); + + if (error || !data) { + throw new Error(`Failed to insert catalog: ${error?.message ?? "no row returned"}`); + } + + return data; +} diff --git a/lib/supabase/catalogs/selectCatalogById.ts b/lib/supabase/catalogs/selectCatalogById.ts new file mode 100644 index 000000000..39f961e3d --- /dev/null +++ b/lib/supabase/catalogs/selectCatalogById.ts @@ -0,0 +1,19 @@ +import supabase from "../serverClient"; +import { Tables } from "@/types/database.types"; + +/** + * Selects a single `catalogs` row by id, or null if none exists. + * + * @param id - Catalog id + * @returns The catalog row, or null if not found + * @throws Error if the query fails + */ +export async function selectCatalogById(id: string): Promise | null> { + const { data, error } = await supabase.from("catalogs").select("*").eq("id", id).maybeSingle(); + + if (error) { + throw new Error(`Failed to fetch catalog: ${error.message}`); + } + + return data; +} diff --git a/lib/supabase/song_measurements/selectSongMeasurements.ts b/lib/supabase/song_measurements/selectSongMeasurements.ts index 24d50bd44..1e118d14b 100644 --- a/lib/supabase/song_measurements/selectSongMeasurements.ts +++ b/lib/supabase/song_measurements/selectSongMeasurements.ts @@ -2,13 +2,14 @@ import supabase from "../serverClient"; import { Tables } from "@/types/database.types"; /** - * Select measurements newest-first. Filter by one song or a batch of songs - * (one is required), and optionally by platform/metric. Uses the - * (song, platform, metric, captured_at DESC) series index; pass `limit: 1` - * for the latest capture, omit for the full series. + * Select measurements newest-first. Filter by one song, a batch of songs, or a + * snapshot (one of the three is required), and optionally by platform/metric. + * Uses the (song, platform, metric, captured_at DESC) series index; pass + * `limit: 1` for the latest capture, omit for the full series. * * @param params.song - Single song ISRC filter * @param params.songs - Batch of song ISRCs (e.g. all tracks on an album) + * @param params.snapshot - Snapshot id filter (all measurements captured for a run) * @param params.platform - Optional platform filter (e.g. "spotify") * @param params.metric - Optional metric filter (e.g. "platform_displayed_play_count") * @param params.limit - Optional cap on returned rows @@ -17,17 +18,19 @@ import { Tables } from "@/types/database.types"; export async function selectSongMeasurements({ song, songs, + snapshot, platform, metric, limit, }: { song?: string; songs?: string[]; + snapshot?: string; platform?: string; metric?: string; limit?: number; }): Promise[]> { - if (!song && (!songs || songs.length === 0)) return []; + if (!song && (!songs || songs.length === 0) && !snapshot) return []; let query = supabase .from("song_measurements") @@ -36,6 +39,7 @@ export async function selectSongMeasurements({ if (song) query = query.eq("song", song); if (songs && songs.length > 0) query = query.in("song", songs); + if (snapshot) query = query.eq("snapshot", snapshot); if (platform) query = query.eq("platform", platform); if (metric) query = query.eq("metric", metric); if (limit) query = query.limit(limit); From a520a1ddac9b7dd4f3bf9d040c2270ecdb7c1dbe Mon Sep 17 00:00:00 2001 From: "sweetman.eth" Date: Thu, 18 Jun 2026 17:00:50 -0500 Subject: [PATCH 5/7] fix: LEFT-join artists in catalog-songs read (materialized tracks were hidden) (#681) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: LEFT-join artists in catalog-songs read so materialized tracks surface selectCatalogSongsWithArtists used song_artists!inner -> accounts!inner, so valuation-captured tracks (which have songs + song_measurements but no song_artists yet) were filtered out — a materialized catalog read back as 0 songs (verified live on api#677). Drop the two !inner so artist-less songs return with artists: []; songs!inner stays (catalog_songs.song FK guarantees it). Closes the read-path half of the song_artists follow-up in recoupable/chat#1801. Longer-term (option a): the capture pipeline should also write song_artists. * Update lib/supabase/catalog_songs/selectCatalogSongsWithArtists.ts --- lib/supabase/catalog_songs/selectCatalogSongsWithArtists.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/supabase/catalog_songs/selectCatalogSongsWithArtists.ts b/lib/supabase/catalog_songs/selectCatalogSongsWithArtists.ts index 8639eb405..47cc14206 100644 --- a/lib/supabase/catalog_songs/selectCatalogSongsWithArtists.ts +++ b/lib/supabase/catalog_songs/selectCatalogSongsWithArtists.ts @@ -55,9 +55,9 @@ export async function selectCatalogSongsWithArtists( album, notes, updated_at, - song_artists!inner ( + song_artists ( artist, - accounts!inner ( + accounts ( id, name, timestamp From 709ea0cb339e425cdf4b27cbbd34bfaccf374319 Mon Sep 17 00:00:00 2001 From: "sweetman.eth" Date: Thu, 18 Jun 2026 17:58:44 -0500 Subject: [PATCH 6/7] feat: add X (Twitter) + LinkedIn to the Composio connector whitelist (chat#1793) (#679) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add X (Twitter) + LinkedIn to the Composio connector whitelist (chat#1793) Expand the existing whitelist pattern to two new platforms — no architecture changes: - SUPPORTED_TOOLKITS (getConnectors.ts) + ENABLED_TOOLKITS (getComposioTools.ts) - CONNECTOR_DISPLAY_NAMES: twitter → "X (Twitter)", linkedin → "LinkedIn" - buildAuthConfigs() reads COMPOSIO_TWITTER_AUTH_CONFIG_ID + COMPOSIO_LINKEDIN_AUTH_CONFIG_ID - document both env vars in .env.example TDD: new buildAuthConfigs unit + expanded getConnectors / handler / ENABLED_TOOLKITS assertions, RED before GREEN. Full lib/composio suite green (157 tests). Implements the contract from docs#244. Co-Authored-By: Claude Opus 4.8 (1M context) * chore: fix lint/format — relocate ENABLED_TOOLKITS test block, reformat toolkit array - Move the ENABLED_TOOLKITS describe block below the imports (import/first) - Prettier-format the expanded toolkits array in getConnectors.test.ts Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .env.example | 2 + .../__tests__/buildAuthConfigs.test.ts | 48 +++++++++++++++++++ .../__tests__/getConnectors.test.ts | 22 ++++++++- .../__tests__/getConnectorsHandler.test.ts | 2 + lib/composio/connectors/buildAuthConfigs.ts | 6 +++ lib/composio/connectors/getConnectors.ts | 2 + .../connectors/getConnectorsHandler.ts | 2 + .../__tests__/getComposioTools.test.ts | 9 +++- lib/composio/toolRouter/getComposioTools.ts | 2 + 9 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 lib/composio/connectors/__tests__/buildAuthConfigs.test.ts diff --git a/.env.example b/.env.example index 9d6de84d9..80bca6864 100644 --- a/.env.example +++ b/.env.example @@ -34,6 +34,8 @@ COMPOSIO_INSTAGRAM_AUTH_CONFIG_ID= COMPOSIO_GOOGLE_SHEETS_AUTH_CONFIG_ID= COMPOSIO_GOOGLE_DOCS_AUTH_CONFIG_ID= COMPOSIO_GOOGLE_DRIVE_AUTH_CONFIG_ID= +COMPOSIO_TWITTER_AUTH_CONFIG_ID= +COMPOSIO_LINKEDIN_AUTH_CONFIG_ID= # Slack (coding agent) SLACK_BOT_TOKEN= diff --git a/lib/composio/connectors/__tests__/buildAuthConfigs.test.ts b/lib/composio/connectors/__tests__/buildAuthConfigs.test.ts new file mode 100644 index 000000000..451dc92ed --- /dev/null +++ b/lib/composio/connectors/__tests__/buildAuthConfigs.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { buildAuthConfigs } from "../buildAuthConfigs"; + +const AUTH_CONFIG_ENV_KEYS = [ + "COMPOSIO_TIKTOK_AUTH_CONFIG_ID", + "COMPOSIO_INSTAGRAM_AUTH_CONFIG_ID", + "COMPOSIO_GOOGLE_SHEETS_AUTH_CONFIG_ID", + "COMPOSIO_GOOGLE_DOCS_AUTH_CONFIG_ID", + "COMPOSIO_GOOGLE_DRIVE_AUTH_CONFIG_ID", + "COMPOSIO_YOUTUBE_AUTH_CONFIG_ID", + "COMPOSIO_TWITTER_AUTH_CONFIG_ID", + "COMPOSIO_LINKEDIN_AUTH_CONFIG_ID", +]; + +describe("buildAuthConfigs", () => { + let saved: Record; + + beforeEach(() => { + // Snapshot then clear every auth-config env var so the test is isolated + // from whatever .env.local provides. + saved = {}; + for (const key of AUTH_CONFIG_ENV_KEYS) { + saved[key] = process.env[key]; + delete process.env[key]; + } + }); + + afterEach(() => { + for (const key of AUTH_CONFIG_ENV_KEYS) { + if (saved[key] === undefined) delete process.env[key]; + else process.env[key] = saved[key]; + } + }); + + it("returns undefined when no auth-config env vars are set", () => { + expect(buildAuthConfigs()).toBeUndefined(); + }); + + it("reads the Twitter and LinkedIn auth configs from env", () => { + process.env.COMPOSIO_TWITTER_AUTH_CONFIG_ID = "ac_twitter_123"; + process.env.COMPOSIO_LINKEDIN_AUTH_CONFIG_ID = "ac_linkedin_456"; + + expect(buildAuthConfigs()).toEqual({ + twitter: "ac_twitter_123", + linkedin: "ac_linkedin_456", + }); + }); +}); diff --git a/lib/composio/connectors/__tests__/getConnectors.test.ts b/lib/composio/connectors/__tests__/getConnectors.test.ts index 762c07e07..b637f1026 100644 --- a/lib/composio/connectors/__tests__/getConnectors.test.ts +++ b/lib/composio/connectors/__tests__/getConnectors.test.ts @@ -37,7 +37,16 @@ describe("getConnectors", () => { expect(getComposioClient).toHaveBeenCalled(); expect(mockComposio.create).toHaveBeenCalledWith("account-123", { - toolkits: ["googlesheets", "googledrive", "googledocs", "tiktok", "instagram", "youtube"], + toolkits: [ + "googlesheets", + "googledrive", + "googledocs", + "tiktok", + "instagram", + "youtube", + "twitter", + "linkedin", + ], }); expect(result).toEqual([ { @@ -92,7 +101,16 @@ describe("getConnectors", () => { await getConnectors("account-123"); expect(mockComposio.create).toHaveBeenCalledWith("account-123", { - toolkits: ["googlesheets", "googledrive", "googledocs", "tiktok", "instagram", "youtube"], + toolkits: [ + "googlesheets", + "googledrive", + "googledocs", + "tiktok", + "instagram", + "youtube", + "twitter", + "linkedin", + ], authConfigs: { tiktok: "ac_tiktok_123", instagram: "ac_instagram_456", diff --git a/lib/composio/connectors/__tests__/getConnectorsHandler.test.ts b/lib/composio/connectors/__tests__/getConnectorsHandler.test.ts index d6c3fe31f..bc68d0af6 100644 --- a/lib/composio/connectors/__tests__/getConnectorsHandler.test.ts +++ b/lib/composio/connectors/__tests__/getConnectorsHandler.test.ts @@ -72,6 +72,8 @@ describe("getConnectorsHandler", () => { googlesheets: "Google Sheets", googledrive: "Google Drive", googledocs: "Google Docs", + twitter: "X (Twitter)", + linkedin: "LinkedIn", }, }); }); diff --git a/lib/composio/connectors/buildAuthConfigs.ts b/lib/composio/connectors/buildAuthConfigs.ts index 2c31d571a..4edb893c0 100644 --- a/lib/composio/connectors/buildAuthConfigs.ts +++ b/lib/composio/connectors/buildAuthConfigs.ts @@ -23,5 +23,11 @@ export function buildAuthConfigs(): Record | undefined { if (process.env.COMPOSIO_YOUTUBE_AUTH_CONFIG_ID) { configs.youtube = process.env.COMPOSIO_YOUTUBE_AUTH_CONFIG_ID; } + if (process.env.COMPOSIO_TWITTER_AUTH_CONFIG_ID) { + configs.twitter = process.env.COMPOSIO_TWITTER_AUTH_CONFIG_ID; + } + if (process.env.COMPOSIO_LINKEDIN_AUTH_CONFIG_ID) { + configs.linkedin = process.env.COMPOSIO_LINKEDIN_AUTH_CONFIG_ID; + } return Object.keys(configs).length > 0 ? configs : undefined; } diff --git a/lib/composio/connectors/getConnectors.ts b/lib/composio/connectors/getConnectors.ts index dcb5fb38e..d692bf40c 100644 --- a/lib/composio/connectors/getConnectors.ts +++ b/lib/composio/connectors/getConnectors.ts @@ -23,6 +23,8 @@ const SUPPORTED_TOOLKITS = [ "tiktok", "instagram", "youtube", + "twitter", + "linkedin", ]; /** diff --git a/lib/composio/connectors/getConnectorsHandler.ts b/lib/composio/connectors/getConnectorsHandler.ts index d92f1293d..86633e32b 100644 --- a/lib/composio/connectors/getConnectorsHandler.ts +++ b/lib/composio/connectors/getConnectorsHandler.ts @@ -12,6 +12,8 @@ const CONNECTOR_DISPLAY_NAMES: Record = { googlesheets: "Google Sheets", googledrive: "Google Drive", googledocs: "Google Docs", + twitter: "X (Twitter)", + linkedin: "LinkedIn", }; /** diff --git a/lib/composio/toolRouter/__tests__/getComposioTools.test.ts b/lib/composio/toolRouter/__tests__/getComposioTools.test.ts index f49aff1d1..ae8d5d718 100644 --- a/lib/composio/toolRouter/__tests__/getComposioTools.test.ts +++ b/lib/composio/toolRouter/__tests__/getComposioTools.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { getComposioTools } from "../getComposioTools"; +import { getComposioTools, ENABLED_TOOLKITS } from "../getComposioTools"; import { getComposioClient } from "../../client"; import { getCallbackUrl } from "../../getCallbackUrl"; @@ -235,3 +235,10 @@ describe("getComposioTools", () => { expect(result).toHaveProperty("COMPOSIO_MULTI_EXECUTE_TOOL"); }); }); + +describe("ENABLED_TOOLKITS", () => { + it("includes twitter and linkedin", () => { + expect(ENABLED_TOOLKITS).toContain("twitter"); + expect(ENABLED_TOOLKITS).toContain("linkedin"); + }); +}); diff --git a/lib/composio/toolRouter/getComposioTools.ts b/lib/composio/toolRouter/getComposioTools.ts index 61f0f887f..7f74b4c2f 100644 --- a/lib/composio/toolRouter/getComposioTools.ts +++ b/lib/composio/toolRouter/getComposioTools.ts @@ -16,6 +16,8 @@ export const ENABLED_TOOLKITS = [ "tiktok", "instagram", "youtube", + "twitter", + "linkedin", ]; const SHARED_ACCOUNT_ID = "recoup-shared-767f498e-e1e9-43c6-a152-a96ae3bd8d07"; From 66cc2fe2518ae526d15f12a70528de93410af815 Mon Sep 17 00:00:00 2001 From: "sweetman.eth" Date: Thu, 18 Jun 2026 18:30:11 -0500 Subject: [PATCH 7/7] chore: remove unused ALLOWED_ARTIST_CONNECTORS from api (chat#1793) (#680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: allow artists to connect X (Twitter); keep LinkedIn label-only (chat#1793) Add `twitter` to ALLOWED_ARTIST_CONNECTORS — artist-facing social, same class as tiktok/instagram/youtube. `linkedin` is intentionally left out (label/owner-only). TDD: isAllowedArtistConnector.test.ts asserts twitter allowed + linkedin excluded, RED before GREEN. Full lib/composio suite green (157 tests). Co-Authored-By: Claude Opus 4.8 (1M context) * feat: allow artists to connect LinkedIn too (chat#1793) Reversal of the earlier "LinkedIn label/owner-only" call: per owner decision 2026-06-18, LinkedIn is now an artist-facing connector like the others. Add `linkedin` to ALLOWED_ARTIST_CONNECTORS. TDD: flipped the linkedin assertions (now allowed/included), RED before GREEN. Full lib/composio suite green (159 tests). Co-Authored-By: Claude Opus 4.8 (1M context) * chore: remove unused ALLOWED_ARTIST_CONNECTORS from api (chat#1793) The api copy of the artist connector allow-list had no runtime consumer — only its definition, test, and an (also-unused) barrel re-export. The connector routes are unopinionated (allow any connector for any account); the allow-list that actually drives the artist Connectors tab lives in `chat` (`lib/composio/allowedArtistConnectors.ts`). Removing the dead code. Supersedes the earlier plan to add twitter/linkedin to this api constant (decision: owner, 2026-06-18) — the artist allow-list is chat-only. Deletes isAllowedArtistConnector.ts + its test, and the barrel re-export. lib/composio suite green (149); no new tsc errors vs test (198 baseline). Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../isAllowedArtistConnector.test.ts | 43 ------------------- lib/composio/connectors/index.ts | 5 --- .../connectors/isAllowedArtistConnector.ts | 16 ------- 3 files changed, 64 deletions(-) delete mode 100644 lib/composio/connectors/__tests__/isAllowedArtistConnector.test.ts delete mode 100644 lib/composio/connectors/isAllowedArtistConnector.ts diff --git a/lib/composio/connectors/__tests__/isAllowedArtistConnector.test.ts b/lib/composio/connectors/__tests__/isAllowedArtistConnector.test.ts deleted file mode 100644 index 596407d20..000000000 --- a/lib/composio/connectors/__tests__/isAllowedArtistConnector.test.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { isAllowedArtistConnector, ALLOWED_ARTIST_CONNECTORS } from "../isAllowedArtistConnector"; - -describe("isAllowedArtistConnector", () => { - it("should return true for 'tiktok'", () => { - expect(isAllowedArtistConnector("tiktok")).toBe(true); - }); - - it("should return true for 'instagram'", () => { - expect(isAllowedArtistConnector("instagram")).toBe(true); - }); - - it("should return true for 'youtube'", () => { - expect(isAllowedArtistConnector("youtube")).toBe(true); - }); - - it("should return false for connectors not in ALLOWED_ARTIST_CONNECTORS", () => { - expect(isAllowedArtistConnector("googlesheets")).toBe(false); - expect(isAllowedArtistConnector("googledrive")).toBe(false); - expect(isAllowedArtistConnector("random")).toBe(false); - }); - - it("should return false for empty string", () => { - expect(isAllowedArtistConnector("")).toBe(false); - }); - - it("should be case-sensitive", () => { - expect(isAllowedArtistConnector("TikTok")).toBe(false); - expect(isAllowedArtistConnector("TIKTOK")).toBe(false); - }); -}); - -describe("ALLOWED_ARTIST_CONNECTORS", () => { - it("should include tiktok, instagram, and youtube", () => { - expect(ALLOWED_ARTIST_CONNECTORS).toContain("tiktok"); - expect(ALLOWED_ARTIST_CONNECTORS).toContain("instagram"); - expect(ALLOWED_ARTIST_CONNECTORS).toContain("youtube"); - }); - - it("should be a readonly array", () => { - expect(Array.isArray(ALLOWED_ARTIST_CONNECTORS)).toBe(true); - }); -}); diff --git a/lib/composio/connectors/index.ts b/lib/composio/connectors/index.ts index 848b357ba..be0178edb 100644 --- a/lib/composio/connectors/index.ts +++ b/lib/composio/connectors/index.ts @@ -5,9 +5,4 @@ export { type AuthorizeConnectorOptions, } from "./authorizeConnector"; export { disconnectConnector, type DisconnectConnectorOptions } from "./disconnectConnector"; -export { - ALLOWED_ARTIST_CONNECTORS, - isAllowedArtistConnector, - type AllowedArtistConnector, -} from "./isAllowedArtistConnector"; export { verifyConnectorOwnership } from "./verifyConnectorOwnership"; diff --git a/lib/composio/connectors/isAllowedArtistConnector.ts b/lib/composio/connectors/isAllowedArtistConnector.ts deleted file mode 100644 index f3d83d82d..000000000 --- a/lib/composio/connectors/isAllowedArtistConnector.ts +++ /dev/null @@ -1,16 +0,0 @@ -/** - * List of toolkit slugs that artists are allowed to connect. - * Only these connectors will be shown in the artist-connectors API. - */ -export const ALLOWED_ARTIST_CONNECTORS = ["tiktok", "instagram", "youtube"] as const; - -export type AllowedArtistConnector = (typeof ALLOWED_ARTIST_CONNECTORS)[number]; - -/** - * Check if a connector slug is an allowed artist connector. - * - * @param slug - */ -export function isAllowedArtistConnector(slug: string): slug is AllowedArtistConnector { - return (ALLOWED_ARTIST_CONNECTORS as readonly string[]).includes(slug); -}