diff --git a/CHANGELOG.md b/CHANGELOG.md index f17d6be..dec6ed1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,6 +108,146 @@ All notable changes to bestASR are documented here. The format follows ### Fixed +- **The regression gate reported exit 0 on real regressions (#134)**: three + payloads made `scripts/lib/baseline-compare.py` print `✓ all corpora within + tolerance` and **exit 0** while a 0.99 measurement sat against a 0.05 golden. + This is fail-**open**, and it 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.** Python's `json.load` accepts bare `NaN` / + `Infinity`, and *every* comparison against NaN is false — so `diff > tol` + was false and the entry "passed". Validation now happens **after** + `float()`, which is the only place it works: rejecting the literal at parse + time via `parse_constant` does not close the class, because `{"golden": + "NaN"}` is a legal JSON *string* that never triggers it while + `float("NaN")` still yields nan (likewise `1e999` → inf). + - **An 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, indistinguishable from a real one. + `tolerance` is now capped at **0.25**, the tightest per-field bound of the + three, against a committed baseline that uses 0.02 throughout. Stated + plainly, since the point of this change is that the prose must not flatter + the gate: both that ceiling and the comparison are *inclusive*, so a + regression of exactly 0.25 against a golden of 0 does pass. The bound + refuses the absurd values; the tolerance now rendered on every passing line + is what exposes the merely-too-lenient ones. + - **A garbage numeric field** raised `ValueError` mid-run. A traceback is not + a verdict; numeric fields now fail as a named gate error. (Scoped honestly: + this covers the *numeric* converter. Malformed document shapes — a + top-level array, a missing `corpus` or `language` key — still surface as + tracebacks. They all exit non-zero, so none is fail-open, but the class is + not uniformly handled and a shape guard after `json.load` is the follow-up.) + + `error_rate` is bounded differently on purpose: it is hazardous only when + *non-finite*, because a large measurement correctly **fails**. Its ceiling is + a garbage filter, not a security boundary. + + A cross-model verification round found four more ways the same verdict could + come out wrong, each fixed here: + + - **A run that compared nothing reported success.** With the baseline and the + measured set both empty, every loop was a no-op and the gate printed + `all 0 corpora within tolerance` and exited 0 — so 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 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 — a decoy + in the one file whose purpose is to be auditable by reading it. 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 as an ordinary + number. + + A second verification round found that bounding `tolerance` had closed one + handle of a two-handled lever, and named the other: + + - **An inflated `golden` bought exactly the slack a wide `tolerance` was + refused.** 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 newly rendered on the pass line read a reassuring + `0.0200`, because the number that had moved was not the one being shown. + Reproduced against the real committed baseline with a single field edited. + The bound is now on **the sum** (`≤ 0.5`, against a committed maximum of + 0.1749) — the only bound invariant to trading one handle for the other. + `golden`'s separate ceiling was **deleted**: with the sum bounded it could + never be the guard that fired, and mutation testing put it at 0 of 34 + assertions. A constant no test can distinguish from its absence is not a + guard. + - **A truncated comparison still reported success.** Closing the empty case + closed cardinality 0, not 1-of-N. Both sides of the comparison derive from + the same `baseline.json` — the worklist stage reads it and the assembler + iterates it — 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 corpus + set is now pinned in `benchmarks/baseline-meta.json` and supplied to the + compare stage by the gate, so truncating the baseline requires a second, + deliberate edit to the file that carries the seeding provenance. A run with + no anchor says so on stdout rather than passing quietly — a completeness + check that can be skipped by omitting a key is the shape of the hole it + closes. + - **`OverflowError` escaped the numeric converter.** JSON has no integer + width limit, so a bare 400-digit literal parses to an arbitrary-precision + `int` and `float()` raises — neither `TypeError` nor `ValueError`. Exits + non-zero, so never a bypass, but it reached the log as a stack trace where + the verdict belongs. Universal, not version-specific. Rejected values are + now also truncated at 120 characters, so a hostile field cannot become the + log. + + A third round found that the completeness anchor could be switched off by + omitting a key — the shape the fix was written against, one layer out — and + that three guards had no test that could tell them from their absence: + + - **The anchor is now an invariant of the gate, not a nicety.** + `scripts/regression-gate.sh` knows which baseline it is running, so for the + repo's own it now **fails** when `baseline-meta.json` is missing, + unreadable, or its `corpora` is absent / null / not a non-empty array of + unique strings; an overridden `BESTASR_BASELINE` warns instead. The compare + stage keeps tolerating an absent anchor with a printed NOTE, because a stdin + filter genuinely cannot know whether it was handed a whole sweep — the + invariant belongs where the answer is knowable. This also upgrades the #48 + model-artifact pin, which used to be skipped **silently** when that same + file was missing. + - **A satisfied anchor now says so** (`completeness: N corpora pinned by … — + verified`). Its only previous evidence was the *absence* of the warning, + which asks a reader to already know the warning exists — the same argument + this entry makes for rendering the tolerance on passing lines. + - **`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. Every other position became + redundantly covered once `golden` was bounded at 0.5 — which had quietly + made the existing boolean test vacuous. + - `expected_corpora` is shape-checked before use (a bare string would have + been iterated character-wise; an int or a nested list raised a traceback), + the anchor names the file it actually read rather than the default path, its + corpus lists are length-capped, and the sum-bound message renders at 17 + significant digits so a rejected value cannot print as *equal* to the + threshold it exceeded. + + Every guard is non-vacuous, measured by deleting each independently and + counting failed assertions: effective-threshold bound **9**, completeness + anchor **7**, boolean guard **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. + + Those numbers come from **`scripts/mutation-check.sh`**, which is committed + alongside them. A count a reader cannot re-run is the same defect as a + threshold a reader cannot see, one level out; the script deletes each guard, + re-runs the suite, restores the tree, and fails if anything scores zero. + + Passing lines now render **the tolerance they were judged against**. Auditing + a pass is the whole purpose of this log, and the one number that could expose + a rigged pass appeared only on *failing* lines — a reader checking a green run + had no way to see that the bar had been lowered to meet it. + - **`--backend apple-speech` was silently substituted (#121)**: `Router`'s backend-membership list omitted the new backend, so an explicit `--backend apple-speech` was discarded and another backend's output was written under diff --git a/Tests/BestASRKitTests/RegressionBaselineTests.swift b/Tests/BestASRKitTests/RegressionBaselineTests.swift index 18fa0d7..2c38f3d 100644 --- a/Tests/BestASRKitTests/RegressionBaselineTests.swift +++ b/Tests/BestASRKitTests/RegressionBaselineTests.swift @@ -59,12 +59,16 @@ struct RegressionBaselineTests { // MARK: - compare stage (spec: Regression gate fails on accuracy regression) - private func runCompare(baseline: [[String: Any]], measured: [[String: Any]]) throws -> ( + private func runCompare( + baseline: [[String: Any]], measured: [[String: Any]], + expectedCorpora: [String]? = nil + ) throws -> ( exit: Int32, output: String ) { let script = Self.repoRoot.appendingPathComponent("scripts/lib/baseline-compare.py") - let input = try JSONSerialization.data( - withJSONObject: ["baseline": baseline, "measured": measured]) + var payload: [String: Any] = ["baseline": baseline, "measured": measured] + if let expectedCorpora { payload["expected_corpora"] = expectedCorpora } + let input = try JSONSerialization.data(withJSONObject: payload) let p = Process() p.executableURL = URL(fileURLWithPath: "/usr/bin/python3") p.arguments = [script.path] @@ -82,6 +86,39 @@ struct RegressionBaselineTests { return (p.terminationStatus, out) } + /// Feed the compare stage RAW json text. Needed because the fail-open + /// payloads in #134 cannot be built with JSONSerialization: bare `NaN` / + /// `Infinity` are not legal JSON, and Foundation refuses to emit them — + /// yet Python's `json.load` accepts both, which is precisely the hole. + private func runCompareRaw(_ json: String) throws -> (exit: Int32, output: String) { + let script = Self.repoRoot.appendingPathComponent("scripts/lib/baseline-compare.py") + let p = Process() + p.executableURL = URL(fileURLWithPath: "/usr/bin/python3") + p.arguments = [script.path] + let inPipe = Pipe() + let outPipe = Pipe() + p.standardInput = inPipe + p.standardOutput = outPipe + p.standardError = outPipe + try p.run() + inPipe.fileHandleForWriting.write(Data(json.utf8)) + inPipe.fileHandleForWriting.closeFile() + let out = String( + data: outPipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? "" + p.waitUntilExit() + return (p.terminationStatus, out) + } + + /// One baseline entry + one measured entry, with `golden` / `tolerance` / + /// `error_rate` injected as raw JSON tokens. + private func payload(golden: String, tolerance: String, errorRate: String) -> String { + """ + {"baseline":[{"corpus":"c1","language":"en","model":"large-v3-turbo",\ + "metric":"wer","golden":\(golden),"tolerance":\(tolerance)}],\ + "measured":[{"corpus":"c1","metric":"wer","error_rate":\(errorRate)}]} + """ + } + private let entry: [String: Any] = [ "corpus": "c1", "language": "zh", "model": "large-v3-turbo", "metric": "cer", "golden": 0.10, "tolerance": 0.02, @@ -193,4 +230,525 @@ struct RegressionBaselineTests { #expect(!r.output.contains("\r"), "carriage return reached the log") #expect(r.output.contains("\\x1b"), "the offending bytes should stay visible") } + + // MARK: - fail-OPEN (#134): the gate reporting exit 0 on a real regression + // + // These are 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 + // that still produces the reassuring output of a present one. + + @Test func `a non-finite golden cannot make a real regression pass`() throws { + // measured 0.99 against golden 0.10 is a catastrophic regression. + // NaN makes `diff > tol` false — every comparison against NaN is false — + // so the gate printed a green check and exited 0. + for token in ["NaN", "Infinity", "-Infinity", "1e999"] { + let r = try runCompareRaw(payload(golden: token, tolerance: "0.02", errorRate: "0.99")) + #expect(r.exit != 0, "golden \(token) let a 0.99-vs-0.10 regression pass") + #expect(r.output.contains("GATE ERROR"), "golden \(token) produced no gate error") + } + } + + @Test func `a non-finite value spelled as a JSON string is caught too`() throws { + // The subtle half: `{"golden": "NaN"}` is a legal JSON *string*, so + // `json.load(parse_constant=...)` never fires — but `float("NaN")` + // still yields nan. Validation must therefore happen AFTER conversion, + // not by trying to reject the literal at parse time. + for token in ["\"NaN\"", "\"Infinity\"", "\"1e999\""] { + let r = try runCompareRaw(payload(golden: token, tolerance: "0.02", errorRate: "0.99")) + #expect(r.exit != 0, "golden \(token) (string form) let a regression pass") + } + } + + @Test func `a non-finite measured error rate is a gate error, not a pass`() throws { + let r = try runCompareRaw(payload(golden: "0.05", tolerance: "0.02", errorRate: "NaN")) + #expect(r.exit != 0) + #expect(r.output.contains("GATE ERROR")) + } + + @Test func `a tolerance large enough to swallow any regression is itself an error`() throws { + // The path that needs no non-finite number at all, and is therefore the + // harder one to notice: a wide tolerance renders a 94-point regression + // as an ordinary pass line. + for token in ["1e308", "0.9", "1.0", "42"] { + let r = try runCompareRaw(payload(golden: "0.05", tolerance: token, errorRate: "0.99")) + #expect(r.exit != 0, "tolerance \(token) swallowed a 0.99-vs-0.05 regression") + #expect(r.output.contains("GATE ERROR"), "tolerance \(token) produced no gate error") + } + } + + @Test func `a non-positive tolerance is rejected rather than silently gating everything`() + throws + { + for token in ["0", "-0.02"] { + let r = try runCompareRaw(payload(golden: "0.05", tolerance: token, errorRate: "0.05")) + #expect(r.exit != 0, "tolerance \(token) accepted") + } + } + + @Test func `a garbage numeric field fails cleanly instead of raising`() throws { + // `float("abc")` raises ValueError. A traceback is not a verdict — the + // gate must say which corpus and which field, on stdout, like every + // other gate error. + let r = try runCompareRaw(payload(golden: "\"abc\"", tolerance: "0.02", errorRate: "0.05")) + #expect(r.exit != 0) + #expect(r.output.contains("GATE ERROR"), "expected a gate error, got: \(r.output)") + #expect(!r.output.contains("Traceback"), "raised instead of reporting") + #expect(r.output.contains("c1"), "the failing corpus must be named") + } + + @Test func `the passing line shows the tolerance it was judged against`() throws { + // Auditing a PASS is the whole point of this log, and the one number + // that could expose a rigged pass — the threshold — appeared only on + // FAILING lines. A reader checking a green run had no way to see that + // the bar had been lowered to meet it. + let r = try runCompare( + baseline: [entry], + measured: [["corpus": "c1", "metric": "cer", "error_rate": 0.11]]) + #expect(r.exit == 0) + #expect(r.output.contains("tolerance"), "pass line hides its own threshold") + #expect(r.output.contains("0.0200"), "the tolerance VALUE must be visible, not just the word") + } + + @Test func `a run that compared nothing is not a pass`() throws { + // #134 verify (cross-model leg): with both sides empty every loop is a + // no-op, so the gate printed "all 0 corpora within tolerance" and exited + // 0. A fetch that produced no measurements, or a worklist that silently + // emptied, therefore read as "this release is safe". + let r = try runCompareRaw(#"{"baseline":[],"measured":[]}"#) + #expect(r.exit != 0, "an empty comparison reported success") + #expect(r.output.contains("GATE ERROR")) + } + + @Test func `a boolean is not laundered into a measurement`() throws { + // `bool` subclasses `int`, so `float(true)` is 1.0 and a bare `true` + // passed every bound as an ordinary number. + let r = try runCompareRaw(payload(golden: "true", tolerance: "0.02", errorRate: "0.99")) + #expect(r.exit != 0) + #expect(r.output.contains("GATE ERROR")) + } + + @Test func `a boolean false is not laundered into a perfect score`() throws { + // The case above stopped proving anything once `golden` was bounded at + // 0.5: `float(true)` is 1.0, which the bounds now refuse before the type + // check is ever consulted. Every bool position is redundantly covered + // that way — except this one. `float(false)` is **0.0**, which is inside + // every bound, so the type guard is the only thing standing between a + // bare `false` and a measurement of *zero errors*. + // + // That is also the only direction that manufactures a PASS rather than a + // failure: with the guard deleted this payload prints + // `✓ … golden 0.0500 → measured 0.0000 (-0.0500 ≤ tolerance 0.0200)` + // and exits 0. + let r = try runCompareRaw(payload(golden: "0.05", tolerance: "0.02", errorRate: "false")) + #expect(r.exit != 0, "a boolean false was accepted as a perfect measurement") + #expect(r.output.contains("GATE ERROR")) + #expect(r.output.contains("boolean")) + } + + @Test func `a boolean false is not laundered into a golden either`() throws { + // The other verdict-flipping bool position, and a different mechanism + // worth keeping distinct from the one above. `golden: false` → 0.0 makes + // the THRESHOLD maximally strict, so removing the guard flips the exit + // code while leaving the accuracy comparison conservative: a type hole. + // `error_rate: false` → 0.0 makes the MEASUREMENT a perfect score, so an + // arbitrarily bad system is admitted as flawless: an accuracy hole. + // Only the second is a fail-open in substance; both are worth pinning. + let r = try runCompareRaw(payload(golden: "false", tolerance: "0.02", errorRate: "0.01")) + #expect(r.exit != 0, "a boolean false was accepted as a golden of zero") + #expect(r.output.contains("GATE ERROR")) + #expect(r.output.contains("boolean")) + } + + @Test func `a measurement is never judged against a different metric's golden`() throws { + // metric was validated on the baseline side only and never compared + // across, so a WER measurement could be judged against a CER golden — + // passing or failing on a number that means something else. + let r = try runCompareRaw( + #"{"baseline":[{"corpus":"c1","language":"zh","model":"m","metric":"cer","golden":0.05,"tolerance":0.02}],"measured":[{"corpus":"c1","metric":"wer","error_rate":0.06}]}"#) + #expect(r.exit != 0, "a wer measurement was judged against a cer golden") + #expect(r.output.contains("GATE ERROR")) + } + + @Test func `a repeated JSON key cannot hide the real threshold`() throws { + // Python keeps the LAST value for a duplicate key, so + // {"golden": 0.9, "golden": 0.01} silently becomes 0.01 — a decoy in + // the one file whose purpose is to be auditable by reading it. The + // duplicate-CORPUS check does not see this: that compares entries, this + // is one key repeated inside a single entry. + let r = try runCompareRaw( + #"{"baseline":[{"corpus":"c1","language":"en","model":"m","metric":"wer","golden":0.9,"golden":0.01,"tolerance":0.02}],"measured":[{"corpus":"c1","metric":"wer","error_rate":0.02}]}"#) + #expect(r.exit != 0, "a duplicate key silently took the last value") + #expect(r.output.contains("duplicate key")) + } + + @Test func `the real committed baseline passes the new numeric validation`() throws { + // The bounds must not reject the file the gate actually runs on. + let url = Self.repoRoot.appendingPathComponent("benchmarks/baseline.json") + let data = try Data(contentsOf: url) + let entries = try #require( + try JSONSerialization.jsonObject(with: data) as? [[String: Any]]) + let measured = entries.map { + ["corpus": $0["corpus"]!, "metric": $0["metric"]!, "error_rate": $0["golden"]!] + } + let r = try runCompare(baseline: entries, measured: measured) + #expect(r.exit == 0, "the committed baseline was rejected: \(r.output)") + } + + // MARK: - the effective threshold is `golden + tolerance` (#134 verify, F1) + + // `diff = actual - golden` and the test is `diff > tol`, i.e. the gate + // passes anything at or below `golden + tolerance`. The two fields are one + // lever, so bounding only `tolerance` leaves the sum — the number that + // actually decides the verdict — unbounded in the other direction. + + @Test func `an inflated golden cannot buy slack a tolerance would be refused`() throws { + // The R1 reproduction, kept as a regression lock. Note which mechanism + // actually fires: `golden` reuses the effective threshold as its + // per-field ceiling, so 2.0 is refused there and this payload never + // reaches the sum check. It pins the verdict, not the sum path — the + // case below is what covers the sum. + let r = try runCompareRaw(payload(golden: "2.0", tolerance: "0.02", errorRate: "0.99")) + #expect(r.exit != 0, "a golden of 2.0 waved through a 0.99 measurement") + #expect(r.output.contains("GATE ERROR")) + #expect(!r.output.contains("Traceback")) + } + + @Test func `the sum path fires for a golden that clears its own ceiling`() throws { + // `golden` 0.4 is comfortably inside the per-field ceiling and only the + // SUM (0.4 + 0.15 = 0.55) exceeds the bound, so this is the case that + // exercises the sum check rather than shadowing it. Asserting the + // distinguishing message is the point: without it the test could not + // tell which of the two mechanisms rejected the payload. + let r = try runCompareRaw(payload(golden: "0.4", tolerance: "0.15", errorRate: "0.5")) + #expect(r.exit != 0) + #expect( + r.output.contains("golden + tolerance is"), + "expected the sum-bound message, got: \(r.output)") + } + + @Test func `the sum bound catches the modest inflation the bound on golden alone misses`() + throws + { + // The realistic shape: no extreme value anywhere, an innocuous-looking + // pass line. `golden` alone clears its own ceiling; only the sum does not. + let r = try runCompareRaw(payload(golden: "0.49", tolerance: "0.02", errorRate: "0.50")) + #expect(r.exit != 0, "golden 0.49 + tolerance 0.02 exceeded the effective-threshold bound") + #expect(r.output.contains("GATE ERROR")) + } + + @Test func `the effective-threshold bound is inclusive at its edge`() throws { + // 0.48 + 0.02 == 0.5 exactly: accepted, so the bound refuses only what + // is strictly beyond it (consistent with the `≤ tolerance` rendering). + let r = try runCompareRaw(payload(golden: "0.48", tolerance: "0.02", errorRate: "0.49")) + #expect(r.exit == 0, "the sum bound rejected its own boundary: \(r.output)") + } + + // MARK: - the per-field ceilings are load-bearing (#134 verify, F4) + + // Both were removable with a green suite: every non-finite payload is + // caught earlier by `math.isfinite`, and no test used a FINITE value above + // either ceiling. A guard no test can distinguish from its absence is not + // a guard. (`golden`'s own ceiling failed that test even after the sum + // bound landed — the sum subsumes it entirely — so it was deleted rather + // than given a test written to justify it. `golden` is now bounded by the + // effective threshold, which is the number that actually decides.) + + @Test func `a finite golden beyond any usable threshold is refused by name`() throws { + let r = try runCompareRaw(payload(golden: "2.001", tolerance: "0.02", errorRate: "0.01")) + #expect(r.exit != 0) + #expect(r.output.contains("GATE ERROR")) + // Assert the DISTINGUISHING substring, not just "golden". The sum-bound + // message is "golden + tolerance is …", so `contains("golden")` is + // satisfied by either path and cannot verify the "by name" this test is + // named for — it would pass with the per-field bound deleted. + #expect( + r.output.contains("must be at most"), + "expected the per-field message, got: \(r.output)") + } + + @Test func `a tolerance inside the sum bound is still refused on its own terms`() throws { + // The region where TOLERANCE_MAX does work the sum bound cannot: + // 0.1 + 0.3 = 0.4 clears the effective-threshold ceiling, but a + // tolerance of 0.3 means "accept a 30-point regression on this corpus" + // regardless of what it actually scores. A large tolerance is worse + // than a large golden of the same size, because it applies whatever + // the measurement turns out to be — hence the tighter bound. + let r = try runCompareRaw(payload(golden: "0.1", tolerance: "0.3", errorRate: "0.2")) + #expect(r.exit != 0, "a 30-point tolerance passed because the sum happened to fit") + #expect(r.output.contains("GATE ERROR")) + // `contains("tolerance")` alone can never fail here — the PASSING line + // renders "… ≤ tolerance 0.3000", so it holds with the ceiling removed. + #expect( + r.output.contains("tolerance must be at most"), + "expected the tolerance ceiling message, got: \(r.output)") + } + + @Test func `a finite error rate above the per-field ceiling is refused`() throws { + let r = try runCompareRaw(payload(golden: "0.05", tolerance: "0.02", errorRate: "101")) + #expect(r.exit != 0) + #expect(r.output.contains("error_rate")) + #expect(r.output.contains("GATE ERROR")) + } + + // MARK: - a traceback is not a verdict, for integers too (#134 verify, F5) + + @Test func `a huge bare integer fails as a named gate error, not an OverflowError`() throws { + // `float()` raises OverflowError on a large Python int — neither + // TypeError nor ValueError, so it escaped the converter's except + // clause and reached the CI log as a stack trace where the verdict + // belongs. Reproduces on every CPython (the 4300-digit int↔str cap is + // far above this literal and never fires). + let huge = "1" + String(repeating: "0", count: 400) + let r = try runCompareRaw(payload(golden: huge, tolerance: "0.02", errorRate: "0.99")) + #expect(r.exit != 0) + #expect(!r.output.contains("Traceback"), "raw traceback instead of a verdict: \(r.output)") + #expect(r.output.contains("GATE ERROR")) + // The echo cap is a guard too, and the three assertions above hold with + // or without it — so without this one it would be exactly the "constant + // no test can distinguish from its absence" the compare script condemns. + #expect(r.output.contains("(401 chars)"), "the rejected value was not truncated") + #expect(!r.output.contains(String(repeating: "0", count: 200))) + } + + // MARK: - completeness: a truncated comparison is not a pass (#134 verify, F2) + + // The empty-set guard closed cardinality 0. It did not close 1-of-N: both + // sides of the comparison are derived from the same `baseline.json`, so + // deleting entries shrinks the baseline, the worklist and the measured set + // together and the gate reports "all 1 corpora within tolerance". + // The anchor has to come from outside that file. + + @Test func `a baseline missing corpora the provenance record pins is a gate error`() throws { + let r = try runCompare( + baseline: [entry], + measured: [["corpus": "c1", "metric": "cer", "error_rate": 0.10]], + expectedCorpora: ["c1", "c2", "c3"]) + #expect(r.exit != 0, "a 1-of-3 comparison reported success") + #expect(r.output.contains("GATE ERROR")) + #expect(r.output.contains("c2")) + #expect(r.output.contains("c3")) + } + + @Test func `a baseline carrying corpora the provenance record does not pin is a gate error`() + throws + { + // The other direction: an entry added to baseline.json without + // re-seeding is equally a divergence between the two files. + let r = try runCompare( + baseline: [entry], + measured: [["corpus": "c1", "metric": "cer", "error_rate": 0.10]], + expectedCorpora: []) + #expect(r.exit != 0) + #expect(r.output.contains("GATE ERROR")) + #expect(r.output.contains("c1")) + } + + @Test func `a matching corpus set passes and says the completeness check ran`() throws { + let r = try runCompare( + baseline: [entry], + measured: [["corpus": "c1", "metric": "cer", "error_rate": 0.10]], + expectedCorpora: ["c1"]) + #expect(r.exit == 0, "\(r.output)") + // The second half of this test's name used to be unverifiable: a + // satisfied anchor printed nothing, so the only evidence it ran was the + // absence of the NOTE. The script now says so positively, and this + // asserts it — otherwise the name claims more than the test checks. + #expect( + r.output.contains("completeness:") && r.output.contains("verified"), + "a satisfied anchor left no trace: \(r.output)") + } + + @Test func `the gate script actually wires the anchor into the payload it pipes`() throws { + // The seam neither the compare-stage tests nor the file-coupling test + // reach. Those inject `expected_corpora` as a parameter and compare the + // two JSON files in Swift; NEITHER runs `regression-gate.sh`, and + // `ci.yml` deliberately does not run the gate either. So renaming one + // token in the assembler — `.get("corpora")` → `.get("corpora_SET")` — + // silently reverts the whole guard with a 100% green suite. + // + // This extracts the assembler heredoc from the real script and runs it, + // so a change to that block has to survive a test. + let root = Self.repoRoot + let gate = try String( + contentsOf: root.appendingPathComponent("scripts/regression-gate.sh"), encoding: .utf8) + let body = try #require( + gate.range(of: "import json, os, sys").flatMap { start in + gate.range(of: "\nPY\n", range: start.lowerBound.. ( + exit: Int32, output: String + ) { + let gate = try String( + contentsOf: Self.repoRoot.appendingPathComponent("scripts/regression-gate.sh"), + encoding: .utf8) + let lines = gate.components(separatedBy: "\n") + guard + let defLine = lines.firstIndex(where: { $0.hasPrefix("DEFAULT_BASELINE=") }), + let start = lines.firstIndex(where: { $0.hasPrefix("META=\"${BESTASR_BASELINE_META") }), + let end = lines.firstIndex(where: { $0.hasPrefix("CACHE_ROOT=") }) + else { throw CocoaError(.fileReadCorruptFile) } + // `$0` must be the real script path: DEFAULT_BASELINE resolves the repo + // root from `dirname "$0"`, so passing anything else silently sends every + // case down the "overridden baseline" branch and the test proves nothing. + let script = """ + set -euo pipefail + BASELINE="$1" + TMP=$(mktemp -d); trap 'rm -rf "$TMP"' EXIT + \(lines[defLine]) + \(lines[start.. tol` is false and the entry "passes". +# The decision is `diff = actual - golden > tol`, i.e. the gate passes anything +# at or below `golden + tolerance`. THAT SUM is the effective threshold, and the +# two fields enter it identically — they are one lever with two handles. Bounding +# only `tolerance` (as the first pass at #134 did) leaves the other handle free: +# `golden: 2.0` with an honest `tolerance: 0.02` admits every physically possible +# error rate, and the newly-rendered tolerance reads a reassuring `0.0200` on the +# rigged line, because the number that moved is not the one being rendered. +# +# So the bound that matters is on the sum. It is the only one invariant to +# trading one handle against the other; a per-field bound will always leave its +# partner as a partial substitute. The committed baseline's widest entry is +# `golden 0.1549 + tolerance 0.02 = 0.1749`, so 0.5 leaves 2.9x headroom. +# +# The error directions are not comparable, which is why this errs tight: a bound +# set too tight fails LOUDLY at `swift test`, on every PR, naming the file. A +# bound set too loose fails SILENTLY at release, as a green line. +EFFECTIVE_THRESHOLD_MAX = 0.5 + +# `golden` has no separate CONSTANT, deliberately — it reuses the effective +# threshold as its per-field ceiling (see `validate_numbers`). The sum bound +# already forces `golden < EFFECTIVE_THRESHOLD_MAX` (tolerance is strictly +# positive), so any separate ceiling above 0.5 can never be the guard that +# fires: mutation testing put the old `GOLDEN_MAX = 2.0` at **0 failed +# assertions** both before and after this change. It was dead either way, and a +# constant that no test can distinguish from its absence is not a guard, it is a +# comment with syntax. Removed rather than propped up with a test written to +# justify it. +# +# The surviving per-field bound on `golden` is a MESSAGE SELECTOR, not a verdict +# — it exists so an absurd golden reports as an absurd golden rather than as a +# sum violation. Judged as a verdict guard it would also score zero; that is the +# wrong instrument for it, and the test that pins it asserts the distinguishing +# message rather than the field name. +# +# (Its stated justification was wrong as well as inert: "CER can exceed 1.0 via +# insertions" describes a MEASUREMENT, which `ERROR_RATE_MAX` governs. A golden +# is a pinned expectation chosen by the maintainers. Verified that removing it +# costs nothing real: an honest golden of 0.05 against a measured 1.5 still +# fails as a REGRESSION, which is the correct verdict, rather than being +# rejected as out of bounds.) +# +# `tolerance` keeps its own ceiling because it does independent work: `golden 0` +# with `tolerance 0.4` sums to 0.4 and clears the sum bound, while 0.25 refuses +# it. `error_rate` keeps its own because the sum bound governs the baseline side +# only and never touches a measurement. +TOLERANCE_MAX = 0.25 +ERROR_RATE_MAX = 100.0 # garbage filter only — a real regression must still fail loudly + # The accuracy metrics the gate knows how to judge. Same vocabulary as Swift's # `MetricKind` and the bench validator's `metric_kind`, so an unknown value is a # loud gate error rather than a label nobody notices (#117). @@ -35,6 +101,11 @@ _CONTROL_CHARS[0x7F] = "\\x7f" _CONTROL_CHARS.update({c: f"\\x{c:02x}" for c in range(0x80, 0xA0)}) +# Longest rejected value echoed back into the gate log. Comfortably above any +# legitimate field (the widest committed value is 6 characters) and far below +# the point where a rejected value becomes the log. +_MAX_ECHO = 120 + def safe(value) -> str: """Neutralize ANSI/terminal control codes in a baseline-supplied value. @@ -51,14 +122,215 @@ def safe(value) -> str: corpus and language are `fullmatch`ed upstream and metric is bounded above. And it must be called EXPLICITLY at each interpolation — a new f-string added below is not covered until someone wraps it (#117). + + It also caps length. A rejected value is echoed so a human can see what was + wrong with it, and there is no length at which that stops being the point + and starts being a flood: a bare 400-digit integer or a multi-megabyte + string in `golden` would otherwise be reproduced in full into a CI log + somebody has to read (#134 verify). The head is the diagnostic part. + """ + text = str(value).translate(_CONTROL_CHARS) + if len(text) > _MAX_ECHO: + return f"{text[:_MAX_ECHO]}… ({len(text)} chars)" + return text + + +def _name_list(names, limit=12): + """Render a corpus-name list without letting it become the log. + + `safe()` caps each NAME at `_MAX_ECHO`, which does nothing about the number + of them: a meta file pinning 300 corpora would emit one 4000-character line. + Same flood, one level up. + """ + shown = ", ".join(safe(n) for n in names[:limit]) + if len(names) > limit: + return f"{shown}, … (+{len(names) - limit} more)" + return shown + + +def number(value, field, corpus, low, high, *, low_exclusive=False): + """Convert and bounds-check one numeric baseline/measured field. + + Returns `(value, None)` or `(None, reason)`. + + Validation happens AFTER `float()`, deliberately. Rejecting the literal at + parse time (`json.load(parse_constant=...)`) does not close this class: + `{"golden": "NaN"}` is a legal JSON *string*, so `parse_constant` never + fires, and `float("NaN")` / `float("1e999")` still produce nan / inf. The + only reliable place to check is on the resulting float. + """ + # `bool` is a subclass of `int`, so `float(True)` is 1.0 and a bare `true` + # would sail through every bound below as a perfectly ordinary number. A + # boolean is never a measurement; refusing it removes the type confusion + # rather than letting it be laundered into one. + if isinstance(value, bool): + return None, f"{field} is a boolean, not a measurement (got {safe(value)!r})" + try: + number = float(value) + except (TypeError, ValueError, OverflowError): + # OverflowError is neither of the other two and had escaped: JSON has no + # integer width limit, so a bare 400-digit literal parses to an + # arbitrary-precision `int` and `float()` raises. It exits non-zero, so + # it was never a bypass — but it reached the CI log as a stack trace + # where the verdict belongs, and `regression-gate.sh` does not capture + # stderr. Universal, not version-specific: CPython 3.11+'s 4300-digit + # int-str cap sits far above this literal and never fires. + return None, (f"{field} is not a number (got {safe(value)!r})") + if not math.isfinite(number): + # Covers nan, inf and -inf, whether they arrived as JSON literals + # (Python's json accepts bare NaN/Infinity), as strings, or via + # overflow from a finite-looking literal such as 1e999. + return None, f"{field} is not a finite number (got {safe(value)!r})" + if low_exclusive and number <= low: + return None, f"{field} must be greater than {low:g} (got {number:g})" + if not low_exclusive and number < low: + return None, f"{field} must be at least {low:g} (got {number:g})" + if number > high: + return None, f"{field} must be at most {high:g} (got {number:g})" + return number, None + + +def validate_numbers(data): + """All numeric-field problems across both sides, as printable messages. + + Every problem is collected rather than returning on the first: a run whose + baseline has one bad field usually has more, and reporting them one release + at a time is its own kind of gate failure. """ - return str(value).translate(_CONTROL_CHARS) + problems = [] + for e in data.get("baseline", []): + corpus = e.get("corpus") + # `high` is the effective-threshold ceiling: a golden at or above it + # cannot be part of any acceptable (golden, tolerance) pair anyway, and + # naming the field beats reporting it as a sum violation. + golden, why = number( + e.get("golden"), "golden", corpus, 0.0, EFFECTIVE_THRESHOLD_MAX) + if why: + problems.append((corpus, why)) + # A tolerance wide enough to absorb any plausible regression is not a + # lenient threshold, it is a disabled gate that still prints a green line. + tol, why = number( + e.get("tolerance"), "tolerance", corpus, 0.0, TOLERANCE_MAX, low_exclusive=True) + if why: + problems.append((corpus, why)) + # The bound that actually decides whether the gate can still fail: the + # sum is the effective threshold, so neither handle can be traded for + # the other. Only checked when both parsed — a NaN golden has already + # been reported above and `nan + 0.02 > 0.5` would add nothing but noise. + if golden is not None and tol is not None and golden + tol > EFFECTIVE_THRESHOLD_MAX: + # {:.17g} rather than {:.4f}: a value rejected for exceeding a bound + # must not RENDER as equal to it. `0.4999999999999999 + 2e-16` is + # refused and would print "is 0.5000, above … of 0.5" at 4dp — the + # same defect this file exists to fix, reproduced inside the guard + # that fixes it. + problems.append(( + corpus, + f"golden + tolerance is {golden + tol:.17g}, above the maximum effective " + f"threshold of {EFFECTIVE_THRESHOLD_MAX:g} — the gate would pass any " + f"measurement at or below that, which is not a threshold but a " + f"disabled gate")) + for m in data.get("measured", []): + _, why = number(m.get("error_rate"), "error_rate", m.get("corpus"), 0.0, ERROR_RATE_MAX) + if why: + problems.append((m.get("corpus"), why)) + return problems + + +def reject_duplicate_keys(pairs): + """`object_pairs_hook` that refuses a JSON object with a repeated key. + + Python keeps the LAST value for a duplicate key, so + `{"golden": 0.9, "golden": 0.01}` silently becomes 0.01 — a real threshold + hidden behind a decoy in the very file whose purpose is to be auditable by + reading it. The duplicate-CORPUS check further down does not see this: that + one compares entries, this is one key repeated inside a single entry. + """ + seen = set() + for key, _ in pairs: + if key in seen: + raise ValueError(f"duplicate key {key!r} in a baseline/measured entry") + seen.add(key) + return dict(pairs) def main() -> int: - data = json.load(sys.stdin) + try: + data = json.load(sys.stdin, object_pairs_hook=reject_duplicate_keys) + except ValueError as error: + print(f"✗ GATE ERROR: cannot read the comparison input: {safe(error)}") + print("\n✗ regression gate: 1 failure(s).") + return 1 failures = 0 + # A run that compared NOTHING is not a pass (#134 verify). With both sides + # empty every loop below is a no-op, so the gate printed "all 0 corpora + # within tolerance" and exited 0 — a fetch that produced no measurements, or + # a worklist that silently emptied, read as "this release is safe". + if not data.get("baseline") and not data.get("measured"): + print("✗ GATE ERROR: nothing to compare — the baseline and the measured " + "set are both empty. A gate that verified no corpora has not " + "passed, it has not run.") + print("\n✗ regression gate: 1 failure(s).") + return 1 + + # Cardinality 0 was the easy half. The gate reports on whatever it is + # handed, and BOTH sides of that are derived from the same baseline.json — + # the worklist stage reads it, and the assembler iterates it — so deleting + # entries shrinks the baseline, the work list and the measured set + # together, and "all 1 corpora within tolerance" is printed over a release + # that verified one twelfth of what it should have. Nothing inside a stdin + # filter can notice that; the expected set has to arrive from somewhere the + # same edit does not reach. `regression-gate.sh` supplies it from + # benchmarks/baseline-meta.json, the file that already carries the seeding + # provenance — so truncating the baseline now requires a second, deliberate + # edit to a file whose name says what it is for (#134 verify). + expected = data.get("expected_corpora") + if expected is None: + # Silence must not read as a clean bill of health. A completeness check + # that can be skipped by leaving a key out — quietly — is the same shape + # as the hole it closes. + print("NOTE: no expected-corpus anchor supplied — completeness of this " + "comparison was NOT verified (this run cannot tell a full sweep " + "from a truncated one).") + elif not isinstance(expected, list) or not all(isinstance(c, str) for c in expected): + # Shape-check before `set()`. Without this an int or bool raises + # `TypeError: not iterable` and a nested list raises `unhashable type` — + # raw tracebacks reachable from a hand-edited field in the very file this + # guard made verdict-critical. And a bare string would be iterated + # CHARACTER-wise, so `"ab"` would silently mean the corpora {a, b}. + print(f"✗ GATE ERROR: the expected-corpus anchor is not a list of strings " + f"(got {safe(type(expected).__name__)}) — the completeness check cannot " + f"run against it") + print("\n✗ regression gate: 1 failure(s).") + return 1 + else: + # `anchor_source` names the file the anchor actually came from. The path + # is overridable (BESTASR_BASELINE_META), so hardcoding the default would + # send a reader to a file this run never read. + source = safe(data.get("anchor_source") or "the expected-corpus anchor") + present = {e.get("corpus") for e in data.get("baseline", [])} + missing = sorted(str(c) for c in set(expected) - present) + extra = sorted(str(c) for c in present - set(expected)) + if missing: + print(f"✗ GATE ERROR: baseline is missing {len(missing)} corpus/corpora that " + f"{source} pins: {_name_list(missing)} " + f"— a comparison over a subset is not a pass. Re-seed and update the " + f"provenance record, or restore the entries.") + failures += 1 + if extra: + print(f"✗ GATE ERROR: baseline carries {len(extra)} corpus/corpora absent from " + f"{source}: {_name_list(extra)} " + f"— an entry added without re-seeding has no provenance.") + failures += 1 + if failures: + print(f"\n✗ regression gate: {failures} failure(s).") + return 1 + # A satisfied guard that prints nothing can only be observed by the + # ABSENCE of its warning, which asks the reader to already know it + # exists. Same argument this script makes for rendering the tolerance on + # passing lines: auditing a pass is what the log is for. + print(f"completeness: {len(expected)} corpora pinned by {source} — verified.") + # `metric` is the one rendered field with no validation ANYWHERE — the # worklist stage checks corpus and language, nothing checks this. Unlike # golden/tolerance/error_rate it never passes through float(), so a hostile @@ -86,6 +358,17 @@ def main() -> int: print(f"\n✗ regression gate: {failures} failure(s).") return 1 + # Numeric sanity BEFORE any comparison (#134). Comparing with a nan or an + # unbounded tolerance does not produce a wrong verdict so much as no verdict + # at all, so there is nothing to salvage by continuing. + for corpus, why in validate_numbers(data): + print(f"✗ GATE ERROR: corpus '{safe(corpus)}': {why} — the gate cannot " + f"judge a regression against this value") + failures += 1 + if failures: + print(f"\n✗ regression gate: {failures} failure(s).") + return 1 + baseline = {e["corpus"]: e for e in data.get("baseline", [])} measured = {m["corpus"]: m for m in data.get("measured", [])} @@ -96,6 +379,17 @@ def main() -> int: f"— add it to benchmarks/baseline.json (never silently pass)") failures += 1 continue + # The two sides must be talking about the same yardstick (#134 verify). + # `metric` was validated on the baseline side only, and never compared + # across — so a WER measurement could be judged against a CER golden and + # pass or fail on a number that means something else entirely. + if m.get("metric") != b.get("metric"): + print(f"✗ GATE ERROR: corpus '{safe(corpus)}' was measured as " + f"{safe(m.get('metric'))!r} but its baseline pins " + f"{safe(b.get('metric'))!r} — these are different metrics and " + f"cannot be compared") + failures += 1 + continue golden, tol = float(b["golden"]), float(b["tolerance"]) actual = float(m["error_rate"]) diff = actual - golden @@ -105,8 +399,14 @@ def main() -> int: f"(+{diff:.4f} > tolerance {tol:.4f})") failures += 1 else: + # The tolerance is rendered on the PASSING line too (#134). Auditing + # a pass is the whole purpose of this log, and the one number that + # could expose a rigged pass — the threshold it was judged against — + # used to appear only on failing lines. A reader checking a green run + # had no way to see that the bar had been lowered to meet it. print(f"✓ {safe(corpus)} [{safe(b['language'])}] {safe(b['metric'])}: " - f"golden {golden:.4f} → measured {actual:.4f} ({diff:+.4f})") + f"golden {golden:.4f} → measured {actual:.4f} " + f"({diff:+.4f} ≤ tolerance {tol:.4f})") for corpus in baseline: if corpus not in measured: diff --git a/scripts/mutation-check.sh b/scripts/mutation-check.sh new file mode 100755 index 0000000..28e189c --- /dev/null +++ b/scripts/mutation-check.sh @@ -0,0 +1,92 @@ +#!/usr/bin/env bash +# Non-vacuousness check for the regression gate's guards (#134). +# +# Deletes each guard in scripts/lib/baseline-compare.py (and one in +# scripts/regression-gate.sh) independently, re-runs RegressionBaselineTests, +# and prints how many assertions each deletion breaks. A guard that scores 0 is +# indistinguishable from its absence — the compare script's own words for that +# are "not a guard, it is a comment with syntax", and #134 exists because the +# gate's reassuring output had outrun what it could actually prove. +# +# The counts in CHANGELOG.md and in PR #140 come from this script. It is +# committed so a reader holding only the diff can re-run the claim rather than +# trust it, which is the same standard the change asks of the gate's own log. +# +# ./scripts/mutation-check.sh +# +# Mutation is done on COPIES; the working tree is restored and verified after +# every case. Requires a clean tree (the guards are patched in place while a +# case runs, so an unrelated edit would be reverted along with the mutation). +set -euo pipefail + +cd "$(dirname "$0")/.." + +if [ -n "$(git status --porcelain scripts/lib/baseline-compare.py scripts/regression-gate.sh)" ]; then + echo "✗ refusing to run: baseline-compare.py or regression-gate.sh has uncommitted changes." >&2 + echo " This script patches them in place and restores from its own backup;" >&2 + echo " running it over unsaved work would be indistinguishable from losing it." >&2 + exit 1 +fi + +COMPARE=scripts/lib/baseline-compare.py +GATE=scripts/regression-gate.sh +BACKUP=$(mktemp -d) +trap 'cp "$BACKUP/compare" "$COMPARE"; cp "$BACKUP/gate" "$GATE"; rm -rf "$BACKUP"' EXIT +cp "$COMPARE" "$BACKUP/compare" +cp "$GATE" "$BACKUP/gate" + +# Build once so each case is a test run, not a rebuild. +swift build --build-tests >/dev/null 2>&1 + +count() { + swift test --skip-build --filter RegressionBaselineTests 2>&1 | grep -c 'recorded an issue' || true +} + +restore() { cp "$BACKUP/compare" "$COMPARE"; cp "$BACKUP/gate" "$GATE"; } + +printf '%-34s %s\n' "guard deleted" "assertions broken" +printf '%-34s %s\n' "----------------------------------" "-----------------" +printf '%-34s %s\n' "(none — control)" "$(count)" + +# file:sed-expression:label +CASES=( + "$COMPARE:s/^EFFECTIVE_THRESHOLD_MAX = 0.5/EFFECTIVE_THRESHOLD_MAX = float(\"inf\")/:effective-threshold sum bound" + "$COMPARE:s/^ expected = data.get(\"expected_corpora\")/ expected = None/:completeness anchor" + "$COMPARE:s/if not math.isfinite(number):/if False:/:isfinite" + "$COMPARE:s/ if isinstance(value, bool):/ if False:/:boolean guard" + "$COMPARE:s/^TOLERANCE_MAX = 0.25/TOLERANCE_MAX = float(\"inf\")/:TOLERANCE_MAX" + "$COMPARE:s/except (TypeError, ValueError, OverflowError):/except (TypeError, ValueError):/:OverflowError" + "$COMPARE:s/^ERROR_RATE_MAX = 100.0/ERROR_RATE_MAX = float(\"inf\")/:ERROR_RATE_MAX" + "$COMPARE:s/^_MAX_ECHO = 120/_MAX_ECHO = 10**9/:echo cap" + "$GATE:s/\.get(\"corpora\")\$/.get(\"corpora_SET\")/:gate anchor wiring" +) + +FAILED=0 +for case in "${CASES[@]}"; do + file="${case%%:*}"; rest="${case#*:}" + expr="${rest%:*}"; label="${rest##*:}" + sed -i '' "$expr" "$file" + if ! git diff --quiet "$file"; then + n=$(count) + else + n="PATTERN NOT FOUND" # the guard moved; the mutation silently applied to nothing + FAILED=1 + fi + printf '%-34s %s\n' "$label" "$n" + [ "$n" = "0" ] && FAILED=1 + restore +done + +echo +if [ -n "$(git status --porcelain "$COMPARE" "$GATE")" ]; then + echo "✗ working tree not restored — inspect before continuing." >&2 + exit 1 +fi +echo "✓ working tree restored." +if [ "$FAILED" = "1" ]; then + echo "✗ a guard scored 0, or a mutation pattern no longer matches." >&2 + echo " Either the guard is inert and should be deleted rather than kept," >&2 + echo " or it needs a test that can tell it from its absence." >&2 + exit 1 +fi +echo "✓ every guard is load-bearing." diff --git a/scripts/regression-gate.sh b/scripts/regression-gate.sh index 99390d7..19f7160 100755 --- a/scripts/regression-gate.sh +++ b/scripts/regression-gate.sh @@ -22,7 +22,8 @@ set -euo pipefail BIN="${BESTASR_BIN:-bestasr}" DEST="${BESTASR_CORPORA_DIR:-$HOME/.bestasr/corpora}" -BASELINE="${BESTASR_BASELINE:-$(cd "$(dirname "$0")/.." && pwd)/benchmarks/baseline.json}" +DEFAULT_BASELINE="$(cd "$(dirname "$0")/.." && pwd)/benchmarks/baseline.json" +BASELINE="${BESTASR_BASELINE:-$DEFAULT_BASELINE}" COMPARE="$(cd "$(dirname "$0")" && pwd)/lib/baseline-compare.py" WORKLIST="$(cd "$(dirname "$0")" && pwd)/lib/baseline-worklist.py" @@ -56,6 +57,54 @@ echo "regression gate: reference model = whisperkit/$MODEL, baseline = $BASELINE # benchmark spends minutes producing a misleading accuracy diff. Unpinned # metas warn (TOFU): run scripts/pin-reference-model.sh to pin. META="${BESTASR_BASELINE_META:-$(dirname "$BASELINE")/baseline-meta.json}" + +# The compare stage is a stdin filter: it cannot know whether the payload it was +# handed is the whole sweep, so when no expected-corpus anchor arrives it prints +# a NOTE and continues. That is the right call THERE and the wrong stopping point +# HERE, because this script knows exactly which baseline it is running. For the +# repo's own baseline the anchor is an invariant, not a nicety — a release run +# that cannot prove it compared the full corpus set has not passed, it has not +# run, which is the same sentence #134 is named after. Absent or malformed meta +# is therefore fatal on the default path and a warning on an overridden one, +# where a caller has deliberately supplied their own inputs. +# +# This also upgrades the model-artifact pin below (#48): a MISSING meta file used +# to skip that whole block with no message at all. Two verdict-critical inputs now +# live in this file; neither may vanish quietly. +META_PROBLEM="" +if [ ! -f "$META" ]; then + META_PROBLEM="missing at $META" +elif ! /usr/bin/python3 -c " +import json, sys +try: + meta = json.load(open(sys.argv[1])) +except Exception as e: + sys.exit(f'unreadable ({e.__class__.__name__})') +if not isinstance(meta, dict): + sys.exit('is not a JSON object') +c = meta.get('corpora') +if c is None: + sys.exit(\"has no 'corpora' key\") +if not isinstance(c, list) or not c: + sys.exit(\"'corpora' is not a non-empty array\") +if not all(isinstance(x, str) and x.strip() for x in c): + sys.exit(\"'corpora' contains a non-string or empty entry\") +if len(set(c)) != len(c): + sys.exit(\"'corpora' contains duplicates\") +" "$META" 2>"$TMP/meta_why"; then + META_PROBLEM="$(cat "$TMP/meta_why")" +fi +if [ -n "$META_PROBLEM" ]; then + if [ "$BASELINE" = "$DEFAULT_BASELINE" ]; then + echo "x gate error: baseline-meta $META_PROBLEM" >&2 + echo " the completeness anchor and the model-artifact pin both live in that file;" >&2 + echo " without it this run cannot show it compared the full corpus set (#134)" >&2 + exit 1 + fi + echo "! warning: baseline-meta $META_PROBLEM — completeness NOT anchored" >&2 + echo " (tolerated because BESTASR_BASELINE overrides the repo baseline)" >&2 +fi + CACHE_ROOT="${BESTASR_MODEL_CACHE_DIR:-$HOME/Documents/huggingface/models/argmaxinc/whisperkit-coreml}" MODEL_DIR="$CACHE_ROOT/openai_whisper-$(echo "$MODEL" | sed 's/-\([^-]*\)$/_\1/')" if [ -f "$META" ]; then @@ -158,11 +207,19 @@ echo "" # compare implementation. set -e would kill the script on a failing pipeline # BEFORE any assignment ran; capture the status through if/else so the # FAILED_RUNS combination stays explicit. -if /usr/bin/python3 - "$BASELINE" "$TMP/results" <<'PY' | /usr/bin/python3 "$COMPARE" +if /usr/bin/python3 - "$BASELINE" "$TMP/results" "$META" <<'PY' | /usr/bin/python3 "$COMPARE" import json, os, sys baseline = json.load(open(sys.argv[1])) results_dir = sys.argv[2] +# The expected corpus set travels from the PROVENANCE record, not from the +# baseline itself — an anchor derived from the file it is meant to check would +# shrink with it (#134 verify). A meta file without the key is not silently +# tolerated: compare prints an explicit "completeness NOT verified" note. +meta_path = sys.argv[3] +expected = None +if os.path.exists(meta_path): + expected = json.load(open(meta_path)).get("corpora") measured = [] for entry in baseline: path = os.path.join(results_dir, entry["corpus"] + ".json") @@ -178,7 +235,11 @@ for entry in baseline: "metric": r["metric_kind"], "error_rate": r["error_rate"], }) -json.dump({"baseline": baseline, "measured": measured}, sys.stdout) +payload = {"baseline": baseline, "measured": measured} +if expected is not None: + payload["expected_corpora"] = expected + payload["anchor_source"] = meta_path +json.dump(payload, sys.stdout) PY then COMPARE_RC=0; else COMPARE_RC=$?; fi [ "$FAILED_RUNS" -gt 0 ] && exit 1