diff --git a/lib/research/measurement_jobs/__tests__/createMeasurementJob.test.ts b/lib/research/measurement_jobs/__tests__/createMeasurementJob.test.ts index 88fd7bd8..1bc2fcb2 100644 --- a/lib/research/measurement_jobs/__tests__/createMeasurementJob.test.ts +++ b/lib/research/measurement_jobs/__tests__/createMeasurementJob.test.ts @@ -58,4 +58,46 @@ describe("createMeasurementJob", () => { const r = await createMeasurementJob(req("current")); expect(r).toEqual({ error: "cap reached", status: 429 }); }); + + // Preview verification of chat#1912 row 4 showed this endpoint dropping the + // reuse flag: a handed-back capture was indistinguishable from a fresh one + // except by its zero cost. Callers polling for a new capture need to know. + it("passes the reuse flag through for a handed-back capture", async () => { + vi.mocked(createSnapshot).mockResolvedValue({ + data: { + status: "success", + snapshot_id: "snap_reused", + state: "done", + album_count: 2, + estimated_cost_usd: 0, + reused: true, + }, + }); + + const result = await createMeasurementJob({ + accountId: "acc_1", + body: { source: "current", scope: { album_ids: ["a1"] }, platforms: ["spotify"] }, + } as never); + + expect((result as { data: Record }).data.reused).toBe(true); + }); + + it("omits the reuse flag for a fresh capture", async () => { + vi.mocked(createSnapshot).mockResolvedValue({ + data: { + status: "success", + snapshot_id: "snap_new", + state: "queued", + album_count: 2, + estimated_cost_usd: 0.006, + }, + }); + + const result = await createMeasurementJob({ + accountId: "acc_1", + body: { source: "current", scope: { album_ids: ["a1"] }, platforms: ["spotify"] }, + } as never); + + expect((result as { data: Record }).data.reused).toBeUndefined(); + }); }); diff --git a/lib/research/measurement_jobs/createMeasurementJob.ts b/lib/research/measurement_jobs/createMeasurementJob.ts index adb383e0..240d7e4d 100644 --- a/lib/research/measurement_jobs/createMeasurementJob.ts +++ b/lib/research/measurement_jobs/createMeasurementJob.ts @@ -9,6 +9,8 @@ type SnapshotData = { state: string; album_count: number; estimated_cost_usd: number; + /** Present only when an existing capture was handed back (chat#1912 row 4). */ + reused?: boolean; }; /** @@ -42,6 +44,9 @@ export async function createMeasurementJob( state: d.state, album_count: d.album_count, estimated_cost_usd: d.estimated_cost_usd, + // Surfaced so a caller can tell a handed-back capture from a fresh one + // without inferring it from the zero cost. + ...(d.reused ? { reused: true } : {}), }, }; } diff --git a/lib/research/playcounts/__tests__/createSnapshot.test.ts b/lib/research/playcounts/__tests__/createSnapshot.test.ts index d67f1706..f3a0baed 100644 --- a/lib/research/playcounts/__tests__/createSnapshot.test.ts +++ b/lib/research/playcounts/__tests__/createSnapshot.test.ts @@ -91,4 +91,93 @@ describe("createSnapshot", () => { }); expect(start).not.toHaveBeenCalled(); }); + + // A capture 30 minutes old is reused: play counts do not move meaningfully + // inside an hour, so re-scraping the same albums is pure waste (chat#1912 + // row 4). This would have failed under the original 15-minute window. + it("reuses an identical capture from 30 minutes ago instead of scraping again", async () => { + vi.mocked(resolveSnapshotAlbums).mockResolvedValue(["a1", "a2"]); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ + { + id: "snap_existing", + album_ids: ["a2", "a1"], + platforms: ["spotify"], + schedule: "once", + state: "done", + album_count: 2, + estimated_cost_usd: 0.006, + created_at: new Date(Date.now() - 30 * 60 * 1000).toISOString(), + }, + ] as never); + + const result = await createSnapshot({ + accountId: "acc_1", + body: { album_ids: ["a1", "a2"], platforms: ["spotify"], schedule: "once" }, + }); + + expect(result).toEqual({ + data: { + status: "success", + snapshot_id: "snap_existing", + state: "done", + album_count: 2, + estimated_cost_usd: 0, + reused: true, + }, + }); + expect(insertPlaycountSnapshot).not.toHaveBeenCalled(); + expect(start).not.toHaveBeenCalled(); + }); + + // Review finding (coderabbit / cubic, 2026-07-30): the candidate list used to + // be the monthly-cap query, so in the first hour of a UTC month the previous + // month's captures were invisible and an identical request re-scraped. + it("looks back past the month boundary when the reuse window crosses it", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-08-01T00:10:00.000Z")); + vi.mocked(resolveSnapshotAlbums).mockResolvedValue(["a1"]); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([] as never); + + await createSnapshot({ + accountId: "acc_1", + body: { album_ids: ["a1"], platforms: ["spotify"], schedule: "once" }, + }); + + const { createdAfter } = vi.mocked(selectPlaycountSnapshots).mock.calls[0][0]; + // 60 minutes before 00:10 on the 1st is 23:10 on the previous month's last day. + expect(new Date(createdAfter as string).toISOString()).toBe("2026-07-31T23:10:00.000Z"); + vi.useRealTimers(); + }); + + // The cap must still count only the current month, even though the lookup + // now reaches into the previous one. + it("excludes previous-month spend from the monthly cap", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-08-01T00:10:00.000Z")); + process.env.SNAPSHOT_MONTHLY_CAP_USD = "1"; + vi.mocked(resolveSnapshotAlbums).mockResolvedValue(["a1"]); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ + { + id: "last_month", + album_ids: ["zzz"], + platforms: ["spotify"], + schedule: "once", + state: "done", + album_count: 1, + estimated_cost_usd: 5, + created_at: "2026-07-31T23:50:00.000Z", + }, + ] as never); + + const result = await createSnapshot({ + accountId: "acc_1", + body: { album_ids: ["a1"], platforms: ["spotify"], schedule: "once" }, + }); + + expect(result).not.toEqual({ + error: "Per-organization monthly snapshot cap reached", + status: 429, + }); + vi.useRealTimers(); + }); }); diff --git a/lib/research/playcounts/__tests__/findReusableSnapshot.test.ts b/lib/research/playcounts/__tests__/findReusableSnapshot.test.ts new file mode 100644 index 00000000..2ff59a52 --- /dev/null +++ b/lib/research/playcounts/__tests__/findReusableSnapshot.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect } from "vitest"; +import { findReusableSnapshot } from "@/lib/research/playcounts/findReusableSnapshot"; +import type { Tables } from "@/types/database.types"; + +const NOW = new Date("2026-07-30T14:33:32.000Z"); + +const snapshot = (over: Partial>) => + ({ + id: "snap-1", + account: "acc-1", + catalog: null, + album_ids: ["a1", "a2"], + isrcs: null, + platforms: ["spotify"], + schedule: "once", + state: "done", + album_count: 2, + estimated_cost_usd: 0.006, + created_at: "2026-07-30T14:30:35.000Z", + updated_at: "2026-07-30T14:30:35.000Z", + ...over, + }) as Tables<"playcount_snapshots">; + +const find = (rows: Tables<"playcount_snapshots">[], albumIds = ["a1", "a2"]) => + findReusableSnapshot({ + snapshots: rows, + albumIds, + platforms: ["spotify"], + schedule: "once", + windowMinutes: 15, + now: NOW, + }); + +describe("findReusableSnapshot", () => { + // chat#1912 row 4: one pass through /setup measured the same 24 albums twice + // — the seeding valuation at 14:30:35, then the first-task pre-run at + // 14:33:32 — billing the scraper twice for an identical capture. + it("reuses a capture of the same albums taken minutes ago", () => { + expect(find([snapshot({})])?.id).toBe("snap-1"); + }); + + it("ignores album order when comparing scope", () => { + expect(find([snapshot({ album_ids: ["a2", "a1"] })])?.id).toBe("snap-1"); + }); + + it("does not reuse a capture of a different album set", () => { + expect(find([snapshot({ album_ids: ["a1"] })])).toBeNull(); + expect(find([snapshot({ album_ids: ["a1", "a2", "a3"] })])).toBeNull(); + }); + + it("does not reuse a capture older than the window", () => { + expect(find([snapshot({ created_at: "2026-07-30T14:00:00.000Z" })])).toBeNull(); + }); + + it("does not reuse a failed capture", () => { + expect(find([snapshot({ state: "failed" })])).toBeNull(); + }); + + // A queued capture is still the right thing to hand back: the caller polls + // for measurements either way, and starting a second scrape is the waste. + it("reuses a capture that is still in flight", () => { + expect(find([snapshot({ state: "queued" })])?.id).toBe("snap-1"); + }); + + it("does not reuse across platforms", () => { + expect(find([snapshot({ platforms: ["apple"] })])).toBeNull(); + }); + + it("does not reuse a scheduled run for a one-off request", () => { + expect(find([snapshot({ schedule: "monthly" })])).toBeNull(); + }); + + it("prefers the newest eligible capture", () => { + const older = snapshot({ id: "old", created_at: "2026-07-30T14:25:00.000Z" }); + const newer = snapshot({ id: "new", created_at: "2026-07-30T14:31:00.000Z" }); + expect(find([older, newer])?.id).toBe("new"); + }); + + it("returns null when nothing matches", () => { + expect(find([])).toBeNull(); + }); +}); diff --git a/lib/research/playcounts/buildReusedSnapshotResult.ts b/lib/research/playcounts/buildReusedSnapshotResult.ts new file mode 100644 index 00000000..eb51ffae --- /dev/null +++ b/lib/research/playcounts/buildReusedSnapshotResult.ts @@ -0,0 +1,19 @@ +import type { Tables } from "@/types/database.types"; + +/** + * The 202-shaped payload for a request satisfied by an existing capture. + * `estimated_cost_usd` is 0 because nothing was scraped, and `reused` lets + * callers tell a fresh capture from a handed-back one (chat#1912 row 4). + */ +export function buildReusedSnapshotResult(snapshot: Tables<"playcount_snapshots">) { + return { + data: { + status: "success", + snapshot_id: snapshot.id, + state: snapshot.state, + album_count: snapshot.album_count, + estimated_cost_usd: 0, + reused: true, + }, + }; +} diff --git a/lib/research/playcounts/createSnapshot.ts b/lib/research/playcounts/createSnapshot.ts index b5f6b309..53cb4a1d 100644 --- a/lib/research/playcounts/createSnapshot.ts +++ b/lib/research/playcounts/createSnapshot.ts @@ -3,11 +3,20 @@ import { resolveSnapshotAlbums } from "@/lib/research/playcounts/resolveSnapshot import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots"; import { insertPlaycountSnapshot } from "@/lib/supabase/playcount_snapshots/insertPlaycountSnapshot"; import { playcountSnapshotWorkflow } from "@/app/workflows/playcountSnapshotWorkflow"; +import { findReusableSnapshot } from "@/lib/research/playcounts/findReusableSnapshot"; +import { buildReusedSnapshotResult } from "@/lib/research/playcounts/buildReusedSnapshotResult"; +import { getMonthlySpendUsd } from "@/lib/research/playcounts/getMonthlySpendUsd"; import { CreateSnapshotBody } from "@/lib/research/playcounts/validateCreateSnapshotRequest"; /** Actor pricing: ~$3 per 1k album URLs. */ const COST_PER_ALBUM_USD = 0.003; const DEFAULT_MONTHLY_CAP_USD = 25; +/** + * Play counts do not move meaningfully inside an hour, so an identical capture + * requested within this window reuses the earlier one rather than re-scraping + * (chat#1912 row 4). + */ +const REUSE_WINDOW_MINUTES = 60; export type CreateSnapshotResult = { data: unknown } | { error: string; status: number }; @@ -32,15 +41,35 @@ export async function createSnapshot(params: { }; } - const monthStart = new Date(); + const now = new Date(); + const monthStart = new Date(now); monthStart.setUTCDate(1); monthStart.setUTCHours(0, 0, 0, 0); - const monthSnapshots = await selectPlaycountSnapshots({ + const reuseCutoff = new Date(now.getTime() - REUSE_WINDOW_MINUTES * 60 * 1000); + + // One query serves both needs, so the lower bound is whichever reaches + // further back. Without this, a request in the first hour of a UTC month + // could not see the previous month's captures and re-scraped them. + const lookbackStart = reuseCutoff < monthStart ? reuseCutoff : monthStart; + const snapshots = await selectPlaycountSnapshots({ account: params.accountId, - createdAfter: monthStart.toISOString(), + createdAfter: lookbackStart.toISOString(), }); - const spentUsd = monthSnapshots.reduce((sum, row) => sum + (row.estimated_cost_usd ?? 0), 0); + + const spentUsd = getMonthlySpendUsd(snapshots, monthStart); const estimatedCostUsd = Number((albumIds.length * COST_PER_ALBUM_USD).toFixed(4)); + + // Hand back an identical recent capture rather than scraping the same albums + // twice. + const reusable = findReusableSnapshot({ + snapshots, + albumIds, + platforms: params.body.platforms, + schedule: params.body.schedule, + windowMinutes: REUSE_WINDOW_MINUTES, + now, + }); + if (reusable) return buildReusedSnapshotResult(reusable); const capUsd = Number(process.env.SNAPSHOT_MONTHLY_CAP_USD) || DEFAULT_MONTHLY_CAP_USD; if (spentUsd + estimatedCostUsd > capUsd) { return { error: "Per-organization monthly snapshot cap reached", status: 429 }; diff --git a/lib/research/playcounts/findReusableSnapshot.ts b/lib/research/playcounts/findReusableSnapshot.ts new file mode 100644 index 00000000..8cd8ee4b Binary files /dev/null and b/lib/research/playcounts/findReusableSnapshot.ts differ diff --git a/lib/research/playcounts/getMonthlySpendUsd.ts b/lib/research/playcounts/getMonthlySpendUsd.ts new file mode 100644 index 00000000..30fabc01 --- /dev/null +++ b/lib/research/playcounts/getMonthlySpendUsd.ts @@ -0,0 +1,17 @@ +import type { Tables } from "@/types/database.types"; + +/** + * Scraper spend for the current calendar month, in USD. + * + * The caller's snapshot lookup can reach into the previous month (the reuse + * window crosses the boundary), so rows are filtered by `monthStart` here + * rather than relying on the query's lower bound (chat#1912 row 4). + */ +export function getMonthlySpendUsd( + snapshots: Tables<"playcount_snapshots">[], + monthStart: Date, +): number { + return snapshots + .filter(row => !!row.created_at && new Date(row.created_at) >= monthStart) + .reduce((sum, row) => sum + (row.estimated_cost_usd ?? 0), 0); +}