From d10babb900c7497da80766b52621e28d726d99ff Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Thu, 24 Sep 2026 17:40:24 +0000 Subject: [PATCH] Fix the banner size, the byte budget and the scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects in the animated banners, all visible in the first real renders. Size. Every unit came out two pixels short in each dimension — 298x248 for a 300x250 — because the capturer screenshots `.stage` and `.stage` was on the inner content div, inside the unit's 1px border. An off-size creative is rejected outright by ad networks, so this was the difference between an asset and a file. `.stage` is now the unit itself. Bytes. 327KB for the rectangle against a 150KB ceiling. The cause was not the frame count, it was that everything moved on every frame: a full-width sweep running the whole loop, plus a drifting background wash, changed every pixel of every frame, so GIF's per-frame rectangle diff had nothing to elide. Motion is now confined to three short windows — entrance, one sweep, the CTA beat — with the unit at rest between them, which reads as a deliberate beat rather than a limitation. The wash is static; animating it cost most of the file for motion nobody could see. 10fps and 40 frames rather than 12.5 and 50, because 10fps is exactly 10 centiseconds and GIF stores delays in hundredths. A test now asserts that most of the loop is perfectly still. That is a size requirement wearing a timeline's clothes: lose it and the banners quietly stop fitting. Scope. Product ads only was being enforced by the backfill alone. The dashboard save and the shared creator behind the public API queued a render for any campaign, so campaigns pointing at blog posts and social profiles were getting video and animated banners that were explicitly out of scope — the first banner rendered in production advertised a dev.to article. The rule now lives in queueCampaignVideo, where all three paths pass through, for the same reason the API gap taught: a rule enforced at one of three call sites holds only until someone adds a fourth. A caller that passes no destination still renders, so a forgotten argument produces an extra video rather than a campaign that mysteriously never gets one. Co-Authored-By: Claude Opus 5 (1M context) --- app/actions/ads.ts | 1 + lib/ads/campaigns.ts | 1 + lib/ads/gif/compose.ts | 67 +++++++++++++++++++++------------- lib/ads/video/jobs.ts | 20 ++++++++++ lib/ads/video/profiles.ts | 15 +++++--- scripts/backfill-ad-videos.ts | 1 + tests/ads-gif-pipeline.test.ts | 26 ++++++++++--- tests/ads-video-jobs.test.ts | 66 +++++++++++++++++++++++++++++++++ 8 files changed, 160 insertions(+), 37 deletions(-) diff --git a/app/actions/ads.ts b/app/actions/ads.ts index 43c91c7..b3bd5b3 100644 --- a/app/actions/ads.ts +++ b/app/actions/ads.ts @@ -493,6 +493,7 @@ async function requeueCampaignVideo( domain: (campaign.destination_domain as string | null) ?? domainOf(campaign.destination_url as string), + destinationUrl: campaign.destination_url as string, creatives: rows.map((r) => ({ format: r.format, headline: r.headline ?? "", diff --git a/lib/ads/campaigns.ts b/lib/ads/campaigns.ts index 221ba67..2d5bb37 100644 --- a/lib/ads/campaigns.ts +++ b/lib/ads/campaigns.ts @@ -277,6 +277,7 @@ export async function createCampaignForUrl(input: { campaignId: campaign.id, ownerId: userId, domain, + destinationUrl: request.url, creatives: generated.creatives.map((c) => ({ format: c.format, headline: c.headline ?? "", diff --git a/lib/ads/gif/compose.ts b/lib/ads/gif/compose.ts index a635d12..dd7b910 100644 --- a/lib/ads/gif/compose.ts +++ b/lib/ads/gif/compose.ts @@ -13,9 +13,9 @@ // leaderboard and mobile banner run as a row. // // The motion is deliberately the pre-roll's, not a new vocabulary: a short -// entrance rise, a slow accent drift through the hold, and a CTA that gains -// emphasis on the final beat. An advertiser who has seen their video should -// recognise the banner as the same campaign. +// entrance rise, one accent sweep, and a CTA that gains emphasis on the final +// beat. An advertiser who has seen their video should recognise the banner as +// the same campaign. import { GIF_FPS, GIF_FRAMES } from "../video/profiles"; import { escapeHtml, safeDataUri } from "../video/compose"; @@ -42,10 +42,21 @@ export function gifUnit(id: string): GifUnit { return u; } -/** Beat boundaries, in milliseconds of a 4s loop. */ +/** + * Beat boundaries, in milliseconds of a 4s loop. + * + * The still stretches between them are deliberate and are the reason the file + * fits. GIF stores a frame as a rectangle of changed pixels, so a frame + * identical to the one before it costs almost nothing — while a full-width + * sweep running the whole loop makes every single frame a full frame. Motion is + * therefore confined to short windows with the unit at rest between them, which + * reads as a deliberate beat rather than as a limitation. + */ export const GIF_TIMELINE = { - entranceEndMs: 700, - holdEndMs: 2800, + entranceEndMs: 600, + sweepStartMs: 2200, + sweepEndMs: 3200, + ctaStartMs: 3200, endMs: 4000, } as const; @@ -53,7 +64,6 @@ export type GifFrameState = { frame: number; timeMs: number; entrance: number; - drift: number; cta: number; /** The accent sweep's position, -1 to 2 across the unit. */ sweep: number; @@ -80,17 +90,18 @@ export function gifFrameState(frame: number, reducedMotion: boolean): GifFrameSt if (reducedMotion) { // A still banner, held at its resting state. It still reads as the finished // ad — headline up, CTA emphasised — it simply never moves toward it. - return { frame, timeMs, entrance: 1, drift: 0, cta: 1, sweep: 2 }; + return { frame, timeMs, entrance: 1, cta: 1, sweep: 2 }; } return { frame, timeMs, entrance: easeOut(phase(timeMs, 0, GIF_TIMELINE.entranceEndMs)), - drift: phase(timeMs, GIF_TIMELINE.entranceEndMs, GIF_TIMELINE.holdEndMs), - cta: easeOut(phase(timeMs, GIF_TIMELINE.holdEndMs, GIF_TIMELINE.endMs)), - // One pass across the unit during the hold. Starts off-screen and ends - // off-screen, so the loop point never shows a sweep frozen mid-unit. - sweep: -1 + 3 * phase(timeMs, GIF_TIMELINE.entranceEndMs, GIF_TIMELINE.holdEndMs), + cta: easeOut(phase(timeMs, GIF_TIMELINE.ctaStartMs, GIF_TIMELINE.endMs)), + // One pass across the unit, confined to its own window. Off-screen at both + // ends so the loop point never shows a sweep frozen mid-unit — and, just as + // importantly, every frame outside that window is identical to its + // neighbour and costs the encoder almost nothing. + sweep: -1 + 3 * phase(timeMs, GIF_TIMELINE.sweepStartMs, GIF_TIMELINE.sweepEndMs), }; } @@ -160,8 +171,8 @@ export function gifDocument(input: GifComposeInput): string { const cta = `
${escapeHtml(ctaText)}
`; const inner = unit.row - ? `
${mark}${copy}${cta}
` - : `
+ ? `
${mark}${copy}${cta}
` + : `
${mark}
${copy}
${cta}
@@ -174,10 +185,15 @@ export function gifDocument(input: GifComposeInput): string { /* Text rendering is pinned so a frame captured now matches one captured on a differently-configured container. */ -webkit-font-smoothing:antialiased;text-rendering:geometricPrecision} + /* .stage is what the capturer screenshots, so it must BE the unit. It used + to sit on the inner content div, inside this element's 1px border, and + every banner came out two pixels short in each dimension — an off-size + creative, which ad networks reject outright. */ #unit{position:relative;width:${unit.width}px;height:${unit.height}px;overflow:hidden; border:1px solid ${accentColor}33} /* The accent wash keeps the middle of a flat unit from reading as a dead - block, and it is what the drift moves. */ + block. It is deliberately static: animating it changed every pixel of + every frame, which is a full frame of palette each time. */ #wash{position:absolute;inset:0;z-index:0; background:radial-gradient(120% 140% at 12% 0%, ${accentColor}22, transparent 60%)} /* A single specular pass. Cheap in GIF terms because it is the same few @@ -185,11 +201,11 @@ export function gifDocument(input: GifComposeInput): string { #sweep{position:absolute;top:0;bottom:0;width:38%;z-index:1;pointer-events:none; background:linear-gradient(100deg, transparent, ${accentColor}1f 45%, transparent); transform:translateX(-120%)} - .stage{position:relative;z-index:2} + .content{position:relative;z-index:2} #domain{position:absolute;right:6px;bottom:4px;z-index:3;font-size:9px;letter-spacing:.06em; color:${fgColor};opacity:.45} -
+
${inner} @@ -203,13 +219,12 @@ export function gifDocument(input: GifComposeInput): string { function phase(ms,a,b){if(b<=a)return ms>=b?1:0;return Math.min(1,Math.max(0,(ms-a)/(b-a)));} function state(frame){ var timeMs=(frame/GIF_FPS)*1000; - if(REDUCED)return{timeMs:timeMs,entrance:1,drift:0,cta:1,sweep:2}; + if(REDUCED)return{timeMs:timeMs,entrance:1,cta:1,sweep:2}; return{ timeMs:timeMs, entrance:easeOut(phase(timeMs,0,T.entranceEndMs)), - drift:phase(timeMs,T.entranceEndMs,T.holdEndMs), - cta:easeOut(phase(timeMs,T.holdEndMs,T.endMs)), - sweep:-1+3*phase(timeMs,T.entranceEndMs,T.holdEndMs) + cta:easeOut(phase(timeMs,T.ctaStartMs,T.endMs)), + sweep:-1+3*phase(timeMs,T.sweepStartMs,T.sweepEndMs) }; } // Drive every animated value from the frame index. No CSS transitions or @@ -221,7 +236,6 @@ export function gifDocument(input: GifComposeInput): string { var mark = document.getElementById('mark'); var cta = document.getElementById('cta'); var sweep = document.getElementById('sweep'); - var wash = document.getElementById('wash'); // Entrance: copy rises a few pixels into place and fades up. Small, because // a banner is read in a glance and a long entrance wastes most of the loop. @@ -230,9 +244,10 @@ export function gifDocument(input: GifComposeInput): string { copy.style.opacity = (0.15 + 0.85 * s.entrance).toFixed(3); mark.style.opacity = (0.3 + 0.7 * s.entrance).toFixed(3); - // Hold: the wash drifts slowly so the unit is never completely static, - // which is the whole reason an animated banner outperforms a flat one. - wash.style.transform = 'translateX(' + (s.drift * 4).toFixed(2) + 'px)'; + // The wash is fixed. It used to drift across the hold, which changed every + // pixel of every frame and accounted for most of the file size, in service + // of motion nobody could see. The sweep is the thing that moves, and only + // inside its window. sweep.style.transform = 'translateX(' + (s.sweep * 120).toFixed(2) + '%)'; // Final beat: the CTA lifts slightly and reaches full strength. It ends the diff --git a/lib/ads/video/jobs.ts b/lib/ads/video/jobs.ts index 51fd77e..fbc0d38 100644 --- a/lib/ads/video/jobs.ts +++ b/lib/ads/video/jobs.ts @@ -7,6 +7,7 @@ // never a precondition for one. import type { SupabaseClient } from "@supabase/supabase-js"; +import { classifyCampaign } from "./classify"; import type { AdCreative } from "../formats"; import { VIDEO_FORMAT_ID } from "../formats"; import { MAX_HEADLINE_WORDS, renderHash, validateSnapshot, type VideoDesignSnapshot } from "./snapshot"; @@ -339,11 +340,30 @@ export async function queueCampaignVideo( campaignId: string; ownerId: string; domain: string; + /** + * The campaign's destination, used to decide whether it gets media at all. + * Optional only so a caller without it degrades to rendering rather than to + * silently skipping. + */ + destinationUrl?: string | null; creatives: Parameters[0]["creatives"]; bumpRevision: boolean; }, ): Promise { try { + // Product ads only. The backfill classified campaigns before queueing them, + // but every other path — the dashboard save, and the shared creator behind + // the public API — queued a render for anything, so campaigns pointing at + // blog posts and social profiles were getting video and animated banners + // that were explicitly out of scope. + // + // Deciding it here rather than in each caller is the same lesson the API + // gap taught: a rule enforced at one of three call sites is a rule that + // holds until someone adds a fourth. + if (args.destinationUrl && classifyCampaign(args.destinationUrl) !== "product") { + return null; + } + const snapshot = snapshotFromCreatives({ creatives: args.creatives, domain: args.domain }); if (!snapshot) return null; diff --git a/lib/ads/video/profiles.ts b/lib/ads/video/profiles.ts index 35c07b3..dcd03d0 100644 --- a/lib/ads/video/profiles.ts +++ b/lib/ads/video/profiles.ts @@ -85,13 +85,16 @@ export function isGifProfile(id: string): id is GifProfileId { * Animated banners run slower and shorter than the pre-roll. * * GIF stores an inter-frame delay in hundredths of a second, so only a handful - * of frame rates are exactly representable: 12.5fps is 8cs and lands on a whole - * number, where 12 or 15 would drift and make the loop stutter. Four seconds - * keeps the file inside what ad networks accept — every frame is a full frame - * of palette, so duration is the main lever on size. + * of frame rates are exactly representable: 10fps is exactly 10cs, where 12 or + * 15 would drift and make the loop stutter. + * + * 40 frames rather than 50, because size is the binding constraint and every + * frame costs. What actually keeps the file small is not the frame count + * though — it is holding most of those frames still, so the encoder's + * per-frame diff is a small rectangle instead of the whole unit. */ -export const GIF_FPS = 12.5; -export const GIF_FRAMES = 50; +export const GIF_FPS = 10; +export const GIF_FRAMES = 40; export const GIF_MS = (GIF_FRAMES / GIF_FPS) * 1000; // 4000 export type VideoProfile = { diff --git a/scripts/backfill-ad-videos.ts b/scripts/backfill-ad-videos.ts index 19c41e9..ae74164 100644 --- a/scripts/backfill-ad-videos.ts +++ b/scripts/backfill-ad-videos.ts @@ -136,6 +136,7 @@ for (const [i, c] of targets.entries()) { campaignId: c.id, ownerId: c.owner_id, domain: c.destination_domain ?? new URL(c.destination_url).hostname.replace(/^www\./, ""), + destinationUrl: c.destination_url, creatives: usable.map((r) => ({ format: r.format, headline: r.headline ?? "", diff --git a/tests/ads-gif-pipeline.test.ts b/tests/ads-gif-pipeline.test.ts index 3224fc1..91a0dc9 100644 --- a/tests/ads-gif-pipeline.test.ts +++ b/tests/ads-gif-pipeline.test.ts @@ -40,14 +40,30 @@ describe("the banner timeline is a pure function of the frame index", () => { // the impressions of anyone who scrolls past during the entrance. expect(first.entrance).toBeGreaterThanOrEqual(0); expect(last.entrance).toBe(1); - // Nearly 1, deliberately not exactly 1. The 50 frames cover [0, 4000) at - // 80ms each, so the final frame sits at 3920ms and the 4000ms mark IS + // Nearly 1, deliberately not exactly 1. The 40 frames cover [0, 4000) at + // 100ms each, so the final frame sits at 3900ms and the 4000ms mark IS // frame 0 of the next loop. A timeline that put a frame exactly on the end // would render the loop point twice and the banner would hitch once per // cycle. expect(last.cta).toBeGreaterThan(0.99); expect(last.cta).toBeLessThan(1); - expect(last.timeMs).toBe(3920); + expect(last.timeMs).toBe(3900); + }); + + it("holds most of the loop perfectly still", () => { + // This is what keeps the file inside the budget. GIF stores a frame as the + // rectangle of pixels that changed, so a frame identical to its neighbour + // is nearly free — and a sweep running the whole loop would make every + // frame a full frame. Asserted because it is a size requirement wearing a + // timeline's clothes: lose it and the banners silently stop fitting. + const t = gifTimeline(false); + let moving = 0; + for (let i = 1; i < t.length; i++) { + const a = t[i - 1]; + const b = t[i]; + if (a.entrance !== b.entrance || a.cta !== b.cta || a.sweep !== b.sweep) moving++; + } + expect(moving).toBeLessThan(t.length * 0.62); }); it("ends the loop with the sweep off the unit", () => { @@ -68,7 +84,7 @@ describe("the banner timeline is a pure function of the frame index", () => { it("holds every beat at rest under reduced motion", () => { for (const f of [0, 10, GIF_FRAMES - 1]) { const s = gifFrameState(f, true); - expect(s).toMatchObject({ entrance: 1, drift: 0, cta: 1 }); + expect(s).toMatchObject({ entrance: 1, cta: 1, sweep: 2 }); } }); }); @@ -163,7 +179,7 @@ describe("validation refuses what cannot be trafficked", () => { }); it("rejects the wrong size, a short loop and an oversized file", () => { expect(validateGif({ ...base, width: 728 })[0]).toMatch(/expected 300x250/); - expect(validateGif({ ...base, frames: 12 })[0]).toMatch(/expected 50 frames/); + expect(validateGif({ ...base, frames: 12 })[0]).toMatch(/expected 40 frames/); expect(validateGif({ ...base, byteSize: 200 * 1024 })[0]).toMatch(/exceeds/); }); }); diff --git a/tests/ads-video-jobs.test.ts b/tests/ads-video-jobs.test.ts index ca28865..40131d0 100644 --- a/tests/ads-video-jobs.test.ts +++ b/tests/ads-video-jobs.test.ts @@ -6,6 +6,7 @@ import { ensureVideoCreative, MAX_RENDER_ATTEMPTS, renderStateLabel, + queueCampaignVideo, snapshotFromCreatives, streamingReady, trimHeadlineForVideo, @@ -332,3 +333,68 @@ describe("status presentation", () => { } }); }); + +describe("only product campaigns get media", () => { + const creatives = [design("banner_300x250")]; + + it("skips a campaign that points at a blog post", async () => { + // The backfill classified before queueing, but the dashboard save and the + // public API queued anything — so blog campaigns were getting video and + // animated banners that were explicitly out of scope. The rule belongs + // where all three paths pass through. + const db = fakeDb({ existingJob: null, existingCreative: null }); + const res = await queueCampaignVideo(db.client as never, { + campaignId: "c", + ownerId: "o", + domain: "dev.to", + destinationUrl: "https://dev.to/chovy/some-post-abc", + creatives, + bumpRevision: false, + }); + expect(res).toBeNull(); + // Nothing written at all: no creative row, no job. + expect(db.inserts).toHaveLength(0); + }); + + it("skips a social profile", async () => { + const db = fakeDb({ existingJob: null, existingCreative: null }); + const res = await queueCampaignVideo(db.client as never, { + campaignId: "c", + ownerId: "o", + domain: "x.com", + destinationUrl: "https://x.com/someone", + creatives, + bumpRevision: false, + }); + expect(res).toBeNull(); + expect(db.inserts).toHaveLength(0); + }); + + it("renders a product campaign", async () => { + const db = fakeDb({ existingJob: null, existingCreative: null }); + const res = await queueCampaignVideo(db.client as never, { + campaignId: "c", + ownerId: "o", + domain: "moshcoding.com", + destinationUrl: "https://moshcoding.com/", + creatives, + bumpRevision: false, + }); + expect(res).not.toBeNull(); + }); + + it("renders when no destination is known, rather than silently skipping", async () => { + // Degrading to "render it" is the safer default: a caller that forgets to + // pass the URL produces an extra video, not a campaign that mysteriously + // never gets one. + const db = fakeDb({ existingJob: null, existingCreative: null }); + const res = await queueCampaignVideo(db.client as never, { + campaignId: "c", + ownerId: "o", + domain: "example.com", + creatives, + bumpRevision: false, + }); + expect(res).not.toBeNull(); + }); +});