From 0abfd6bc031737178d94b395fc938428ed51958b Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 2 Aug 2026 08:01:02 +0800 Subject: [PATCH 1/3] fix: regression gate reported exit 0 on real regressions (#134) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 32 +++++ .../RegressionBaselineTests.swift | 124 ++++++++++++++++++ scripts/lib/baseline-compare.py | 97 +++++++++++++- 3 files changed, 252 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f17d6be..082c57a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,6 +108,38 @@ 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` now has the tightest bound of the three fields — the committed + baseline uses 0.02, and 0.5 already accepts a 50-point jump. + - **A garbage numeric field** raised `ValueError` mid-run. A traceback is not + a verdict; it now fails as a named gate error like every other. + + Bounds are deliberately **asymmetric**, because the danger is: `golden` and + `tolerance` are hazardous when *large* (both make the comparison vacuous), + while `error_rate` is hazardous only when *non-finite* — a large measurement + correctly fails, so its ceiling is a garbage filter, not a security boundary. + + 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..258c298 100644 --- a/Tests/BestASRKitTests/RegressionBaselineTests.swift +++ b/Tests/BestASRKitTests/RegressionBaselineTests.swift @@ -82,6 +82,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 +226,95 @@ 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 `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)") + } } diff --git a/scripts/lib/baseline-compare.py b/scripts/lib/baseline-compare.py index c9aadd4..7bd238c 100755 --- a/scripts/lib/baseline-compare.py +++ b/scripts/lib/baseline-compare.py @@ -12,8 +12,30 @@ pipes into it and RegressionBaselineTests exercises it via Process. """ import json +import math import sys +# Numeric sanity bounds (#134). The gate reported exit 0 on real regressions, +# which 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. +# +# The two directions are NOT symmetric, and that asymmetry is what the bounds +# encode: +# +# * `golden` and `tolerance` are dangerous when LARGE. A huge golden makes +# `diff = actual - golden` hugely negative; a huge tolerance makes the +# comparison vacuous. Either way every corpus passes. +# * `error_rate` is dangerous only when NON-FINITE. A large measured value is +# safe — it correctly fails — so its upper bound is a garbage filter, not a +# security boundary. +# +# Non-finite values defeat the comparison a third way: EVERY comparison against +# NaN is false, so `diff > tol` is false and the entry "passes". +GOLDEN_MAX = 2.0 # CER can exceed 1.0 via insertions; beyond 2.0 it is not a baseline +TOLERANCE_MAX = 0.5 # the committed baseline uses 0.02; 0.5 already accepts a 50-pt jump +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). @@ -55,6 +77,62 @@ def safe(value) -> str: return str(value).translate(_CONTROL_CHARS) +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. + """ + try: + number = float(value) + except (TypeError, ValueError): + 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. + """ + problems = [] + for e in data.get("baseline", []): + corpus = e.get("corpus") + _, why = number(e.get("golden"), "golden", corpus, 0.0, GOLDEN_MAX) + if why: + problems.append((corpus, why)) + # `tolerance` gets the tightest bound of the three. 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. + _, why = number( + e.get("tolerance"), "tolerance", corpus, 0.0, TOLERANCE_MAX, low_exclusive=True) + if why: + problems.append((corpus, why)) + 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 main() -> int: data = json.load(sys.stdin) failures = 0 @@ -86,6 +164,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", [])} @@ -105,8 +194,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: From 7c84d28204ec3d161ca9b91b67299612b8de1205 Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 2 Aug 2026 08:13:35 +0800 Subject: [PATCH 2/3] fix: four more fail-open paths found by cross-model verify (#134) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 25 ++++++++ .../RegressionBaselineTests.swift | 40 +++++++++++++ scripts/lib/baseline-compare.py | 60 ++++++++++++++++++- 3 files changed, 123 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 082c57a..4401c57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,31 @@ All notable changes to bestASR are documented here. The format follows while `error_rate` is hazardous only when *non-finite* — a large measurement correctly fails, so 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. + + `TOLERANCE_MAX` also tightened from 0.5 to **0.25** on the same review. No + fixed ceiling can separate *generous* from *disabled* on its own, which is why + the bound handles the absurd values and the newly-rendered tolerance handles + the merely-too-lenient ones. + 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 diff --git a/Tests/BestASRKitTests/RegressionBaselineTests.swift b/Tests/BestASRKitTests/RegressionBaselineTests.swift index 258c298..3633dd6 100644 --- a/Tests/BestASRKitTests/RegressionBaselineTests.swift +++ b/Tests/BestASRKitTests/RegressionBaselineTests.swift @@ -305,6 +305,46 @@ struct RegressionBaselineTests { #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 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") diff --git a/scripts/lib/baseline-compare.py b/scripts/lib/baseline-compare.py index 7bd238c..d2c7b46 100755 --- a/scripts/lib/baseline-compare.py +++ b/scripts/lib/baseline-compare.py @@ -33,7 +33,13 @@ # Non-finite values defeat the comparison a third way: EVERY comparison against # NaN is false, so `diff > tol` is false and the entry "passes". GOLDEN_MAX = 2.0 # CER can exceed 1.0 via insertions; beyond 2.0 it is not a baseline -TOLERANCE_MAX = 0.5 # the committed baseline uses 0.02; 0.5 already accepts a 50-pt jump +# The committed baseline uses 0.02 uniformly. 0.25 leaves an order of magnitude +# of headroom while still refusing a threshold that would wave through a +# quarter-of-the-transcript regression. No fixed ceiling can separate "generous" +# from "disabled" on its own, which is why the tolerance is now RENDERED on +# passing lines too — the bound stops the absurd values, visibility handles the +# merely-too-lenient ones. +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 @@ -88,6 +94,12 @@ def number(value, field, corpus, low, high, *, low_exclusive=False): 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): @@ -133,10 +145,43 @@ def validate_numbers(data): 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 + # `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 @@ -185,6 +230,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 From 39a9a466c3448d5295fe78469198255d9c39972b Mon Sep 17 00:00:00 2001 From: che cheng Date: Sun, 2 Aug 2026 13:54:55 +0800 Subject: [PATCH 3/3] fix: bound the effective threshold, not one of its two handles (#134) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 105 ++++- .../RegressionBaselineTests.swift | 400 +++++++++++++++++- benchmarks/baseline-meta.json | 17 +- scripts/lib/baseline-compare.py | 177 +++++++- scripts/mutation-check.sh | 92 ++++ scripts/regression-gate.sh | 67 ++- 6 files changed, 826 insertions(+), 32 deletions(-) create mode 100755 scripts/mutation-check.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 4401c57..dec6ed1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -125,15 +125,23 @@ All notable changes to bestASR are documented here. The format follows - **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` now has the tightest bound of the three fields — the committed - baseline uses 0.02, and 0.5 already accepts a 50-point jump. + `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; it now fails as a named gate error like every other. + 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.) - Bounds are deliberately **asymmetric**, because the danger is: `golden` and - `tolerance` are hazardous when *large* (both make the comparison vacuous), - while `error_rate` is hazardous only when *non-finite* — a large measurement - correctly fails, so its ceiling is a garbage filter, not a security boundary. + `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: @@ -155,10 +163,85 @@ All notable changes to bestASR are documented here. The format follows `float(true)` is 1.0 and a bare `true` passed every bound as an ordinary number. - `TOLERANCE_MAX` also tightened from 0.5 to **0.25** on the same review. No - fixed ceiling can separate *generous* from *disabled* on its own, which is why - the bound handles the absurd values and the newly-rendered tolerance handles - the merely-too-lenient ones. + 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 diff --git a/Tests/BestASRKitTests/RegressionBaselineTests.swift b/Tests/BestASRKitTests/RegressionBaselineTests.swift index 3633dd6..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] @@ -323,6 +327,38 @@ struct RegressionBaselineTests { #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 — @@ -357,4 +393,362 @@ struct RegressionBaselineTests { 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". -GOLDEN_MAX = 2.0 # CER can exceed 1.0 via insertions; beyond 2.0 it is not a baseline -# The committed baseline uses 0.02 uniformly. 0.25 leaves an order of magnitude -# of headroom while still refusing a threshold that would wave through a -# quarter-of-the-transcript regression. No fixed ceiling can separate "generous" -# from "disabled" on its own, which is why the tolerance is now RENDERED on -# passing lines too — the bound stops the absurd values, visibility handles the -# merely-too-lenient ones. +# 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 @@ -63,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. @@ -79,8 +122,30 @@ 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. """ - return str(value).translate(_CONTROL_CHARS) + 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): @@ -102,7 +167,14 @@ def number(value, field, corpus, low, high, *, low_exclusive=False): return None, f"{field} is a boolean, not a measurement (got {safe(value)!r})" try: number = float(value) - except (TypeError, ValueError): + 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 @@ -128,16 +200,35 @@ def validate_numbers(data): problems = [] for e in data.get("baseline", []): corpus = e.get("corpus") - _, why = number(e.get("golden"), "golden", corpus, 0.0, GOLDEN_MAX) + # `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)) - # `tolerance` gets the tightest bound of the three. 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. - _, why = number( + # 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: @@ -182,6 +273,64 @@ def main() -> int: 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 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