Back to Blog

Pin One Hook Into Every Variant: Fixing a Hook-Management Builder

By · 7 min read
Frontend Engineering Video Production TypeScript UX

The ad-variant export tool exists to reassemble sped-up ad variants with a swappable opening "hook" — an editor picks a fast-cut visual, pairs it against a library of alternate hooks, and the tool builds every combination as its own export. Eight PRs over three weeks fixed it from "editors keep reporting the same class of bug in different clothes" to something closer to what they'd actually asked for — and the most interesting finding wasn't a UI bug at all, it was a test suite that had been silently asserting nothing for months.

A green test suite is a claim about what ran, not about what passed. Check the first thing before trusting the second.

A naming bug that erased the library

PR #1 opens with the kind of bug that looks impossible until you see the exact ordering: _clip_from_file ran parse_sop() before it read the file's own stamped slot-type metadata. Any filename shaped like <adname>_<product>_<format> — which is exactly how the tool names its own short leads, e.g. SL872_PRO99_916.mp4 — got reclassified as a finished ad purely from its filename shape, before the code ever checked the field that actually said what it was. Every short lead an editor exported vanished from the builder's own library, including leads the tool had generated itself moments earlier. The same PR also ported hook reordering and per-ad prelead handling from the tool's sibling "Export" repo, and fixed archiving that had been silently unreliable — three real fixes bundled together, with a fourth request (adding a product tracker) explicitly scoped out to a different repo rather than absorbed here.

Pin one hook, not five clicks

PR #2 tackled the feature editors actually wanted: apply one hook across an entire batch of variants in a single action, instead of dragging it into each one by hand. The PR also fixed something unrelated that had been quietly broken for a while — the preview panel stopped updating after the first build of a session, because recompute() short-circuited into the finished-job list whenever a prior batch's job list was non-empty, and nothing ever cleared that list once a job finished. Every keystroke, every product change, every rearrangement after that first build just repainted the old result. The only way to see a new arrangement was to commit to another full render — verifying your edit by paying for it.

Built the wrong shape, twice — on purpose, not by accident

PR #4 is a clean example of shipping something reasonable that still isn't what was asked for, and treating that as a spec problem rather than a regression. What landed in #2 was "pin to all" — one hook joins every other variant. What the editor actually needed was a subset: one specific hook applied to three of five variants, not all five. The fix — tick the hooks that should get it, then Add a hook to each of 3… — is asserted twice: once against the exact expected filename list in the unit suite, and again by driving the real browser controls end to end, so the assertion can't quietly stop matching what a user actually clicks.

PR #5 found the same feature was still subtly wrong one layer down. The picker that chose which clip to apply listed individual clips, not lanes — so if the source hook was a two-part opener, picking "one of its clips" split the pair in half: one half joined the target variants, the other was left behind as an orphaned alternate nobody asked for. Measured on a real five-hook batch, the fix took the lane count from six back down to the correct five. The picker now offers lanes, not clips, which makes "half an opener" structurally impossible to select.

A test suite that reported success having asserted nothing

PR #3 is the one worth sitting with longest. Two follow-up audits after #2 shipped — an accessibility/UX pass and a speed-mode pass — surfaced a batch of an editor's work disappearing entirely on a mid-task mode switch, and table rows wrapping awkwardly throughout the interface. But the finding that undermines trust in everything shipped before it is this: test_builder_e2e.py could report success having verified nothing at all. Flask auto-loads a local .env file, and on any machine where HUB_SHARED_SECRET happened to be set, every request the test suite made came back 403. That 403 arrived as an HTTPError — a subclass of URLError — so the suite's own retry loop caught it, printed a friendly SKIP: the Flask dev server did not come up, and exited zero. Green. The suite that had been specifically added because a DOM stub is blind to real layout was itself blind, on exactly the machines most likely to have a shared secret configured — which is to say, on anything resembling a real environment. The fix forces the secret empty for the suite's own server and distinguishes a closed connection from an actual HTTP rejection, so a real failure can no longer disguise itself as an environment quirk.

Small frictions, fixed for the reason an editor gave

PR #6 closed a naming inconsistency that had been open since an earlier task: two different ways of including a hook in a batch — "include in every hook" and "add a hook to each" — defaulted to opposite ordering, producing two different filenames for what editors meant as the same operation. The fix wasn't a preference call; it was resolved from the editors' own worked example, and the reasoning is concrete: buildSopName emits the timeline in reverse, so a code placed last in the name plays first in the render — meaning "plays-first" was the only default that made both routes agree.

PR #7 investigated a report that the library picker "reloads" every time a filter is touched, and the finding was that it never actually reloads — the listing is fetched once per mode and cached, and a filter click issues zero network requests. What was real: reopening a cached library flashed a loading state it didn't need, and one specific filter path was needlessly rebuilding 480 row nodes in three seconds for a change that touched none of them. Three plausible-sounding causes, only one of which was doing real work — the kind of bug report that's only solvable by actually instrumenting the claim instead of guessing at it.

PR #8 closed the loop on a claim that turned out to be about a missing feature, not a broken one. A report read "apply hook to all — this is not applied," but the feature had been live and tested since #4. What was actually missing was the specific route to it that the sibling Export tool already had: a ⧉ Duplicate gesture, stacking each copy with its visual. Rather than re-invent that interaction from scratch, this PR ported the row controls that already existed one repo over — the same workflow, available here for the first time, built the way it had already been proven to work elsewhere.

The pattern: check what the test actually exercised

Most of these bugs are unremarkable individually — a metadata read in the wrong order, a stale short-circuit, a picker granularity mismatch. What ties them together is that nearly every fix in this batch came from checking a claim against reality rather than trusting that a passing test, a shipped feature, or a plausible-sounding bug report was accurately describing what was actually happening. The test suite that reported green while testing nothing is the sharpest version of that lesson, but PR #2's stale preview, PR #7's "it reloads" report, and PR #8's "it's not applied" report are all the same shape: the thing that looked true on the surface wasn't, and the fix in every case started with measuring, not assuming.

Related Articles