Write the asset rows, and notice when they are not written - #290
Merged
Merged
Conversation
There is a unique index on ad_video_assets (creative_id, revision, profile), so re-rendering a revision — exactly what a RENDERER_VERSION bump asks for — violated it on every row. That failure was invisible: the insert's return value was discarded, so the statement failed, the job was still marked ready, and the revision kept the assets of the render it was supposed to replace. Every fix since the version bump has been landing in storage and then being dropped here in silence. The animated banners were rendered, validated, uploaded to the bucket, and then not recorded, which is why the renderer reported "produced 3" while the table showed nothing and the campaign page showed nothing. It is also why the corrected HLS codec strings never reached the existing revisions. The rows are now upserted on that index, for the same reason the objects are overwritten: the design did not change, the renderer did. And the error is checked — a revision whose rows were not written points at nothing, so it fails loudly instead of being marked ready. 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.
This is the root cause of everything since the
RENDERER_VERSIONbump.There is a unique index on
ad_video_assets (creative_id, revision, profile). Re-rendering a revision — exactly what a version bump asks for — violates it on every row.That failure was invisible, because the insert's return value was discarded:
So the statement failed, the job was still marked
ready, and the revision kept the assets of the render it was supposed to replace.What this explains
Every fix since the version bump has been landing in storage and then being dropped here in silence:
animated banners: produced 3while the table showed nothing and the campaign page showed nothing.I spent several deploys treating the symptom because the failure path was a discarded return value.
The change
Rows are upserted on that index — the same reasoning as overwriting the objects: the design did not change, the renderer did. And the error is now checked: a revision whose rows were not written points at nothing, so it fails loudly instead of being marked ready.
Verification
2570 passed / 1 failed repo-wide — the pre-existing
tracker-geofailure. Both typechecks clean.Confirmed directly in the production container before writing this:
renderPrerollthere returns all 8 profiles with 0 problems, and the three banners come out at exactly 300×250 / 728×90 / 320×50, 40 frames, 144 KB / 116 KB / 40 KB — all inside the 150 KB budget. The rendering was never the problem after the earlier fixes; the recording was.