mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-01 19:42:03 +00:00
feat(lint): dense motion re-sampling for content_overlap (#2746)
* feat(lint): dense motion re-sampling for content_overlap Transient text-on-text collisions during continuous motion (e.g. an orbiting label card crossing the center card) overlap for a fraction of a second that the sparse 9-point layout grid seeks straight past. The content_overlap detector is correct; it just never gets a sample at the crossing moment. Rerun ONLY content_overlap on an 8fps grid (text-only, cheap) when the composition animates; findings feed the existing persistence tiering unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(lint): unconditional dense content_overlap pass + honor 500ms floor Round-1 blocker: the dense motion-overlap re-pass was gated on sparse-grid geometry fingerprints changing, so an animation aliased to the sparse grid (identical fingerprints, yet colliding between samples) bypassed the pass — exactly the transient false-negative it was built to catch. Remove the gate: the dense pass now runs unconditionally (bounded, text-only), driven by the composition timeline rather than a fingerprint heuristic. Round-2 follow-ups: - Persistence-tier drift: at 8fps, occurrences>=2 spans only ~125ms, not the ~500ms the design intends, and it short-circuited before the ms floor. content_overlap promotion now requires BOTH occurrences>=2 AND a literal firstSeen..lastSeen span >= 500ms, so the wall-clock floor is honored at any sampling density. Comment block updated to match. - Sample cap scales to hold a true 8fps grid up to ~75s (raised 120 -> 600) with an explicit note that longer comps degrade below 8fps to stay bounded. Tests: - Replaced the trivial "warning at every sample" test with a real between-grid regression: a collision living only inside (3.5,4.5) — a gap the sparse grid seeks past — is detected and, held ~750ms, promoted to error. - Replaced the now-invalid "skips when static" test with one asserting the dense pass runs even when sparse fingerprints are identical (aliased motion). - Added a tiering regression: two dense occurrences spanning ~125ms stay a warning (not error). Both new guards verified red before the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * perf(lint): settle-free geometry seek for dense content_overlap pass The dense overlap re-pass did up to OVERLAP_MAX_SAMPLES full-settle seeks (120ms paint settle each, ~72s of pure sleep at the ceiling) even though collectOverlap only reads getBoundingClientRect geometry, valid synchronously after the timeline setTime. Add a settle-free DENSE_GEOMETRY_SEEK_OPTIONS + driver.seekGeometry used only by the dense loop; the base grid keeps full-settle driver.seek. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(lint): document + cover content_overlap 500ms boundary for sparse callers The occurrences>=2 AND heldMs>=500 promotion rule is a semantics change for sparse callers (--samples 20, --at, short comps) whose two samples can land <500ms apart. Document the change in the tiering comment and add boundary tests: 499ms span stays warning, 500ms span promotes to error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(lint): make dense content_overlap seek genuinely geometry-only Per review: DENSE_GEOMETRY_SEEK_OPTIONS only overrode settleMs, still inheriting animationFrameSettle:double + waitForFontsMs:500 → ~3 frame waits + font wait per seek → ~30s at the 600-sample cap. Geometry (getBoundingClientRect) is valid synchronously post-setTime, so drop all post-seek waits (animationFrameSettle:none, waitForFontsMs:0, settleMs:0). Add options-level regression locking the geometry-only contract. Also fix a stale comment name (detectMotionTextOverlap → collectMotionOverlapSamples). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style: collapse multi-line comments to single lines Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
3b3d4f559c
commit
72e2f08f15
@@ -3,6 +3,8 @@ import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
AUDIT_SEEK_OPTIONS,
|
||||
DENSE_GEOMETRY_SEEK_OPTIONS,
|
||||
captureRegionCrop,
|
||||
clampCropRegion,
|
||||
installPageFunctionGuard,
|
||||
@@ -374,3 +376,15 @@ describe("installPageFunctionGuard", () => {
|
||||
expect(shim(marker)).toBe(marker);
|
||||
});
|
||||
});
|
||||
|
||||
describe("DENSE_GEOMETRY_SEEK_OPTIONS", () => {
|
||||
it("is genuinely geometry-only — no post-seek settle waits at the 600-sample cap", () => {
|
||||
// Dense pass must not inherit AUDIT's post-seek waits — geometry is valid synchronously after setTime, and waits multiply by sample count.
|
||||
expect(DENSE_GEOMETRY_SEEK_OPTIONS.animationFrameSettle).toBe("none");
|
||||
expect(DENSE_GEOMETRY_SEEK_OPTIONS.waitForFontsMs).toBe(0);
|
||||
expect(DENSE_GEOMETRY_SEEK_OPTIONS.settleMs).toBe(0);
|
||||
// AUDIT (the full-settle path used by the base grid) must still carry the waits.
|
||||
expect(AUDIT_SEEK_OPTIONS.animationFrameSettle).toBe("double");
|
||||
expect(AUDIT_SEEK_OPTIONS.settleMs).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -18,6 +18,14 @@ export const AUDIT_SEEK_OPTIONS = {
|
||||
settleMs: 120,
|
||||
} as const;
|
||||
|
||||
// Geometry-only seek for the dense content_overlap grid: getBoundingClientRect is valid synchronously after setTime, so drop all post-seek waits (rAF/font/sleep) that would multiply across the dense grid.
|
||||
export const DENSE_GEOMETRY_SEEK_OPTIONS = {
|
||||
...AUDIT_SEEK_OPTIONS,
|
||||
animationFrameSettle: "none",
|
||||
waitForFontsMs: 0,
|
||||
settleMs: 0,
|
||||
} as const;
|
||||
|
||||
export interface SeekCompositionTimelineOptions {
|
||||
fallbackToBridgeAndTimelines?: boolean;
|
||||
waitForPreferredSeekTargetMs?: number;
|
||||
|
||||
@@ -147,7 +147,9 @@ function fakeDriver(overrides: Partial<CheckAuditDriver> = {}): CheckAuditDriver
|
||||
getCanvas: vi.fn(async () => ({ width: 1920, height: 1080 })),
|
||||
findAmbiguousSelectors: vi.fn(async (_selectors: string[]) => []),
|
||||
seek: vi.fn(async (_time: number) => undefined),
|
||||
seekGeometry: vi.fn(async (_time: number) => undefined),
|
||||
collectLayout: vi.fn(async (_time: number, _tolerance: number) => []),
|
||||
collectOverlap: vi.fn(async (_time: number) => []),
|
||||
collectLayoutGeometry: vi.fn(async () => `geometry-${geometryCallCount++}`),
|
||||
collectRotationSample: vi.fn(async (_time: number) => []),
|
||||
collectOffPivotRotationSample: vi.fn(async (time: number) => ({ time, samples: [] })),
|
||||
@@ -1343,3 +1345,42 @@ describe("contrast candidate round-trip", () => {
|
||||
expect(source).not.toMatch(/prepared\.map\(\(entry\) => entry\.candidate\)/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("dense motion-overlap re-sampling", () => {
|
||||
// Collision lives inside (3.5, 4.5), a gap the sparse base grid seeks past; only the 8fps dense pass observes it.
|
||||
const inBetweenGridWindow = (time: number): boolean => time >= 3.6 && time <= 4.4;
|
||||
|
||||
it("detects a content_overlap that occurs ONLY between two sparse grid samples", async () => {
|
||||
const driver = fakeDriver({
|
||||
// Sparse base grid sees nothing at any base sample time.
|
||||
collectLayout: vi.fn(async (_time: number) => []),
|
||||
// The transient exists only strictly between base samples 3.5 and 4.5.
|
||||
collectOverlap: vi.fn(async (time: number) =>
|
||||
inBetweenGridWindow(time)
|
||||
? [layoutIssue("warning", { time, code: "content_overlap" })]
|
||||
: [],
|
||||
),
|
||||
});
|
||||
const { report } = await runScenario(driver);
|
||||
expect(driver.collectOverlap).toHaveBeenCalled();
|
||||
expect(report.layout.findings.some((f) => f.code === "content_overlap")).toBe(true);
|
||||
// Held ~750ms across the dense grid (>= the 500ms floor) -> promoted.
|
||||
expect(report.layout.errorCount).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("runs the dense pass even when sparse fingerprints are identical (aliased motion)", async () => {
|
||||
// Aliased motion has identical fingerprints yet still collides between samples — the false-negative the removed gate caused.
|
||||
const driver = fakeDriver({
|
||||
collectLayoutGeometry: vi.fn(async () => "static"),
|
||||
collectLayout: vi.fn(async (_time: number) => []),
|
||||
collectOverlap: vi.fn(async (time: number) =>
|
||||
inBetweenGridWindow(time)
|
||||
? [layoutIssue("warning", { time, code: "content_overlap" })]
|
||||
: [],
|
||||
),
|
||||
});
|
||||
const { report } = await runScenario(driver);
|
||||
expect(driver.collectOverlap).toHaveBeenCalled();
|
||||
expect(report.layout.findings.some((f) => f.code === "content_overlap")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1425,6 +1425,16 @@
|
||||
return issues;
|
||||
};
|
||||
|
||||
// Reruns only the overlap detector (same threshold, no new surface) on a fine grid for the dense motion re-sampling pass.
|
||||
window.__hyperframesOverlapAudit = function auditOverlap(options) {
|
||||
const time = options && typeof options.time === "number" ? options.time : 0;
|
||||
const root =
|
||||
document.querySelector("[data-composition-id][data-width][data-height]") ||
|
||||
document.querySelector("[data-composition-id]") ||
|
||||
document.body;
|
||||
return contentOverlapIssues(root, time);
|
||||
};
|
||||
|
||||
// Frozen-sweep guard (#U10, checkPipeline.ts): a compact per-sample
|
||||
// fingerprint of every visible element's box + opacity, in DOM order. Node
|
||||
// calls this once per seeked grid point and compares the strings across the
|
||||
|
||||
@@ -3,6 +3,7 @@ import { join, resolve } from "node:path";
|
||||
import type { Page } from "puppeteer-core";
|
||||
import {
|
||||
AUDIT_SEEK_OPTIONS,
|
||||
DENSE_GEOMETRY_SEEK_OPTIONS,
|
||||
DEFAULT_ZOOM_PADDING_PX,
|
||||
DEFAULT_ZOOM_SCALE,
|
||||
captureRegionCrop,
|
||||
@@ -348,7 +349,12 @@ function createPageDriver(page: Page, setTime: (time: number) => void): CheckAud
|
||||
setTime(time);
|
||||
await seekCompositionTimeline(page, time, AUDIT_SEEK_OPTIONS);
|
||||
},
|
||||
seekGeometry: async (time) => {
|
||||
setTime(time);
|
||||
await seekCompositionTimeline(page, time, DENSE_GEOMETRY_SEEK_OPTIONS);
|
||||
},
|
||||
collectLayout: (time, tolerance) => collectLayout(page, time, tolerance),
|
||||
collectOverlap: (time) => collectOverlap(page, time),
|
||||
collectLayoutGeometry: () => collectLayoutGeometry(page),
|
||||
collectRotationSample: (time) => collectRotationSample(page, time),
|
||||
collectOffPivotRotationSample: (time) => collectOffPivotRotationSample(page, time),
|
||||
@@ -464,6 +470,19 @@ async function collectLayout(
|
||||
return anchorLayoutIssues(page, raw.flatMap(parseLayoutIssue));
|
||||
}
|
||||
|
||||
async function collectOverlap(page: Page, time: number): Promise<AnchoredLayoutIssue[]> {
|
||||
const raw = await page.evaluate(
|
||||
(options: { time: number }) => {
|
||||
const audit = Reflect.get(window, "__hyperframesOverlapAudit");
|
||||
if (typeof audit !== "function") return [];
|
||||
const result = Reflect.apply(audit, window, [options]);
|
||||
return Array.isArray(result) ? result : [];
|
||||
},
|
||||
{ time },
|
||||
);
|
||||
return anchorLayoutIssues(page, raw.flatMap(parseLayoutIssue));
|
||||
}
|
||||
|
||||
async function collectLayoutGeometry(page: Page): Promise<string> {
|
||||
return page.evaluate(() => {
|
||||
const geometry = Reflect.get(window, "__hyperframesLayoutGeometry");
|
||||
|
||||
@@ -417,9 +417,42 @@ async function collectGridSamples(
|
||||
collected.screenshots.push({ time, pngBase64: capture.pngBase64 });
|
||||
}
|
||||
}
|
||||
await collectMotionOverlapSamples(driver, grid, collected);
|
||||
return collected;
|
||||
}
|
||||
|
||||
// Dense grid catches mid-motion text collisions the sparse layout grid seeks past; text-only overlap collection is cheap enough to afford it.
|
||||
const OVERLAP_SAMPLE_FPS = 8;
|
||||
// Ceiling that bounds dense seeks; past ~75s the grid degrades below 8fps rather than growing the seek budget without limit.
|
||||
const OVERLAP_MAX_SAMPLES = 600;
|
||||
|
||||
function buildOverlapSampleTimes(duration: number): number[] {
|
||||
if (!Number.isFinite(duration) || duration <= 0) return [];
|
||||
const count = Math.min(
|
||||
OVERLAP_MAX_SAMPLES,
|
||||
Math.max(2, Math.ceil(duration * OVERLAP_SAMPLE_FPS) + 1),
|
||||
);
|
||||
const step = duration / (count - 1);
|
||||
return mergeSampleTimes(
|
||||
Array.from({ length: count }, (_, index) => Math.round(index * step * 1000) / 1000),
|
||||
);
|
||||
}
|
||||
|
||||
/** Reruns content_overlap on a fine grid, unconditionally rather than gated on fingerprint change, since an animation aliased to the sparse grid collides between identical-fingerprint samples. */
|
||||
async function collectMotionOverlapSamples(
|
||||
driver: CheckAuditDriver,
|
||||
grid: SampleGrid,
|
||||
collected: GridSamples,
|
||||
): Promise<void> {
|
||||
const baseTimes = new Set(grid.layoutSamples);
|
||||
for (const time of buildOverlapSampleTimes(grid.duration)) {
|
||||
if (baseTimes.has(time)) continue;
|
||||
// Settle-free seek: collectOverlap reads getBoundingClientRect geometry, valid synchronously after setTime, so the dense pass skips the per-seek paint settle.
|
||||
await driver.seekGeometry(time);
|
||||
collected.layoutIssues.push(...(await driver.collectOverlap(time)));
|
||||
}
|
||||
}
|
||||
|
||||
// Frozen-sweep guard (#U10): compositions this short can legitimately hold a
|
||||
// single static frame the whole time (a title card) — never flag those.
|
||||
const SWEEP_STATIC_MIN_DURATION_SEC = 3;
|
||||
|
||||
@@ -173,7 +173,11 @@ export interface CheckAuditDriver {
|
||||
getCanvas(): Promise<Canvas>;
|
||||
findAmbiguousSelectors(selectors: string[]): Promise<AnchoredLayoutIssue[]>;
|
||||
seek(time: number): Promise<void>;
|
||||
/** Settle-free seek for the geometry-only dense content_overlap pass; only collectOverlap consumes it, and getBoundingClientRect is valid synchronously after setTime. */
|
||||
seekGeometry(time: number): Promise<void>;
|
||||
collectLayout(time: number, tolerance: number): Promise<AnchoredLayoutIssue[]>;
|
||||
/** content_overlap only, for the dense re-sampling grid — catches transient text collisions the sparse grid seeks past. */
|
||||
collectOverlap(time: number): Promise<AnchoredLayoutIssue[]>;
|
||||
/** Frozen-sweep guard (#U10): an opaque per-sample geometry+opacity
|
||||
* fingerprint of the current seeked state, for detecting a timeline that
|
||||
* never advances under seek. See layout-audit.browser.js. */
|
||||
|
||||
@@ -227,6 +227,47 @@ describe("persistence-tiered severity (#U10)", () => {
|
||||
expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 2 });
|
||||
});
|
||||
|
||||
it("keeps a content_overlap that spans under the 500ms floor as a warning, even with 2 occurrences", () => {
|
||||
// Two dense-pass occurrences ~125ms apart are under the held-duration floor, so occurrences>=2 alone must not promote to error.
|
||||
const collapsed = collapseStaticLayoutIssues(
|
||||
[
|
||||
{ ...issue("content_overlap", "warning"), time: 4.0 },
|
||||
{ ...issue("content_overlap", "warning"), time: 4.125 },
|
||||
],
|
||||
73,
|
||||
);
|
||||
|
||||
expect(collapsed).toHaveLength(1);
|
||||
expect(collapsed[0]).toMatchObject({ severity: "warning", occurrences: 2 });
|
||||
});
|
||||
|
||||
it("does NOT promote content_overlap whose two occurrences span exactly 499ms (under the floor)", () => {
|
||||
// Boundary: a span one millisecond short of the 500ms floor stays a warning — guards the AND-tighten for sparse callers.
|
||||
const collapsed = collapseStaticLayoutIssues(
|
||||
[
|
||||
{ ...issue("content_overlap", "warning"), time: 4.0 },
|
||||
{ ...issue("content_overlap", "warning"), time: 4.499 },
|
||||
],
|
||||
73,
|
||||
);
|
||||
|
||||
expect(collapsed).toHaveLength(1);
|
||||
expect(collapsed[0]).toMatchObject({ severity: "warning", occurrences: 2 });
|
||||
});
|
||||
|
||||
it("promotes content_overlap whose two occurrences span exactly 500ms (at the floor)", () => {
|
||||
const collapsed = collapseStaticLayoutIssues(
|
||||
[
|
||||
{ ...issue("content_overlap", "warning"), time: 4.0 },
|
||||
{ ...issue("content_overlap", "warning"), time: 4.5 },
|
||||
],
|
||||
73,
|
||||
);
|
||||
|
||||
expect(collapsed).toHaveLength(1);
|
||||
expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 2 });
|
||||
});
|
||||
|
||||
it("promotes a held, canvas-scale canvas_overflow breach from info to warning", () => {
|
||||
const breach = {
|
||||
...issue("canvas_overflow", "info"),
|
||||
|
||||
@@ -181,21 +181,7 @@ export function dedupeLayoutIssues(issues: LayoutIssue[]): LayoutIssue[] {
|
||||
return result;
|
||||
}
|
||||
|
||||
// Persistence-tier thresholds (#U10, adapted from Adam Rosler's visual-linter
|
||||
// design). The approach doc frames these as held-duration floors — ignore
|
||||
// under ~250ms, re-promote content_overlap at >= ~500ms — measured against
|
||||
// the SAME firstSeen/lastSeen span this collapse step already tracks. At the
|
||||
// default 9-sample grid over a multi-second composition, a single collapsed
|
||||
// occurrence is held 0ms (one entrance/exit transient sample) and two
|
||||
// collapsed occurrences are already >= one sample-to-sample gap, which is
|
||||
// well past 500ms — so "held under 250ms" reduces to `occurrences <= 1` and
|
||||
// "held >= 500ms" reduces to `occurrences >= 2`. Tiering below is written in
|
||||
// those sample-count terms (the mapping the approach doc asks to document),
|
||||
// with the literal ms span (CONTENT_OVERLAP_HELD_ERROR_MS) kept as a fallback
|
||||
// for callers whose samples really are spaced close enough together for the
|
||||
// ms floor to matter on its own (dense `--at`/`--at-transitions` runs). The
|
||||
// ~250ms ignore floor needs no separate constant — see the occurrences <= 1
|
||||
// branch below.
|
||||
// Persistence-tier thresholds (#U10): occurrences>=2 alone can't imply the 500ms floor under dense 8fps sampling, so content_overlap promotion requires a literal firstSeen..lastSeen span >= 500ms — stricter for sparse callers too.
|
||||
const CONTENT_OVERLAP_HELD_ERROR_MS = 500;
|
||||
const HELD_ACROSS_SAMPLES_MIN_OCCURRENCES = 2;
|
||||
|
||||
@@ -317,11 +303,10 @@ function isCanvasBreachHeldLarge(issue: LayoutIssue, occurrences: number): boole
|
||||
return overlapX > 0 && overlapY > 0;
|
||||
}
|
||||
|
||||
// Split out of applyPersistenceTier so the two independent "held long enough"
|
||||
// signals (sample count vs. wall-clock span) read as one boolean question
|
||||
// instead of adding a third compound branch to the tiering ladder above.
|
||||
// Split out of applyPersistenceTier so the compound "held long enough" test reads as one boolean question.
|
||||
function isContentOverlapHeldLongEnough(issue: LayoutIssue, occurrences: number): boolean {
|
||||
if (occurrences >= HELD_ACROSS_SAMPLES_MIN_OCCURRENCES) return true;
|
||||
// Two samples measure a span, but under dense 8fps sampling that span must still clear the wall-clock floor.
|
||||
if (occurrences < HELD_ACROSS_SAMPLES_MIN_OCCURRENCES) return false;
|
||||
const firstSeen = issue.firstSeen ?? issue.time;
|
||||
const lastSeen = issue.lastSeen ?? issue.time;
|
||||
const heldMs = (lastSeen - firstSeen) * 1000;
|
||||
|
||||
Reference in New Issue
Block a user