diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c5b2b04..c7e4fef 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,8 +4,9 @@ name: CI # actually deploys (`bun run build` on every push to `main`), but until this # workflow existed nothing checked a PR *before* it reached `main` — a broken # build only surfaced as a failed deploy. This job runs the same install and -# build as the deploy, plus `astro check` (types + content schema) and the -# data-layer tests, so a required status check named `build` can gate `main`. +# build as the deploy, plus the benchmark-cell gate (`bun run validate`), +# `astro check` (types + content schema) and the data-layer tests, so a +# required status check named `build` can gate `main`. # # It is deliberately read-only: no deploy, no token use, no writes. @@ -44,6 +45,17 @@ jobs: - name: Install (frozen lockfile) run: bun install --frozen-lockfile + # The benchmark-cell gate. `bun run build` is deliberately lenient — a + # malformed cell is warned about, marked "excluded:invalid" and dropped, + # and the build still exits 0 — because one corrupt cell must not take + # the whole deploy down. That leniency is exactly why a bad cell could + # vanish from the dashboard with every check green: `--validate-only` + # walks the same tree, emits nothing and exits non-zero instead. It runs + # before Build so the first thing to speak is the gate, and because it + # costs under a second. + - name: Validate benchmark cells + run: bun run validate + # `build` = build:data -> astro build -> pagefind. It must run before # `check` because `astro check` type-checks imports of the gitignored # src/data/generated/*.json that build:data emits. diff --git a/.github/workflows/sync-benchmarks.yml b/.github/workflows/sync-benchmarks.yml index 0ef2a56..3d4bd32 100644 --- a/.github/workflows/sync-benchmarks.yml +++ b/.github/workflows/sync-benchmarks.yml @@ -1,17 +1,26 @@ name: Sync Benchmarks -# Acknowledge a benchmark publish from probatorium. +# Verify a benchmark publish from probatorium. # # The publisher (probatorium `mage Publish`) commits the four split files of a # benchmark cell directly to this repo under results////, # then fires a tiny pointer repository_dispatch. The static site reads the # results/ tree directly at BUILD time (it derives all dashboard assets itself), -# so this workflow maintains no derived index/latest mirror. +# so this workflow maintains no derived index/latest mirror, and deployment is +# Cloudflare Workers Builds' job (it rebuilds on every push to `main`, including +# commits created via the GitHub API). # -# Deployment is handled by Cloudflare Workers Builds, which rebuilds and deploys -# the site on every push to `main` (the publish commit IS such a push, including -# commits created via the GitHub API). There is therefore nothing to trigger -# here — this workflow only leaves a log trail so a publish is visible in Actions. +# What is left for this workflow is the part nothing else does: check that the +# cell the pointer describes actually LANDED, and that the tree it landed in +# still validates. Between 2026-06-28 and this change the job was two `echo` +# statements — no checkout, no network call, unconditionally green on all 7 runs +# — while v1.5.9, v1.5.10 and v1.5.11 shipped without ever reaching results/. +# A dispatch for a cell that is not on disk now fails loudly instead. +# +# This is a post-hoc check, not a merge gate: the publisher pushes straight to +# main, so by the time the dispatch arrives the data is already committed. The +# pre-merge gate for anything that arrives by pull request is the same +# `bun run validate` in ci.yml. # # Canonical event type (must match probatorium mage_publish.go's # repository_dispatch event_type): @@ -22,6 +31,23 @@ on: repository_dispatch: types: [benchmark-published] workflow_dispatch: + inputs: + version: + description: "Cell version to assert is present (e.g. v1.6.0). Empty = validate the whole tree only." + required: false + default: "" + date: + description: "Cell date yyyymmdd (required when version is given)." + required: false + default: "" + arch: + description: "Cell arch (x86_64 | arm64)." + required: false + default: x86_64 + run_id: + description: "Run id (run-1 is canonical; the flat layout is accepted too)." + required: false + default: run-1 permissions: contents: read @@ -31,15 +57,77 @@ concurrency: cancel-in-progress: false jobs: - acknowledge: - name: Acknowledge benchmark publish + verify: + name: Verify published benchmark tree runs-on: ubuntu-latest + timeout-minutes: 20 steps: - - name: Note publish + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: main + # Read-only: this job never pushes and never calls the API. + persist-credentials: false + + # The publisher pushes, then dispatches. The two are not atomic, and the + # dispatch can be served before the push is visible to a fresh checkout, + # so re-fetch results/ a few times before declaring a cell missing — + # otherwise this gate would invent failures of its own. + - name: Wait for the published cell to be visible + if: github.event.client_payload.version != '' || github.event.inputs.version != '' + env: + P_VERSION: ${{ github.event.client_payload.version || github.event.inputs.version }} + P_DATE: ${{ github.event.client_payload.date || github.event.inputs.date }} + P_ARCH: ${{ github.event.client_payload.arch || github.event.inputs.arch }} + P_RUN: ${{ github.event.client_payload.run_id || github.event.inputs.run_id }} + run: | + set -euo pipefail + if [ -z "${P_DATE:-}" ] || [ -z "${P_ARCH:-}" ]; then + echo "::error::pointer gave version=${P_VERSION} but date=${P_DATE:-} arch=${P_ARCH:-}; cannot locate the cell" + exit 1 + fi + base="results/${P_VERSION}/${P_DATE}/${P_ARCH}" + run="${P_RUN:-run-1}" + for attempt in 1 2 3 4 5; do + # run-N/ subdir layout, or the flat run-1 layout (files directly + # under /) that walk.ts also accepts. + if [ -f "${base}/${run}/summary.json" ]; then + echo "found ${base}/${run}/summary.json (attempt ${attempt})" + exit 0 + fi + if [ "$run" = "run-1" ] && [ -f "${base}/summary.json" ]; then + echo "found ${base}/summary.json — flat run-1 layout (attempt ${attempt})" + exit 0 + fi + echo "attempt ${attempt}: ${base}/${run}/summary.json not on disk yet; re-fetching main" + sleep 15 + git fetch --quiet origin main + git checkout --quiet origin/main -- results/ + done + echo "::error::publish dispatched for ${P_VERSION}/${P_DATE}/${P_ARCH}/${run} but no summary.json is committed under ${base}" + echo "Tree actually present for that version:" + ls -R "results/${P_VERSION}" 2>/dev/null || echo " (results/${P_VERSION} does not exist)" + exit 1 + + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: latest + + - name: Install (frozen lockfile) + run: bun install --frozen-lockfile + + # Same gate ci.yml runs, over the tree as it stands after the publish: + # every committed cell is opened and validated, and the job fails if any + # of them is malformed or if it somehow examined no cells at all. + - name: Validate benchmark cells + run: bun run validate + + # Deliberately NOT `if: always()`: this line may only appear when the two + # checks above actually passed. + - name: Summary env: - P_VERSION: ${{ github.event.client_payload.version }} - P_DATE: ${{ github.event.client_payload.date }} - P_ARCH: ${{ github.event.client_payload.arch }} + P_VERSION: ${{ github.event.client_payload.version || github.event.inputs.version }} + P_DATE: ${{ github.event.client_payload.date || github.event.inputs.date }} + P_ARCH: ${{ github.event.client_payload.arch || github.event.inputs.arch }} run: | - echo "Benchmark published: ${P_VERSION:-manual}/${P_DATE:-} ${P_ARCH:-}" - echo "Cloudflare Workers Builds deploys automatically on the push to main; no action needed." + echo "Benchmark publish verified: ${P_VERSION:-}/${P_DATE:-} ${P_ARCH:-}" + echo "Cloudflare Workers Builds deploys automatically on the push to main; no deploy action needed here." diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 76e3523..61af9bb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,13 +16,18 @@ bun run demo # same, against a synthesized demo dataset Before opening a pull request, run what CI runs: ```sh +bun run validate # benchmark-cell gate: every committed results/ cell bun run build # build:data -> astro build -> pagefind bun run check # astro check (types + content schema); needs build:data first bun test # data-layer tests ``` -`bun run validate` checks every committed `results/` cell without emitting -anything — it is the gate the benchmark publisher relies on. +`bun run validate` opens every committed `results/` cell without emitting +anything and exits non-zero if one is malformed. It is the benchmark-data +gate, and it is the one check the plain build cannot be: `bun run build` is +deliberately lenient (a bad cell is warned about, excluded and the build still +succeeds) so that one corrupt cell never takes the deploy down. It runs in the +`build` CI job and again in `Sync Benchmarks` when the publisher pushes a run. Documentation pages live in `src/content/docs/**/*.{md,mdx}` and are validated against the frontmatter schema described in the [README](README.md#content-structure). diff --git a/README.md b/README.md index d409dad..eeab842 100644 --- a/README.md +++ b/README.md @@ -75,8 +75,18 @@ Every task is a Bun script (see [`package.json`](package.json)): | `bun run start` | Alias for `bun run dev` | `validate` runs `build-data --validate-only`: it walks and validates every cell but -emits nothing, exiting non-zero if any cell fails validation. It is what gates a -publish. +emits nothing, exiting non-zero if any cell fails validation (and with status 2 if +it somehow examined no cells at all, so a gate that inspected nothing can never +report success). Unlike `build`, it also scans versions listed in +`DEACTIVATED_VERSIONS` — hiding a version from the dashboard is a presentation +decision, not a licence for its committed data to rot. + +It is the benchmark-data gate, and it runs in two places: the `build` job of +[`ci.yml`](.github/workflows/ci.yml) on every pull request and push to `main`, and +[`sync-benchmarks.yml`](.github/workflows/sync-benchmarks.yml) on every publish +dispatch. `bun run build` deliberately does **not** fail on a bad cell — it warns, +marks the cell `excluded:invalid` and carries on, so one corrupt cell cannot take +the whole deploy down. That is precisely why the separate red check exists. ## Tech @@ -171,11 +181,11 @@ by `bun test`. `{ version, arch, date, run_id, path, commit }`). 2. That commit **is** a push to `main`, so **Cloudflare Workers Builds** rebuilds and redeploys the site — which reads the new `results/` tree at build time. - -The only GitHub Actions workflow, -[`sync-benchmarks.yml`](.github/workflows/sync-benchmarks.yml), does **not** trigger -a deploy. It only leaves a log trail (`contents: read`) so a publish is visible in -Actions; the deploy is entirely handled by the push to `main`. +3. [`sync-benchmarks.yml`](.github/workflows/sync-benchmarks.yml) listens for that + dispatch and **verifies** the publish (`contents: read`, no deploy): it asserts + that the cell the pointer describes is actually committed — a dispatch for a + cell that never landed fails the job — and then runs `bun run validate` over + the tree. It does **not** trigger the deploy; the push to `main` already did. ## Deploy & hosting diff --git a/results/README.md b/results/README.md index a98896f..b0558d0 100644 --- a/results/README.md +++ b/results/README.md @@ -27,6 +27,16 @@ directly at build time and derives every dashboard asset itself — there is no committed manifest or `latest/` mirror to maintain. Producers commit the four files of a cell directly, then fire a -`benchmark-published` `repository_dispatch`; `.github/workflows/sync-benchmarks.yml` -pings the Cloudflare Pages deploy hook so the published run appears on the -dashboard. The site build validates every cell it reads and skips malformed ones. +`benchmark-published` `repository_dispatch`. Cloudflare Workers Builds rebuilds +and deploys the site on that push by itself, so nothing here has to trigger a +deploy; `.github/workflows/sync-benchmarks.yml` instead **verifies** the +publish — it asserts that the cell the dispatch points at is really on disk +(a dispatch for a cell that never landed fails the job) and then runs +`bun run validate` over the whole tree. + +The site build itself stays lenient: it validates every cell it reads and +*skips* malformed ones, so one corrupt cell cannot take the deploy down. The +red check is `bun run validate` (`scripts/build-data.ts --validate-only`), +which walks the same tree, emits nothing and exits non-zero if any cell fails +validation — it runs in `ci.yml` on every pull request and push to `main`, and +in `sync-benchmarks.yml` on every publish. diff --git a/scripts/build-data.test.ts b/scripts/build-data.test.ts index c36d5b8..6f82c65 100644 --- a/scripts/build-data.test.ts +++ b/scripts/build-data.test.ts @@ -279,3 +279,123 @@ describe("loader (real preserved sample)", () => { } }); }); + +describe("publish gate (build-data --validate-only)", () => { + const fixture = join(import.meta.dir, "..", "test", "fixtures", "sample-cell"); + const script = join(import.meta.dir, "build-data.ts"); + + /** Lay the sample cell down at results//// in a temp tree. */ + function tree(version: string, date: string, arch: string): { root: string; dir: string; tmp: string } { + const tmp = mkdtempSync(join(tmpdir(), "celeris-gate-")); + const root = join(tmp, "results"); + const dir = join(root, version, date, arch); + mkdirSync(dir, { recursive: true }); + for (const f of ["summary.json", "env.json", "timeseries.json.gz", "histograms.json.gz"]) { + copyFileSync(join(fixture, f), join(dir, f)); + } + return { root, dir, tmp }; + } + + function validate(root: string): { code: number; stderr: string } { + const p = Bun.spawnSync({ + cmd: ["bun", script, "--validate-only"], + cwd: join(import.meta.dir, ".."), + env: { ...process.env, RESULTS_ROOT: root }, + }); + return { code: p.exitCode, stderr: p.stderr.toString() }; + } + + test("accepts a well-formed cell and reports how many it examined", () => { + if (!existsSync(join(fixture, "summary.json"))) return; + const { root, tmp } = tree("v1.4.15", "20260610", "x86_64"); + try { + const { code, stderr } = validate(root); + expect(stderr).toContain("1 cell(s) examined"); + expect(stderr).toContain("0 validation error(s)"); + expect(code).toBe(0); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + test("rejects a summary with the wrong schema major", () => { + if (!existsSync(join(fixture, "summary.json"))) return; + const { root, dir, tmp } = tree("v1.4.15", "20260610", "x86_64"); + try { + const summary = JSON.parse(readFileSync(join(dir, "summary.json"), "utf8")); + summary.schema_version = "4.1"; + writeFileSync(join(dir, "summary.json"), JSON.stringify(summary)); + // Assert the injection landed before trusting the verdict. + expect(JSON.parse(readFileSync(join(dir, "summary.json"), "utf8")).schema_version).toBe("4.1"); + const { code, stderr } = validate(root); + expect(stderr).toContain("summary schema_version must be 5.x"); + expect(code).toBe(1); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + test("rejects unparseable JSON", () => { + if (!existsSync(join(fixture, "summary.json"))) return; + const { root, dir, tmp } = tree("v1.4.15", "20260610", "x86_64"); + try { + writeFileSync(join(dir, "summary.json"), "{ truncated"); + expect(readFileSync(join(dir, "summary.json"), "utf8")).toBe("{ truncated"); + const { code, stderr } = validate(root); + expect(stderr).toContain("summary.json parse failed"); + expect(code).toBe(1); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + test("rejects a cell whose env.json disagrees with its own path", () => { + if (!existsSync(join(fixture, "summary.json"))) return; + const { root, dir, tmp } = tree("v1.4.15", "20260610", "x86_64"); + try { + const env = JSON.parse(readFileSync(join(dir, "env.json"), "utf8")); + env.arch = "arm64"; // the mislabelled-cell corruption that had to be scrubbed by hand + writeFileSync(join(dir, "env.json"), JSON.stringify(env)); + expect(JSON.parse(readFileSync(join(dir, "env.json"), "utf8")).arch).toBe("arm64"); + const { code, stderr } = validate(root); + expect(stderr).toContain("env arch arm64 != path x86_64"); + expect(code).toBe(1); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + test("fails rather than passing when it examined no cells at all", () => { + const tmp = mkdtempSync(join(tmpdir(), "celeris-gate-empty-")); + try { + const root = join(tmp, "results"); + mkdirSync(root, { recursive: true }); + const { code, stderr } = validate(root); + expect(stderr).toContain("0 cell(s) examined"); + expect(stderr).toContain("FATAL validate-only examined 0 cells"); + expect(code).toBe(2); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + test("a malformed cell is still only a warning on the lenient build path", () => { + if (!existsSync(join(fixture, "summary.json"))) return; + const { root, dir, tmp } = tree("v1.4.15", "20260610", "x86_64"); + const out = mkdtempSync(join(tmpdir(), "celeris-gate-out-")); + try { + writeFileSync(join(dir, "summary.json"), "{ truncated"); + const p = Bun.spawnSync({ + cmd: ["bun", script], + cwd: out, // emit into a throwaway cwd, never the repo + env: { ...process.env, RESULTS_ROOT: root }, + }); + // This is the defect the gate exists for: the build tolerates it by design. + expect(p.exitCode).toBe(0); + expect(p.stderr.toString()).toContain("summary.json parse failed"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + rmSync(out, { recursive: true, force: true }); + } + }); +}); diff --git a/scripts/build-data.ts b/scripts/build-data.ts index 10bd3ab..955eb71 100644 --- a/scripts/build-data.ts +++ b/scripts/build-data.ts @@ -10,8 +10,13 @@ * public/data/v//.json — per-version aggregated payload * * Never crashes on empty/partial/malformed data: bad cells are skipped (warned), - * and an empty tree yields valid empty assets. Run with --validate-only to gate - * a publish without emitting (exits non-zero if any cell fails validation). + * and an empty tree yields valid empty assets — a deploy must not be taken down + * by one corrupt cell. The red check is `--validate-only` instead: it walks the + * same tree, emits nothing, and exits non-zero if any cell fails validation (1) + * or if it somehow examined no cells at all (2). `.github/workflows/ci.yml` + * runs it on every pull request and push to main, and + * `.github/workflows/sync-benchmarks.yml` runs it on the publisher's + * benchmark-published dispatch. */ import { mkdirSync, writeFileSync, rmSync, existsSync } from "node:fs"; import { join, dirname, resolve } from "node:path"; @@ -72,6 +77,8 @@ const PUB_DATA = join(repoRoot, "public", "data"); let warnings = 0; let validationErrors = 0; +/** Cells (version/date/arch/run) actually opened and validated this run. */ +let cellsExamined = 0; function warn(msg: string) { warnings++; process.stderr.write(`build-data: WARN ${msg}\n`); @@ -92,8 +99,11 @@ function build() { rmSync(join(PUB_DATA, "v"), { recursive: true, force: true }); } + // Deactivation is a presentation decision, not a data-integrity one: the + // build hides those versions from the site, but --validate-only still opens + // every committed cell so a hidden one cannot rot unnoticed. const versions = listVersions(root) - .filter((v) => !DEACTIVATED_VERSIONS.has(v)) + .filter((v) => validateOnly || !DEACTIVATED_VERSIONS.has(v)) .sort(versionCmpDesc); const adapters = new Map(); @@ -114,6 +124,7 @@ function build() { { version, date, arch, runId }, { minDurationNs: CONFIG.minDurationNs }, ); + cellsExamined++; for (const e of errors) { warn(`${version}/${date}/${arch}/${runId}: ${e}`); validationErrors++; @@ -228,8 +239,17 @@ function build() { if (validateOnly) { process.stderr.write( - `build-data: validate-only — ${manifest.versions.length} version(s), ${validationErrors} validation error(s), ${warnings} warning(s)\n`, + `build-data: validate-only — ${cellsExamined} cell(s) examined in ${versions.length} version(s), ` + + `${validationErrors} validation error(s), ${warnings} warning(s)\n`, ); + // A gate that inspected nothing must not report success. results/ is never + // legitimately empty in this repo, so zero cells means the tree is missing + // (bad checkout, wrong RESULTS_ROOT) or the walker broke — either way the + // green check would be a lie about coverage. + if (cellsExamined === 0) { + process.stderr.write(`build-data: FATAL validate-only examined 0 cells under ${root}\n`); + process.exit(2); + } process.exit(validationErrors > 0 ? 1 : 0); } @@ -238,7 +258,8 @@ function build() { writeJSON(join(SRC_DATA, "scenarios.json"), scenarioRegistry); process.stderr.write( - `build-data: ${manifest.versions.length} version(s), ${adapters.size} adapter(s), ${scenarios.size} scenario(s), ` + + `build-data: ${manifest.versions.length} version(s), ${cellsExamined} cell(s), ${adapters.size} adapter(s), ` + + `${scenarios.size} scenario(s), ` + `default=${manifest.default ? `${manifest.default.version}/${manifest.default.arch}` : "none"}, ${warnings} warning(s)\n`, ); } diff --git a/src/lib/results/load.ts b/src/lib/results/load.ts index d4756eb..b50cf6b 100644 --- a/src/lib/results/load.ts +++ b/src/lib/results/load.ts @@ -2,13 +2,18 @@ * Load + validate a single benchmark cell from disk. Never throws on bad input: * a malformed summary returns null (cell skipped); a corrupt/optional timeseries * degrades to null while the cell's scalars are still used. + * + * `errors` is the publish gate's channel: the lenient build only warns about + * them, while `build-data --validate-only` exits non-zero if any cell reported + * one. A cell can therefore be returned AND carry errors (mislabelled env) — + * the site keeps rendering what it always rendered, the gate goes red. */ import { existsSync, readFileSync } from "node:fs"; import { gunzipSync } from "node:zlib"; import { join } from "node:path"; import type { LoadedCell, RawEnv, RawSummary, RawTimeseries } from "./types"; import { runDir } from "./walk"; -import { validateSummary, validateTimeseriesDoc, type CellExpect } from "./validate"; +import { validateEnv, validateSummary, validateTimeseriesDoc, type CellExpect } from "./validate"; export interface LoadResult { cell: LoadedCell | null; @@ -55,10 +60,18 @@ export function loadCell( return { cell: null, errors: summaryErrs, warnings }; } + // env.json is optional: absent or unparseable is a warning and the cell still + // loads from the summary. But a *present and parseable* env.json that + // disagrees with its own path (env says arm64, path says x86_64) is a + // mislabelled cell — the exact corruption that had to be scrubbed by hand in + // d3424f4 — so validateEnv's findings are errors, which is what makes + // `--validate-only` reject them. let env: RawEnv = { schema_version: "env/1" }; try { - if (existsSync(join(dir, "env.json"))) env = readJSON(join(dir, "env.json")); - else warnings.push("env.json missing; using config from summary"); + if (existsSync(join(dir, "env.json"))) { + env = readJSON(join(dir, "env.json")); + errors.push(...validateEnv(env, expect)); + } else warnings.push("env.json missing; using config from summary"); } catch (e) { warnings.push(`env.json parse failed: ${(e as Error).message}`); }