fix(engine): scale static-dedup verification density with run length (#1903)

* fix(engine): scale static-dedup verification density with run length

Reported symptom: a 10-scene template composition (shared card layout,
per-scene text/progress-bar content) rendered scene 1 correctly, but
every scene after that had its text/progress-bar card missing from
the final MP4 -- even though snapshot and validate showed correct
per-scene content when seeking directly to those timestamps. Setting
HF_STATIC_DEDUP=false fixed every scene. Render log showed a large,
mostly-reusable static-frame run engaging (2430 frames, 34% reusable).

verifyStaticFramesSafe already does a real, pixel-exact comparison
(anchor vs. candidate screenshot) before trusting a predicted-static
run -- the reuse mechanism itself is correct and already regression-
locked (frameCapture-staticDedupIndex.test.ts). The gap was sample
density: interior checks per run were capped at a flat
min(sampleCount, 8) points, so the stride between checks grew with
the run's span. A 2000+ frame run (plausible for a 10-scene comp
where computeStaticFrameSet's GSAP-tween-only interval walk can't see
whatever mechanism swaps each scene's text) could space checks ~285
frames apart, letting a real content change hide between two verified
points and get the whole run wrongly trusted as static.

Fix: extract the point-selection into a pure, exported
computeStaticVerificationPoints(a, b, sampleCount), and bound the
STRIDE by sampleCount (HF_STATIC_DEDUP_SAMPLES) instead of just the
point count, so density scales with run length. Short/typical runs
are unaffected (the two formulas agree there); long runs get
proportionally denser checks. The existing hardCap safety valve is
untouched -- if this makes verification too expensive for a
pathological composition, dedup still disarms entirely rather than
trusting a sparsely-checked set.

Test: new frameCapture-staticDedupVerifyDensity.test.ts asserts the
max gap between consecutive verification points never exceeds
sampleCount on long runs (would fail pre-fix at span=2000/10000),
matches the prior stride on short runs, and always includes both
run endpoints. Full engine suite (845 tests) passes.

* fix(engine): decouple verification density scaling from sampleCount polarity

Addresses review feedback on the static-dedup density fix (PR #1903):

1. The prior revision bounded the interior-check STRIDE by sampleCount
   directly, which inverted HF_STATIC_DEDUP_SAMPLES' polarity: raising
   it widened the allowed gap between checks instead of narrowing it,
   and the "raise HF_STATIC_DEDUP_SAMPLES to verify more" log guidance
   became backwards for exactly the long runs it's meant to help.

   Fix: introduce a fixed STATIC_VERIFY_REFERENCE_STRIDE (24 frames,
   independent of sampleCount) that drives the length-scaling behavior
   -- this alone fixes the original bug (long runs going nearly
   unverified) regardless of how sampleCount is configured. sampleCount
   is now purely a per-run point-count FLOOR: raising it only ever
   increases density, restoring correct, monotonic polarity.

2. hardCap wasn't re-tuned for the new cost model. The old flat 8-point
   cap cost ~8 checks/run; the new density costs ~span/24 checks/run --
   ~103 for the reported 2430-frame run, ~417 for a 10k-frame run.
   Sizing the budget only off sampleCount (which no longer drives
   density for long runs) would make a genuinely-static long
   composition spuriously disarm under the new, more thorough checking.
   hardCap now also scales with the total predicted-static frame count,
   with a 3x margin over the expected minimum verification cost.
   Softened the budget-exhausted log message accordingly -- it no
   longer prescribes raising sampleCount, which would often just add
   cost without proportionally raising the now length-driven budget.

3. The 5 existing tests only asserted sample-point geometry (gaps,
   endpoints, stride shape), not the actual point of the fix -- that a
   real content change hiding between the OLD sample gaps now gets
   caught. Added a behavior-level test: mocks pageScreenshotCapture to
   simulate a transient content change at a frame the pre-fix formula
   would have skipped (reconstructed locally in the test, commented as
   historical-only) but the new formula samples, and asserts the real
   verifyStaticFramesSafe (now exported) detects it via the real
   computeStaticVerificationPoints -- not a reimplementation. Also
   added a direct polarity regression test (raising sampleCount past
   the length-scaled floor must strictly tighten the gap) and reworded
   the short-run test to reflect the corrected formula.

Full engine suite (847 tests) passes.
This commit is contained in:
Miguel Ángel
2026-07-03 12:30:38 -07:00
committed by GitHub
parent 7e8a1466c3
commit df221c1fd6
2 changed files with 215 additions and 17 deletions
+65 -17
View File
@@ -1525,6 +1525,49 @@ async function computeStaticFrameSet(
};
}
// Fixed density target for verification checks: never leave a gap wider than this
// many frames within a run, independent of the user-tunable sampleCount. This is
// what fixes long runs going nearly unverified — deliberately NOT derived from
// sampleCount, so that knob's effect on density stays monotonic (see below).
const STATIC_VERIFY_REFERENCE_STRIDE = 24;
/**
* Interior verification points for a run [a..b], plus the always-included end `b`.
* Density used to be a flat point-count cap (min(sampleCount, 8)), so a run's
* stride grew with its span — on a long run (many merged static frames), two
* checks could land hundreds of frames apart. A genuine content change in
* between (e.g. text swapped by a mechanism computeStaticFrameSet's GSAP-only
* tween walk can't see) then hides between samples and the whole run gets
* wrongly trusted as static.
*
* `sampleCount` (HF_STATIC_DEDUP_SAMPLES) is a per-run point-count FLOOR, not a
* stride cap — raising it always increases density, never decreases it. (An
* earlier revision of this fix bounded the stride BY sampleCount directly, which
* inverted that: raising sampleCount widened the allowed gap instead of shrinking
* it, and the "raise HF_STATIC_DEDUP_SAMPLES to verify more" log guidance became
* backwards for exactly the long runs it's meant to help.) The length-scaling
* fix itself comes from STATIC_VERIFY_REFERENCE_STRIDE, which is independent of
* sampleCount, so density scales with run length regardless of how that knob is
* set; sampleCount only ever raises density further above that floor.
*
* Pure and exported so its scaling behavior is unit-testable without a real
* page/browser.
*/
export function computeStaticVerificationPoints(
a: number,
b: number,
sampleCount: number,
): number[] {
const span = b - a;
const lengthScaledPoints = span > 0 ? Math.ceil(span / STATIC_VERIFY_REFERENCE_STRIDE) + 1 : 1;
const perRun = Math.max(3, sampleCount, lengthScaledPoints);
const stride = span > 0 ? Math.max(1, Math.floor(span / (perRun - 1))) : 1;
const pts = new Set<number>();
for (let f = a; f <= b; f += stride) pts.add(f);
pts.add(b);
return [...pts].sort((x, y) => x - y);
}
/**
* Empirically verify the predicted-static set before trusting it. Group static frames
* into runs; each run [a..b] reuses anchor a-1. CRITICAL: compare against the ANCHOR,
@@ -1534,7 +1577,7 @@ async function computeStaticFrameSet(
* mismatch ⇒ the run isn't truly static ⇒ disable dedup whole-comp. Capture-mode-
* independent (seeks + screenshots in normal DOM). Returns the first bad frame, or null.
*/
async function verifyStaticFramesSafe(
export async function verifyStaticFramesSafe(
session: CaptureSession,
page: Page,
staticFrames: Set<number>,
@@ -1561,31 +1604,36 @@ async function verifyStaticFramesSafe(
};
// Verify EVERY run in order (no longest-first truncation that would leave runs armed
// but unverified). Per run, compare the FIRST reused frame `a`, the END `b` (max
// accumulated drift), and interior points at a stride — against the anchor the run
// actually reuses. `sampleCount` sets the interior density (points per run ~ that many
// for a long run); a hard cap bounds pathological run counts, and hitting it DISABLES
// dedup (conservative: never trust an unverified set).
const perRun = Math.max(3, Math.min(sampleCount, 8));
const hardCap = Math.max(sampleCount * 8, 400);
// accumulated drift), and interior points at a stride (see computeStaticVerificationPoints)
// — against the anchor the run actually reuses.
//
// hardCap bounds pathological cases and hitting it DISABLES dedup (conservative:
// never trust an unverified set). It must scale with the new density model:
// each run now costs roughly span/STATIC_VERIFY_REFERENCE_STRIDE + 1 checks (plus
// one anchor), not the ~8 the old flat point cap cost — sizing the budget only off
// sampleCount (which no longer drives density for long runs) would make a
// genuinely-static long composition spuriously disarm under the new, more
// thorough checking. `frames.length` approximates total interior checks; a 3x
// margin absorbs per-run anchor overhead and the 3-point floor on short runs.
const hardCap = Math.max(
sampleCount * 8,
400,
Math.ceil(frames.length / STATIC_VERIFY_REFERENCE_STRIDE) * 3 + runs.length,
);
let spent = 0;
for (const { a, b } of runs) {
const anchor = a - 1;
if (anchor < 0) continue;
const anchorBuf = await seekCapture(anchor);
spent++;
const span = b - a;
const stride = span > 0 ? Math.max(1, Math.floor(span / (perRun - 1))) : 1;
const pts = new Set<number>();
for (let f = a; f <= b; f += stride) pts.add(f);
pts.add(b); // always include the end (max drift)
for (const f of [...pts].sort((x, y) => x - y)) {
for (const f of computeStaticVerificationPoints(a, b, sampleCount)) {
const cur = await seekCapture(f);
spent++;
if (!anchorBuf.equals(cur)) return { badFrame: f, budgetExhausted: false };
}
// Budget exhausted → can't fully verify → disarm. Reported distinctly from real
// drift so a `verification_budget` spike in telemetry signals "tune HF_STATIC_DEDUP_SAMPLES",
// not "compositions are non-static".
// Budget exhausted → can't fully verify → disarm, distinct from real drift so a
// `verification_budget` spike in telemetry reads as "this composition has a lot
// of static material to verify," not "compositions are non-static."
if (spent > hardCap) return { badFrame: a, budgetExhausted: true };
}
return null;
@@ -1664,7 +1712,7 @@ async function armStaticDedup(
logInitPhase(
verdict.budgetExhausted
? `static-frame dedup: disabled (verification budget exhausted before frame ${verdict.badFrame}; ` +
`raise HF_STATIC_DEDUP_SAMPLES to verify more)`
`too much predicted-static material to fully verify — this is the safe fallback, not an error)`
: `static-frame dedup: disabled (verification failed — content drifts from anchor at ` +
`predicted-static frame ${verdict.badFrame})`,
);