Audit the gates themselves, not just the product - #1718
Merged
Alex Weininger (alexweininger) merged 8 commits intoAug 26, 2026
Conversation
A gate that has never once passed is more likely broken than the product is to be uniformly incapable of exactly that one thing. gate-health.ts reads past MSBench runs and reports, per gate, whether it has ever actually done its job. Ports the idea from #1669, whose input file does not exist here. MSBench records only passed/failed plus a nullable error, so the distinctions are reconstructed from eval.json, the exec table in session.sqlite, error.json and final-agent-config.json. Two differences from #1669 matter: - Void instances discard their passes as well as their failures. A rate-limited run that produced nothing still records a PASS for a negative assertion, because COUNT(*) = 0 is trivially true against an empty table. 7 of 26 instances in the corpus are void. - Not-applicable results never enter the pass numerator. N/A graders exit 0, so MSBench scores them as passes; an always-N/A gate would otherwise report 16-for-16 while testing nothing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Alex Weininger (alexweininger)
August 26, 2026 05:16
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new MSBench utility (npm run gate-health) that audits gate health across past runs to detect “instrument failures” (e.g., gates that never pass, gates that never fail, always-N/A gates being mis-scored as passes, and gates that are never attempted), plus documentation describing how to use and interpret the report.
Changes:
- Add
evals/msbench/gate-health.tsto extract and analyze prior MSBench runs, classify per-gate health verdicts, and optionally emit machine-readable JSON (failing only on confidentnever-passed). - Add an
npm run gate-healthscript inevals/package.json. - Document gate-health usage, verdict semantics, data sources, and flags in
evals/msbench/README.md.
Show a summary per file
| File | Description |
|---|---|
| evals/package.json | Adds a gate-health npm script to run the new auditor. |
| evals/msbench/README.md | Adds a detailed “Auditing the gates themselves” section explaining verdicts, inputs, and flags. |
| evals/msbench/gate-health.ts | Implements the gate-health auditor (run discovery/extraction, per-instance parsing, per-gate tallies/verdicts, report + JSON mode). |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (1)
evals/msbench/gate-health.ts:45
- The header comment hard-codes a specific void-instance count ("Six of twenty-six...") which is already inconsistent with the README section added in this PR ("Seven of twenty-six...") and will drift over time as the corpus grows. Consider removing the exact count (or at least keeping it in sync with the README) to avoid stale documentation.
* passes it never earned. Six of twenty-six instances in the current corpus are void.
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+558
to
+575
| const execRows = readExecRows(join(instance.vscOutput, 'session.sqlite')); | ||
|
|
||
| for (const detail of details) { | ||
| const command = execCommandOf(detail.comment); | ||
| const exec = command ? execRows.get(command) : undefined; | ||
| const gate = gateIdentity(detail.comment, exec?.stdErr, identityByComment); | ||
| const tally = tallyOf(tallies, gate); | ||
| tally.runs.add(runId); | ||
| tally.instances++; | ||
|
|
||
| const notApplicable = exec ? parseNotApplicable(exec.stdErr) : undefined; | ||
|
|
||
| // Order matters. A void instance discards everything, including passes: the agent produced | ||
| // nothing, so a negative assertion passed trivially rather than meaningfully. | ||
| if (fault) { | ||
| tally.notAttempted++; | ||
| bump(tally.notAttemptedReasons, fault); | ||
| } else if (notApplicable) { |
| * Four signals matter, and each maps to a distinct instrument failure: | ||
| * never-passed — the gate may be impossible to satisfy (broken probe, wrong credential, bad fixture) | ||
| * never-failed — the gate may be vacuous; it has never discriminated between good and bad output | ||
| * always-n/a — the gate is dead weight; no scenario has ever exercised it |
The liveness sentinel is declared in all five stimuli and appears in zero of the 21 audited runs, because every run predates #1706. That is benign now and is expected to resolve when the scaffold runs land — but if it is still never-attempted after those, the same verdict means a real bug. A verdict here is a question with a date on it, not a finding. Also records the measured before/after for gate identity: four validate-requirements.ts variants as separate rows (three never-failed on one or two runs each) versus one requirements gate reading 5 pass / 1 fail / 7 runs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Not-applicable graders now exit 3 rather than 0, and carry an explicit class=outOfScope|environmentGap alongside gate= and reason=. Because detection keys off the stderr marker rather than the exit code, the reversal needed no change to the bucketing — but three things did change: - Marker tokens are parsed order-independently. The live format is gate= class= reason= detail=, and a fixed-order regex would have silently stopped recognising N/A if a producer reordered them. - class=environmentGap is tallied as notAttempted, not notApplicable. A missing func binary is not dead weight: nobody decided the gate was unnecessary. An absent or unrecognised class is read as environmentGap, which is the safe direction — it says something is in the way rather than this gate is pointless. - never-attempted is now grouped by cause, as always-not-applicable already was, since that is where the five runtime-* gates land. Under exit 3 MSBench records N/A as passed: false, so the risk this protects against is inverted: N/A must now stay out of the failed bucket rather than the passed one, or the product gets charged for a missing prerequisite. Verified against fixtures covering both classes, a genuinely crashed grader (exit 3, no marker) and a real product failure (exit 1) — all four discriminate, and neither N/A lands in passed or failed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Applicability is decided at wiring time, in the stack declaration, not discovered by a gate at runtime. So a gate reporting class=outOfScope was attached to a stack it cannot answer for — a config bug with an owner. "Dead weight, consider deleting" is only correct if the gate is out of scope for every stack in the corpus, which this report cannot determine on its own, so it now says so rather than implying the stronger conclusion. Also drops stray markdown emphasis from terminal output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ecosystemNotSupported is classified by its producer as a gap to close, not as out of scope: a Go project has a plan, a tree and a real fidelity question, and there is simply no analyser for it. Calling that dead weight would suggest deleting the gate when the correct action is to write the analyser. The docs here said otherwise; the code never did, because it buckets on class= alone and never interprets reason codes. Also stops the out-of-scope section implying more than it knows. The report sees the runs it was given, so it can say "out of scope for the stacks observed" and no more — "dead weight everywhere" is a coverage claim it has no evidence for. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
An extraction with no instances had two causes collapsed into one quiet log line: the archive holds instance output we failed to load, or the run genuinely has none yet. Those need opposite responses, and collapsing them is this tool's own version of the failure class it exists to find — a silently dropped run under-reports coverage invisibly, and does so towards "never executed", the loudest verdict here. The reader now cross-checks the cached results.zip. An archive containing *-output.zip members while the extraction yielded no instances is reported as a READER FAULT in the preamble, naming the runs and stating that no tally below should be quoted. An archive with no output member is reported as pending or missing, which is a corpus fact rather than a bug. Also hardens marker parsing: field scanning stops before detail=, so a detail string containing the literal text class= or reason= cannot be read as a field. detail= is now JSON.stringify on both emitters and is treated as opaque. Verified: findInstances is order-independent (total scan, no first-match) against directories with -output entries at adversarial readdir positions; and a marker whose detail embeds a decoy class=outOfScope is bucketed by the real class. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Bucketing anything that is not explicitly outOfScope as a gap is the safe default and makes the coverageGap rename a no-op. It also means a future third class would land in the gap bucket silently, so that consequence is now written where the next reader will see it: a third class needs an explicit ruling and this line updated to match. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keying exec: gates on the grader filename stops that half of the corpus from forking histories when an assertion is reworded. It does nothing for SQL assertions over files/toolCalls/llm_responses, which have no stderr and no grader file — and those are the majority. That limit is measured rather than feared: three pairs in the corpus are one gate wearing two names, because sibling stimuli word the same assertion differently. All three are SQL assertions, so no identity scheme available here can merge them. The real fix is upstream — one canonical string per shared gate plus a drift check. The report now warns about it where it actually surfaces: a gate reworded in one stimulus appears under "declared but never seen" with its old text while running fine under the new one, and "this wording has never run" is indistinguishable from "this gate has never run" without checking. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Alex Weininger (alexweininger)
merged commit Aug 26, 2026
0d844fb
into
feat/CoR
3 of 4 checks passed
Alex Weininger (alexweininger)
deleted the
alexweininger-gate-health-audit
branch
August 26, 2026 06:22
Alex Weininger (alexweininger)
added a commit
that referenced
this pull request
Aug 26, 2026
CI went red on this branch without this branch touching the file:
Error: evals/graders/graderHarness.ts imports 'mysterious failure' from
node_modules.
#1718 added a doc comment to graderHarness.ts reading "turns a red from
'mysterious failure' into 'known gap, here is the fix'", and the specifier scan
in stage-graders.ts matched `from "mysterious failure"` inside it. There is no
such package; there is no import at all. A sentence broke the build.
The regex ran against raw source, and prose is full of the word "from" followed
by a quoted phrase. That is not a harmless over-report: this scanner's job is to
be conservative about bare specifiers, so every false positive is a hard error by
construction. Documentation is the most-edited part of these files, which makes
this a recurring break rather than a one-off.
Fixed by blanking comments and string bodies before scanning. Deliberately a
small state machine rather than a cleverer regex: `//` occurs inside strings
(every https:// URL) and quotes occur inside comments (every "don't"), so the two
cases defeat each other and both shapes are common in real files. Quote
characters and their delimiters are preserved so genuine specifiers still match,
while their contents cannot be confused with prose.
Verified both directions: the tree now stages clean, and a real bare import added
to graderHarness.ts is still rejected. A fix that only silenced the error would
have disabled the guard.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The story
A gate in a sibling suite failed 16 times in a row. Sixteen runs, sixteen red marks, zero passes. Everyone read it as "the product can't do this yet."
The product was fine. The gate's storage probe was signing its requests with a corrupted account key, so the storage emulator answered 403 to every single request. No generated app could ever have passed it, no matter how good. Ten percent of the corpus was being charged for a bug in the measuring instrument.
Nobody noticed, because nothing was watching the gates themselves. Everything was watching the product.
This PR adds the thing that watches the gates.
What it does
npm run gate-healthreads runs that already happened — free, no model tokens, no new run — and asks one question per gate: has this gate ever actually done its job?Then, underneath, the part that matters — WHAT TO GO AND LOOK AT, with the specific gate, the reason, and the command to reproduce it:
How to read the five verdicts
Limits — please read these before quoting anything
None of these verdicts proves a defect. Every one of them is a reason to look, not a finding. A gate can be legitimately never-failed because the product is genuinely good at that thing.
Three runs minimum. Below
--min-runs(default 3) a verdict is printed but marked(low confidence)and never fails the build. Two runs agreeing is a coincidence with a label on it.A young suite looks "broken" by these metrics and isn't. Right now 25 of 30 gates are never-failed. That is what a three-week-old suite with 1–14 observations per gate looks like. The number worth watching is whether that ratio survives the corpus growing — not its value today.
Only
never-passed, with enough runs behind it, sets exit 1.The porting problem, and two things found on the way
#1669 reads a
cor-validation.jsonwith a per-gate status and an explicitnotAttemptedflag. That file does not exist in our world. MSBench records onlypassed: true|falseplus a nullableerror, so all five distinctions are reconstructed from four artifacts:eval.json, theexectable insession.sqlite(which is where exit 1 and exit 3 stay distinct),error.json, andfinal-agent-config.json(so a run that produced noeval.jsoncan still name the gates that never ran).Two findings came out of that, and both are bigger than the tool.
1. Our historical results are wrong in both directions
#1669 models cascade failures — downstream gates recorded as failed when they never executed. We have that too, but we have something it doesn't model: void runs also manufacture fake passes.
2026082583236973Agent should not fall back to the chat question tool2026082467189297A
COUNT(*) = 0assertion is trivially true against an empty table. So a run where the agent never ran collects free passes on every negative check. 7 of 26 instances in the corpus are void. This tool discards everything from a void instance, passes included.The liveness sentinel (#1706) fixes this going forward. It does nothing for what's already recorded — so any run predating #1706 may contain vacuous passes, and anyone trend-plotting historical runs should know the older half of the corpus is contaminated in both directions.
2. Not-applicable results must never be counted as verdicts
The incoming fidelity and runtime gates emit
NOT_APPLICABLE gate=… class=… reason=…on stderr and exit 3, which MSBench records aspassed: false. So an N/A is scored as a failure, and a gate that's N/A across the whole corpus reads 0-for-16 — the story at the top of this PR, except the gate is fine and the environment is the problem. Not hypothetical: the fiveruntime-*gates emitfunctionsHostUnavailableon every current stimulus, because all four are Azure Functions and the container has nofuncbinary.This convention was reversed mid-review, and the reversal is the interesting part. Exit 0 was ruled first, explicitly on the grounds that this tool's always-not-applicable verdict made it safe. That premise was false, and this PR is what showed it: MSBench writes
exitCode = 0aspassed: true,resolvedderives from it, and the run-analysis site,msbench-cli reportand Kusto all publish that number. This report could say "not applicable" while the headline said green — and nobody investigates green. Observing inflation is not the same as being able to undo it. Exit 3 is pessimistic and recoverable; exit 0 was optimistic and unrecoverable.Because detection keys off the marker, not the exit code, the reversal needed no code change to the bucketing. Three behaviours are a contract:
passed— and under exit 3, never intofailed, which is now the live risk and would charge the product for a missing binary.ratecolumn printsn/a, meaning nothing was judged — not 0%, not 100%.class=splits the two kinds, and the mapping is mechanical rather than a lookup table here, so a new reason code can't silently default into the wrong bucket:class=outOfScopenotApplicableenvironmentGapnotAttemptedAbsent or unrecognised
class=reads asenvironmentGap— the safe direction, sincenoProjectManifestFoundmost likely means the tree was never staged, and calling that dead weight would invite deleting a gate to fix a staging bug.Verified against fixtures, since no grader emits the marker yet and this branch is load-bearing. All four cases discriminate correctly, and MSBench recorded every one of them as
passed: false:class=environmentGap, exit 3notAttempted, reasonfunctionsHostUnavailable— not a failureclass=outOfScope, exit 3notApplicable— not a failurenotAttempted,GRADER_EXIT_3— not a product failurefailedThe five-gate Functions case collapses to exactly one actionable line instead of five mystery failures:
Without the grouping, that reads as five gates failing on every run — the fastest route to somebody switching them off.
Where the data lives
The worry going in was that this would only ever see one laptop's runs. It's better than that, and the detail is worth knowing:
msbench-cli extractis backend-served, so any run id you have access to can be audited from any machine. The local cache is a cache, not the source.list runs --kusto, butMSBench Userdoesn't appear to grant Kusto read:Principal 'aaduser=…' is not authorized to read database 'ces_telemetry_prod'(and'msbench'on AME). That's a filable access gap and the whole fix for discovery.details[]is ingested nowhere.So the tool is run-id-driven and indifferent to provenance. The day Kusto read lands,
list runs --kustopiped intonpm run gate-healthworks with no change to the tool.Two things found by interrogating the tool, not by admiring it
A prediction that was wrong, and the real bug it exposed
It was predicted that the sentinel gate would be mis-detected: the stimulus YAML declares
SELECT COUNT(*) > 0 FROM llm_responses, the harness appendsWHERE stepIndex = :stepIndex, so any declared-vs-stored comparison done by equality on query text would never match and the gate would report never-attempted against runs that genuinely carry it.Tested, and the prediction was false — that code path was never in play. The declared-gate catalogue matches on the assertion comment, never on query text, so the appended filter is irrelevant to it. That retires the equality-vs-substring concern properly: not "we think it's fine", but "here is why it cannot arise".
The test exposed a different, real defect. The sentinel did not flip to healthy, for a cause nobody had predicted — sibling stimuli word the same gate differently:
Sentinel; session data must exist or the negative checks below are vacuousSentinel; session data must exist or every check below is vacuousRun
2026082618693091carries the second and passes it, so the sentinel is executing. What remains never-seen is the first wording — correctly, since those stimuli haven't been re-run since #1706. Two wordings of one gate, two histories, and the report was right both times.The two scaffold runs show the same drift on two further gates:
20260826186930912026082619460117Agent should not open the plan view to approve the plan itselfAgent should not take over planning by opening the plan viewAgent should not fall back to the chat question toolAgent should refuse with a message, not by asking a chat questionThis is the comment-identity fragility documented below, live, three commits after it was written down as theoretical — and it is outside what filename identity can fix, because these are SQL assertions with no grader file. The fix belongs upstream: one canonical string per shared gate, plus a drift check. This report is the evidence that the divergence is real.
A
missingrun — chased to ground, and the guard it producedAuditing two scaffold runs,
2026082619460117extracted with onlyrun_metadata.jsonand no instances, while being reported elsewhere as 7/7resolved: true. A run reporting green while its artifact is absent is the recurring failure class here, so it was chased rather than assumed.Two hypotheses were on the table: reconciliation lag (the blob hadn't landed when it was read) and an order-dependent reader (the archives differ in member order —
output.zipis first in the run that read fine, third in the one that didn't).It was lag, and it is settled by three independent checks:
results.zipwritten 22:28:22; the read was ~22:23:30 — five minutes before the file existed2026082620153444,output.zipat index 4, complete artifact2026082603685803, output blobs at indices 4, 6, 10findInstancesis a totalreaddirSyncscan — nobreak, no first-match, no index assumption — and was separately verified against a directory whose-outputentries are interleaved with decoys in adversarialreaddirorder. It also never opens an archive;msbench-cli extractdoes that.The clinching detail is in the original observation: the extraction produced
run_metadata.json, which is member 4, behindoutput.zipat member 3. A reader that bailed on a non-matching member could not have reached member 4.The full corpus was re-run under both readers. Zero verdicts moved — 25 runs, 30 instances, 46 gates, identical tallies.
The guard is in anyway, because the underlying criticism was correct. "No instances" previously meant two different things in one quiet log line: we failed to load data that exists and there is no data. Those need opposite responses. The reader now cross-checks the cached
results.zip:*-output.zipbut the extraction yielded nothing → READER FAULT, loud, in the preamble, naming the runs and stating that no tally below should be quoted.A silently dropped run under-reports coverage invisibly, and it does so towards never-executed — this report's loudest verdict. That is this tool's own version of the failure class it was built to find, so it is now checked rather than assumed.
What the tool did right is the durable point. Under a transient partial artifact it printed
no instances foundand audited one run, instead of quietly auditing one while reporting as though it had seen two. A tool that averaged over missing data would have published a clean bill of health for half a corpus.Known limitation: gate identity
MSBench carries no gate id — an assertion is identified only by its comment. That's unstable:
requirements.json should be valid JSON carrying a questions arrayandrequirements.json satisfies the requirements contractare the same gate before and after it moved from SQL toexec:, so by comment they look like two gates with 7 and 3 runs instead of one with 10. A gate can silently reset its own history by being reworded.The identity fix, measured
Same 21 runs, same data, two identity schemes:
--identity comment(the naive view)requirements.json satisfies the requirements contractrequirements.json describes an API-only Cosmos + Blob projectrequirements.json should decompose into exactly three servicesrequirements.json should recommend no datastore at all--identity gate(default)requirementsThose are four invocations of the same grader,
validate-requirements.ts, differing only in flags. Under comment identity they fragment into four rows, three of which look like never-failed gates resting on one or two runs — which is exactly the "gate silently resets its own history" defect. Only the second table is true.Default
--identity gatemitigates it by keyingexec:gates on the grader filename — the same idgate=is derived from — which also recovers stable identity for runs recorded before the convention existed. It's deliberately coarser (allvalidate-requirements.tsinvocations are one gate);--identity commentgives the raw view. It only helpsexec:gates — SQL assertions have no stderr and stay comment-keyed. Retrofitting existing stimuli is a separate follow-up.Scope
Three files:
evals/msbench/gate-health.ts, apackage.jsonscript, and a README section. Nothing inconfig/,graders/,src/artifacts/orgrader-certification/, which sibling sessions are editing.npm run typecheckpasses. Verified against all 21 runs / 26 instances in the local corpus, plus a synthetic fixture for the N/A path.