fix: regression gate reported exit 0 on real regressions (#134) - #140
fix: regression gate reported exit 0 on real regressions (#134)#140kiki830621 wants to merge 3 commits into
Conversation
Three payloads made the compare stage print "all corpora within
tolerance" and exit 0 with a 0.99 measurement against a 0.05 golden.
Fail-open is categorically worse than a noisy log: a gate that says
pass when it should say fail is not a weakened gate, it is an absent
one still producing the reassuring output of a present one.
Non-finite numbers: json.load accepts bare NaN/Infinity, and every
comparison against NaN is false, so `diff > tol` was false and the
entry passed. Validation now runs AFTER float() — the only place it
works, since {"golden": "NaN"} is a legal JSON string that never
triggers parse_constant while float("NaN") still yields nan.
Unbounded tolerance: this path needs no unusual number at all, which
is what made it hard to see. tolerance 1e308 rendered a 94-point
regression as an ordinary pass line. tolerance now carries the
tightest bound of the three fields.
Bounds are asymmetric by design: golden and tolerance are dangerous
when large (both make the comparison vacuous), error_rate only when
non-finite (a large measurement correctly fails).
Passing lines now render the tolerance they were judged against.
Auditing a pass is this log's whole purpose, and the one number that
could expose a rigged pass appeared only on failing lines.
Refs #134
A run that compared NOTHING reported success: with both sides empty
every loop was a no-op, so the gate printed "all 0 corpora within
tolerance" and exited 0. A fetch that produced no measurements read as
"this release is safe". A gate that verified no corpora has not
passed, it has not run.
A measurement could be judged against a different metric's golden —
`metric` was validated on the baseline side and never compared across,
so a WER number could be scored against a CER threshold.
A repeated JSON key hid the real threshold: Python keeps the last
value, so {"golden": 0.9, "golden": 0.01} silently became 0.01. The
existing duplicate-corpus check does not see this — that compares
entries, this is one key repeated inside a single entry.
A boolean laundered into a measurement: bool subclasses int, so
float(true) is 1.0 and a bare `true` passed every bound.
TOLERANCE_MAX tightened 0.5 → 0.25. No fixed ceiling separates
generous from disabled on its own, so the bound handles absurd values
and the newly-rendered tolerance handles merely-too-lenient ones.
Refs #134
4428d3b to
7c84d28
Compare
Verify Report — PR #140Enginemanual fan-out (4 lens Agents + sequenced Devil's Advocate, model: opus, file-based output) + Codex ( Backend note: canonical Diff-freshness gate (#228): PASS — frozen Process notes
AggregateFAIL — 2 blocking, 5 follow-up. The PR closes everything issue #134 enumerated. It does not close the class the issue is named after, and both blocking findings are instances of that class reached without any anomalous value. Requirements coverage — issue #134Every atomic requirement is FULLY addressed and was independently reproduced: bare Both mutation-testing claims in the PR body were verified exactly, not accepted — dropping the The umbrella requirement — 「gate 對『真實 regression』不得 exit 0」 — is PARTIALLY addressed. See F1 and F2. The issue's warning that this work would collide with #132's byte-identical lock is wrong, and the PR's rebuttal is right — verified by exhaustion rather than accepted: the only exact-stdout assertions in the repo are three in Findings (merged, deduped; severity = max, except where the Devil's Advocate adjudicated)
Adversarial pass — the severity dispute, adjudicatedThe four lenses split on F1: logic said CRITICAL, security HIGH / fix before merge, requirements MEDIUM, regression MEDIUM and explicitly not blocking. Because all four saw the same evidence and the same reproduction, taking the maximum would have buried a genuine disagreement about reasoning inside a Ruling: HIGH, fix before merge — upholding security and overturning the other three.
The decisive argument was cost, and it was measured. Replaying all 57 assertions against tightened bounds: The code comment justifying On F2, and an honest limit of the adversarial passThe Devil's Advocate ran a systematic 207-probe fuzz — every JSON type substituted into every field position, each probe carrying a real 0.99-vs-0.05 regression so that any But it has a scope limit the pass did not state: the probe dimension is type substitution with both sides present. Cardinality was never a probe dimension, so that fuzz could not have found F2 regardless of how many probes it ran. The cross-model leg found it from a different starting question — what entitles this decider to believe it received a complete input — and F2 was then confirmed independently by reading Recorded because a negative result is only as strong as the space it actually covered. Scope checkClean. Three files, +374/−2, all within issue #134's surface. The re-cut that moved the FluidAudio pin to PR #142 left no residue — verified: Sister issues are untouched and not made harder: The four extra paths from the second commit exceed the issue's three enumerated "Expected" items, but the issue's own 命名 section fixes the scope as 「gate 對真實 regression 回報 exit 0」 rather than "NaN handling", and all four are instances of that verdict going wrong. Not scope creep by the issue's own definition. One near-miss worth recording: NextF1 and F2 gate the merge. F1 is one constant plus three assertions at a measured cost of zero failing tests; F5 is one word and should ride along. F6's documentation defects should be corrected before merge for the reason the PR itself argues: in a change whose thesis is "the reassuring output of a gate that is not what it says", shipping a body that misstates its own bound is the wrong note to end on. F3 is the durable answer to the whole F1 class and belongs in a follow-up issue. Verify was run at |
7a4b2ea to
f9155c9
Compare
Verification found that bounding `tolerance` had closed one handle of a two-handled lever and left the other free. The decision is `actual - golden > tol`, so the gate passes anything at or below `golden + tolerance` — that sum is the effective threshold, and the two fields enter it identically. A golden of 2.0 with an honest tolerance of 0.02 admitted every physically possible error rate, and the tolerance this same issue had just put on the passing line read a reassuring 0.0200, because the number that moved was not the one being rendered. Reproduced against the real committed baseline with one field edited. The bound is now on the sum (<= 0.5, against a committed maximum of 0.1749). It is the only bound invariant to trading one handle for the other; a per-field bound always leaves its partner as a substitute. `golden`'s own constant is deleted rather than kept. With the sum bounded it can never be the guard that fires, and mutation testing put it at zero failed assertions both before and after. A constant no test can distinguish from its absence is not a guard. Its stated justification was also wrong: "CER can exceed 1.0 via insertions" describes a measurement, which ERROR_RATE_MAX governs. A truncated comparison also still reported success. Closing the empty case closed cardinality 0, not 1-of-N: both sides derive from the same baseline.json, so deleting entries shrinks the baseline, the work list and the measured set together and the gate prints "all 1 corpora within tolerance" over a release that verified one twelfth of what it should have — by pure deletion, with no unusual value anywhere. The expected set is pinned in baseline-meta.json and, for the repo's own baseline, is now an INVARIANT of regression-gate.sh: a missing, unreadable or malformed meta is fatal there, an overridden BESTASR_BASELINE only warns. The compare stage still tolerates an absent anchor with a printed NOTE, because a stdin filter cannot know whether it was handed a whole sweep — the invariant belongs where the answer is knowable. That conditional also upgrades the #48 model-artifact pin, which was skipped SILENTLY when the same file was missing. A satisfied anchor now says so. Its only previous evidence was the absence of the warning, which asks a reader to already know the warning exists — the argument this issue makes for rendering the tolerance on passing lines, applied to the guard it just added. `error_rate: false` was the one boolean position still able to flip a verdict, and nothing tested it: float(false) is 0.0, a boolean laundered into a PERFECT SCORE rather than a bad one. Bounding golden at 0.5 had quietly made the existing boolean test vacuous, which is the general hazard — tightening one bound can hollow out another guard's only test. OverflowError escaped the numeric converter; a bare 400-digit integer parses to an arbitrary-precision int and float() raises neither TypeError nor ValueError. Non-zero exit, so never a bypass, but it reached the log as a stack trace where the verdict belongs. Rejected values are capped at 120 characters and corpus lists at 12 names, the anchor is shape-checked before use, it names the file it actually read rather than the default path, and the sum-bound message renders at 17 significant digits so a rejected value cannot print as equal to the threshold it exceeded. A second verify round found the anchor could still be switched off by omitting a key, and that three guards had no test able to tell them from their absence. The gate now REFUSES an unanchored run on the repo's own baseline (missing, unreadable, or corpora absent/null/not a unique non-empty string array); an overridden BESTASR_BASELINE warns instead. The compare stage keeps tolerating an absent anchor with a NOTE, because a stdin filter cannot know whether it was handed a whole sweep — the invariant belongs where the answer is knowable, and both halves now have a test so the permissive half is no longer the whole specification. Every guard is non-vacuous, measured by deleting each independently: sum bound 9, completeness 7, boolean 6, isfinite 5, TOLERANCE_MAX 3, OverflowError 3, ERROR_RATE_MAX 2, echo cap 2, and the gate's own anchor wiring 1 — a one-token typo in that shell heredoc used to revert the whole completeness guard with a green suite, because nothing ran regression-gate.sh. Those counts come from scripts/mutation-check.sh, committed alongside them. A count the reader cannot re-run is the same defect as a threshold the reader cannot see, one level out. The PR body has been regenerated from this tree. An earlier version of this commit message claimed the body had already been corrected when only the CHANGELOG had been; that claim was false when written and is the reason this message was amended rather than followed by another commit. 474 tests / 88 suites green (+29 over main). Refs #134
f9155c9 to
39a9a46
Compare
Verify Report — PR #140, Round 2Re-verify after the round-1 blocking findings were addressed. This report describes Engine4 lens Agents (opus) + sequenced Devil's Advocate + Codex ( Process notes
AggregateFAIL at Both round-1 blockers were genuinely closed and the code was in better shape than round 1 left it. What failed was narrower and is stated precisely in the adjudication below. Round-1 findings — dispositionEvery R1 finding was checked for silent claiming, and none was claimed fixed without being fixed. Each deferral is either unmentioned or explicitly scoped in the CHANGELOG.
Issue #134's three enumerated requirements: FULLY addressed, all independently reproduced. The umbrella requirement — 「gate 對『真實 regression』不得 exit 0」 — is where the adjudication below lands. The adjudication: is F2 closed?This is the round's substantive disagreement and it split the ensemble 5–1. Four Claude lenses and the Devil's Advocate's first pass: CLOSED. The named attack — pure deletion from Codex: PARTIALLY, and blocking. Its framing: the two env vars being overridable is not the problem — anyone with shell control can substitute inputs. The problem is that the supported missing-file / missing-key state is designed to succeed. It listed five non-adversarial paths, of which the fourth is ordinary local triage: copy the baseline to Ruling, after the Devil's Advocate re-derived the argument: Codex is right. F2 is PARTIALLY closed. The reasoning that decided it was a layering argument nobody had made:
F2's requirement lives at the middle layer, and that layer did not implement it. So the finding was not closed at the layer where closing it is possible. The Devil's Advocate's account of why the majority erred is worth preserving verbatim, because it is a failure mode this ensemble had already ruled against once:
Where the ruling did not follow Codex. Codex's first proposed remedy — make the compare stage exit non-zero on a missing anchor — was rejected: it would break the filter's documented contract and eleven tests, and would put the invariant at the layer that cannot justify it. Codex's own second branch (an explicit opt-in, with the release wrapper never enabling it) is the one that was taken. Codex also bundled R1-F7/F9 (corpus type confusion, shape guards) into its blocking list; those are pre-existing, explicitly deferred follow-ups and were kept there. Findings (merged; the ~20 filed items dedupe to these)
Negative space — attacks that failedRecorded so the surviving verdicts carry weight, and because two of these are why the first CLOSED ruling looked defensible.
Two verifiers also corrected their own earlier work: one revised a warnings comparison after realising its first reading came from an incremental build, and the Devil's Advocate discarded its own boolean-payload conclusion after Codex found a case its grid's shape had masked — a negative result is only as strong as the space it covered, applied by its author to itself. What changed in response (
|
1 participant
The release gate said pass when it should have said fail. That is fail-open — categorically worse than a noisy log, because a gate that produces the reassuring output of a present gate while being absent is worse than no gate at all: everything downstream of it was trusting a verdict that could not fail.
Refs #134
Reproduced before fixing
All three printed
✓ regression gate: all 1 corpora within toleranceand exited 0, with a 0.99 measurement against a 0.05 golden:The third line is the one worth staring at. It contains no unusual number and is byte-for-byte indistinguishable from a genuine pass — because the threshold it was judged against was never printed.
Ten paths, found over three rounds
Round 1 fixed the three above plus four more. Two verify rounds then found that the fix had closed one handle of a two-handled lever, and that the empty-set guard had closed only cardinality zero.
Non-finite numbers.
json.loadaccepts bareNaN/Infinity, and every comparison against NaN is false. Validation happens afterfloat(), the only place it works:{"golden": "NaN"}is a legal JSON string, soparse_constantnever fires whilefloat("NaN")still yields nan.An unbounded
tolerance. Needs no unusual number and defeats every existing defence. Capped at0.25.An inflated
goldenbought exactly the slack a widetolerancewas refused. The decision isactual - golden > tol, so the gate passes anything at or belowgolden + tolerance— that sum is the effective threshold and the two fields enter it identically. Agoldenof 2.0 with an honesttoleranceof 0.02 admitted every physically possible error rate, and the tolerance newly rendered on the pass line read a reassuring0.0200, because the number that had moved was not the one being shown. The bound is now on the sum (≤ 0.5against a committed maximum of 0.1749).A truncated comparison still reported success. Both sides of the comparison derive from the same
baseline.json, so deleting entries shrinks the baseline, the work list and the measured set together and the gate printsall 1 corpora within tolerance— by pure deletion, no unusual value anywhere. The expected corpus set is now pinned inbenchmarks/baseline-meta.jsonand is an invariant of the gate: for the repo's own baseline a missing or malformed meta is fatal, not a warning.Also closed: a run that compared nothing, a measurement judged against a different metric's golden, a repeated JSON key hiding the real threshold, a boolean laundered into a measurement (including
false, which laundered into a perfect score), andOverflowErrorescaping the numeric converter.Bounds
golden + tolerance(0, 0.5]tolerance(0, 0.25]golden 0.1 + tolerance 0.3sums to 0.4 and clears it, but a 30-point tolerance applies whatever the measurement turns out to beerror_rate[0, 100]goldenhas no separate constant. The sum bound forcesgolden < 0.5, so any ceiling above that can never fire — mutation testing put the oldGOLDEN_MAX = 2.0at 0 failed assertions before and after. It was removed rather than propped up with a test written to justify it. (Its stated rationale was wrong as well as inert: "CER can exceed 1.0 via insertions" describes a measurement, whichERROR_RATE_MAXgoverns.)Passing lines now show their threshold
Auditing a pass is what this log is for, and the single number capable of exposing a rigged pass appeared only on failing lines. The same argument later applied to the completeness anchor, which now prints
completeness: 12 corpora pinned by … — verifiedinstead of being observable only by the absence of its warning.Stated plainly, since the point is that the prose must not flatter the gate: the tolerance ceiling and the comparison are both inclusive, so a regression of exactly 0.25 against a golden of 0 does pass. The bound refuses the absurd values; the rendered tolerance exposes the merely-too-lenient ones.
One correction to the issue
#134 warns that this conflicts with #132's byte-identical regression lock. It does not — that lock (
RegressionWorklistTests.pinnedWorklist) pins the worklist stage's TSV, a different script and a different stage. Verified by exhaustion: the only exact-stdout assertions in the repo targetbaseline-worklist.py.Tests
474 tests / 88 suites green (+29 over
main).Non-vacuousness by mutation — each guard deleted independently, counting failed assertions. Re-runnable:
./scripts/mutation-check.sh, committed with the change, which restores the tree and fails if anything scores zero.isfiniteTOLERANCE_MAXOverflowErrorERROR_RATE_MAXThat last row is the one that mattered most: a one-token typo in the shell heredoc (
.get("corpora")→.get("corpora_SET")) used to revert the entire completeness guard with a 100% green suite, because nothing ranregression-gate.sh.Not in scope
#133(gate does not enforce design D2's single fixed reference model) and#127(pin-reference-model.shpath sink) are separate open issues on the same script family. Malformed document shapes — a top-level array, a missingcorpusorlanguagekey — still surface as tracebacks rather than named gate errors; all fail closed, and a shape guard afterjson.loadis the follow-up. Coupling goldens to the provenance record (so a golden that moves whileseeded_atstands still is itself a gate error) is the durable answer to the residual inflation window and is likewise a follow-up.