Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions packages/cli/src/commands/check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
38 changes: 29 additions & 9 deletions packages/cli/src/utils/checkPipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -175,13 +178,30 @@ async function planMotionSampling(
duration: number,
): Promise<MotionPlan> {
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 {
Expand Down Expand Up @@ -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,
Expand Down
42 changes: 29 additions & 13 deletions packages/cli/src/utils/motionAudit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,26 +240,42 @@ 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[];
} {
const selectors = new Set<string>();
const scopes = new Set<string>();
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] };
}
Loading