Queue a pre-roll render when a campaign is saved or edited (package C) - #275
Merged
Merged
Conversation
Something finally calls the render pipeline. Saving a campaign now creates its video creative and queues a render; editing copy or regenerating bumps the revision and queues another; the campaign page shows progress and, once ready, a Download MP4 button. The rule this is built around is that a render is an extra output of saving a campaign, never a precondition for one. queueCampaignVideo returns null on every failure rather than throwing, the row is written before the enqueue is attempted so a missing REDIS_URL loses throughput rather than the request, and no path waits on an encode. An advertiser who saves a campaign gets the same response they always did. The snapshot is derived from a creative the advertiser already approved rather than generated afresh, so the pre-roll makes the same claim in the same palette as the display ads. The medium rectangle is preferred as the source and banner_320x50 avoided, for the same reason the feed backfill avoids it: that format carries the shortened mobile headline, and a 1920x1080 frame has no width problem that would justify truncated copy. Display headlines are capped by characters, which is a different constraint from what five seconds can hold, so trimHeadlineForVideo clips to eight words on a word boundary. Artwork is deliberately not carried yet. Recording a hero URL with a null content hash would be worse than recording neither: the dedupe key trusts the hash, and a URL sitting beside a null hash invites a later change to treat the URL as sufficient when the same URL can serve different bytes. The compositor renders its accent-tinted fallback until hashing lands. Dedupe is on the render hash, not the campaign, so a design previewed, saved, then regenerated without edits is one encode. The unique index on (render_hash, output_profile) is what makes that hold under concurrency; the select-then- insert is the fast path and the conflict re-select is the correct one. ensureVideoCreative bumps requested_revision on edits and leaves published_revision alone. That bump is what gives the worker's compare-and-swap something to compare: two quick edits produce revisions N and N+1, and a slow render of N publishes nothing when it lands second. Writing published_revision here would mark a creative servable before any bytes existed. Also fixes a leak this package would otherwise have introduced. The campaign detail page maps ad_creatives rows straight into <AdPreview>, which renders markup — so the video creative row this package creates would have been drawn as a banner there. The dashboard forms already iterate DESIGN_FORMATS for that reason, but the detail page reads from the database and was not covered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan48 finding(s) HIGH/CRITICAL: 2 | MEDIUM: 31 | LOW: 15
Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Package C of the Crawlproof five-second streaming ads spec. Something finally calls the render pipeline.
Saving a campaign creates its video creative and queues a render; editing copy or regenerating bumps the revision and queues another; the campaign page shows progress and, once ready, a Download MP4 button.
Spec exit condition for C: Generate Ads automatically creates video; download works before activation.
The rule it's built around
A render is an extra output of saving a campaign, never a precondition for one:
queueCampaignVideoreturnsnullon every failure rather than throwingREDIS_URLloses throughput rather than the requestAn advertiser saving a campaign gets exactly the response they always did, even if rendering is completely down.
Design decisions worth a reviewer's eye
The snapshot comes from copy the advertiser already approved, not a fresh generation pass. A campaign whose banner and pre-roll make different claims is a compliance problem, not a design inconsistency. The medium rectangle is the preferred source and
banner_320x50is avoided — for the same reason the feed backfill avoids it: that format carries the shortened mobile headline, and a 1920×1080 frame has no width problem that would justify truncated copy.Headlines are re-clipped for video. Display headlines are capped by characters; a pre-roll has to be legible at 480p in about three seconds.
trimHeadlineForVideoclips to eight words on a word boundary, and a test asserts the clipped result actually satisfiesvalidateSnapshot— otherwise every long-headline campaign would queue a job the renderer then refuses.Artwork is deliberately not carried yet. Recording a hero URL with a null content hash would be worse than recording neither: the dedupe key trusts the hash, and a URL beside a null hash invites a later change to treat the URL as sufficient — when the same URL can serve different bytes. The compositor uses its accent-tinted fallback until hashing lands.
Dedupe is on the render hash, not the campaign, so a design previewed, saved, then regenerated without edits is one encode. The unique index on
(render_hash, output_profile)makes that hold under concurrency; select-then-insert is the fast path, the conflict re-select is the correct one.ensureVideoCreativebumpsrequested_revisionon edits and leavespublished_revisionalone. That bump is what gives the worker's compare-and-swap something to compare — two quick edits produce revisions N and N+1, and a slow render of N publishes nothing when it lands second. Writingpublished_revisionhere would mark a creative servable before any bytes existed.A leak this package would otherwise have introduced
The campaign detail page maps
ad_creativesrows straight into<AdPreview>, which renders markup — so the video creative row this package creates would have been drawn as a banner there. The dashboard forms already iterateDESIGN_FORMATSfor exactly that reason, but the detail page reads from the database and wasn't covered by package A. Fixed here withisStreamingFormat, and the render card takes its place in the layout.Verification
18 new tests in
tests/ads-video-jobs.test.tscovering snapshot derivation and source preference, headline clipping, dedupe/reuse, row-before-enqueue, Redis-unavailable, snapshot rejection, revision bump semantics, and the download-vs-streamable distinction.Repo-wide: 2538 passed / 1 failed — the failure is the pre-existing
tests/contract/tracker-geo.test.ts(@ip-location-db/geolite2-city-mmdbnot installed locally), unrelated. Root and worker typechecks both clean.Not done here
No selection, decision endpoint, serving, player integration or accounting — packages D–F. A rendered MP4 is downloadable by its advertiser; nothing can serve one to a viewer yet, and
published_revisionis still only ever set by the worker after ffprobe validates an encode.Preview-before-save renders (the spec's owner-scoped preview job from the Generate Ads screen) are not wired yet either — the render is queued at save time.
ensureRenderJobalready supports a null campaign for that, so it's a small follow-up.