diff --git a/packages/cli/src/commands/check.test.ts b/packages/cli/src/commands/check.test.ts index 8d9a07f97e..2a9c6f77b1 100644 --- a/packages/cli/src/commands/check.test.ts +++ b/packages/cli/src/commands/check.test.ts @@ -934,6 +934,91 @@ describe("check pipeline", () => { expect(browser).toHaveBeenCalledTimes(1); }); + // An ambiguous selector used to zero the sample grid, so every other assertion in the sidecar + // went unevaluated with no trace — the section still reported `enabled`. + it("keeps evaluating the unambiguous assertions when one selector is ambiguous", async () => { + const motion: MotionSpecResolution = { + kind: "valid", + path: "/project/index.motion.json", + spec: { + assertions: [ + { kind: "appearsBy", selector: ".item", bySec: 1 }, + { kind: "appearsBy", selector: "#hero", bySec: 0.2 }, + ], + }, + }; + const driver = fakeDriver({ + getDuration: vi.fn(async () => 1), + findAmbiguousSelectors: vi.fn(async () => [ + { ...layoutIssue("error", { code: "motion_selector_ambiguous" }), selector: ".item" }, + ]), + collectMotionFrame: vi.fn(async (time: number) => heroMotionFrame(time, (t) => t >= 0.5)), + }); + const { report } = await runScenario(driver, {}, { motion }); + + expect(report.motion.samples).toBeGreaterThan(0); + expect(report.motion.findings.map((finding) => finding.code).sort()).toEqual([ + "motion_appears_late", + "motion_selector_ambiguous", + ]); + expect(report.ok).toBe(false); + // The grid is sampled again, so the report must say on its own that part of the spec was dropped. + expect( + report.motion.findings.find((finding) => finding.code === "motion_selector_ambiguous") + ?.message, + ).toContain("1 assertion(s) naming it were not evaluated"); + // Only the surviving assertion's selector is sampled. + expect(driver.collectMotionFrame).toHaveBeenCalledWith(expect.any(Number), ["#hero"], []); + }); + + it("drops a before assertion when either side is ambiguous, keeping the rest", async () => { + const motion: MotionSpecResolution = { + kind: "valid", + path: "/project/index.motion.json", + spec: { + assertions: [ + { kind: "before", a: "#hero", b: ".item" }, + { kind: "appearsBy", selector: "#hero", bySec: 0.2 }, + ], + }, + }; + const driver = fakeDriver({ + getDuration: vi.fn(async () => 1), + findAmbiguousSelectors: vi.fn(async () => [ + { ...layoutIssue("error", { code: "motion_selector_ambiguous" }), selector: ".item" }, + ]), + collectMotionFrame: vi.fn(async (time: number) => heroMotionFrame(time, (t) => t >= 0.5)), + }); + const { report } = await runScenario(driver, {}, { motion }); + + expect(report.motion.findings.map((finding) => finding.code).sort()).toEqual([ + "motion_appears_late", + "motion_selector_ambiguous", + ]); + // `#hero` survives via the appearsBy assertion even though the before naming it was dropped. + expect(driver.collectMotionFrame).toHaveBeenCalledWith(expect.any(Number), ["#hero"], []); + }); + + it("does not sample when every assertion names an ambiguous selector", async () => { + const motion: MotionSpecResolution = { + kind: "valid", + path: "/project/index.motion.json", + spec: { assertions: [{ kind: "appearsBy", selector: ".item", bySec: 1 }] }, + }; + const driver = fakeDriver({ + getDuration: vi.fn(async () => 1), + findAmbiguousSelectors: vi.fn(async () => [ + { ...layoutIssue("error", { code: "motion_selector_ambiguous" }), selector: ".item" }, + ]), + }); + const { report } = await runScenario(driver, {}, { motion }); + + expect(report.motion.samples).toBe(0); + expect(report.motion.findings.map((finding) => finding.code)).toEqual([ + "motion_selector_ambiguous", + ]); + }); + it("reports a failing appearsBy sidecar as motion_appears_late", async () => { const motion: MotionSpecResolution = { kind: "valid", diff --git a/packages/cli/src/utils/checkPipeline.ts b/packages/cli/src/utils/checkPipeline.ts index a02fb2fa5b..80a7bd6f04 100644 --- a/packages/cli/src/utils/checkPipeline.ts +++ b/packages/cli/src/utils/checkPipeline.ts @@ -15,12 +15,13 @@ import { type LayoutRect, } from "./layoutAudit.js"; import { + assertionTargets, collectSamplingTargets, evaluateMotion, type Canvas, type MotionFrame, } from "./motionAudit.js"; -import { findMotionSpec, readMotionSpec } from "./motionSpec.js"; +import { findMotionSpec, readMotionSpec, type MotionAssertion } from "./motionSpec.js"; import { normalizeErrorMessage } from "./errorMessage.js"; import { parseColorRGBA, @@ -167,6 +168,8 @@ interface MotionPlan { selectors: string[]; livenessScopes: string[]; preflightIssues: AnchoredLayoutIssue[]; + /** Assertions naming no ambiguous selector; a zero-match selector survives and reports as missing. */ + assertions: MotionAssertion[]; } async function planMotionSampling( @@ -175,13 +178,30 @@ async function planMotionSampling( duration: number, ): Promise { if (motion.kind !== "valid") { - return { times: [], selectors: [], livenessScopes: [], preflightIssues: [] }; + return { times: [], selectors: [], livenessScopes: [], preflightIssues: [], assertions: [] }; } - const targets = collectSamplingTargets(motion.spec.assertions); - const preflightIssues = await driver.findAmbiguousSelectors(targets.selectors); + const preflightIssues = await driver.findAmbiguousSelectors( + collectSamplingTargets(motion.spec.assertions).selectors, + ); + // One ambiguous selector used to zero the grid, so every other assertion went unevaluated in + // silence. Drop only the assertions that name it; the rest still run. + const ambiguous = new Set(preflightIssues.map((issue) => issue.selector)); + const assertions = motion.spec.assertions.filter( + (assertion) => + !assertionTargets(assertion).selectors.some((selector) => ambiguous.has(selector)), + ); + noteSkippedAssertions(preflightIssues, motion.spec.assertions.length - assertions.length); + const targets = collectSamplingTargets(assertions); const times = - preflightIssues.length === 0 ? buildMotionSampleTimes(motion.spec.duration ?? duration) : []; - return { times, ...targets, preflightIssues }; + assertions.length > 0 ? buildMotionSampleTimes(motion.spec.duration ?? duration) : []; + return { times, ...targets, preflightIssues, assertions }; +} + +/** Filtering a spec down must not be silent: a sampled grid no longer says on its own that part of the spec went unevaluated. */ +function noteSkippedAssertions(issues: AnchoredLayoutIssue[], skipped: number): void { + if (skipped <= 0) return; + const suffix = ` ${skipped} assertion(s) naming it were not evaluated.`; + for (const issue of issues) issue.message += suffix; } interface GridSamples { @@ -1000,13 +1020,13 @@ export async function runAuditGrid( const seekLoopMs = Date.now() - seekLoopStart; let motionIssues = plan.preflightIssues; - if (motion.kind === "valid" && motionIssues.length === 0 && collected.motionFrames.length > 0) { + if (motion.kind === "valid" && plan.assertions.length > 0 && collected.motionFrames.length > 0) { const evaluated = evaluateMotion( collected.motionFrames, - motion.spec.assertions, + plan.assertions, await driver.getCanvas(), ); - motionIssues = await driver.anchorMotionIssues(evaluated); + motionIssues = [...motionIssues, ...(await driver.anchorMotionIssues(evaluated))]; } const sweepFindings = detectSweepStatic( grid.duration, diff --git a/packages/cli/src/utils/motionAudit.ts b/packages/cli/src/utils/motionAudit.ts index 368de902ee..03d6d09ae0 100644 --- a/packages/cli/src/utils/motionAudit.ts +++ b/packages/cli/src/utils/motionAudit.ts @@ -240,6 +240,32 @@ export function evaluateMotion( * Selectors and liveness scopes the in-page sampler must read for a spec. * Selectors feed the per-element matrix; scopes feed keepsMoving liveness. */ +/** + * What one assertion resolves against: `selectors` are preflighted for ambiguity + * and sampled per element, `scopes` drive liveness. Exhaustive by design — a new + * assertion kind must declare its targets here or fail to compile, because a + * kind missing from this map would have its ambiguous selector reported AND + * still evaluated against an arbitrary first match. + */ +export function assertionTargets(assertion: MotionAssertion): { + selectors: string[]; + scopes: string[]; +} { + switch (assertion.kind) { + case "appearsBy": + case "staysInFrame": + return { selectors: [assertion.selector], scopes: [] }; + case "before": + return { selectors: [assertion.a, assertion.b], scopes: [] }; + case "keepsMoving": + return { selectors: [], scopes: [assertion.withinSelector ?? "*"] }; + default: { + const exhaustive: never = assertion; + return exhaustive; + } + } +} + export function collectSamplingTargets(assertions: MotionAssertion[]): { selectors: string[]; livenessScopes: string[]; @@ -247,19 +273,9 @@ export function collectSamplingTargets(assertions: MotionAssertion[]): { const selectors = new Set(); const scopes = new Set(); for (const assertion of assertions) { - switch (assertion.kind) { - case "appearsBy": - case "staysInFrame": - selectors.add(assertion.selector); - break; - case "before": - selectors.add(assertion.a); - selectors.add(assertion.b); - break; - case "keepsMoving": - scopes.add(assertion.withinSelector ?? "*"); - break; - } + const targets = assertionTargets(assertion); + for (const selector of targets.selectors) selectors.add(selector); + for (const scope of targets.scopes) scopes.add(scope); } return { selectors: [...selectors], livenessScopes: [...scopes] }; }