Skip to content

Commit 3e0c480

Browse files
xuanruliclaude
andcommitted
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>
1 parent 862691e commit 3e0c480

4 files changed

Lines changed: 80 additions & 42 deletions

File tree

packages/cli/src/commands/check.test.ts

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,30 +1346,44 @@ describe("contrast candidate round-trip", () => {
13461346
});
13471347

13481348
describe("dense motion-overlap re-sampling", () => {
1349-
it("surfaces a held content_overlap that only the dense grid observes", async () => {
1350-
// collectLayout (sparse base grid) sees nothing; collectOverlap (dense
1351-
// grid) reports the collision — as it would for a mid-motion crossing the
1352-
// base samples seek past. Held across the dense grid, it promotes to error.
1349+
// The default grid is 9 base samples at index+0.5 (0.5,1.5,...,8.5) over a 9s
1350+
// composition; the collision below lives entirely inside (3.5, 4.5), a gap
1351+
// the sparse grid seeks straight past. Only the 8fps dense pass observes it.
1352+
const inBetweenGridWindow = (time: number): boolean => time >= 3.6 && time <= 4.4;
1353+
1354+
it("detects a content_overlap that occurs ONLY between two sparse grid samples", async () => {
13531355
const driver = fakeDriver({
1356+
// Sparse base grid sees nothing at any base sample time.
13541357
collectLayout: vi.fn(async (_time: number) => []),
1355-
collectOverlap: vi.fn(async (time: number) => [
1356-
layoutIssue("warning", { time, code: "content_overlap" }),
1357-
]),
1358+
// The transient exists only strictly between base samples 3.5 and 4.5.
1359+
collectOverlap: vi.fn(async (time: number) =>
1360+
inBetweenGridWindow(time)
1361+
? [layoutIssue("warning", { time, code: "content_overlap" })]
1362+
: [],
1363+
),
13581364
});
13591365
const { report } = await runScenario(driver);
13601366
expect(driver.collectOverlap).toHaveBeenCalled();
13611367
expect(report.layout.findings.some((f) => f.code === "content_overlap")).toBe(true);
1368+
// Held ~750ms across the dense grid (>= the 500ms floor) -> promoted.
13621369
expect(report.layout.errorCount).toBeGreaterThan(0);
13631370
});
13641371

1365-
it("skips the dense overlap pass when the composition never animates", async () => {
1366-
// A constant geometry fingerprint means the timeline never advanced — no
1367-
// transient crossing is possible, so the dense pass must not run.
1372+
it("runs the dense pass even when sparse fingerprints are identical (aliased motion)", async () => {
1373+
// A constant geometry fingerprint no longer suppresses the pass: an
1374+
// animation aliased to the sparse grid has identical fingerprints yet still
1375+
// collides between samples — the false-negative the removed gate caused.
13681376
const driver = fakeDriver({
13691377
collectLayoutGeometry: vi.fn(async () => "static"),
1370-
collectOverlap: vi.fn(async (_time: number) => []),
1378+
collectLayout: vi.fn(async (_time: number) => []),
1379+
collectOverlap: vi.fn(async (time: number) =>
1380+
inBetweenGridWindow(time)
1381+
? [layoutIssue("warning", { time, code: "content_overlap" })]
1382+
: [],
1383+
),
13711384
});
1372-
await runScenario(driver);
1373-
expect(driver.collectOverlap).not.toHaveBeenCalled();
1385+
const { report } = await runScenario(driver);
1386+
expect(driver.collectOverlap).toHaveBeenCalled();
1387+
expect(report.layout.findings.some((f) => f.code === "content_overlap")).toBe(true);
13741388
});
13751389
});

packages/cli/src/utils/checkPipeline.ts

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -426,12 +426,15 @@ async function collectGridSamples(
426426
// mid-orbit text-on-text crossing that only overlaps for a fraction of a
427427
// second: an in-corpus orbit (samples/fuzz016) collides at 28% area for ~0.4s,
428428
// entirely between two adjacent base samples. 8fps (~0.125s spacing) lands
429-
// >= 2 samples inside a window that narrow, which is what persistence tiering
430-
// needs to promote the finding to error. Overlap collection is text-only
431-
// (collectSolidTextBlocks), far cheaper than a full layout audit, so a fine
432-
// grid here is affordable where densifying every detector would not be.
429+
// enough samples inside a window that narrow to observe it. Overlap collection
430+
// is text-only (collectSolidTextBlocks), far cheaper than a full layout audit,
431+
// so a fine grid here is affordable where densifying every detector would not.
433432
const OVERLAP_SAMPLE_FPS = 8;
434-
const OVERLAP_MAX_SAMPLES = 120;
433+
// Absolute ceiling on dense seeks so the pass stays bounded. This holds a true
434+
// 8fps grid for compositions up to OVERLAP_MAX_SAMPLES / OVERLAP_SAMPLE_FPS
435+
// (~75s); longer compositions degrade below 8fps rather than growing the seek
436+
// budget without limit. (Corpus compositions run 7-8s, well inside 8fps.)
437+
const OVERLAP_MAX_SAMPLES = 600;
435438

436439
function buildOverlapSampleTimes(duration: number): number[] {
437440
if (!Number.isFinite(duration) || duration <= 0) return [];
@@ -449,19 +452,24 @@ function buildOverlapSampleTimes(duration: number): number[] {
449452
* Dense motion-overlap re-sampling. Reruns ONLY content_overlap on a fine time
450453
* grid so transient text collisions during continuous motion are observed at
451454
* all — the detector itself is unchanged (same 0.2-area threshold), only the
452-
* sampling density is. Gated on the composition actually animating (the
453-
* frozen-sweep geometry fingerprints differ across the base grid): a static
454-
* card produces no new work and no new findings. Findings feed the existing
455-
* collapse/persistence tiering, so a one-sample graze stays info while a held
456-
* collision (samples/fuzz016) re-promotes to error. Skips times already in the
457-
* base grid to avoid double-collecting the overlaps collectLayout already found.
455+
* sampling density is.
456+
*
457+
* This runs UNCONDITIONALLY (bounded + text-only), NOT gated on sparse-grid
458+
* geometry fingerprints changing. That gate was the motivating false-negative:
459+
* an animation aliased to the sparse grid (the same pose sampled at every base
460+
* point) has identical fingerprints yet still collides *between* those samples,
461+
* so gating on fingerprint change skipped the exact transient this pass exists
462+
* to catch. A static composition simply yields no overlaps at the extra times,
463+
* so the only cost of running always is a bounded set of cheap text-only seeks.
464+
* Findings feed the existing collapse/persistence tiering (a graze stays info,
465+
* a held collision re-promotes to error). Skips times already in the base grid
466+
* to avoid double-collecting overlaps collectLayout already found.
458467
*/
459468
async function collectMotionOverlapSamples(
460469
driver: CheckAuditDriver,
461470
grid: SampleGrid,
462471
collected: GridSamples,
463472
): Promise<void> {
464-
if (new Set(collected.geometrySignatures).size <= 1) return;
465473
const baseTimes = new Set(grid.layoutSamples);
466474
for (const time of buildOverlapSampleTimes(grid.duration)) {
467475
if (baseTimes.has(time)) continue;

packages/cli/src/utils/layoutAudit.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,21 @@ describe("persistence-tiered severity (#U10)", () => {
227227
expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 2 });
228228
});
229229

230+
it("keeps a content_overlap that spans under the 500ms floor as a warning, even with 2 occurrences", () => {
231+
// Two occurrences from the dense 8fps re-pass span only ~125ms — under the
232+
// held-duration floor, so occurrences>=2 alone must NOT promote to error.
233+
const collapsed = collapseStaticLayoutIssues(
234+
[
235+
{ ...issue("content_overlap", "warning"), time: 4.0 },
236+
{ ...issue("content_overlap", "warning"), time: 4.125 },
237+
],
238+
73,
239+
);
240+
241+
expect(collapsed).toHaveLength(1);
242+
expect(collapsed[0]).toMatchObject({ severity: "warning", occurrences: 2 });
243+
});
244+
230245
it("promotes a held, canvas-scale canvas_overflow breach from info to warning", () => {
231246
const breach = {
232247
...issue("canvas_overflow", "info"),

packages/cli/src/utils/layoutAudit.ts

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -183,19 +183,17 @@ export function dedupeLayoutIssues(issues: LayoutIssue[]): LayoutIssue[] {
183183

184184
// Persistence-tier thresholds (#U10, adapted from Adam Rosler's visual-linter
185185
// design). The approach doc frames these as held-duration floors — ignore
186-
// under ~250ms, re-promote content_overlap at >= ~500ms — measured against
187-
// the SAME firstSeen/lastSeen span this collapse step already tracks. At the
188-
// default 9-sample grid over a multi-second composition, a single collapsed
189-
// occurrence is held 0ms (one entrance/exit transient sample) and two
190-
// collapsed occurrences are already >= one sample-to-sample gap, which is
191-
// well past 500ms — so "held under 250ms" reduces to `occurrences <= 1` and
192-
// "held >= 500ms" reduces to `occurrences >= 2`. Tiering below is written in
193-
// those sample-count terms (the mapping the approach doc asks to document),
194-
// with the literal ms span (CONTENT_OVERLAP_HELD_ERROR_MS) kept as a fallback
195-
// for callers whose samples really are spaced close enough together for the
196-
// ms floor to matter on its own (dense `--at`/`--at-transitions` runs). The
197-
// ~250ms ignore floor needs no separate constant — see the occurrences <= 1
198-
// branch below.
186+
// under ~250ms, re-promote content_overlap at >= ~500ms — measured against the
187+
// SAME firstSeen/lastSeen span this collapse step already tracks. `occurrences`
188+
// is a NECESSARY guard (one sample can't span any duration), but it is NOT a
189+
// sufficient proxy for the 500ms floor: the dense content_overlap re-pass
190+
// (checkPipeline collectMotionOverlapSamples) samples at 8fps, so two adjacent
191+
// occurrences there span only ~125ms — the old "occurrences >= 2 => held >=
192+
// 500ms" shortcut held only for the coarse ~1s-spaced base grid and breaks
193+
// under dense sampling. content_overlap promotion therefore requires BOTH
194+
// occurrences >= 2 AND a literal firstSeen..lastSeen span >= 500ms, so the
195+
// wall-clock floor is honored regardless of sampling density. The ~250ms
196+
// ignore floor needs no separate constant — see the occurrences <= 1 branch.
199197
const CONTENT_OVERLAP_HELD_ERROR_MS = 500;
200198
const HELD_ACROSS_SAMPLES_MIN_OCCURRENCES = 2;
201199

@@ -317,11 +315,14 @@ function isCanvasBreachHeldLarge(issue: LayoutIssue, occurrences: number): boole
317315
return overlapX > 0 && overlapY > 0;
318316
}
319317

320-
// Split out of applyPersistenceTier so the two independent "held long enough"
321-
// signals (sample count vs. wall-clock span) read as one boolean question
322-
// instead of adding a third compound branch to the tiering ladder above.
318+
// Split out of applyPersistenceTier so the compound "held long enough" test
319+
// (>= 2 samples AND wall-clock span >= the ms floor) reads as one boolean
320+
// question instead of adding a compound branch to the tiering ladder above.
323321
function isContentOverlapHeldLongEnough(issue: LayoutIssue, occurrences: number): boolean {
324-
if (occurrences >= HELD_ACROSS_SAMPLES_MIN_OCCURRENCES) return true;
322+
// Need at least two samples to measure a span at all, AND that span must
323+
// clear the wall-clock floor — dense 8fps re-sampling makes occurrences>=2
324+
// alone (potentially ~125ms) too weak to imply a genuinely held collision.
325+
if (occurrences < HELD_ACROSS_SAMPLES_MIN_OCCURRENCES) return false;
325326
const firstSeen = issue.firstSeen ?? issue.time;
326327
const lastSeen = issue.lastSeen ?? issue.time;
327328
const heldMs = (lastSeen - firstSeen) * 1000;

0 commit comments

Comments
 (0)