From 489bd08e5d8bbb262f59053a8a05c695564b5419 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Tue, 18 Aug 2026 10:02:16 +0000 Subject: [PATCH] fix(ads): count free-tier delivery in the stats box, not just paid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four tiles on /dashboard/ads read 0 for 1W and every shorter range while the chart directly beneath them drew thousands of impressions. Nothing failed to load. The tiles counted paid inventory only, and since 2026-07-31 there has been no paid inventory: PR #177 demotes a self-owned campaign to the free tier rather than dropping it, and while every slot and every campaign belong to one account, every single fill is a self-deal. 1M and wider still reached back to genuinely paid days, which is exactly why the break looked like a short-range bug. Three fixes, one per layer: * The tiles now report delivery — paid plus free — with the split named underneath, and CTR is computed on the same totals. Spend stays strictly paid, because it is money. The per-campaign rows follow the same rule so a row can't contradict the header above it. * ad_charge_click recorded a self-deal click as `valid=false, tier='paid'`. Reporting counts `valid` as billed clicks and `not valid and tier='free'` as real-but-unbillable ones, so that combination — the one the bot/duplicate/forged path writes — made every self-deal click invisible. The branches either side of it already write 'free' for the same situation. Migration fixes the branch and reclassifies the 661 rows it mislabelled, scoped so no genuine fraud row moves. * bucketAxis stopped one bucket short of the window start. The RPC filters on `ts >= p_since` then date_bins, so it emits a partial leading bucket; getAccountSeries skips any row without a matching point, so up to a full bucket of real delivery was dropped from both the chart and the totals. Verified against prod: 1W now reports 16,220 impressions and 154 clicks where it reported 1 and 0. Co-Authored-By: Claude Opus 5 (1M context) --- app/(app)/dashboard/ads/page.tsx | 64 ++++-- lib/ads/ranges.ts | 12 +- lib/ads/series.ts | 37 ++++ ...818120000_ad_selfdeal_clicks_free_tier.sql | 182 ++++++++++++++++++ tests/ads-stats-box.test.ts | 134 +++++++++++++ 5 files changed, 407 insertions(+), 22 deletions(-) create mode 100644 supabase/migrations/20260818120000_ad_selfdeal_clicks_free_tier.sql create mode 100644 tests/ads-stats-box.test.ts diff --git a/app/(app)/dashboard/ads/page.tsx b/app/(app)/dashboard/ads/page.tsx index 3be82d35..61ccd709 100644 --- a/app/(app)/dashboard/ads/page.tsx +++ b/app/(app)/dashboard/ads/page.tsx @@ -7,9 +7,14 @@ import { AccountTrend } from "@/components/ads/account-trend"; import { RangeTabs } from "@/components/ads/range-tabs"; import { StatSpark } from "@/components/ads/stat-spark"; import { + deliveredClicks, + deliveredImpressions, + deliverySplitNote, getAccountSeries, getCampaignDailySeries, getCampaignRangeTotals, + pickDeliveredClicks, + pickDeliveredImpressions, sumSeries, EMPTY_TOTALS, type AccountPoint, @@ -123,39 +128,50 @@ export default async function AdsPage({ {range.hint} + {/* Delivery first, revenue second. The tiles count every ad actually + shown — paid inventory plus free backfill — and name the split + underneath, so a range in which nothing was billable reports the + traffic it really carried instead of four zeros. Spend stays + strictly paid: it is money, and free backfill costs none. */}
p.impressions} />} + value={deliveredImpressions(totals).toLocaleString()} + note={deliverySplitNote(totals.impressions, totals.freeImpressions)} + spark={} /> p.clicks} />} + value={deliveredClicks(totals).toLocaleString()} + note={deliverySplitNote(totals.clicks, totals.freeClicks)} + spark={} + /> + - 0 + ? "nothing billable" + : undefined + } spark={ p.spentCents} />} />
- {/* Free backfill is delivery that costs and earns nothing, so it never - belongs in the paid figures above — but hiding it entirely would - make impressions look like they collapsed when a campaign runs dry. */} + {/* Free backfill costs and earns nobody anything, so the reason it is + free is worth one line — otherwise a dashboard full of traffic and + an empty Spend tile reads as a billing fault. */} {(totals.freeImpressions > 0 || totals.freeClicks > 0) && (

- Plus{" "} - - {totals.freeImpressions.toLocaleString()} - {" "} - free-tier impressions and{" "} - - {totals.freeClicks.toLocaleString()} - {" "} - free clicks in this range, at no cost. + Free-tier delivery is backfill: a campaign out of credits or daily + budget, or one running on a slot its own account owns. It fills + requests no paying advertiser wanted, bills nobody and earns nobody.

)} @@ -178,8 +194,10 @@ export default async function AdsPage({ {campaigns.map((c) => { // Range-scoped, so a row never contradicts the header above it. const s = rangeById.get(c.id) ?? EMPTY_TOTALS; - const impr = s.impressions; - const clk = s.clicks; + // Same measure as the header tiles, or a campaign delivering only + // free backfill would read as a dead row under a live chart. + const impr = deliveredImpressions(s); + const clk = deliveredClicks(s); const display = campaignDisplayStatus(c, today, creditsAvailable); return (
  • @@ -212,7 +230,10 @@ export default async function AdsPage({ {s.freeImpressions > 0 && ( - + )}
    {label}
    {value}
    + {note &&
    {note}
    } {spark &&
    {spark}
    } ); diff --git a/lib/ads/ranges.ts b/lib/ads/ranges.ts index f575cc2d..29ad08a9 100644 --- a/lib/ads/ranges.ts +++ b/lib/ads/ranges.ts @@ -69,9 +69,17 @@ export function bucketAxis(range: RangeDef, now: Date = new Date()): number[] { const stepMs = range.bucketSeconds * 1000; const endMs = Math.floor(now.getTime() / stepMs) * stepMs; if (range.windowSeconds == null) return [endMs]; - const count = Math.ceil(range.windowSeconds / range.bucketSeconds); + // Start at the bucket the window's first instant falls into, not at + // `endMs - window`. Those differ whenever the window start lands mid-bucket, + // which it does for every range coarser than a minute: the RPC filters rows + // on `ts >= p_since` and then date_bins them, so it emits a partial leading + // bucket. An axis one bucket short dropped it — getAccountSeries skips any + // row with no matching point — and up to a full bucket of real delivery + // vanished from the chart and the headline totals alike. The leading bucket + // is partial by construction, exactly as the trailing one already is. + const startMs = Math.floor((now.getTime() - range.windowSeconds * 1000) / stepMs) * stepMs; const out: number[] = []; - for (let i = count - 1; i >= 0; i--) out.push(endMs - i * stepMs); + for (let t = startMs; t <= endMs; t += stepMs) out.push(t); return out; } diff --git a/lib/ads/series.ts b/lib/ads/series.ts index 4b5f1368..2e36ba58 100644 --- a/lib/ads/series.ts +++ b/lib/ads/series.ts @@ -114,6 +114,43 @@ export function sumSeries(points: AccountPoint[]): RangeTotals { ); } +/** + * Everything actually shown in the range: paid inventory plus free backfill. + * + * The headline tiles report this rather than the paid figure alone. Paid-only + * was fine while some delivery was paid, and read as a dead dashboard the + * moment none of it was — a network whose slots and campaigns belong to the + * same account books every fill as free tier (serveAd demotes a self-deal), + * so every tile showed 0 while the chart underneath showed thousands of + * impressions. A free-tier impression is still an impression; what it isn't is + * revenue, and Spend is the tile that says so. + */ +export function deliveredImpressions(t: RangeTotals): number { + return t.impressions + t.freeImpressions; +} + +/** Clicks actually taken in the range: billed plus unbillable-but-real. */ +export function deliveredClicks(t: RangeTotals): number { + return t.clicks + t.freeClicks; +} + +/** + * Sub-line for a delivery tile: how its headline total divides into paid and + * free. Silent when there is nothing to divide — a tile reading 0 needs no + * footnote saying it was 0 paid and 0 free, and an all-paid tile is already + * fully described by its own number. + */ +export function deliverySplitNote(paid: number, free: number): string | undefined { + if (free === 0) return undefined; + if (paid === 0) return "all free backfill"; + return `${paid.toLocaleString()} paid · ${free.toLocaleString()} free`; +} + +/** Sparkline accessors, so a tile's shape plots the number above it. */ +export const pickDeliveredImpressions = (p: AccountPoint): number => + p.impressions + p.freeImpressions; +export const pickDeliveredClicks = (p: AccountPoint): number => p.clicks + p.freeClicks; + type CampaignTotalsRow = { campaign_id: string; impressions: number | string; diff --git a/supabase/migrations/20260818120000_ad_selfdeal_clicks_free_tier.sql b/supabase/migrations/20260818120000_ad_selfdeal_clicks_free_tier.sql new file mode 100644 index 00000000..fdb698c1 --- /dev/null +++ b/supabase/migrations/20260818120000_ad_selfdeal_clicks_free_tier.sql @@ -0,0 +1,182 @@ +-- Ad network: a self-deal click is free-tier delivery, not a paid click that +-- happened to fail. +-- +-- ad_charge_click refuses to bill when the same profile owns the slot and the +-- campaign — correct, there is no money to move. But that branch recorded the +-- row as `valid=false, tier='paid'`, and reporting reads exactly two kinds of +-- click: +-- +-- clicks = valid -- billed +-- free_clicks = not valid and tier='free' -- real, unbillable +-- +-- `not valid and tier='paid'` is neither, and it is what the bot/duplicate/ +-- forged path writes. So every self-deal click landed in the bucket reserved +-- for fraud and disappeared from the dashboard entirely. The two branches +-- either side of it already write 'free' for the same situation — a real click +-- nobody can be charged for — so this was an inconsistency, not a policy. +-- +-- serveAd makes the matching call on the impression side: a self-owned +-- campaign is demoted to the free tier rather than dropped (see the comment in +-- lib/ads/serve.ts). This aligns the click side with it. +-- +-- Backfill included, because the misclassification is recent and total: while +-- every slot and every campaign belong to one account, 100% of clicks take +-- this branch, and 661 of them are currently invisible. The update is scoped +-- narrowly enough not to touch a genuine fraud row: +-- +-- * valid = false and tier = 'paid' — the only rows in the wrong bucket; +-- * charged_cents = 0 — never move a row that billed; +-- * slot owner = campaign owner — the self-deal condition itself; +-- * device is distinct from 'bot' — bots are rejected before this branch, +-- so a bot row can only have come from +-- the fraud path in resolveClick. +-- +-- Duplicate-click rows cannot be caught by mistake: that check requires an +-- existing valid=true click on the campaign inside 6h, and the branch being +-- fixed here is precisely why no such click exists. + +create or replace function public.ad_charge_click( + p_campaign uuid, + p_slot uuid, + p_creative uuid, + p_impression uuid, + p_visitor text, + p_ip_hash text, + p_country text, + p_device text, + p_cpc_credits integer, + p_platform_rate numeric +) +returns table(click_id uuid, charged_cents integer, publisher_earn_cents integer, valid boolean) +language plpgsql +security definer +set search_path to 'public' +as $function$ +declare + v_owner uuid; + v_status text; + v_daily int; + v_spend int; + v_date date; + v_paid int; + v_bonus int; + v_promo int; + v_from_bonus int; + v_from_promo int; + v_from_cash int; + v_rest int; + v_slot_owner uuid; + v_charged int; + v_earn int; + v_cut int; + v_click uuid; + v_rack_cents constant int := 5; + v_floor_cents constant numeric := 2.0; +begin + select owner_id, status, daily_budget_cents, spend_today_cents, spend_date + into v_owner, v_status, v_daily, v_spend, v_date + from public.ad_campaigns where id = p_campaign for update; + if not found then return; end if; + + v_charged := p_cpc_credits * v_rack_cents; + if v_date is distinct from current_date then v_spend := 0; end if; + + -- Paused / archived campaign: this click should not have been servable at + -- all, so it stays out of the free-tier figures as well as the paid ones. + if v_status not in ('active', 'exhausted') then + insert into public.ad_clicks(impression_id,slot_id,campaign_id,creative_id,visitor_id,ip_hash,geo_country,device,charged_cents,publisher_earn_cents,platform_cut_cents,valid,tier) + values (p_impression,p_slot,p_campaign,p_creative,p_visitor,p_ip_hash,p_country,p_device,0,0,0,false,'paid') + returning id into v_click; + return query select v_click, 0, 0, false; + return; + end if; + + select owner_id into v_slot_owner from public.ad_slots where id = p_slot; + + -- Self-deal: one account on both sides, so nothing is billed and nothing is + -- earned. Real delivery all the same — free tier, same as the two branches + -- below. + if v_slot_owner is not null and v_slot_owner = v_owner then + insert into public.ad_clicks(impression_id,slot_id,campaign_id,creative_id,visitor_id,ip_hash,geo_country,device,charged_cents,publisher_earn_cents,platform_cut_cents,valid,tier) + values (p_impression,p_slot,p_campaign,p_creative,p_visitor,p_ip_hash,p_country,p_device,0,0,0,false,'free') + returning id into v_click; + return query select v_click, 0, 0, false; + return; + end if; + + if (v_spend + v_charged) > v_daily then + insert into public.ad_clicks(impression_id,slot_id,campaign_id,creative_id,visitor_id,ip_hash,geo_country,device,charged_cents,publisher_earn_cents,platform_cut_cents,valid,tier) + values (p_impression,p_slot,p_campaign,p_creative,p_visitor,p_ip_hash,p_country,p_device,0,0,0,false,'free') + returning id into v_click; + return query select v_click, 0, 0, false; + return; + end if; + + select credits_balance, + coalesce(ad_bonus_credits, 0), + least(coalesce(promo_credits, 0), credits_balance) + into v_paid, v_bonus, v_promo + from public.profiles where id = v_owner for update; + + if coalesce(v_paid, 0) + coalesce(v_bonus, 0) < p_cpc_credits then + insert into public.ad_clicks(impression_id,slot_id,campaign_id,creative_id,visitor_id,ip_hash,geo_country,device,charged_cents,publisher_earn_cents,platform_cut_cents,valid,tier) + values (p_impression,p_slot,p_campaign,p_creative,p_visitor,p_ip_hash,p_country,p_device,0,0,0,false,'free') + returning id into v_click; + return query select v_click, 0, 0, false; + return; + end if; + + v_from_bonus := least(v_bonus, p_cpc_credits); + v_rest := p_cpc_credits - v_from_bonus; + v_from_promo := least(v_promo, v_rest); + v_from_cash := v_rest - v_from_promo; + + update public.profiles + set ad_bonus_credits = ad_bonus_credits - v_from_bonus, + credits_balance = credits_balance - (v_from_promo + v_from_cash), + promo_credits = greatest(0, coalesce(promo_credits, 0) - v_from_promo) + where id = v_owner; + + v_earn := floor(v_from_cash * (1 - p_platform_rate) * v_floor_cents); + v_cut := v_charged - v_earn; + + update public.ad_campaigns + set spend_today_cents = v_spend + v_charged, + spend_date = current_date, + total_spent_cents = coalesce(total_spent_cents,0) + v_charged + where id = p_campaign; + + insert into public.ad_clicks(impression_id,slot_id,campaign_id,creative_id,visitor_id,ip_hash,geo_country,device,charged_cents,publisher_earn_cents,platform_cut_cents,valid,tier) + values (p_impression,p_slot,p_campaign,p_creative,p_visitor,p_ip_hash,p_country,p_device,v_charged,v_earn,v_cut,true,'paid') + returning id into v_click; + + if v_slot_owner is not null and v_earn > 0 then + insert into public.ad_ledger(kind, owner_id, campaign_id, slot_id, amount_cents, ref_click_id) + values ('publisher_accrual', v_slot_owner, p_campaign, p_slot, v_earn, v_click); + end if; + if v_cut > 0 then + insert into public.ad_ledger(kind, owner_id, campaign_id, slot_id, amount_cents, ref_click_id) + values ('platform_fee', null, p_campaign, p_slot, v_cut, v_click); + end if; + + return query select v_click, v_charged, v_earn, true; +end $function$; + +-- create or replace keeps the existing ACL, but state it anyway so a fresh +-- database ends up where 20260731160000_ad_rpc_revoke_public.sql left this one: +-- no PUBLIC execute on a security-definer money function. +revoke execute on function public.ad_charge_click(uuid, uuid, uuid, uuid, text, text, text, text, integer, numeric) from public; +grant execute on function public.ad_charge_click(uuid, uuid, uuid, uuid, text, text, text, text, integer, numeric) to service_role; + +-- Reclassify the rows the old branch mislabelled. See the header for why each +-- clause is here; together they select self-deal clicks and nothing else. +update public.ad_clicks c + set tier = 'free' + from public.ad_campaigns camp, public.ad_slots s + where c.campaign_id = camp.id + and c.slot_id = s.id + and s.owner_id = camp.owner_id + and c.valid = false + and c.tier = 'paid' + and c.charged_cents = 0 + and c.device is distinct from 'bot'; diff --git a/tests/ads-stats-box.test.ts b/tests/ads-stats-box.test.ts new file mode 100644 index 00000000..b658b6c5 --- /dev/null +++ b/tests/ads-stats-box.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it } from "vitest"; +import { + deliveredClicks, + deliveredImpressions, + deliverySplitNote, + getAccountSeries, + pickDeliveredClicks, + pickDeliveredImpressions, + sumSeries, + EMPTY_TOTALS, +} from "@/lib/ads/series"; +import { RANGES, bucketAxis, bucketOf, rangeSince, type RangeId } from "@/lib/ads/ranges"; + +// The stats box above the delivery chart went blank for 1W and every shorter +// range while the chart under it drew thousands of impressions. Nothing was +// failing to load: the tiles counted paid delivery only, and once every slot +// and every campaign belonged to one account, serveAd demoted every fill to the +// free tier as a self-deal. 1M and wider still reached back to genuinely paid +// days, which is exactly why the break looked like a short-range bug. + +const NOW = new Date("2026-08-18T09:50:00.000Z"); +const byId = (id: RangeId) => RANGES.find((r) => r.id === id)!; + +/** Minimal stand-in for the Supabase client: getAccountSeries only calls .rpc. */ +const clientReturning = (rows: unknown[]): any => ({ + rpc: async () => ({ data: rows, error: null }), +}); +const failingClient = (): any => ({ + rpc: async () => ({ data: null, error: { message: "boom" } }), +}); + +const row = (bucket: string, over: Record = {}) => ({ + bucket, + impressions: 0, + free_impressions: 0, + clicks: 0, + free_clicks: 0, + spent_cents: 0, + ...over, +}); + +describe("delivered totals", () => { + it("counts free backfill as delivery, because it was delivered", () => { + const totals = { ...EMPTY_TOTALS, impressions: 0, freeImpressions: 16207 }; + expect(deliveredImpressions(totals)).toBe(16207); + }); + + it("counts an unbillable click as a click", () => { + const totals = { ...EMPTY_TOTALS, clicks: 0, freeClicks: 649 }; + expect(deliveredClicks(totals)).toBe(649); + }); + + it("adds the two tiers rather than preferring one", () => { + const totals = { ...EMPTY_TOTALS, impressions: 12, freeImpressions: 30, clicks: 2, freeClicks: 5 }; + expect(deliveredImpressions(totals)).toBe(42); + expect(deliveredClicks(totals)).toBe(7); + }); + + it("gives the sparklines the same measure as the number above them", () => { + const p = { t: 0, impressions: 3, freeImpressions: 4, clicks: 1, freeClicks: 2, spentCents: 9 }; + expect(pickDeliveredImpressions(p)).toBe(7); + expect(pickDeliveredClicks(p)).toBe(3); + }); +}); + +describe("delivery split note", () => { + it("says nothing when there is no free tier to explain", () => { + expect(deliverySplitNote(500, 0)).toBeUndefined(); + expect(deliverySplitNote(0, 0)).toBeUndefined(); + }); + + it("names the all-free case outright, so a $0 spend reads as intended", () => { + expect(deliverySplitNote(0, 16207)).toBe("all free backfill"); + }); + + it("gives both halves when delivery is mixed", () => { + expect(deliverySplitNote(1200, 300)).toBe("1,200 paid · 300 free"); + }); +}); + +describe("getAccountSeries", () => { + it("keeps free-tier delivery that the paid figure alone would hide", async () => { + const range = byId("1w"); + const axis = bucketAxis(range, NOW); + const rows = [row(new Date(axis.at(-1)!).toISOString(), { free_impressions: 240, free_clicks: 3 })]; + + const points = await getAccountSeries(clientReturning(rows), range, NOW); + const totals = sumSeries(points); + + expect(totals.impressions).toBe(0); // nothing was billable... + expect(deliveredImpressions(totals)).toBe(240); // ...but 240 ads were shown + expect(deliveredClicks(totals)).toBe(3); + }); + + it("keeps the partial bucket at the start of the window", async () => { + // The window opens mid-bucket for every range coarser than a minute, so the + // RPC emits a leading bucket that starts before `since`. The axis used to + // stop one bucket short and getAccountSeries dropped the row on the floor — + // silently, since an unmatched bucket is skipped rather than appended. + for (const range of RANGES) { + if (range.windowSeconds == null) continue; + const since = rangeSince(range, NOW)!; + const rows = [row(new Date(bucketOf(since, range)).toISOString(), { impressions: 7 })]; + + const points = await getAccountSeries(clientReturning(rows), range, NOW); + expect(sumSeries(points).impressions, `${range.id} dropped its first bucket`).toBe(7); + } + }); + + it("still covers the whole window without duplicating a bucket", () => { + for (const range of RANGES) { + if (range.windowSeconds == null) continue; + const axis = bucketAxis(range, NOW); + expect(new Set(axis).size).toBe(axis.length); + expect(axis[0]).toBeLessThanOrEqual(NOW.getTime() - range.windowSeconds * 1000); + // One partial bucket of slack at each end, no more. + expect(axis[0]).toBeGreaterThan( + NOW.getTime() - (range.windowSeconds + range.bucketSeconds) * 1000, + ); + } + }); + + it("zero-fills the whole axis so a quiet range still draws a line", async () => { + const range = byId("1d"); + const points = await getAccountSeries(clientReturning([]), range, NOW); + expect(points).toHaveLength(bucketAxis(range, NOW).length); + expect(sumSeries(points)).toEqual(EMPTY_TOTALS); + }); + + it("renders an empty range rather than throwing when the RPC fails", async () => { + const points = await getAccountSeries(failingClient(), byId("1h"), NOW); + expect(sumSeries(points)).toEqual(EMPTY_TOTALS); + }); +});