diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c1b32ce901a..82a43253169 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,7 +79,7 @@ jobs: codex_plugin: ${{ steps.filter.outputs.codex_plugin }} gcp_beginframe: ${{ steps.filter.outputs.gcp_beginframe }} studio: ${{ steps.filter.outputs.studio }} - edit_accuracy: ${{ steps.filter.outputs.edit_accuracy }} + edit_accuracy: ${{ steps.edit_accuracy_filter.outputs.edit_accuracy }} steps: # Force git-based change detection instead of the pull_request REST API. # The API path can fail the whole workflow on transient listFiles @@ -189,45 +189,70 @@ jobs: - "packages/studio-server/**" - "bun.lock" - ".github/workflows/ci.yml" + gcp_beginframe: + - "packages/gcp-cloud-run/Dockerfile" + - "packages/aws-lambda/scripts/probe-beginframe.ts" + - "packages/engine/src/services/browserManager.ts" + - "package.json" + - "bun.lock" + - ".github/workflows/ci.yml" + # Only what the edit accuracy bench runs: the Studio it drives, the preview server the CLI hosts it from, + # and the engine and producer modules it captures frames with (the import closure of browserManager, + # frameCapture and both fileServers). Encoders, renders and other CLI commands are not on its path. + # Its own base: the merge queue's is the event's base_sha, since that run's HEAD is not a PR merge commit. + - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4 + id: edit_accuracy_filter + if: steps.edit.outputs.passed != 'true' && (github.event_name == 'pull_request' || github.event_name == 'merge_group') + with: + token: "" + base: ${{ github.event_name == 'merge_group' && github.event.merge_group.base_sha || steps.base.outputs.sha }} + predicate-quantifier: some-with-excludes + filters: | edit_accuracy: - "packages/studio/src/**" + - "packages/studio/index.html" + - "packages/studio/vite.*.ts" + - "packages/studio/public/**" + - "packages/studio/tests/e2e/edit-accuracy/**" + - "packages/studio/tests/e2e/webmcp-host.mjs" - "packages/studio-server/src/**" - "packages/core/src/**" - "packages/core/scripts/**" - - "packages/cli/src/**" - - "packages/player/src/**" + - "packages/core/schemas/**" - "packages/parsers/src/**" - - "packages/producer/src/**" - - "packages/engine/src/**" - - "packages/lint/src/**" + - "packages/player/src/**" - "packages/sdk/src/**" - - "packages/studio/index.html" - - "packages/studio/vite.config.ts" - - "packages/studio/public/**" + - "packages/lint/src/**" + - "packages/cli/src/{cli,help,version}.ts" + - "packages/cli/src/renderSetupWorker*.ts" + - "packages/cli/src/commands/{preview*,add,browser}.ts" + - "packages/cli/src/server/**" + - "packages/cli/src/browser/**" + - "packages/cli/src/utils/**" + - "packages/cli/src/ui/**" + - "packages/cli/src/{background-removal,registry,telemetry}/**" + - "packages/cli/src/capture/{captureCompositionFrame,readBoundedResponse}.ts" + - "packages/cli/src/media-use/lib/config-lock.mjs" - "packages/cli/scripts/**" - - "packages/cli/tsup.config.ts" - - "packages/studio/package.json" - - "packages/cli/package.json" - - "packages/core/package.json" - - "packages/producer/package.json" - - "packages/engine/package.json" - - "packages/parsers/package.json" - - "packages/lint/package.json" - - "packages/sdk/package.json" - - "packages/studio-server/package.json" - - "packages/player/package.json" + - "packages/core/stubs/**" + - "packages/engine/src/**" + - "packages/producer/src/**" + - "packages/producer/stubs/**" + - "packages/producer/scripts/build-hf-early-stub.ts" + - "packages/producer/build*.mjs" + - "packages/player/scripts/verify-runtime-pin.mjs" + - "packages/{studio,studio-server,core,parsers,player,sdk,lint,cli,engine,producer}/{package.json,tsconfig.json,tsup.config.ts}" - "packages/*/package-subpaths.json" - "scripts/package-subpaths.mjs" - - "packages/studio/tests/e2e/edit-accuracy/**" - - "bun.lock" - - ".github/workflows/ci.yml" - gcp_beginframe: - - "packages/gcp-cloud-run/Dockerfile" - - "packages/aws-lambda/scripts/probe-beginframe.ts" - - "packages/engine/src/services/browserManager.ts" + - "scripts/ci/base-file.sh" + - "scripts/ci/install-workspace-dependencies.sh" + - ".github/actions/prepare-ffmpeg-bin/**" - "package.json" - "bun.lock" - ".github/workflows/ci.yml" + - "!**/*.{test,spec}.{ts,tsx,mjs,js}" + - "!**/{__tests__,__fixtures__}/**" + - "!**/*.md" gcp-beginframe-contract: name: GCP BeginFrame image contract @@ -263,7 +288,7 @@ jobs: name: Build needs: changes # A skipped required check passes, so a broken change detection must fail it instead. - if: ${{ !cancelled() && (needs.changes.result != 'success' || needs.changes.outputs.code == 'true' || needs.changes.outputs.cli_bundle == 'true') }} + if: ${{ !cancelled() && (needs.changes.result != 'success' || needs.changes.outputs.code == 'true' || needs.changes.outputs.cli_bundle == 'true' || needs.changes.outputs.edit_accuracy == 'true') }} runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -286,7 +311,7 @@ jobs: - run: bun run build - run: bun run verify:packed-manifests - name: Share the built CLI with the edit accuracy shards - if: github.event_name == 'pull_request' && needs.changes.outputs.edit_accuracy == 'true' + if: needs.changes.outputs.edit_accuracy == 'true' uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: edit-accuracy-cli @@ -1102,17 +1127,21 @@ jobs: /tmp/undo-socket-pool.json if-no-files-found: error - # Manual editing accuracy in the built Studio, gated against the base branch's baseline.json. + # Manual editing accuracy in the built Studio, gated against the base branch's baseline.json. A pull request + # runs the `pr` slice in 5 shards; the merge queue runs the full grid in 20, so nothing reaches main unmeasured. + # The gate job below repeats both shard counts. studio-edit-accuracy: - name: "Studio: edit accuracy (${{ matrix.shard }}/20)" + name: "Studio: edit accuracy (${{ matrix.shard }}/${{ strategy.job-total }})" needs: [changes, build] - if: github.event_name == 'pull_request' && needs.changes.outputs.edit_accuracy == 'true' + if: needs.changes.outputs.edit_accuracy == 'true' runs-on: ubuntu-latest timeout-minutes: 45 strategy: fail-fast: false matrix: - shard: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20] + shard: ${{ fromJSON(github.event_name == 'merge_group' && '[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20]' || '[1, 2, 3, 4, 5]') }} + env: + EDIT_ACCURACY_GRID: ${{ github.event_name == 'merge_group' && 'full' || 'pr' }} steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 @@ -1136,8 +1165,8 @@ jobs: - name: Run this shard, then re-run twice each case whose verdict differs from the base branch run: | set -euo pipefail - bench() { bun run --cwd packages/studio test:edit-accuracy -- --grid full --jobs 2 "$@"; } - bench --shard "${{ matrix.shard }}/20" --out /tmp/edit-accuracy/run1 + bench() { bun run --cwd packages/studio test:edit-accuracy -- --grid "$EDIT_ACCURACY_GRID" --jobs 2 "$@"; } + bench --shard "${{ matrix.shard }}/${{ strategy.job-total }}" --out /tmp/edit-accuracy/run1 mapfile -t FLIPPED < <(node packages/studio/tests/e2e/edit-accuracy/ratchet.mjs flipped \ /tmp/base-edit-accuracy.json /tmp/edit-accuracy/run1/results.json) if (( ${#FLIPPED[@]} > 0 )); then @@ -1155,7 +1184,7 @@ jobs: studio-edit-accuracy-gate: name: "Studio: edit accuracy gate" needs: [changes, studio-edit-accuracy] - if: ${{ !cancelled() && github.event_name == 'pull_request' && needs.changes.outputs.edit_accuracy == 'true' }} + if: ${{ !cancelled() && needs.changes.outputs.edit_accuracy == 'true' }} runs-on: ubuntu-latest timeout-minutes: 5 permissions: @@ -1176,16 +1205,19 @@ jobs: path: /tmp/edit-accuracy-shards - name: Compare every run with the base branch id: gate - # Smoothness is reported, not gated; a case fails only if it fails 2 of its 3 runs. + # Smoothness and newly passing cases are reported, not gated; a case fails only if it fails 2 of its 3 runs. + env: + SHARDS: ${{ github.event_name == 'merge_group' && 20 || 5 }} run: | set +e mapfile -t RESULTS < <(find /tmp/edit-accuracy-shards -name results.json | sort) - node packages/studio/tests/e2e/edit-accuracy/ratchet.mjs gate /tmp/base-edit-accuracy.json \ + node packages/studio/tests/e2e/edit-accuracy/ratchet.mjs gate --shards "$SHARDS" /tmp/base-edit-accuracy.json \ packages/studio/tests/e2e/edit-accuracy/baseline.json /tmp/edit-accuracy-gate "${RESULTS[@]}" echo "exit_code=$?" >> "$GITHUB_OUTPUT" cat /tmp/edit-accuracy-gate/comment.md >> "$GITHUB_STEP_SUMMARY" || true - name: Post sticky comment - # Fork PRs get a read-only token; the gate below still fires. + # Fork PRs get a read-only token; the gate below still fires. The merge queue has no PR to comment on. + if: github.event_name == 'pull_request' continue-on-error: true uses: marocchino/sticky-pull-request-comment@773744901bac0e8cbb5a0dc842800d45e9b2b405 # v2.9.4 with: diff --git a/bun.lock b/bun.lock index f8e9f71e31e..2a34d5f276a 100644 --- a/bun.lock +++ b/bun.lock @@ -17,6 +17,7 @@ "lefthook": "^2.1.4", "oxfmt": "^0.41.0", "oxlint": "^1.56.0", + "picomatch": "^4.0.4", "tsx": "^4.21.0", "typescript": "^5.0.0", "vitest": "^4.1.11", diff --git a/package.json b/package.json index 3973612f0f9..3c1f5fb2780 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "player:perf": "bun run --filter @hyperframes/player perf", "format:check": "oxfmt --check .", "knip": "knip", - "test:scripts": "node --import tsx --test scripts/animejs-v4-guidance.test.mjs scripts/check-tracked-artifacts.test.mjs scripts/check-registry-set-delta.test.mjs scripts/check-no-main-deletions.test.mjs scripts/check-pr-captures.test.mjs scripts/check-comment-citations.test.mjs scripts/comments-workflow.test.mjs scripts/comment-ratchet.test.mjs scripts/check-docs-snippet-motion.test.mjs scripts/registry-target-paths.test.mjs scripts/check-workspace-contracts.test.mjs scripts/check-media-use-copy-parity.test.mjs scripts/check-svg-sanitize-parity.test.mjs scripts/check-media-use-svg-sanitize-generated.test.mjs scripts/check-package-cycles.test.mjs scripts/check-cli-process-ownership.test.mjs scripts/check-studio-copy-dashes.test.mjs scripts/check-large-files.test.mjs scripts/prune-fallow-audit-caches.test.mjs scripts/package-subpaths.test.mjs scripts/validate-release-channel.test.mjs scripts/clawhub-sync.test.mjs scripts/publish-workflow.test.mjs scripts/pr-edit-concurrency.test.mjs scripts/merge-queue-workflows.test.mjs scripts/install-workspace-dependencies.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts scripts/catalog-payload-assets.test.ts scripts/host-registry-assets.test.ts scripts/catalog-preview-temp.test.ts scripts/backfill-block-previews.test.ts scripts/catalog-hosted-files.test.ts scripts/player-cdn-pin.test.ts scripts/studio-runtime-smoke.test.mjs scripts/verify-packed-manifests.test.mjs scripts/lint-skills.test.mjs scripts/creator-editing-recipes.test.mjs packages/gcp-cloud-run/check-dockerfile-workspaces.test.mjs packages/core/scripts/writeGeneratedFile.test.ts scripts/catalog-publication.test.mjs scripts/ci/resolve-workflow-pr.test.mjs scripts/ci/base-file.test.mjs scripts/check-catalog-source-pr.test.mjs scripts/generate-registry-items.test.ts scripts/catalog-drift.test.ts scripts/catalog-fetch-mirror.test.ts scripts/catalog-script-inlining.test.ts scripts/catalog-detail.test.ts scripts/generate-catalog-pages.test.ts scripts/verify-catalog-payloads.test.ts scripts/registry-skill-files.test.ts scripts/creator-editing-capabilities.test.mjs scripts/generate-catalog-previews.test.ts scripts/registry-primitive-payloads.test.ts registry/components/pan-stations/pan-stations.test.mjs registry/components/filmed-screen/filmed-screen.test.mjs && vitest run scripts/catalog/ scripts/contrastRatchet.test.ts scripts/generate-catalog-payloads.test.ts", + "test:scripts": "node --import tsx --test scripts/animejs-v4-guidance.test.mjs scripts/check-tracked-artifacts.test.mjs scripts/check-registry-set-delta.test.mjs scripts/check-no-main-deletions.test.mjs scripts/check-pr-captures.test.mjs scripts/check-comment-citations.test.mjs scripts/comments-workflow.test.mjs scripts/comment-ratchet.test.mjs scripts/check-docs-snippet-motion.test.mjs scripts/registry-target-paths.test.mjs scripts/check-workspace-contracts.test.mjs scripts/check-media-use-copy-parity.test.mjs scripts/check-svg-sanitize-parity.test.mjs scripts/check-media-use-svg-sanitize-generated.test.mjs scripts/check-package-cycles.test.mjs scripts/check-cli-process-ownership.test.mjs scripts/check-studio-copy-dashes.test.mjs scripts/check-large-files.test.mjs scripts/prune-fallow-audit-caches.test.mjs scripts/package-subpaths.test.mjs scripts/validate-release-channel.test.mjs scripts/clawhub-sync.test.mjs scripts/publish-workflow.test.mjs scripts/pr-edit-concurrency.test.mjs scripts/merge-queue-workflows.test.mjs scripts/install-workspace-dependencies.test.mjs scripts/check-edit-accuracy-filter.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts scripts/catalog-payload-assets.test.ts scripts/host-registry-assets.test.ts scripts/catalog-preview-temp.test.ts scripts/backfill-block-previews.test.ts scripts/catalog-hosted-files.test.ts scripts/player-cdn-pin.test.ts scripts/studio-runtime-smoke.test.mjs scripts/verify-packed-manifests.test.mjs scripts/lint-skills.test.mjs scripts/creator-editing-recipes.test.mjs packages/gcp-cloud-run/check-dockerfile-workspaces.test.mjs packages/core/scripts/writeGeneratedFile.test.ts scripts/catalog-publication.test.mjs scripts/ci/resolve-workflow-pr.test.mjs scripts/ci/base-file.test.mjs scripts/check-catalog-source-pr.test.mjs scripts/generate-registry-items.test.ts scripts/catalog-drift.test.ts scripts/catalog-fetch-mirror.test.ts scripts/catalog-script-inlining.test.ts scripts/catalog-detail.test.ts scripts/generate-catalog-pages.test.ts scripts/verify-catalog-payloads.test.ts scripts/registry-skill-files.test.ts scripts/creator-editing-capabilities.test.mjs scripts/generate-catalog-previews.test.ts scripts/registry-primitive-payloads.test.ts registry/components/pan-stations/pan-stations.test.mjs registry/components/filmed-screen/filmed-screen.test.mjs && vitest run scripts/catalog/ scripts/contrastRatchet.test.ts scripts/generate-catalog-payloads.test.ts", "typecheck:scripts": "tsc --noEmit -p scripts/tsconfig.json", "test:skills": "HYPERFRAMES_MEDIA_HOME_REQUIRED=1 node --import ./packages/cli/scripts/test-home.mjs --test 'skills/**/*.test.mjs' 'packages/cli/src/media-use/**/*.test.mjs'", "generate:previews": "tsx scripts/generate-template-previews.ts", @@ -78,6 +78,7 @@ "lefthook": "^2.1.4", "oxfmt": "^0.41.0", "oxlint": "^1.56.0", + "picomatch": "^4.0.4", "tsx": "^4.21.0", "typescript": "^5.0.0", "vitest": "^4.1.11", diff --git a/packages/studio/tests/e2e/edit-accuracy/grid.mjs b/packages/studio/tests/e2e/edit-accuracy/grid.mjs index 08dfc17f853..4ea2341f582 100644 --- a/packages/studio/tests/e2e/edit-accuracy/grid.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/grid.mjs @@ -130,7 +130,11 @@ const product = (axes) => const caseId = (c) => [c.gesture, c.gsap, c.placement, `r${c.rotation}`, c.nesting, `z${c.zoom}`].join("-"); -/** `pr` is a smaller slice for CI; `keyframes` is the GSAP-animated set alone, which `full` also runs. */ +/** + * `pr` is the pull request slice, zoom 100 and no rotation in both nestings: it held a regressed case for 10 of the 11 + * pull requests the full gate caught in its first two weeks (the miss was a zoom 50 pick). The merge queue runs `full`. + * `keyframes` is the GSAP-animated set alone, which `full` also runs. + */ export function buildGrid(kind = "full") { if (kind === "keyframes") return keyframedCases(); return ( @@ -139,7 +143,7 @@ export function buildGrid(kind = "full") { .filter((c) => c.placement !== "xpercent" || !["none", "idle"].includes(c.gsap)) .map((c) => ({ id: caseId(c), ...c, other: c.gsap === "idle" })) .concat(dragCases(), keyframedCases()) - .filter((c) => kind !== "pr" || (c.zoom === 100 && c.nesting === "root")) + .filter((c) => kind !== "pr" || (c.zoom === 100 && c.rotation === 0)) ); } diff --git a/packages/studio/tests/e2e/edit-accuracy/ratchet.mjs b/packages/studio/tests/e2e/edit-accuracy/ratchet.mjs index da880d82e72..d96320390c8 100644 --- a/packages/studio/tests/e2e/edit-accuracy/ratchet.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/ratchet.mjs @@ -1,7 +1,7 @@ #!/usr/bin/env node // Edit accuracy gate against the base branch's baseline.json; smoothness is reported, never gated. -// `flipped ` lists cases to re-run twice; `gate ` judges 2 of 3. -// A case whose runs disagree is listed as unstable. +// `flipped ` lists cases to re-run twice; `gate [--shards n] ` +// judges 2 of 3. A case whose runs disagree is listed as unstable. A newly passing case is reported, never gated. import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -57,9 +57,12 @@ export function bankable(runs) { */ export const QUARANTINED = {}; -/** Every run of every case: each shard's run plus the re-runs of the cases it flipped. */ +/** + * Every run of every case: each shard's run plus the re-runs of the cases it flipped. `scope`, for a run of + * part of the grid, limits the base branch's passing count to the cases that ran. + */ // fallow-ignore-next-line complexity -export function gate(base, head, runs, quarantine = QUARANTINED) { +export function gate(base, head, runs, quarantine = QUARANTINED, scope = null) { const seen = new Map(); for (const r of runs) seen.set(r.id, [...(seen.get(r.id) ?? []), entry(r)]); const all = [...seen].map(([id, entries]) => { @@ -78,7 +81,7 @@ export function gate(base, head, runs, quarantine = QUARANTINED) { return { id, fixer, passed: c?.passed ?? null, runs: c?.entries.map(accurate) ?? [] }; }), basePassing: Object.entries(base.cases).filter( - ([id, e]) => !Object.hasOwn(quarantine, id) && accurate(e), + ([id, e]) => !Object.hasOwn(quarantine, id) && (!scope || scope.has(id)) && accurate(e), ).length, headPassing: passing.length, headSmooth: passing.filter((c) => mostlySmooth(c.entries)).length, @@ -89,15 +92,13 @@ export function gate(base, head, runs, quarantine = QUARANTINED) { newlyPassing: passing.filter((c) => !c.basePassed).map((c) => c.id), unbanked: passing.filter((c) => !accurate(head.cases[c.id])).map((c) => c.id), overclaimed: cases.filter((c) => !c.passed && accurate(head.cases[c.id])).map((c) => c.id), - missing: Object.keys(base.cases).filter((id) => !seen.has(id)), + missing: Object.keys(base.cases).filter((id) => (!scope || scope.has(id)) && !seen.has(id)), }; const reasons = [ result.regressed.length && `${result.regressed.length} case(s) that pass on the base branch fail here`, result.headPassing < result.basePassing && `the passing count fell from ${result.basePassing} to ${result.headPassing}`, - result.unbanked.length && - `${result.unbanked.length} newly passing case(s) are not banked in baseline.json`, result.overclaimed.length && `baseline.json marks ${result.overclaimed.length} failing case(s) as passing`, ].filter(Boolean); @@ -124,7 +125,10 @@ export function comment(g) { "", ...list("Regressed", g.regressed), ...list("Newly passing", g.newlyPassing), - ...list("Not banked (commit the artifact's baseline.json)", g.unbanked), + ...list( + "Not banked yet, reported and not gated (bank a full-grid run's baseline.json)", + g.unbanked, + ), ...list("Marked passing in baseline.json but failing", g.overclaimed), ...list("In the base grid but not run", g.missing), `**Quarantined, measured but not gated** (${g.quarantined.length})`, @@ -146,8 +150,14 @@ export function comment(g) { // Strict: CI writes {"cases":{}} for a missing base, so a file that does not parse is broken, not empty. const readJson = (path) => JSON.parse(readFileSync(path, "utf8")); +/** The grid a shard ran, from its first run's meta ("pr shard 2/5"); re-runs record their filter instead. */ +const gridOf = (results) => results.find((r) => !r.meta.rerun)?.meta.grid.split(" ")[0]; + // fallow-ignore-next-line complexity -function main([command, basePath, ...rest]) { +function main(argv) { + const shardsAt = argv.indexOf("--shards"); + const shards = shardsAt === -1 ? null : Number(argv.splice(shardsAt, 2)[1]); + const [command, basePath, ...rest] = argv; const base = readJson(basePath); if (command === "flipped") { for (const id of flipped(base, readJson(rest[0]).cases)) console.log(id); @@ -156,22 +166,31 @@ function main([command, basePath, ...rest]) { const [headPath, out, ...resultPaths] = rest; const runs = resultPaths.map((p) => readJson(p)); if (!runs.length) throw new Error("no results.json from any shard"); + const cases = runs.flatMap((r) => r.cases); + const full = gridOf(runs) === "full"; const g = gate( base, readJson(headPath), - runs.flatMap((r) => r.cases), + cases, + QUARANTINED, + full ? null : new Set(cases.map((c) => c.id)), ); + // A shard that died uploads nothing, so the count of first runs is what shows it. + const firstRuns = runs.filter((r) => !r.meta.rerun).length; + if (shards !== null && firstRuns !== shards) { + g.reasons.push(`${firstRuns} of ${shards} shards reported results`); + g.ok = false; + } mkdirSync(out, { recursive: true }); writeFileSync(join(out, "comment.md"), comment(g)); writeFileSync(join(out, "gate.json"), JSON.stringify(g, null, 1)); - // A baseline.json to commit when cases newly pass. - writeReport( - out, - { ...runs[0].meta, grid: "full (CI)" }, - bankable(runs.flatMap((r) => r.cases)), - 0, - ); + // A baseline.json to commit when cases newly pass; only a full grid holds every case. + if (full) writeReport(out, { ...runs[0].meta, grid: "full (CI)" }, bankable(cases), 0); console.log(comment(g)); + if (g.unbanked.length && process.env.GITHUB_ACTIONS) + console.log( + `::notice title=Edit accuracy::${g.unbanked.length} newly passing case(s) are not banked in baseline.json; not gated.`, + ); return g.ok ? 0 : 1; } diff --git a/packages/studio/tests/e2e/edit-accuracy/ratchet.test.mjs b/packages/studio/tests/e2e/edit-accuracy/ratchet.test.mjs index 24bdadbfbc8..e39e35271c5 100644 --- a/packages/studio/tests/e2e/edit-accuracy/ratchet.test.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/ratchet.test.mjs @@ -62,7 +62,7 @@ describe("quarantine", () => { const fresh = baseline({}); expect(gate(fresh, fresh, [run("a"), run("b")], q)).toMatchObject({ unbanked: ["b"], - ok: false, + ok: true, }); expect( gate(base, baseline({ a: good, b: good }), [run("a", 9), run("b")], q).overclaimed, @@ -120,7 +120,7 @@ describe("gate", () => { expect(lucky.ok).toBe(true); const real = gate(before, before, [run("a"), run("b"), run("b"), run("b", 9)]); expect(real.unbanked).toEqual(["b"]); - expect(real.ok).toBe(false); + expect(real.ok).toBe(true); }); it("fails a regression that fails 2 of 3 runs", () => { @@ -138,10 +138,12 @@ describe("gate", () => { expect(g.ok).toBe(true); }); - it("fails a newly passing case until baseline.json banks it", () => { + it("reports a newly passing case that baseline.json does not bank, without failing on it", () => { const before = baseline({ a: good, b: { ...good, drop: 9 } }); - expect(gate(before, before, [run("a"), run("b")]).unbanked).toEqual(["b"]); - expect(gate(before, base, [run("a"), run("b")]).ok).toBe(true); + const g = gate(before, before, [run("a"), run("b")]); + expect(g).toMatchObject({ unbanked: ["b"], ok: true }); + expect(comment(g)).toContain("- b"); + expect(gate(before, base, [run("a"), run("b")]).unbanked).toEqual([]); }); it("fails when baseline.json claims a pass the run does not reproduce", () => { @@ -155,6 +157,13 @@ describe("gate", () => { expect(g.missing).toEqual(["b"]); expect(g.reasons.join()).toContain("fell from 2 to 1"); }); + + it("counts only the cases that ran when a run covers part of the grid, still failing a regression", () => { + const slice = new Set(["a"]); + expect(gate(base, base, [run("a")], {}, slice)).toMatchObject({ missing: [], ok: true }); + const g = gate(base, base, [run("a", 3), run("a", 3), run("a")], {}, slice); + expect(g).toMatchObject({ regressed: ["a"], ok: false }); + }); }); describe("teleport in the gate", () => { diff --git a/scripts/check-edit-accuracy-filter.test.mjs b/scripts/check-edit-accuracy-filter.test.mjs new file mode 100644 index 00000000000..2c7dcc089b4 --- /dev/null +++ b/scripts/check-edit-accuracy-filter.test.mjs @@ -0,0 +1,247 @@ +import assert from "node:assert/strict"; +import { existsSync, readFileSync, readdirSync, statSync } from "node:fs"; +import { join, posix } from "node:path"; +import { test } from "node:test"; +import { fileURLToPath } from "node:url"; +import picomatch from "picomatch"; + +// The edit accuracy bench is skipped when its path filter matches nothing, so every repo file the +// bench runs, imports or is built from must be in that filter, or a change to it skips the gate. +const REPO = fileURLToPath(new URL("..", import.meta.url)); +const read = (path) => readFileSync(join(REPO, path), "utf8"); +const isFile = (path) => existsSync(join(REPO, path)) && statSync(join(REPO, path)).isFile(); +const ci = read(".github/workflows/ci.yml"); +const BENCH = "packages/studio/tests/e2e/edit-accuracy"; + +function job(name) { + const start = ci.indexOf(`\n ${name}:\n`); + assert.notEqual(start, -1, `job ${name} not found`); + const rest = ci.slice(start + 1); + const end = rest.slice(1).search(/\n {2}[a-z0-9_-]+:\n/); + return end === -1 ? rest : rest.slice(0, end + 1); +} + +function filterGlobs() { + const block = ci.match(/\n {12}edit_accuracy:\n((?: {14}- .*\n)+)/); + assert.ok(block, "edit_accuracy filter not found"); + return [...block[1].matchAll(/- "(.*)"/g)].map((m) => m[1]); +} + +// Matches a path the way dorny/paths-filter does with `predicate-quantifier: some-with-excludes`. +function matcher(globs) { + const all = globs.map((glob) => picomatch(glob, { dot: true }, true)); + const include = all.filter((m) => !m.state.negated); + const exclude = all.filter((m) => m.state.negated); + return (path) => include.some((m) => m(path)) && exclude.every((m) => m(path)); +} + +const PACKAGES = new Map( + readdirSync(join(REPO, "packages")) + .filter((dir) => isFile(`packages/${dir}/package.json`)) + .map((dir) => [JSON.parse(read(`packages/${dir}/package.json`)).name, `packages/${dir}`]), +); +const BUILT = /(?:^|\/)(?:dist|node_modules)\//; +const SOURCE = /\.(?:[cm]?[jt]sx?|html)$/; +const COMMENT = /\/\*[\s\S]*?\*\/|(^|[^:\w"'`])\/\/.*$/gm; +const TYPE_ONLY = /^\s*(?:import|export) type\b[^;]*;/gm; +const SPECIFIER = /(?:\bfrom|\bimport\(?|\brequire\(|new URL\(|\bsrc=)\s*["'`]([^"'`]+)["'`]/g; +const PATH_LITERAL = /["'`]((?:\.{1,2}\/)*[\w@-][\w.@-]*(?:\/[\w.@-]+)+)["'`]/g; +const TOOL_INPUTS = { tsup: ["tsup.config.ts"], vite: ["vite.config.ts", "index.html"] }; +// cli.ts lazy-loads every command; only the commands the bench runs are followed. +const LAZY_COMMANDS = { file: "packages/cli/src/cli.ts", specifier: /^\.\/commands\// }; + +const RUN_SCRIPT = /^bun run (?:--cwd (\S+) )?(?:--filter '?([^' ]+)'? )?([\w:-]+)$/; +const INSTALL = /bun (?:install|run)\b|install-workspace-dependencies/; + +const packageOf = (path) => path.match(/^packages\/[^/]+/)?.[0] ?? ""; +const scriptsOf = (dir) => JSON.parse(read(repoPath(dir, "package.json"))).scripts ?? {}; + +function repoPath(...parts) { + const path = posix.join(...parts); + return path === "." ? "" : path; +} + +function resolveFile(base) { + const stem = base.replace(/\.[cm]?js$/, ""); + const candidates = [base, `${stem}.ts`, `${stem}.tsx`, `${stem}.mts`, `${base}.ts`, `${base}.js`]; + return [...candidates, `${base}/index.ts`, `${base}/index.js`].find(isFile); +} + +function exportTargets(entry) { + if (typeof entry === "string") return [entry]; + return Object.values(entry ?? {}).flatMap(exportTargets); +} + +function exportEntry(manifest, subpath) { + const main = subpath === "." ? manifest.main : undefined; + return manifest.exports?.[subpath] ?? main; +} + +// A workspace import resolves to the source its package exports; built output is covered by the +// package's own build, which `reach` follows. +function resolveWorkspace(specifier) { + const name = specifier.split("/").slice(0, 2).join("/"); + const dir = PACKAGES.get(name); + if (!dir) return []; + const manifest = JSON.parse(read(`${dir}/package.json`)); + const entry = exportEntry(manifest, `.${specifier.slice(name.length)}`); + return [`${dir}/package.json`, ...exportTargets(entry).map((target) => posix.join(dir, target))]; +} + +function resolveSpecifier(from, specifier) { + const bare = specifier.split("?")[0]; + if (bare.startsWith("@hyperframes/")) return resolveWorkspace(bare); + if (bare.startsWith("/")) return [resolveFile(posix.join(packageOf(from), bare))]; + if (!bare.startsWith(".")) return []; + return [resolveFile(posix.join(posix.dirname(from), bare))]; +} + +function imports(path) { + const text = read(path).replace(COMMENT, "$1").replace(TYPE_ONLY, ""); + const lazy = path === LAZY_COMMANDS.file ? LAZY_COMMANDS.specifier : null; + return [...text.matchAll(SPECIFIER)] + .map((m) => m[1]) + .filter((specifier) => !lazy?.test(specifier)) + .flatMap((specifier) => resolveSpecifier(path, specifier)); +} + +// Files a build script names as string literals, such as the stub it compiles. +function namedFiles(path) { + const roots = [posix.dirname(path), packageOf(path), ""]; + return [...read(path).matchAll(PATH_LITERAL)].flatMap((m) => + roots.map((root) => repoPath(root, m[1])).filter(isFile), + ); +} + +// A file leads to the files it imports, and a file a script runs also to the files it names. +const references = (path, entry) => [ + ...(SOURCE.test(path) ? imports(path) : []), + ...(entry ? namedFiles(path) : []), +]; + +function tsconfigs(path) { + if (!isFile(path)) return []; + const parent = read(path).match(/"extends":\s*"(\.[^"]+)"/)?.[1]; + return parent ? [path, ...tsconfigs(posix.join(posix.dirname(path), parent))] : [path]; +} + +class Closure { + files = new Set(); + packages = new Set(); + scripts = new Set(); + + isNew(path) { + return Boolean(path) && !this.files.has(path) && !BUILT.test(path) && isFile(path); + } + + add(path, entry = false) { + if (!this.isNew(path)) return; + this.files.add(path); + this.reach(packageOf(path)); + for (const next of references(path, entry)) this.add(next); + } + + // The build job builds every package, but only the build of a package the bench reaches can change it. + reach(dir) { + if (!dir || this.packages.has(dir)) return; + this.packages.add(dir); + this.add(`${dir}/package.json`); + tsconfigs(`${dir}/tsconfig.json`).forEach((path) => this.add(path)); + if (scriptsOf(dir).build) this.script(dir, "build"); + } + + script(dir, name) { + const key = `${dir}#${name}`; + if (this.scripts.has(key)) return; + this.scripts.add(key); + this.add(repoPath(dir, "package.json")); + const command = scriptsOf(dir)[name]; + assert.ok(command, `${dir || "root"} has no script ${name}`); + for (const part of command.split(/\s*(?:&&|\|\||;)\s*/)) this.command(dir, part); + } + + command(dir, command) { + const words = command.split(/\s+/).filter((word) => !/^\w+=/.test(word)); + const run = words.join(" ").match(RUN_SCRIPT); + if (run) return this.runs(repoPath(dir, run[1] ?? "."), run[2], run[3]); + this.tool(dir, words); + } + + // A tool's own config, then every file named on its command line. + tool(dir, words) { + for (const input of TOOL_INPUTS[words[0]] ?? []) this.add(posix.join(dir, input)); + for (const word of words) this.add(posix.join(dir, word), true); + } + + runs(dir, filter, name) { + if (!filter) return this.script(dir, name); + const selected = picomatch(filter); + for (const [pkg, pkgDir] of PACKAGES) if (selected(pkg)) this.script(pkgDir, name); + } +} + +// Repo files and package scripts the jobs' steps name directly. +function seed(closure, text) { + for (const m of text.matchAll(/uses: \.\/(\S+)/g)) closure.add(`${m[1]}/action.yml`); + for (const m of text.matchAll(/\b(?:bash|sh|node|bun) ((?:scripts|packages)\/\S+)/g)) + closure.add(m[1], true); + for (const m of text.matchAll(/\bbun run --cwd (\S+) ([\w:-]+)/g)) closure.script(m[1], m[2]); +} + +// The bench drives Studio through the built CLI, as `node cli.js `. +const cliCommands = (texts) => + texts.flatMap((text) => + [...text.matchAll(/\bcli\.js (\w+)|\[cli, "(\w+)"/g)].map((m) => m[1] ?? m[2]), + ); + +function benchInputs() { + const closure = new Closure(); + const steps = ["build", "studio-edit-accuracy", "studio-edit-accuracy-gate"].map(job); + steps.forEach((text) => seed(closure, text)); + for (const name of cliCommands([...steps, read(`${BENCH}/case.mjs`)])) + closure.add(`packages/cli/src/commands/${name}.ts`); + closure.add("packages/cli/src/cli.ts"); + if (steps.some((text) => INSTALL.test(text))) + ["package.json", "bun.lock"].forEach((path) => closure.add(path)); + return [...closure.files].sort(); +} + +test("the edit accuracy filter covers every file the bench runs, imports or is built from", () => { + const inputs = benchInputs(); + for (const path of [ + `${BENCH}/run.mjs`, + `${BENCH}/ratchet.mjs`, + "scripts/ci/install-workspace-dependencies.sh", + "packages/cli/src/commands/preview.ts", + "packages/producer/src/services/frameCapture.ts", + "packages/producer/scripts/build-hf-early-stub.ts", + "packages/producer/stubs/hf-early-stub.ts", + ]) + assert.ok(inputs.includes(path), `${path} should be a bench input`); + const covered = matcher(filterGlobs()); + assert.deepEqual( + inputs.filter((path) => !covered(path)), + [], + ); +}); + +test("the filter keeps the root manifest, lockfile and install script, and drops tests and docs", () => { + const covered = matcher(filterGlobs()); + for (const path of [ + "package.json", + "bun.lock", + "scripts/ci/install-workspace-dependencies.sh", + ".github/actions/prepare-ffmpeg-bin/action.yml", + "packages/studio/src/App.tsx", + "packages/producer/src/services/frameCapture.ts", + "packages/producer/scripts/build-hf-early-stub.ts", + "packages/producer/stubs/hf-early-stub.ts", + ]) + assert.ok(covered(path), `${path} should run the bench`); + for (const path of [ + "packages/studio/src/App.test.tsx", + "packages/core/src/README.md", + "docs/index.mdx", + ]) + assert.ok(!covered(path), `${path} should not run the bench`); +}); diff --git a/scripts/check-test-reachability.mjs b/scripts/check-test-reachability.mjs index 95dcd2d7884..0c70d04c595 100644 --- a/scripts/check-test-reachability.mjs +++ b/scripts/check-test-reachability.mjs @@ -30,14 +30,19 @@ export function matches(path, glob) { return regex.test(path); } +// A negated glob is a final exclusion only under paths-filter's `some-with-excludes`; elsewhere it fails closed. function readFilters(source) { return Object.fromEntries( [...source.matchAll(/^ ([\w-]+):\n((?: - .*\n)+)/gm)].map( - ([, name, body]) => { + ({ 1: name, 2: body, index }) => { const globs = [...body.matchAll(/^ - (["'])(.*?)\1(?:\s+#.*)?$/gm)].map( (match) => match[2], ); - if (globs.some((glob) => glob.startsWith("!"))) + const step = source.slice(source.lastIndexOf("- uses: dorny/paths-filter", index), index); + if ( + globs.some((glob) => glob.startsWith("!")) && + !/^\s+predicate-quantifier: some-with-excludes$/m.test(step) + ) throw new Error("Negated path filters need an explicit reachability model"); return [name, globs]; }, @@ -45,6 +50,10 @@ function readFilters(source) { ); } +const filterMatches = (globs, path) => + globs.some((glob) => !glob.startsWith("!") && matches(path, glob)) && + !globs.some((glob) => glob.startsWith("!") && matches(path, glob.slice(1))); + function jobField(body, key) { return body.match(new RegExp(`^ ${key}: (.*)$`, "m"))?.[1] ?? ""; } @@ -82,7 +91,7 @@ function conditionEnabled(condition, path, filters) { const resolved = condition .replace(/needs\.changes\.outputs\.([\w-]+) == 'true'/g, (_, name) => { if (!filters[name]) throw new Error(`Unknown path filter: ${name}`); - return String(filters[name].some((glob) => matches(path, glob))); + return String(filterMatches(filters[name], path)); }) .replace(/always\(\)/g, "true") .replace(/github.event_name == 'pull_request'/g, "true") diff --git a/scripts/check-test-reachability.test.mjs b/scripts/check-test-reachability.test.mjs index 9d78a6dc7fc..7e2ed4f7064 100644 --- a/scripts/check-test-reachability.test.mjs +++ b/scripts/check-test-reachability.test.mjs @@ -173,6 +173,16 @@ test("quoted comments do not widen path filters", () => { assert.match(check(tree)["scripts/parity.test.mjs"].join("\n"), /CI filters exclude skills/); }); +test("negated filter globs exclude only under some-with-excludes, and fail closed elsewhere", () => { + const tree = fixture("node --test scripts/parity.test.mjs", '"**"\n - "!skills/**"'); + assert.throws(() => check(tree), /Negated path filters/); + tree[".github/workflows/ci.yml"] = tree[".github/workflows/ci.yml"].replace( + " with:\n", + " with:\n predicate-quantifier: some-with-excludes\n", + ); + assert.match(check(tree)["scripts/parity.test.mjs"].join("\n"), /CI filters exclude skills/); +}); + test("runner verbs must match exactly", () => { const tree = fixture("vitest run-anything", '"**"'); assert.match(check(tree)["scripts/parity.test.mjs"][0], /no CI runner/);