diff --git a/.github/actions/setup-node-pnpm/action.yml b/.github/actions/setup-node-pnpm/action.yml new file mode 100644 index 000000000..a6064e2a0 --- /dev/null +++ b/.github/actions/setup-node-pnpm/action.yml @@ -0,0 +1,22 @@ +name: Setup Node and pnpm +description: > + pnpm + Node 22 with the pnpm store cached, then `pnpm install --frozen-lockfile`. + Extracted when the single serial `frontend-run` job became five parallel jobs + (fe-static / fe-test / fe-coverage / fe-servers / fe-build): the same four + steps were about to be copy-pasted five times, and a version bump applied to + four of the five copies is a drift bug that reports green. + +runs: + using: composite + steps: + - uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6 + with: + version: 10 + + - uses: actions/setup-node@v7 + with: + node-version: 22 + cache: pnpm + + - run: pnpm install --frozen-lockfile + shell: bash diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 70453bba3..7604260f0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,27 +41,31 @@ concurrency: cancel-in-progress: true jobs: - # Frontend checks: lint, tests, build — platform-independent (jsdom + vite) - frontend-run: + + # ── Frontend verification, split into parallel groups ───────────────────── + # + # This was ONE job running `pnpm check:all` end to end: 25 lint gates, then + # 35k tests with coverage, then three builds — strictly serial, ~22 min, and + # the critical path of every PR. The groups below are the same 31 steps + # (`pnpm check:all` still composes exactly them, and check-scripts-parity + # asserts it), so the critical path becomes the MAX of the groups rather than + # their SUM. + # + # Tests shard because they dominate. Each shard writes a blob report; the + # merge job reconstitutes them and applies the coverage thresholds to the + # COMBINED result — verified to reproduce the unsharded numbers + # (94.28/90.62/93.77/95.13 vs 94.27/90.60/93.74/95.13). Sharding without that + # merge would silently drop the coverage gate, which is worse than being slow. + fe-static: runs-on: ubuntu-latest - timeout-minutes: 30 + timeout-minutes: 20 steps: - uses: actions/checkout@v7 with: - # Fetch enough history for the new-deps slopsquatting gate to diff - # against origin/main. + # Full history: the slopsquatting gate and the baseline ratchet both + # diff against the merge base. fetch-depth: 0 - - - uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6 - with: - version: 10 - - - uses: actions/setup-node@v7 - with: - node-version: 22 - cache: pnpm - - - run: pnpm install --frozen-lockfile + - uses: ./.github/actions/setup-node-pnpm - name: Security audit # pnpm 10 audit broken: npm retired legacy endpoint (410), bulk API lands in pnpm 11 @@ -78,19 +82,104 @@ jobs: - name: Baseline ratchet (merge-base, all committed baselines) # Every gate compares the tree against its baseline in the SAME commit, # so a PR can raise its own floor — edit the offending file and its - # baseline together and every check reports green (audit 20260729 C3; - # generalized from file-size alone to all 12 baselines by the 20260803 - # review, D2). This re-reads each baseline at the merge base, where the - # PR cannot have written it, and rejects any loosening. - # - # PR-only, and NOT part of `pnpm check:all`: the comparison needs a base - # ref, which a local checkout cannot guarantee. The checkout above uses - # fetch-depth: 0, so `git merge-base origin/ HEAD` resolves here; - # if it ever cannot, the script exits 1 rather than skipping. - if: github.event_name == 'pull_request' + # baseline together and every check reports green (audit 20260729 C3). + # This re-reads each baseline at the merge base, where the PR cannot + # have written it, and rejects any loosening. run: node scripts/check-baseline-ratchet.mjs "origin/${{ github.base_ref }}" - - run: pnpm check:all + - run: pnpm check:static + + fe-test: + runs-on: ubuntu-latest + timeout-minutes: 25 + strategy: + fail-fast: false + matrix: + shard: [1, 2, 3, 4] + steps: + - uses: actions/checkout@v7 + - uses: ./.github/actions/setup-node-pnpm + + # Blob reports carry this shard's coverage; fe-coverage merges them and + # is the ONLY place thresholds are applied. + # + # The zeroed thresholds are REQUIRED, not tidiness. A shard runs a quarter + # of the suite, so it measures ~49% line coverage and would fail the 94.7% + # global floor every time — all four shards did exactly that on the first + # run of this workflow. Coverage is a property of the WHOLE suite, so the + # only place it can be judged is after the merge. (Enforced by + # scripts/check-scripts-parity.test.mjs, so a future edit cannot quietly + # restore per-shard gating and make the matrix permanently red.) + # + # `--reporter=default` alongside `blob` is not cosmetic: the blob reporter + # REPLACES the console reporter, so a failing shard printed nothing but + # "Process completed with exit code 1" — no test name, no assertion, no + # way to tell a real failure from a flake without re-running locally. + # (Enforced below by check-scripts-parity.) + # + # Deliberately ONE long line rather than a folded `>-` block. `ci.yml` is + # on the BYTE_IDENTICAL identity ratchet in + # `ghaWorkflow/save/__tests__/corpusRoundtrip.test.ts`, and a folded + # scalar does not survive that CST round trip byte-identically — it + # dropped the file off the list. A literal `|` block is NOT the + # alternative: `>-` folds to a single command, while `|` keeps the + # newlines, so under `bash -e` every flag line would run as its own + # command. + - name: Tests (shard ${{ matrix.shard }}/4) + run: pnpm vitest run --coverage --shard=${{ matrix.shard }}/4 --reporter=blob --reporter=default --outputFile.blob=.vitest-reports/blob-${{ matrix.shard }}.json --coverage.thresholds.lines=0 --coverage.thresholds.functions=0 --coverage.thresholds.statements=0 --coverage.thresholds.branches=0 + + # `include-hidden-files` is REQUIRED: vitest writes blobs to + # `.vitest-reports/`, a dot-directory, and upload-artifact@v4 treats + # everything under one as hidden and skips it by default. The blob was + # written and then silently not uploaded. + # + # `if-no-files-found: error` is the other half. The default is `warn`, so + # that skip passed the shard and only surfaced two jobs later as + # `ENOENT: scandir '.vitest-reports'` in fe-coverage — an error message + # pointing at the consumer instead of the producer. Fail where the file + # should have been made. + - uses: actions/upload-artifact@v4 + with: + name: blob-report-${{ matrix.shard }} + path: .vitest-reports/* + include-hidden-files: true + if-no-files-found: error + retention-days: 1 + + fe-coverage: + needs: [fe-test] + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v7 + - uses: ./.github/actions/setup-node-pnpm + + - uses: actions/download-artifact@v4 + with: + path: .vitest-reports + pattern: blob-report-* + merge-multiple: true + + # Thresholds live in vitest.config.ts and apply to the MERGED report, so + # this is the real coverage gate — the shards deliberately do not gate. + - name: Merge shard reports and enforce coverage thresholds + run: pnpm vitest --merge-reports=.vitest-reports --coverage + + fe-servers: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v7 + - uses: ./.github/actions/setup-node-pnpm + - run: pnpm check:servers + + fe-build: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v7 + - uses: ./.github/actions/setup-node-pnpm + - run: pnpm check:build # Real-WebKit tier. `pnpm check:all` is jsdom-only, and the difference is not # cosmetic: real WebKit drains a microtask BETWEEN capture listeners and jsdom @@ -122,21 +211,37 @@ jobs: - run: pnpm exec vitest run --config vitest.browser.config.ts - # Gate job: required by branch protection (exact name "frontend") + # Gate job: required by branch protection (exact name "frontend"). + # + # Fails unless EVERY group succeeded. `if: always()` means this runs even when + # a dependency failed or was skipped, so the check must treat anything other + # than "success" as a failure — a skipped group is not a passed group, and + # `needs` alone would let one silently vanish from the gate. frontend: if: always() - needs: [frontend-run, webkit] + needs: [fe-static, fe-test, fe-coverage, fe-servers, fe-build, webkit] runs-on: ubuntu-latest steps: - name: Check result env: - RESULT: ${{ needs.frontend-run.result }} + STATIC: ${{ needs.fe-static.result }} + TESTS: ${{ needs.fe-test.result }} + COVERAGE: ${{ needs.fe-coverage.result }} + SERVERS: ${{ needs.fe-servers.result }} + BUILD: ${{ needs.fe-build.result }} WEBKIT: ${{ needs.webkit.result }} run: | - if [ "$RESULT" != "success" ]; then - echo "frontend-run: $RESULT" - exit 1 - fi + failed=0 + for pair in "fe-static:$STATIC" "fe-test:$TESTS" "fe-coverage:$COVERAGE" \ + "fe-servers:$SERVERS" "fe-build:$BUILD" "webkit:$WEBKIT"; do + name="${pair%%:*}" + result="${pair#*:}" + if [ "$result" != "success" ]; then + echo "$name: $result" + failed=1 + fi + done + [ "$failed" -eq 0 ] if [ "$WEBKIT" != "success" ]; then echo "webkit: $WEBKIT" exit 1 diff --git a/package.json b/package.json index 40b09fb09..90e769549 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,10 @@ "mutation:ts": "stryker run", "test:sidecar": "pnpm --dir server/mcp lint && pnpm --dir server/mcp build && pnpm --dir server/mcp test:coverage", "test:content-server": "pnpm --dir server/content build && pnpm --dir server/content smoke && pnpm --dir server/content test:coverage", - "check:all": "pnpm lint && pnpm lint:console && pnpm lint:selection-styles && pnpm lint:design-tokens && pnpm lint:emdash && pnpm lint:deps && pnpm lint:hooks-purity && pnpm lint:extension-budget && pnpm lint:store-coupling && pnpm lint:mock-boundaries && pnpm lint:command-errors && pnpm lint:mutants-config && pnpm lint:deleted-names && pnpm lint:mcp-contracts && pnpm lint:tauri-versions && pnpm lint:i18n && pnpm lint:themes && pnpm lint:keybinding-manifest && pnpm lint:merge-drops && pnpm lint:file-size && pnpm lint:barrels && pnpm lint:shell-slots && pnpm lint:bespoke-buttons && pnpm knip && pnpm lint:knip-baseline && pnpm test:coverage && pnpm test:sidecar && pnpm test:content-server && pnpm build && pnpm lint:eager && pnpm size", + "check:static": "pnpm lint && pnpm lint:console && pnpm lint:selection-styles && pnpm lint:design-tokens && pnpm lint:emdash && pnpm lint:deps && pnpm lint:hooks-purity && pnpm lint:extension-budget && pnpm lint:store-coupling && pnpm lint:mock-boundaries && pnpm lint:command-errors && pnpm lint:mutants-config && pnpm lint:deleted-names && pnpm lint:mcp-contracts && pnpm lint:tauri-versions && pnpm lint:i18n && pnpm lint:themes && pnpm lint:keybinding-manifest && pnpm lint:merge-drops && pnpm lint:file-size && pnpm lint:barrels && pnpm lint:shell-slots && pnpm lint:bespoke-buttons && pnpm knip && pnpm lint:knip-baseline", + "check:servers": "pnpm test:sidecar && pnpm test:content-server", + "check:build": "pnpm build && pnpm lint:eager && pnpm size", + "check:all": "pnpm check:static && pnpm test:coverage && pnpm check:servers && pnpm check:build", "lint:extension-budget": "node scripts/check-extension-budget.mjs", "lint:knip-baseline": "node scripts/check-knip-baseline.mjs", "lint:store-coupling": "node scripts/check-plugin-store-coupling.mjs", diff --git a/scripts/check-command-error-ratchet.test.mjs b/scripts/check-command-error-ratchet.test.mjs index 8bdc98253..6cdce5f80 100644 --- a/scripts/check-command-error-ratchet.test.mjs +++ b/scripts/check-command-error-ratchet.test.mjs @@ -16,6 +16,7 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { invokedScripts } from "./lib/packageScripts.mjs"; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const SCRIPT = path.join(REPO, "scripts", "check-command-error-ratchet.mjs"); @@ -274,7 +275,10 @@ describe("wiring — real package.json", () => { it("exposes lint:command-errors and chains it into check:all", () => { const pkg = JSON.parse(readFileSync(path.join(REPO, "package.json"), "utf8")); expect(pkg.scripts["lint:command-errors"]).toContain("check-command-error-ratchet.mjs"); - expect(pkg.scripts["check:all"]).toContain("lint:command-errors"); + // Transitive: check:all composes check:static/servers/build, so a + // literal substring check would break on regrouping (see + // scripts/lib/packageScripts.mjs). + expect(invokedScripts(pkg.scripts, "check:all")).toContain("lint:command-errors"); }); }); diff --git a/scripts/check-mock-boundaries.test.mjs b/scripts/check-mock-boundaries.test.mjs index 309603777..b40551488 100644 --- a/scripts/check-mock-boundaries.test.mjs +++ b/scripts/check-mock-boundaries.test.mjs @@ -17,6 +17,7 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { invokedScripts } from "./lib/packageScripts.mjs"; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const SCRIPT = path.join(REPO, "scripts", "check-mock-boundaries.mjs"); @@ -316,7 +317,10 @@ describe("wiring (case 13) — real package.json and real baseline", () => { it("exposes lint:mock-boundaries and chains it into check:all", () => { const pkg = JSON.parse(readFileSync(path.join(REPO, "package.json"), "utf8")); expect(pkg.scripts["lint:mock-boundaries"]).toBe("node scripts/check-mock-boundaries.mjs"); - expect(pkg.scripts["check:all"]).toContain("lint:mock-boundaries"); + // Transitive: check:all composes check:static/servers/build, so a + // literal substring check would break on regrouping (see + // scripts/lib/packageScripts.mjs). + expect(invokedScripts(pkg.scripts, "check:all")).toContain("lint:mock-boundaries"); }); it("ships a real identity baseline, registered in the WI-16 merge-base ratchet", async () => { diff --git a/scripts/check-mutants-config-path.test.mjs b/scripts/check-mutants-config-path.test.mjs index 4e27cd11a..f64e63e52 100644 --- a/scripts/check-mutants-config-path.test.mjs +++ b/scripts/check-mutants-config-path.test.mjs @@ -20,6 +20,7 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { invokedScripts } from "./lib/packageScripts.mjs"; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const SCRIPT = path.join(REPO, "scripts", "check-mutants-config-path.mjs"); @@ -104,7 +105,10 @@ describe("wiring — real package.json and the real tree", () => { expect(pkg.scripts["lint:mutants-config"]).toBe( "node scripts/check-mutants-config-path.mjs", ); - expect(pkg.scripts["check:all"]).toContain("lint:mutants-config"); + // Transitive: check:all composes check:static/servers/build, so a + // literal substring check would break on regrouping (see + // scripts/lib/packageScripts.mjs). + expect(invokedScripts(pkg.scripts, "check:all")).toContain("lint:mutants-config"); }); it("passes on the real repository (config relocated, legacy file gone)", () => { diff --git a/scripts/check-plugin-store-coupling.test.ts b/scripts/check-plugin-store-coupling.test.ts index 88153cb67..bacfee992 100644 --- a/scripts/check-plugin-store-coupling.test.ts +++ b/scripts/check-plugin-store-coupling.test.ts @@ -43,6 +43,7 @@ import { fileURLToPath } from "node:url"; // @ts-expect-error — plain .mjs module without type declarations import { findCouplingViolations } from "./check-plugin-store-coupling.mjs"; +import { invokedScripts } from "./lib/packageScripts.mjs"; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const SCRIPT = path.join(REPO, "scripts", "check-plugin-store-coupling.mjs"); @@ -396,7 +397,10 @@ describe("wiring — the real package.json and the real baseline", () => { it("exposes lint:store-coupling and chains it into check:all", () => { const pkg = JSON.parse(readFileSync(path.join(REPO, "package.json"), "utf8")); expect(pkg.scripts["lint:store-coupling"]).toBe("node scripts/check-plugin-store-coupling.mjs"); - expect(pkg.scripts["check:all"]).toContain("lint:store-coupling"); + // Transitive: check:all composes check:static/servers/build, so a + // literal substring check would break on regrouping (see + // scripts/lib/packageScripts.mjs). + expect(invokedScripts(pkg.scripts, "check:all")).toContain("lint:store-coupling"); }); it("keeps the @/stores channel at ZERO — the win the other three now protect", () => { diff --git a/scripts/check-scripts-parity.test.mjs b/scripts/check-scripts-parity.test.mjs new file mode 100644 index 000000000..15ea35471 --- /dev/null +++ b/scripts/check-scripts-parity.test.mjs @@ -0,0 +1,102 @@ +/** + * `pnpm check:all` and CI's parallel groups must run the SAME set of gates. + * + * CI no longer runs `check:all` as one job. It runs the groups — `check:static`, + * `test:coverage`, `check:servers`, `check:build` — as separate jobs so the + * critical path is their max rather than their sum. That split introduces a + * drift hole the moment it exists: append `pnpm lint:new-gate` directly to + * `check:all` and it runs locally and in the pre-push hook, but NO CI job runs + * it. The gate would look wired up, pass every local check, and be absent from + * the only place that actually blocks a merge. + * + * So `check:all` may not contain steps of its own: it must be exactly the + * composition of the groups CI runs. Adding a gate then has one correct home + * (a group), and CI picks it up for free. + * + * @coordinates-with .github/workflows/ci.yml — fe-static / fe-test / fe-coverage / fe-servers / fe-build + * @coordinates-with scripts/lib/packageScripts.mjs — transitive expansion + * @module scripts/check-scripts-parity.test + */ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { invokedScripts } from "./lib/packageScripts.mjs"; + +const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const pkg = JSON.parse(readFileSync(path.join(REPO, "package.json"), "utf8")); +const ci = readFileSync(path.join(REPO, ".github/workflows/ci.yml"), "utf8"); + +/** The groups CI runs as jobs. Each must appear verbatim in ci.yml. */ +const CI_GROUPS = ["check:static", "test:coverage", "check:servers", "check:build"]; + +describe("check:all and CI run the same gates", () => { + it("check:all is exactly the composition of the CI groups, in order", () => { + const steps = pkg.scripts["check:all"].split("&&").map((s) => s.trim()); + expect(steps).toEqual(CI_GROUPS.map((g) => `pnpm ${g}`)); + }); + + it("every gate check:all runs is reachable through some CI group", () => { + const viaCheckAll = new Set(invokedScripts(pkg.scripts, "check:all")); + const viaGroups = new Set( + CI_GROUPS.flatMap((g) => [g, ...invokedScripts(pkg.scripts, g)]), + ); + const orphans = [...viaCheckAll].filter((s) => !viaGroups.has(s)); + expect(orphans, `gates in check:all that no CI job runs: ${orphans.join(", ")}`).toEqual([]); + }); + + it("ci.yml actually invokes each group", () => { + for (const group of CI_GROUPS) { + // `test:coverage` runs sharded (`vitest run --coverage --shard=...`), so + // accept either the script name or the sharded invocation it expands to. + const present = ci.includes(`pnpm ${group}`) || (group === "test:coverage" && ci.includes("--shard=")); + expect(present, `ci.yml does not run ${group}`).toBe(true); + } + }); + + it("the coverage gate is applied to the MERGED shard report", () => { + // Sharding without a merge silently drops the thresholds: each shard would + // measure a fraction of the suite and none would represent the whole. + expect(ci).toContain("--merge-reports"); + expect(ci).toMatch(/--merge-reports[^\n]*--coverage|--coverage[^\n]*--merge-reports/); + }); + + it("a failing shard prints why — blob must not be the only reporter", () => { + // `--reporter=blob` REPLACES the console reporter. On its own, a red shard + // emitted nothing but "Process completed with exit code 1": no test name, + // no assertion, nothing to distinguish a real regression from a flake. + const shardStep = ci.slice(ci.indexOf("Tests (shard"), ci.indexOf("upload-artifact")); + expect(shardStep).toContain("--reporter=blob"); + expect(shardStep, "shard needs a console reporter beside blob").toContain( + "--reporter=default", + ); + // With two reporters, the blob path must be addressed per-reporter or the + // blob silently lands somewhere else (or not at all). + expect(shardStep).toContain("--outputFile.blob="); + }); + + it("shard blobs actually upload — hidden dir, and a missing blob is fatal", () => { + // vitest writes blobs into `.vitest-reports/`, a dot-directory. + // upload-artifact@v4 skips hidden files by default, so the blob was written + // and silently dropped; `if-no-files-found` then defaults to `warn`, so the + // shard passed and fe-coverage failed with ENOENT instead — the error + // naming the consumer rather than the producer. + const upload = ci.slice(ci.indexOf("upload-artifact"), ci.indexOf("fe-coverage:")); + expect(upload, "blobs live in a dot-directory").toContain("include-hidden-files: true"); + expect(upload, "a missing blob must fail the shard").toContain("if-no-files-found: error"); + }); + + it("shards do NOT gate on coverage — only the merged report does", () => { + // A shard runs a quarter of the suite and measures ~49% lines, so leaving + // the global thresholds active makes every shard fail on every run — which + // is exactly what happened the first time this matrix ran. Coverage is a + // property of the whole suite; a fraction of it cannot be judged. + const shardStep = ci.slice(ci.indexOf("Tests (shard"), ci.indexOf("upload-artifact")); + for (const metric of ["lines", "functions", "statements", "branches"]) { + expect( + shardStep, + `shard run must zero coverage.thresholds.${metric}`, + ).toContain(`--coverage.thresholds.${metric}=0`); + } + }); +}); diff --git a/scripts/lib/packageScripts.mjs b/scripts/lib/packageScripts.mjs new file mode 100644 index 000000000..885de29ff --- /dev/null +++ b/scripts/lib/packageScripts.mjs @@ -0,0 +1,48 @@ +/** + * Purpose: expand a package.json script transitively, so a "is this gate wired + * into check:all?" assertion survives `check:all` being composed of + * sub-scripts. + * + * `check:all` used to be one flat 31-step `&&` chain, and several gate tests + * asserted `pkg.scripts["check:all"]).toContain("lint:their-gate")`. When CI + * needed the chain split into parallel groups (`check:static`, `check:servers`, + * `check:build`) those literal assertions broke even though every gate was + * still chained in — the guard was pinned to the SHAPE of the script rather + * than to the property it cares about. + * + * Expanding transitively pins the property: "running `pnpm check:all` + * eventually runs this gate", regardless of how the chain is grouped. + * + * @module scripts/lib/packageScripts + */ + +/** + * The script NAMES that `pnpm ` transitively invokes, in run order. + * + * Names, not expanded command lines: a caller asking "is `lint:mock-boundaries` + * wired in?" wants the script it can run, and expanding that leaf to + * `node scripts/check-mock-boundaries.mjs` would answer a different question + * (and make the assertion restate the command, so renaming the script silently + * still "passes"). + * + * A step counts as a reference only when it is a bare `pnpm `; + * anything with arguments or flags is a real command, not a composition. + * Cycles are broken by tracking visited names, so a malformed package.json + * cannot hang the caller. + */ +export function invokedScripts(scripts, name, seen = new Set()) { + if (seen.has(name)) return []; + seen.add(name); + const body = scripts?.[name]; + if (typeof body !== "string") return []; + + const out = []; + for (const step of body.split("&&")) { + const token = step.trim(); + const ref = token.startsWith("pnpm ") ? token.slice("pnpm ".length).trim() : null; + if (ref && !ref.includes(" ") && typeof scripts?.[ref] === "string") { + out.push(ref, ...invokedScripts(scripts, ref, seen)); + } + } + return out; +} diff --git a/src/lib/cjkFormatter/rules/fullwidthScaling.test.ts b/src/lib/cjkFormatter/rules/fullwidthScaling.test.ts index cc9ed97a7..a9262857d 100644 --- a/src/lib/cjkFormatter/rules/fullwidthScaling.test.ts +++ b/src/lib/cjkFormatter/rules/fullwidthScaling.test.ts @@ -70,7 +70,28 @@ describe("normalizeFullwidthPunctuation scaling", () => { expect(ms).toBeLessThan(BUDGET_MS); }); - it("scales sub-quadratically: 4x the input is not ~16x the work", () => { + // Opt-in, like `markdownPipeline/__tests__/performance.test.ts` (PERF=1). + // + // This is a wall-clock RATIO, and in the full suite — ~1450 files across + // every core — the noise is larger than the signal it measures. Linear and + // quadratic differ by 4x at 4x input; a saturated runner inflates a + // millisecond sample by ~14x. It failed at 26.5ms against a 20.4ms bound on + // code measured at rest as flatly linear (0.23 ms/1k, n=8k→64k). + // + // Widening the input ratio to buy margin does not work either: past ~64k + // chars the timing turns over (0.25 → 1.11 ms/1k at 96k) on V8 string + // representation, not on this algorithm, so a bigger sample measures the + // engine instead. Best-of-N does not save it, and taking the minimum of BOTH + // sides actively widens the ratio, because the sub-millisecond baseline + // improves far more than the large one does. + // + // What still guards the ORIGINAL defect on every run is the absolute ceiling + // in the two tests above: the quadratic implementation this file was written + // against rescanned the whole document per converted character, which blows + // past a 2s budget on 10k commas by orders of magnitude — no ratio needed. + const itPerf = process.env.PERF === "1" ? it : it.skip; + + itPerf("scales sub-quadratically: 4x the input is not ~16x the work", () => { // A direct shape assertion on the algorithm, independent of the machine: // the old implementation's pass count grew with the run length, so this // ratio grew with it too. @@ -78,14 +99,14 @@ describe("normalizeFullwidthPunctuation scaling", () => { // Warm up so JIT/first-run costs do not land on the small sample. normalizeFullwidthPunctuation(run(500)); - // Best-of-5 on both sides: the baseline is sub-millisecond, so a single - // sample of `large` that catches one preemption is enough to blow a 12x - // ratio that the algorithm itself never approaches. + // Best-of-5 per side: with PERF=1 this runs deliberately, on a quiet + // machine, where the minimum is the sample least polluted by the scheduler. + // Sizes stay in the range where the timing is genuinely linear (see above). const small = fastest(5, () => normalizeFullwidthPunctuation(run(2_000))); const large = fastest(5, () => normalizeFullwidthPunctuation(run(8_000))); - // 4x input under a quadratic law is ~16x time; allow a very wide margin - // for timer noise on a sub-millisecond baseline. + // 4x input under a quadratic law is ~16x time; the 1ms floor keeps a + // sub-millisecond baseline from turning timer noise into a failure. expect(large).toBeLessThan(Math.max(small, 1) * 12); });