Run the result-schema gate, over a scope a new file lands inside by default - #394
Merged
Conversation
CommitLore — record lintTrailers: clean — 1 commit in Active constraints for the paths this PR touchesLimits (47)
Ruled out (129)
Warnings (33)
Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
…efault bench/verify.mjs had no npm script and no CI step: `grep -rn "verify.mjs" .github/workflows/ package.json` returned nothing. So the schema drifted five fields behind the runner and bench/results/m5-off-design-20-tasks.jsonl sat in the tree failing the gate for two days. Reproduced at 6d4e828^, the commit before #390 fixed the schema: 80 problem(s) across 80 rows, every one `must NOT have additional properties`. #390 fixed that drift. This is about the reason nobody saw it, and docs/RELEASE-GATE.md already names the shape -- "A gate nobody can check is a slogan." Running it needs an answer to which files, and bench/results/ is not one population. Seventeen .jsonl files there hold two row families that share a directory and nothing else. Nine are run records -- one row per (task, condition, seed), written by bench/runner.ts, which is what result.schema.json describes in its own words. Eight are metric rows written by bench/deterministic.ts and bench/external/run.ts: one row per measurement, never a run id, a task or a condition. Checking those against this schema is not a drift check, it is a category error, and it fails eight of seventeen files permanently. So the scope is the directory, with two exemptions decided per row. Nothing opts in: `npm run bench:verify` reads bench/results/*.jsonl and a new results file is gated the moment it is committed. A declared list of filenames was the obvious alternative and is the one thing that cannot work -- it is opt-in wearing a different hat, and a file left off it is silently ungated, which is the failure being fixed. The family split keys on `schema_version`, and on its absence rather than its presence. #392 suggested the field as the inclusion marker; it is the opposite. BaseRow (bench/deterministic/types.ts) declares `readonly schema_version: 1`, so every metric row carries it, and result.schema.json is additionalProperties:false with no such property, so no valid run record can. Keying inclusion on presence would have selected exactly the eight files this schema cannot describe. A file carrying it on some rows and not others is failed rather than classified: that is a corrupt file, and it is also the only accident by which a run-record file could leave the gate. Six run-record matrices predate 1073fa4, which made harness_commit and dist_digest required. They cannot be corrected -- bench/results/ holds committed measurements, and ADR-0018 is why a superseded or invalidated matrix stays in the tree rather than being deleted -- but they are not skipped and the schema is not weakened. The gate compiles a second validator from the committed schema with exactly those two `required` entries dropped, and applies it only to rows that identify themselves as old by their own started_at. Every other constraint still holds for all 380 of them. The cutoff is not a judgement call: the last row without provenance started at 2026-07-27T02:21:50.808Z, the first row with it at 2026-07-27T07:21:31.025Z, and 1073fa4 is dated 2026-07-27T02:56:12Z, inside that gap. It cannot creep forward, because started_at comes from the clock when the runner writes the row. Both exemptions are printed per file on every run, so the scope is checkable by running the gate rather than by trusting the comment that defines it -- which is the whole complaint this closes. Record-Id: r-benchscope Limit: nothing checks the shape of the eight metric-row files. This gate names them and steps over them, and bench/deterministic/types.ts is the only definition that family has -- there is no JSON schema for it, so drift on that side is still invisible Limit: the pre-provenance exemption reads started_at, which is data on the row rather than a fact about the file. A row that misreported it would be held to the shorter list of requirements; that is a deliberate falsification rather than the omission this fixes, and nothing here detects it Ruled-out: A declared list of result files, in the gate or in a manifest | opt-in under another name, and a file left off it is silently ungated -- which is precisely the failure #392 reports Ruled-out: Validating only the three files that pass today | a description of the present rather than a rule, and it goes stale the first time a matrix is added or the schema moves Ruled-out: A naming convention on results filenames | the runner's default output is an ISO stamp plus hex and the metric writers' is a family name plus an ISO stamp, but every run-record file in the tree was named by hand through --out (t702-m2, m5-off-design-20-tasks), so the convention is enforced nowhere and cannot carry a gate Ruled-out: A marker file dropped beside each gated result | still a step somebody has to remember, and a forgotten step is the defect being fixed Ruled-out: Widening result.schema.json to describe both row families | they share no required field, so the union would accept a run record with no task and a metric row with no metric, which is a schema that rejects nothing Ruled-out: Making harness_commit and dist_digest optional in the schema | weakens the gate for every future row in order to admit six historical files, and #390's drift was an additionalProperties failure that a laxer schema catches later rather than sooner Ruled-out: Skipping the six pre-provenance files whole | cheaper to write, but it stops checking types, patterns and the closed property set on 380 rows in order to excuse two absent fields Certainty: firm Blast: module Undo: easy Verified: the gap confirmed at 85ea006 -- grep for verify.mjs across .github/workflows/ and package.json exits 1; the drift reproduced at 6d4e828^ (e5d8eed), where verify.mjs reports 80 problem(s) across 80 rows of m5-off-design-20-tasks.jsonl, all additionalProperties; the survey run file by file before any change -- 17 .jsonl, 3 passing and 14 failing, the 14 splitting into 8 metric-row files and 6 run-record files missing only harness_commit and dist_digest; npm run bench:verify now exits 0 over 632 rows in 9 files, naming the 8 metric-row files as out of scope and 380 rows as pre-provenance; the gate bites -- a scratch bench/results/*.jsonl carrying one undeclared field made it exit 1 naming file and line, and deleting that file returned it to exit 0 with bench/results/ clean; the exemption is bounded -- the same row passes dated 2026-07-26 and fails dated 2026-08-01, and a pre-provenance row with an extra field still fails; npx vitest run reports Test Files 83 passed, Tests 2092 passed and 1 skipped, 11 of them new here; npm run typecheck, npx tsc -p bench/tsconfig.json --noEmit and npm run build clean with dist/ byte-unchanged; node scripts/check-readme-numbers.mjs exits 0 and bash spec/verify.sh reports OK on 26 fixtures; no file under bench/results/ was modified Unverified: the CI step has not run on a GitHub runner from this commit -- only the command it invokes has been run locally, on macOS and one Node version; whether the metric family keeps declaring schema_version, which holds because BaseRow requires it today and is enforced nowhere outside bench/deterministic/types.ts; whether any pre-provenance row misstates its own started_at, since the cutoff was derived from the rows in the tree rather than from an independent record of when each matrix ran
MongLong0214
force-pushed
the
fix-issue-392
branch
from
August 1, 2026 20:26
633d054 to
67f4375
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #392.
bench/verify.mjshad no npm script and no CI step, so the result schema drifted five fields behind the runner and a committed matrix failed the gate for two days with nobody looking. #390 fixed that drift; this fixes the reason it went unnoticed.The gap, confirmed
Reproduced at
6d4e828^(the commit before #390 fixed the schema):Survey of
bench/results/17
*.jsonlfiles, 802 rows, and two row families that share a directory and nothing else.schema_versionRunRecord)bench/runner.tsBaseRow)bench/deterministic.ts,bench/external/run.tsThe 6 failing run-record files fail on one axis only:
harness_commitanddist_digest, which1073fa4maderequiredon 2026-07-27. The 8 metric-row files fail onrun_id,task,condand the rest — they are one row per measurement, not per(task, condition, seed)run, soresult.schema.jsonwas never meant to describe them.Scope, and why
The gate is default-in:
npm run bench:verifyreadsbench/results/*.jsonl, all of it. Nothing registers, nothing opts in, and a new results file is gated the moment it is committed. Two exemptions, both decided per row rather than per filename, both printed on every run. The rule for each is stated at the top ofbench/verify.mjs, where the scope is defined.1. A file whose every row carries
schema_versionholds metric rows, and this schema does not describe them. The field is the discriminator because both sides guarantee the answer:BaseRowdeclaresreadonly schema_version: 1, andresult.schema.jsonisadditionalProperties: falsewith no such property. Note the direction — the marker belongs to the other family, so the gate reads its absence as inclusion. Keying inclusion on its presence, as the issue suggested, would select exactly the files this schema cannot describe. A file carrying it on some rows and not others is failed, not classified — that is a corrupt file, and it closes the only accident by which a run-record file could leave the gate.2. Rows recorded before
1073fa4are not required to carry the two provenance fields. Those six matrices cannot be corrected —bench/results/holds committed measurements, and ADR-0018 is why a superseded or invalidated matrix stays in the tree. They are not skipped and the schema is not weakened: the gate compiles a second validator from the committed schema with exactly those tworequiredentries dropped. Every other constraint — every type, every pattern, the closed property set — still applies to all 380 rows. The cutoff is not a judgement call: the last pre-provenance row started at2026-07-27T02:21:50.808Z, the first post-provenance row at2026-07-27T07:21:31.025Z, and1073fa4is dated2026-07-27T02:56:12Z, inside that gap. It cannot creep forward, becausestarted_atcomes from the clock.Ruled out, in the commit trailers: a declared file list (opt-in under another name), validating only what passes today (a description, not a rule), a naming convention (every run-record file in the tree was named by hand through
--out), a marker file (still a step to remember), widening the schema to cover both families (they share no required field, so the union rejects nothing), and making the provenance fields optional (weakens every future row to admit six old files).The gate bites
A scratch file added to
bench/results/, carrying one field the schema does not declare:Removed again:
Eleven tests in
test/bench-result-scope.test.tshold the rules, including the one that matters most: the gate's output must account for every.jsonlon disk, so replacing the directory scan with a list, a prefix filter or a "passes today" set fails immediately.Verification
npx vitest run— Test Files 83 passed, Tests 2092 passed | 1 skipped.npm run typecheck,npx tsc -p bench/tsconfig.json --noEmit,npm run build(dist/ byte-unchanged),node scripts/check-readme-numbers.mjs,bash spec/verify.sh(OK: 26 fixtures),npm run bench:verify— all exit 0.node dist/commitlore.mjs validate -c HEAD— shape ok, references ok.No file under
bench/results/was modified, and neitherREADME.mdnor any translation was touched.