Repository navigation
perf(cli): check's craft pass skips clips the runtime has hidden - #6627
Conversation
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 0 nits · d2aacd2e
Core's [data-hf-clip-hidden][data-hf-clip-hidden] * rule (timedClipHide.ts) sets visibility: hidden !important on everything inside a hidden clip, so neither the craft parts nor the signature lose anything they would have kept. The fails without the fix: test fails on main's scripts (expected 4 to be +0) and passes at head, along with the rest of layout-audit and motion-signature (140/140). Every Actions run at this head is green.
Scope
Read all 3 files and core's timedClipHide.ts and clipReveal.ts. One edge, outside this PR: when an older shader build is on the page, keepInlineHideRulesForOlderShaders narrows the rule to clips whose inline style still says hidden. A clip such a build reveals keeps the attribute while its contents show. The overlap pass already skips those clips, so this PR makes the craft pass consistent with it.
What
hyperframes check's craft pass no longer walks clips the runtime has hidden. Each craft sample read every element in the composition, on-screen or not, twice: once for its text parts and once for the composition signature. On a long edit with many timed clips that cost grows with the whole timeline at every sample (up to 600 per check).Why
A long edit's check spent most of its time in the craft pass: one long-form edit took 87 s of craft, about 190 ms per sample, and agents gave up waiting. Profiling a 60 s composition with 120 timed clips and 3,600 text elements put two thirds of each sample in
compositionSignatureand most of the rest incraftTextParts, almost all of it reading styles of elements inside clips that were not on screen.Related work
The overlap pass already bounds its work this way (
elementsOnScreenScenesinlayout-audit.browser.js).How
craftTextPartswalkselementsOnScreenScenes(root)instead of every element.compositionSignaturetreats a[data-hf-clip-hidden]clip like an element with no box: it and its subtree are skipped before any style is read. The composition root is always measured.Both rely on core's
timedClipHiderule, which hides a marked clip's whole subtree. What a hidden clip would add to the signature was already filtered out as not visible, so the signature and text parts are unchanged; only the work shrinks.Test plan
fails without the fix:
layout-audit.chromium.test.ts"reads no style inside a clip the runtime hid, and still reads the text on screen": with main's two browser scripts it fails (expected 4 to be +0, four style reads inside the hidden clip); with this change it passes.Timings,
hyperframes check --json0.8.210 with the two browser scripts patched in place, Linux, 60 s composition with 120 timed clips (one on screen at a time), 480 craft samples, craft pass only:vitest --project chromeoverlayout-audit.chromium,motion-sample,motion-signature: 148/148, three runs in a row.vitest --project unitoversrc/commands/motion*,src/commands/layout-audit*,src/utils/check*: 180/180, three runs in a row.oxfmt, oxlint and
fallow auditare clean on the changed files.Unit tests added/updated
Manual testing performed
Documentation updated (if applicable)
Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug
Not exercised: a real long-form edit end to end, and video-heavy edits beyond the timing above (video adds its own per-seek cost that this does not touch).
Why under 100 lines
Small on purpose: a lone urgent fix for checks that go silent for minutes on long edits. Both walks switch to the on-screen walk the overlap pass already uses, plus one test that fails without it; nothing else rides along.