From 3a949a60a0a617425ee34be498e79e312c5cc0fd Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Thu, 13 Aug 2026 10:53:06 +0000 Subject: [PATCH 1/3] fix(GATE-PR-SIZE-BINARY): a golden is binary, so the binary guard made every golden-bearing PR unmergeable (#615) `check-pr-size.py` errored on any path git reports as binary -- "is not reviewable as text" -- with no exemption route. `--pr-number` was accepted and validated, then never consulted for the decision. Parity goldens are binary by nature, so no PR that captures an oracle golden could reach main at all. This is a deliberate retirement of a rule, not a red run repaired. Arguing it here because there is no waiver registry: 1. It contradicted this checker's own model. The `asset` class exists for shipped artwork and the `SITE_ASSET` note says so outright -- "the binaries among them have no reviewable line budget at all, so they take the `asset` class the same way any other shipped artwork does." The classifier was built to give binaries a class; the guard then refused every one regardless. 2. It post-dated the precedent it rejected. The guard landed 450a1b696 (2026-08-10); the Tenstorrent golden precedent landed 971d55063 (2026-08-09). The one post-guard golden commit on main, 5080983fd with 16 .npy files, also trips it and reached main by a route the current PR-only ruleset no longer permits. A rule nothing in the tree can satisfy is not enforcing a standard, it is blocking a lane. 3. Its protection was already carried elsewhere, and is kept. `classify_path` runs directly above and raises on any path without a class. An unclassified binary is still refused -- now with a message naming the real defect, which the author can act on, instead of an unfixable property of the file. What was removed is only the blanket refusal of CLASSIFIED binaries. Not an allowlist instead: a roster of blessed binary paths is a shared must-write surface, so every golden-bearing PR would edit it -- exactly the lock AGENTS.md forbids. Classification already partitions by where a file lives, which is the derived-at-read-time shape. This removes no size rule because none was left: the per-class line budgets were retired 2026-08-10 and stay retired. Nothing in this file measures a diff. RED before, on the unmodified checker, for the intended reason -- all four subtests of the new case die on the guard: AssertionError: Lists differ: ["binary change 'website/static/fonts/sora-700.woff2' is not reviewable as text"] != [] 4 failed, 43 passed, 119 subtests passed GREEN after: 43 passed, 123 subtests passed. The wider governance suite holds at 92 passed, 125 subtests. `tests/scripts/test_cpu_kernel_bench.py` fails collection on unmodified main too (it wants a built benchmark binary) and is untouched by this change. `test_binary_changes_fail_closed_instead_of_becoming_free` is deleted because it states the retired rule and cannot survive it. Its replacement, `test_an_unclassified_binary_is_still_refused`, pins the half that stays and is green on both sides -- a rail, not the evidence. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [Claude Code] --- .agents/roadmap_v1.md | 1 + .agents/specs/gate-pr-size-binary.md | 154 +++++++++++++++++++++++++++ scripts/check-pr-size.py | 25 +++-- tests/scripts/test_check_pr_size.py | 54 +++++++--- 4 files changed, 213 insertions(+), 21 deletions(-) create mode 100644 .agents/specs/gate-pr-size-binary.md diff --git a/.agents/roadmap_v1.md b/.agents/roadmap_v1.md index 5fb7978a0..4a000d546 100644 --- a/.agents/roadmap_v1.md +++ b/.agents/roadmap_v1.md @@ -138,6 +138,7 @@ issue is not yet placed. Keyed record: update in place, never append. | [#483](https://github.com/mudler/vllm.cpp/issues/483) | `GATE-PIN-UNPINNED-SNAPSHOTS` | `check-snapshot-pins` is evaded by ordinary modern-C++ punctuation, not by the deliberate escapes: `directory_iterator{snaps}` (brace init), `root /= "snapshots"` (compound assignment), and a lambda body severed by the `[;{}]` statement split | bug | | [#484](https://github.com/mudler/vllm.cpp/issues/484) | `GATE-PIN-UNPINNED-SNAPSHOTS` | The `check-snapshot-pins` CI step has never been observed executing on a real GitHub runner; the spec records this row's GPU/weights limits but not that one | bug | | [#485](https://github.com/mudler/vllm.cpp/issues/485) | `GATE-PIN-UNPINNED-SNAPSHOTS` | `_MARK_FIXPOINT_ROUNDS = 6` silently truncates the binding fixpoint: a 7-link helper chain evades `check-snapshot-pins` with no diagnostic | bug | +| [#615](https://github.com/mudler/vllm.cpp/issues/615) | `GATE-PR-SIZE-BINARY` | `check-pr-size` fail-closes on every binary path with no exemption route, so no golden-bearing PR can merge: it blocks #431 and post-dates the golden precedent it rejects, spec [`gate-pr-size-binary.md`](specs/gate-pr-size-binary.md) | bug | | [#238](https://github.com/mudler/vllm.cpp/issues/238) | `SAMPLE-LOGPROB-TOKEN-IDS` | `logprobs_mode`: three of four modes are runtime-refused stubs | bug | | [#264](https://github.com/mudler/vllm.cpp/issues/264) | `SAMPLE-LOGPROB-TOKEN-IDS` | `logprob_token_ids`: generative scoring over an explicit token set is unported | feature | | [#365](https://github.com/mudler/vllm.cpp/issues/365) | `PERF-27B-DENSE-MARLIN-GATEUP` | 27B gap decomposed vs vLLM's own decode profile; dense W4A16 MLP bypassed the fused gate_up seam (spec `specs/perf-27b-dense-marlin-gateup.md`) | bug | diff --git a/.agents/specs/gate-pr-size-binary.md b/.agents/specs/gate-pr-size-binary.md new file mode 100644 index 000000000..7f9dfbeb5 --- /dev/null +++ b/.agents/specs/gate-pr-size-binary.md @@ -0,0 +1,154 @@ +# GATE-PR-SIZE-BINARY — retire the fail-closed binary guard + +**Row:** `GATE-PR-SIZE-BINARY` +**Issue:** [#615](https://github.com/mudler/vllm.cpp/issues/615) +**Base:** `origin/main` `7572b0f4e` +**Status:** ACTIVE, 2026-08-13 + +## 1. Scope + +One behavioural change to `scripts/check-pr-size.py`: remove the error raised +for a changed path that git reports as binary. + +**In scope.** The `change.lines is None` branch in `change_errors`, the header +sentence that advertises it, the `SITE_ASSET` comment that contradicts it, and +the two cases in `tests/scripts/test_check_pr_size.py` that pin it. + +**Out of scope.** Explicit path classification, the checker-evidence contract, +the role checks, and the retired line budget. None of them changes. This is not +a size rule and it does not reopen one — the per-class budgets were retired on +2026-08-10 by developer decision and stay retired. + +## 2. Anchors + +Local, not upstream — this is a project governance checker with no vLLM +counterpart. + +| What | Where | +|---|---| +| The guard | `scripts/check-pr-size.py:480-481` | +| Its advertisement | `scripts/check-pr-size.py:56-57` | +| The classifier that already protects us | `scripts/check-pr-size.py` `classify_path`, raises `ValueError` on any unclassified path | +| The `asset` class the guard contradicts | `scripts/check-pr-size.py:160-165` (`SITE_ASSET`) | +| The guard's landing commit | `450a1b696`, 2026-08-10 | +| The golden precedent it post-dates | `971d55063`, 2026-08-09 | +| Blocked work | [#431](https://github.com/mudler/vllm.cpp/pull/431) | + +## 3. Design + +`change_errors` currently short-circuits on binaries before any class-specific +rule runs: + +```python +if change.lines is None: + errors.append(f"binary change {change.path!r} is not reviewable as text") + continue +``` + +Delete the branch. Everything downstream already tolerates `lines is None` — +the checker-evidence contract tests `evidence_change.lines is None` explicitly +rather than assuming an int, so a binary simply cannot serve as mutation +evidence, which remains correct. + +Classification runs *before* this branch and is unchanged, so the ordering after +the edit is: classify (raise on unknown) → class-specific rules. An unclassified +binary is still refused, by the classifier, with the message that names the real +defect — an unclassified path — instead of one that names an unfixable property +of the file. + +**Why the guard is not load-bearing.** Its stated job is that a binary "is not +reviewable as text". True, and irrelevant: nothing else in this checker reviews +text either. It classifies paths and enforces an evidence contract. The property +that keeps an unreviewable blob out of the tree is that it must first earn a +class, and that check is the one being kept. + +**Why not an exemption list instead.** An allowlist of blessed binary paths is a +shared must-write surface — every golden-bearing PR would edit it, which is +precisely the lock AGENTS.md forbids. Classification already partitions these +paths by *where they live*, which is the derived-at-read-time shape. + +## 4. Risks and decisions + +| Risk | Assessment | +|---|---| +| A large unreviewed binary lands in a product path | The classifier still refuses any path without a class, and `product` requires arriving on a PR. A binary in a classified location was always intended to be legal — see the `SITE_ASSET` comment. | +| This reads as weakening a gate to go green | It is a deliberate retirement, argued in the commit message per the no-waiver-registry rule, not a repair of a red run. No PR of mine is unblocked by it; the beneficiaries are #431 and future golden work. | +| Goldens become unreviewable in practice | Unchanged by this edit — they are unreviewable as text either way. Golden provenance is enforced by the parity gates and the oracle-identity requirements, which is where it belongs. | +| The retirement is silently reversed later | The RED-first test in §5 asserts the new behaviour directly, so a reintroduction turns it red. | + +## 5. Tests + +RED-first, in `tests/scripts/test_check_pr_size.py`: + +1. `test_a_classified_binary_is_accepted` — a binary at a classified path + (`tests/parity/goldens/.../our_ids.npy`, and a `website/static/` asset) + produces **no** error. **RED before the change** for the intended reason: + the guard fires. +2. `test_an_unclassified_binary_is_still_refused` — a binary at an unclassified + path still errors, and the error names classification, not binaryness. This + is the guard rail that keeps the retirement scoped. Green both before and + after (the classifier raises first), so it is a regression pin, not evidence. +3. Rewrite `test_retiring_the_budget_did_not_retire_the_other_contracts` so its + binary clause asserts the *classified* binary passes while the *unclassified* + one fails, keeping the other two contracts pinned exactly as they are. +4. Delete `test_binary_changes_fail_closed_instead_of_becoming_free`, which + states the retired rule and cannot survive it. + +## 6. Gates + +- `python3 -m pytest tests/scripts/test_check_pr_size.py` green, with case 1 + shown RED on the unmodified checker first. +- `python3 scripts/check-pr-size.py --base --head ` classifies this + PR's own change without error. +- `scripts/agent-preflight.sh --staged` clean. +- The checker-evidence contract must be satisfied *by this very PR*: it changes + a `governance_checker`, so it must ship executable mutation evidence in + `tests/scripts/test_check_pr_size.py`. It does. + +## 7. Evidence + +**RED before**, on the unmodified checker, for the intended reason — all four +subtests of `test_a_classified_binary_is_accepted` die on the guard: + +``` +AssertionError: Lists differ: + ["binary change 'website/static/fonts/sora-700.woff2' is not reviewable as text"] != [] +4 failed, 43 passed, 119 subtests passed +``` + +`test_an_unclassified_binary_is_still_refused` was already green here, as §5 +predicted — classification runs first, so it is a rail and not the evidence. + +**GREEN after:** `43 passed, 123 subtests passed`. Wider governance suite +(`test_check_pr_size` + `test_agent_record`): `92 passed, 125 subtests`. + +**The retirement does what it is for**, checked directly against a +golden-bearing change: + +``` +golden-bearing PR errors -> NONE (was: 2 refusals) +unclassified binary -> ["unclassified repository path 'junk/blob.bin'"] +``` + +**The checker accepts its own diff**, satisfying the evidence contract it +enforces on `governance_checker` paths: +`check-pr-size.py --base 7572b0f4e --head ` → `OK`, exit 0. +`check-commit-trailers.py` → `OK: commit trailer contract`, exit 0. + +**Stop condition §8 checked, not assumed.** The full `tests/scripts/` suite is +byte-identical before and after: `8 failed, 20 passed, 2 skipped` on the +modified worktree *and* on unmodified `main` `7572b0f4e`, across +`test_gen_vulkan_spirv`, `test_mlx_system_headers` and `test_now_render`. All +pre-existing; this change adds no failure. `test_cpu_kernel_bench.py` fails +collection on unmodified main too (it wants a built benchmark binary). +`test_cpu_x86_llamacpp_floor` and `test_now_render` are order-dependent under a +loaded parallel run and pass in isolation on both trees. + +## 8. Stop conditions + +- If removing the branch turns any other case in the suite red for a reason not + named in §5, stop — that is a load-bearing use of the guard this spec did not + find, and the design in §3 is wrong. +- If the checker cannot classify its own diff after the edit, stop. +- If a reviewer judges that classification alone does not carry the protection, + stop and escalate rather than widening the change. diff --git a/scripts/check-pr-size.py b/scripts/check-pr-size.py index 0618320ab..7bd9037b1 100755 --- a/scripts/check-pr-size.py +++ b/scripts/check-pr-size.py @@ -1,10 +1,11 @@ #!/usr/bin/env python3 """Enforce explicit path classification and the checker-evidence contract. -The per-class LINE BUDGETS this file used to enforce were retired 2026-08-10; -see the note where they stood. What remains: every changed path must classify -explicitly, binaries fail closed, a governance-checker change must carry -executable mutation evidence, and product paths must arrive on a PR.""" +The per-class LINE BUDGETS this file used to enforce were retired 2026-08-10, +and the fail-closed BINARY GUARD was retired 2026-08-13; see the notes where +each stood. What remains: every changed path must classify explicitly, a +governance-checker change must carry executable mutation evidence, and product +paths must arrive on a PR. Nothing here measures the size of a diff.""" from __future__ import annotations @@ -54,8 +55,8 @@ # # Everything else this checker enforces is unchanged and is NOT a size rule: # explicit path classification (no blanket directory exemptions), the -# fail-closed binary guard, the checker-change mutation-evidence contract, and -# the role checks that keep product paths on a PR. +# checker-change mutation-evidence contract, and the role checks that keep +# product paths on a PR. # Machine-generated artifacts, each of which MUST be (a) emitted by a tracked # generator in this repository, (b) reproduced byte-for-byte by a gate that runs @@ -477,9 +478,15 @@ def change_errors( except ValueError as exc: errors.append(str(exc)) continue - if change.lines is None: - errors.append(f"binary change {change.path!r} is not reviewable as text") - continue + # NO BINARY GUARD. A `lines is None` path used to error here as "not + # reviewable as text" (GATE-PR-SIZE-BINARY, #615). Retired 2026-08-13: + # it made every golden-bearing PR unmergeable by construction -- parity + # goldens are binary by nature -- while adding nothing, because the + # protection that matters is classification, which runs directly above + # and still refuses any path without a class. The classifier was always + # built to give binaries a class; see the `SITE_ASSET` note. Note + # `lines is None` still matters downstream: the evidence contract below + # tests it, so a binary cannot serve as mutation evidence. if path_class == "governance_checker": evidence = recognized_evidence(change.path) evidence_change = changed_paths.get(evidence) diff --git a/tests/scripts/test_check_pr_size.py b/tests/scripts/test_check_pr_size.py index f66ac0f58..5e2842f69 100755 --- a/tests/scripts/test_check_pr_size.py +++ b/tests/scripts/test_check_pr_size.py @@ -341,27 +341,57 @@ def test_no_line_budget_is_enforced_for_any_class(self) -> None: self.assertEqual(checker.change_errors([huge]), []) def test_retiring_the_budget_did_not_retire_the_other_contracts(self) -> None: - """The three rules that share this checker must still bite. + """The rules that share this checker must still bite. - Dropping a size gate is not licence to drop classification, the binary - guard, or checker-evidence with it, which is exactly the kind of thing - that goes unnoticed when a constant is deleted. + Dropping a size gate is not licence to drop classification or + checker-evidence with it, which is exactly the kind of thing that goes + unnoticed when a constant is deleted. """ unknown = checker.ChangedPath("no/such/surface.txt", 1, 0) self.assertTrue(checker.change_errors([unknown])) - # Asserted on the ERROR, not its wording: this is a regression guard - # that must hold on both sides of the retirement, so it must not be - # coupled to a message string that the retirement itself reworded. - binary = checker.ChangedPath("assets/logo.png", None, None) - self.assertTrue(checker.change_errors([binary])) lone_checker = self.change("scripts/check-pr-size.py", 10) self.assertTrue( any("mutation evidence" in e for e in checker.change_errors([lone_checker])) ) - def test_binary_changes_fail_closed_instead_of_becoming_free(self) -> None: - errors = checker.change_errors([checker.ChangedPath("docs/image.png", None, None)]) - self.assertTrue(any("binary" in error for error in errors), errors) + def test_a_classified_binary_is_accepted(self) -> None: + """A binary at a classified path is not an error (GATE-PR-SIZE-BINARY, #615). + + RED before the retirement: `change_errors` short-circuited on every + `lines is None` path, so a captured parity golden could not reach main + at all and #431 was unmergeable by construction. The classifier was + always built to give binaries a class -- the `SITE_ASSET` comment says + so in as many words -- and the guard refused them anyway. + """ + for path in ( + "tests/parity/goldens/qwen3_greedy_0_6b/our_ids.npy", + "tests/parity/goldens/qwen35_greedy_0_8b/neartie_gap_mnats.npy", + "tests/parity/goldens/qwen3_greedy_0_6b/p0_prompt.i32", + "website/static/fonts/sora-700.woff2", + ): + with self.subTest(path=path): + # Asserted through classify_path rather than a hardcoded class + # so this stays true if a golden is later reclassified. + checker.classify_path(path) + binary = checker.ChangedPath(path, None, None) + self.assertEqual(checker.change_errors([binary]), []) + + def test_an_unclassified_binary_is_still_refused(self) -> None: + """Retiring the guard must not turn an unclassified path into a free one. + + This is the rail that keeps the retirement scoped: the protection was + never "binaries are unreviewable", it was "every path earns a class". + Green on both sides of the change -- classification runs first -- so it + is a regression pin, not the evidence for the retirement. + """ + errors = checker.change_errors([checker.ChangedPath("no/such/surface.png", None, None)]) + self.assertTrue(errors) + # The message must name the real defect. "Not reviewable as text" told + # the author to fix something about the file; an unclassified path is + # something they can actually act on. + self.assertFalse( + any("not reviewable as text" in error for error in errors), errors + ) def test_checker_change_requires_its_recognized_mutation_test(self) -> None: changed = [ From c63d121ad046cbe6ac410a68a71261a46411c052 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Thu, 13 Aug 2026 14:44:11 +0000 Subject: [PATCH 2/3] fix(GATE-PR-SIZE-BINARY): three descriptions of the retired guard, and a mislabelled evidence number (#615) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repairs from the fresh review of #619. No behaviour change; the checker and its tests are untouched. Three places still described the guard as live. Retiring a rule and leaving its description standing is the same defect as leaving a refuted rationale in a spec, so these are fixed in the same flow rather than deferred: - `.github/workflows/ci.yml:403-407` still advertised "the fail-closed binary guard" as one of four things the required `pr-size` job enforces. That is the first place anyone auditing the gate from CI reads. - `website/README.md:60` still justified shipping no woff2 on the grounds that `check-pr-size.py` "refuses binary changes outright", and cited a waiver registry that was itself retired earlier. Both halves were false, and this PR's own new test asserts `website/static/fonts/sora-700.woff2` is accepted -- a shipped rationale contradicted by a test in the same commit. The decision not to self-host a face stands; it is now argued on its own merits. - Spec §5 item 3 said the multi-contract test would be REWRITTEN to keep a binary clause. What shipped deletes that clause, because cases 1 and 2 carry the coverage directly and duplicating it would hide which contract failed. The spec now records what shipped. Also corrected, and worth naming plainly: spec §7 recorded the stop-condition check as `8 failed, 20 passed, 2 skipped` on both trees. That was a THREE-FILE SUBSET mislabelled as the whole suite -- no stated command produced it. Run properly and sequentially on both trees, `tests/scripts/` is: HEAD 9 failed, 1276 passed, 3 skipped, 1510 subtests BASE 9 failed, 1275 passed, 3 skipped, 1506 subtests with IDENTICAL failure sets, and the +1/+4 delta being exactly this change (one test deleted, two added, four new subtests). The substantive claim the number was offered for -- that this adds no failure -- holds; the number did not. Two spec claims were wider than the evidence and are narrowed: - §4 now states the real scope. Goldens live under `tests/`, so they classify as `product`; permitting them necessarily permits binaries across `src/`, `scripts/`, `tools/` and `benchmarks/`, and the same delete admits them to `procedure`, `project_record`, `ci` and `vendored_dependency`. Measured, not inferred: a blob at `src/vllm/blob.dat` and binary bytes replacing `.agents/workflow.md` are refused at BASE and pass at HEAD. Recorded as an accepted trade with what still refuses them, not left implicit. - §3's appeal to the `SITE_ASSET` comment over-generalized: that comment speaks for `website/static/`, not for every class. §4 now names the narrower alternative (a derived regex class for the goldens directory) and says why it was not taken, instead of rebutting only the allowlist strawman. §1's scope list claimed the `SITE_ASSET` comment was in scope; it is untouched, correctly, because it never described the guard and reads correctly without it. Verified: 375 passed, 1072 subtests across every suite that reads `ci.yml` or `website/README.md` plus `test_check_pr_size`. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [Claude Code] --- .agents/specs/gate-pr-size-binary.md | 54 ++++++++++++++++++++-------- .github/workflows/ci.yml | 9 ++--- website/README.md | 9 +++-- 3 files changed, 50 insertions(+), 22 deletions(-) diff --git a/.agents/specs/gate-pr-size-binary.md b/.agents/specs/gate-pr-size-binary.md index 7f9dfbeb5..f8d49abc3 100644 --- a/.agents/specs/gate-pr-size-binary.md +++ b/.agents/specs/gate-pr-size-binary.md @@ -10,9 +10,14 @@ One behavioural change to `scripts/check-pr-size.py`: remove the error raised for a changed path that git reports as binary. -**In scope.** The `change.lines is None` branch in `change_errors`, the header -sentence that advertises it, the `SITE_ASSET` comment that contradicts it, and -the two cases in `tests/scripts/test_check_pr_size.py` that pin it. +**In scope.** The `change.lines is None` branch in `change_errors`, the two +places in that file that advertise it, the two cases in +`tests/scripts/test_check_pr_size.py` that pin it, and the three descriptions of +the guard that live outside it: the `pr-size` job comment in +`.github/workflows/ci.yml` and the font rationale in `website/README.md`. + +The `SITE_ASSET` comment is deliberately **not** edited: it never described the +guard, and it reads correctly once the guard is gone. **Out of scope.** Explicit path classification, the checker-evidence contract, the role checks, and the retired line budget. None of them changes. This is not @@ -71,7 +76,8 @@ paths by *where they live*, which is the derived-at-read-time shape. | Risk | Assessment | |---|---| -| A large unreviewed binary lands in a product path | The classifier still refuses any path without a class, and `product` requires arriving on a PR. A binary in a classified location was always intended to be legal — see the `SITE_ASSET` comment. | +| **The retirement is wider than the problem it solves** | Accepted, and the operator should merge knowing it. Goldens live under `tests/`, so they classify as `product` — permitting them necessarily permits binaries across `src/`, `scripts/`, `tools/` and `benchmarks/`, and the same delete admits them to `procedure`, `project_record`, `ci` and `vendored_dependency` too. Measured before/after, not inferred: an opaque blob at `src/vllm/blob.dat`, and binary bytes replacing `.agents/workflow.md` or `ci.yml`, were refused at BASE and pass at HEAD. What still refuses them: `.gitignore` eats the realistic accidental blob, every change arrives on a PR, and a binary in a diff is maximally visible to a reviewer. Judged an acceptable trade because the guard could not tell a golden from a rootkit, so keeping it meant blocking correctness evidence — which a correctness-first project should not do. | +| A narrower fix was available | True, and it is the honest limit of this change. A derived regex class for `tests/parity/goldens/…` would be the same derived-at-read-time shape this spec praises in `SITE_ASSET`, and would not admit binaries to `src/`. It was not taken because it leaves the `asset` and `website/static/` cases still refused and would need a second class the next time a lane needs a binary — but it is a real alternative, and §3's earlier appeal to the `SITE_ASSET` comment over-generalized: that comment speaks for `website/static/` only, not for every class. | | This reads as weakening a gate to go green | It is a deliberate retirement, argued in the commit message per the no-waiver-registry rule, not a repair of a red run. No PR of mine is unblocked by it; the beneficiaries are #431 and future golden work. | | Goldens become unreviewable in practice | Unchanged by this edit — they are unreviewable as text either way. Golden provenance is enforced by the parity gates and the oracle-identity requirements, which is where it belongs. | | The retirement is silently reversed later | The RED-first test in §5 asserts the new behaviour directly, so a reintroduction turns it red. | @@ -88,9 +94,12 @@ RED-first, in `tests/scripts/test_check_pr_size.py`: path still errors, and the error names classification, not binaryness. This is the guard rail that keeps the retirement scoped. Green both before and after (the classifier raises first), so it is a regression pin, not evidence. -3. Rewrite `test_retiring_the_budget_did_not_retire_the_other_contracts` so its - binary clause asserts the *classified* binary passes while the *unclassified* - one fails, keeping the other two contracts pinned exactly as they are. +3. `test_retiring_the_budget_did_not_retire_the_other_contracts` **drops its + binary clause entirely**, because cases 1 and 2 now carry that coverage + directly and duplicating it inside a multi-contract test would hide which + contract failed. Its classification and checker-evidence clauses are + untouched and still bite. (Drafted as "rewrite the clause"; deleting it was + the better shape once 1 and 2 existed, and this line records what shipped.) 4. Delete `test_binary_changes_fail_closed_instead_of_becoming_free`, which states the retired rule and cannot survive it. @@ -135,14 +144,29 @@ enforces on `governance_checker` paths: `check-pr-size.py --base 7572b0f4e --head ` → `OK`, exit 0. `check-commit-trailers.py` → `OK: commit trailer contract`, exit 0. -**Stop condition §8 checked, not assumed.** The full `tests/scripts/` suite is -byte-identical before and after: `8 failed, 20 passed, 2 skipped` on the -modified worktree *and* on unmodified `main` `7572b0f4e`, across -`test_gen_vulkan_spirv`, `test_mlx_system_headers` and `test_now_render`. All -pre-existing; this change adds no failure. `test_cpu_kernel_bench.py` fails -collection on unmodified main too (it wants a built benchmark binary). -`test_cpu_x86_llamacpp_floor` and `test_now_render` are order-dependent under a -loaded parallel run and pass in isolation on both trees. +**Stop condition §8 checked, not assumed.** `python3 -m pytest tests/scripts/ +--ignore=tests/scripts/test_cpu_kernel_bench.py`, run sequentially on both +trees: + +| tree | result | +|---|---| +| HEAD | `9 failed, 1276 passed, 3 skipped, 1510 subtests` | +| BASE `7572b0f4e` | `9 failed, 1275 passed, 3 skipped, 1506 subtests` | + +The **failure sets are identical** — the same six `test_gen_vulkan_spirv` shader +subfailures, the same `test_check_windows_portability` subfailure, and the same +`test_mlx_system_headers` and `test_now_render` failures. All pre-existing; this +change adds no failure. The `+1 passed / +4 subtests` delta is exactly this +change: one test deleted, two added, four new subtests. + +`test_cpu_kernel_bench.py` fails collection on unmodified main too (it wants a +built benchmark binary). `test_cpu_x86_llamacpp_floor` is order-dependent under +a loaded parallel run and passes sequentially on both trees. + +An earlier draft of this section recorded `8 failed, 20 passed, 2 skipped` for +this check. That was a **three-file subset** mislabelled as the whole suite; no +stated command produced it. The substantive claim it was offered for — identical +failure sets before and after — is unchanged and is what the table above shows. ## 8. Stop conditions diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 014e625cc..8b25b7f0d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -401,10 +401,11 @@ jobs: echo "OK: all new commits carry FOLLOWING_AGENTS_PROTOCOL." pr-size: # PR-only. The per-class LINE BUDGETS this job used to enforce were retired - # 2026-08-10 (see scripts/check-pr-size.py); the job name is kept because it - # is a required check. What it enforces now: explicit path classification, - # the fail-closed binary guard, the checker-change mutation-evidence - # contract, and the role check that keeps product paths on a PR. + # 2026-08-10, and the fail-closed BINARY GUARD was retired 2026-08-13 (see + # scripts/check-pr-size.py for both); the job name is kept because it is a + # required check. What it enforces now: explicit path classification, the + # checker-change mutation-evidence contract, and the role check that keeps + # product paths on a PR. Nothing here measures the size of a diff. if: github.event_name == 'pull_request' runs-on: ubuntu-latest steps: diff --git a/website/README.md b/website/README.md index fc5da46f4..e4601c94f 100644 --- a/website/README.md +++ b/website/README.md @@ -57,9 +57,12 @@ The site loads nothing from another host. No CDN, no font service, no analytics. Typography is font stacks only: a serif does the display work and the monospace carries labels, kickers and table headers. -Shipping woff2 files was considered and rejected: `check-pr-size.py` refuses -binary changes outright ("binary change has no reviewable line budget"), and a -waiver is for one-time migrations with an expiry, not for a permanent asset. +Shipping woff2 files was considered and rejected, though no longer for the +original reason: `check-pr-size.py` used to refuse binary changes outright, and +that guard was retired 2026-08-13 (#615), so a woff2 under `website/static/` +would now pass the gate as an `asset`. It stays rejected on its own merits — +a self-hosted face is bytes every visitor pays for and a licence someone has to +keep tracking, and the stacks below already carry the design. The logos and favicon are **mounted** from the repository's `assets/`, not copied here, the same rule as `docs/`: a logo refreshed upstream must not From 90e195d6fd6aba482e0d715d3cc38b2b987034aa Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Thu, 13 Aug 2026 20:45:11 +0000 Subject: [PATCH 3/3] fix(GATE-PR-SIZE-BINARY): .agents/oracles/* had no path class, so classification failed on main itself (#668) Found while re-merging main for #615, and fixed in the same flow per AGENTS.md: small, obvious, adds no semantics, and currently reds `main`. The secondary-oracle registry (AGENTS.md, "When vLLM has no implementation") landed `.agents/oracles/.md` with no pattern in `check-pr-size.py`, so all eight tracked files are unclassified and `classify_path` RAISES. Reproduced on UNMODIFIED origin/main before touching anything, so this is not inherited from the binary-guard work in this branch: test_every_tracked_and_current_change_path_is_classified AssertionError: Lists differ: ['.agents/oracles/README.md', '.agents/ora[200 chars].md'] != [] First list contains 8 additional elements. Consequence: any PR recording an oracle pin is refused by a required check with "unclassified repository path". The whole point of one-file-per-oracle is that pinning is cheap; an unclassified path makes it impossible, so the registry was unusable as designed from the commit that introduced it. `.agents/oracles/*.md` is structurally identical to `.agents/specs/` and `.agents/claims/` -- one file per key, globbed for reading, deliberately not a shared table every change must write -- so it takes the same `procedure` class they already take. No new class, no new semantics. Deliberately a pattern and not a directory exemption: a non-.md file or a nested path under `.agents/oracles/` still fails closed, because AGENTS.md forbids hiding mutable files behind a blanket directory rule. RED before, on this branch with only the one-line wiring reverted -- all eight oracle files plus the tracked-paths test: 9 failed, 44 passed, 126 subtests passed GREEN after: 45 passed, 134 subtests passed. Two tests, not one. `test_every_secondary_oracle_file_classifies` asserts over `git ls-files .agents/oracles/` rather than a hardcoded sample, so a ninth oracle added without a class is caught here instead of in someone's PR. `test_oracles_is_a_pattern_not_a_blanket_directory_exemption` pins the fail- closed half, so a later widening to the whole directory turns it red. Closes #668. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [Claude Code] --- scripts/check-pr-size.py | 7 ++++++ tests/scripts/test_check_pr_size.py | 35 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/scripts/check-pr-size.py b/scripts/check-pr-size.py index 7bd9037b1..1b3eebaf0 100755 --- a/scripts/check-pr-size.py +++ b/scripts/check-pr-size.py @@ -174,6 +174,12 @@ # this. A claim in its own file has one writer and cannot collide. Classified # with the other per-row records it now resembles. CLAIM = re.compile(r"\.agents/claims/[A-Za-z0-9_.-]+\.md\Z") +# One file per secondary oracle (AGENTS.md, "When vLLM has no implementation"). +# Same shape and therefore the same class as SPEC and CLAIM: a per-key record +# globbed for reading, deliberately NOT a shared table every change must write. +# Absent until #668 -- the registry landed with no pattern here, so every one of +# its files was unclassified and a required check refused any PR touching a pin. +ORACLE = re.compile(r"\.agents/oracles/[A-Za-z0-9_.-]+\.md\Z") # Retired state evidence, moved wholesale under completed/ when history became # git. It is archived evidence, classified like every other completed record. COMPLETED_STATE_EVENT = re.compile( @@ -368,6 +374,7 @@ def classify_path(path: str) -> str: path in PROCEDURE_FILES or SPEC.fullmatch(path) or CLAIM.fullmatch(path) + or ORACLE.fullmatch(path) or COMPLETED.fullmatch(path) or COMPLETED_STATE_EVENT.fullmatch(path) ): diff --git a/tests/scripts/test_check_pr_size.py b/tests/scripts/test_check_pr_size.py index 5e2842f69..e14cdae75 100755 --- a/tests/scripts/test_check_pr_size.py +++ b/tests/scripts/test_check_pr_size.py @@ -354,6 +354,41 @@ def test_retiring_the_budget_did_not_retire_the_other_contracts(self) -> None: any("mutation evidence" in e for e in checker.change_errors([lone_checker])) ) + def test_every_secondary_oracle_file_classifies(self) -> None: + """One file per oracle must classify (GATE-PR-SIZE-BINARY follow-on, #668). + + RED before the fix on EVERY tracked file under .agents/oracles/: the + secondary-oracle registry landed with no pattern in the checker, so a + required check refused any PR that recorded a pin -- which is the one + thing the registry exists to make cheap. Asserted on the whole tracked + set rather than a sample, so a ninth oracle added without a class is + caught here and not in someone's PR. + """ + tracked = subprocess.run( + ["git", "ls-files", ".agents/oracles/"], + capture_output=True, text=True, check=True, cwd=checker.ROOT, + ).stdout.split() + self.assertTrue(tracked, "expected tracked .agents/oracles/ files") + for path in tracked: + with self.subTest(path=path): + self.assertEqual(checker.classify_path(path), "procedure") + + def test_oracles_is_a_pattern_not_a_blanket_directory_exemption(self) -> None: + """The class is earned by shape, not by living under .agents/oracles/. + + AGENTS.md forbids hiding mutable files behind a blanket directory + exemption, so a non-.md file or a nested path there must still fail + closed rather than inherit `procedure`. + """ + for path in ( + ".agents/oracles/pin.txt", + ".agents/oracles/vllm.json", + ".agents/oracles/nested/dir.md", + ): + with self.subTest(path=path): + with self.assertRaises(ValueError): + checker.classify_path(path) + def test_a_classified_binary_is_accepted(self) -> None: """A binary at a classified path is not an error (GATE-PR-SIZE-BINARY, #615).