From 3af32436acf02ee6b0318a48bb49903831543798 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Wed, 16 Sep 2026 03:28:38 +0000 Subject: [PATCH] Fix ad starvation beyond the first 100 creatives --- lib/ads/candidates.ts | 32 +++++++++++++++ lib/ads/serve.ts | 11 +---- tests/contract/ads-self-deal.test.ts | 61 ++++++++++++++++++++++++++-- 3 files changed, 91 insertions(+), 13 deletions(-) create mode 100644 lib/ads/candidates.ts diff --git a/lib/ads/candidates.ts b/lib/ads/candidates.ts new file mode 100644 index 00000000..e7b16b85 --- /dev/null +++ b/lib/ads/candidates.ts @@ -0,0 +1,32 @@ +import type { SupabaseClient } from "@supabase/supabase-js"; +import type { AdFormatId } from "./creative"; + +// A page size, never a cap on the auction. The old LIMIT 100 excluded newer +// campaigns entirely once a format had more than 100 ready creatives. +const PAGE_SIZE = 500; +const COLUMNS = "id, campaign_id, format, headline, body, cta_text, image_url, logo_url, bg_color, fg_color, accent_color, light_bg_color, light_fg_color, light_accent_color, font_family, ad_campaigns!inner(id, owner_id, status, ref_slug, destination_url, daily_budget_cents, spend_today_cents, spend_date, bid_credits)"; + +export async function loadServingCreatives(sb: SupabaseClient, format: AdFormatId) { + const rows = []; + let after: string | undefined; + for (;;) { + let query = sb.from("ad_creatives") + .select(COLUMNS) + .eq("format", format) + .eq("status", "ready") + .in("ad_campaigns.status", ["active", "exhausted"]) + .order("id", { ascending: true }) + .limit(PAGE_SIZE); + if (after) query = query.gt("id", after); + const { data, error } = await query; + if (error) { + // Never run an auction against a silently incomplete candidate pool. + console.error("[ads] creative inventory unavailable", error.message); + return []; + } + if (!data?.length) return rows; + rows.push(...data); + if (data.length < PAGE_SIZE) return rows; + after = data[data.length - 1].id as string; + } +} diff --git a/lib/ads/serve.ts b/lib/ads/serve.ts index 94111b03..898301fb 100644 --- a/lib/ads/serve.ts +++ b/lib/ads/serve.ts @@ -1,4 +1,5 @@ import crypto from "node:crypto"; +import { loadServingCreatives } from "./candidates"; import { serviceClient } from "@/lib/supabase/service"; import { env } from "@/lib/env"; import { @@ -221,15 +222,7 @@ export async function serveAd( // Campaigns with a ready creative in this format. 'exhausted' is a legacy // status no longer written by ad_charge_click — rows still carrying it are // live campaigns that ran dry, and belong on the free tier rather than dark. - const { data: creatives } = await sb - .from("ad_creatives") - .select( - "id, campaign_id, format, headline, body, cta_text, image_url, logo_url, bg_color, fg_color, accent_color, light_bg_color, light_fg_color, light_accent_color, font_family, ad_campaigns!inner(id, owner_id, status, ref_slug, destination_url, daily_budget_cents, spend_today_cents, spend_date, bid_credits)", - ) - .eq("format", format) - .eq("status", "ready") - .in("ad_campaigns.status", ["active", "exhausted"]) - .limit(100); + const creatives = await loadServingCreatives(sb, format); // No paid creative for this format → default CrawlProof house ad. if (!creatives || creatives.length === 0) return houseFill(format, theme); diff --git a/tests/contract/ads-self-deal.test.ts b/tests/contract/ads-self-deal.test.ts index 11601c15..34de88e4 100644 --- a/tests/contract/ads-self-deal.test.ts +++ b/tests/contract/ads-self-deal.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; // Regression: a self-owned campaign (same profile owns the slot and the // campaign) used to be filtered out of the candidate list entirely. That is @@ -22,6 +22,7 @@ const H = vi.hoisted(() => { slotOwner: OWNER as string | null, campaignOwners: [OWNER] as string[], credits: 9999, + inventoryErrorAfter: false, inserted: [] as Record[], }, }; @@ -87,10 +88,26 @@ vi.mock("@/lib/supabase/service", () => ({ }); } if (table === "ad_creatives") { - return chain({ - data: state.campaignOwners.map((o, i) => creativeFor(o, i)), - error: null, + let limit = 1000; // PostgREST's default response cap. + let after = ""; + const query: unknown = new Proxy({}, { + get(_target, prop) { + if (prop === "then") { + const rows = state.campaignOwners.map((o, i) => creativeFor(o, i)) + .sort((a, b) => a.id.localeCompare(b.id)) + .filter((r) => r.id > after).slice(0, limit); + const result = after && state.inventoryErrorAfter + ? { data: null, error: { message: "inventory timeout" } } + : { data: rows, error: null }; + const promise = Promise.resolve(result); + return promise.then.bind(promise); + } + if (prop === "limit") return (n: number) => { limit = n; return query; }; + if (prop === "gt") return (_key: string, id: string) => { after = id; return query; }; + return () => query; + }, }); + return query; } if (table === "profiles") { return chain({ @@ -187,3 +204,39 @@ describe("self-deal still loses to a real advertiser", () => { expect(state.inserted.some((r) => r.tier === "paid")).toBe(true); }); }); + + +describe("inventory beyond the first page", () => { + afterEach(() => { vi.restoreAllMocks(); state.inventoryErrorAfter = false; }); + + it.each([374, 500, 1201])("allows the last of %i campaigns to win and records its impression", async (count) => { + vi.resetModules(); + state.slotOwner = OWNER; + state.campaignOwners = Array(count).fill(OWNER); + state.credits = 9999; + state.inserted = []; + // Select the last candidate in the ordered pool, past both the former + // 100-row cutoff and (in the larger fixture) PostgREST's 1000-row cap. + vi.spyOn(Math, "random").mockReturnValue(0.999999); + const last = state.campaignOwners.map((owner, i) => creativeFor(owner, i)) + .sort((a, b) => a.id.localeCompare(b.id)).at(-1)!; + const fill = await serve(); + expect(fill?.campaignId).toBe(last.campaign_id); + expect(fill?.tier).toBe("free"); + expect(state.inserted).toEqual([expect.objectContaining({ campaign_id: last.campaign_id })]); + }); + + it("does not auction a partial pool when a later inventory page fails", async () => { + vi.resetModules(); + state.slotOwner = OWNER; + state.campaignOwners = Array(501).fill(OWNER); + state.inventoryErrorAfter = true; + state.inserted = []; + const log = vi.spyOn(console, "error").mockImplementation(() => {}); + const fill = await serve(); + expect(fill?.campaignId).toBe("house"); + expect(state.inserted).toEqual([]); + expect(log).toHaveBeenCalledWith("[ads] creative inventory unavailable", "inventory timeout"); + }); + +});