Add a CI quality gate for evaluation specs and fixtures - #953
Merged
Evangelink merged 3 commits intoJul 29, 2026
Merged
Conversation
Contributor
|
✅ Evaluation passed for |
Evangelink
force-pushed
the
dev/amauryleve/eval-quality-gate
branch
from
July 28, 2026 08:31
9b62611 to
9dad14c
Compare
Split out of #945, where every defect class below was first found the hard way: as a skill mysteriously losing to its own baseline, with the eval specs parsing cleanly and skill-validator passing throughout. ## Why An audit of all 97 eval specs found: | Finding | Scale | | --- | --- | | Skills with no eval at all | 5 | | Evals at n=1 (one judge call decides pass/fail) | 18 | | Evals at n=2 | 10 | | Evals at n=3 | 21 | | **Underpowered (n<=3) of 93 skill evals** | **49 (53%)** | | Orphaned fixtures (committed, never referenced) | 12 | | Cobertura methods whose line-rate contradicts own <lines> | 6 | None of this was visible to any existing check. ## What fails the build All four are structural — they inspect file existence, git state, or YAML keys, never prose — so they cannot fire spuriously on a well-written eval: 1. A referenced fixture missing on disk. 2. A referenced fixture not tracked by git. `.gitignore` carries `coverage*.xml` (correct for Coverlet output) and silently swallowed a committed Cobertura *fixture*: `git add -A` reported success, the eval passed locally, and three scenarios would have failed at setup in CI. Only the git index catches this. 3. A Cobertura fixture whose declared `line-rate` contradicts its own `<lines>`. The crap-score skill documents both parse paths, so the two arms of a comparison can read different coverage inputs and the eval measures the disagreement instead of the skill. Observed live: a scenario lost -40% with the judge writing "Response B made a critical error by manually counting line hits (12/15 = 80%) instead of using the XML's recorded line-rate of 0.55" — the fixture was wrong, not the response. 4. A dormancy guard (`expect_activation: false`) that also sets `reject_skills`. That forces the skilled arm skill-free, making it identical to the baseline arm, so the score is judge noise. The same guard scored -0.4, +0.4, +0.4 and 0 across four evals and twice cost a skill its pass. ## What only warns Statistical power, orphaned fixtures, uncovered skills, and dormancy guards that appear to lack an anti-hijack rubric item. Two deliberate calls here: - The anti-hijack check began as an error and immediately false-positived on a well-formed guard in `system-text-json-net11` whose rubric reads "Does NOT load or reference the system-text-json-net11 skill". Detecting that property needs phrase matching over free text and will always have false positives, so it warns. A gate that blocks a PR spuriously is a gate the team switches off. - Failing on power would break 28 existing evals across 8 plugins, and the fix (raising `runs`) is a CI cost decision. See #952. ## Self-test `selftest_eval_quality.py` injects each defect into a scratch tree, asserts the gate rejects it, and asserts a clean tree passes. CI runs the self-test *before* the gate, so the gate cannot silently stop working. Refs #899, #952. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1947263a-0ef9-47bd-ac53-5af5afa3ddaa
git_tracked_files() unioned `git ls-files` with `git diff --cached --name-only`. The second arm is redundant for additions (ls-files already reports the index) and wrong for deletions: a fixture staged for removal but left on disk appears in diff --cached and was counted back as "tracked", a false negative for the exact untracked-fixture class the check exists to catch. It was invisible because scratch() ran `git init; git add -A` with no commit, so there was no HEAD, diff --cached errored, and the arm was inert. scratch() now commits. Verified: with the commit in place and the two-arm implementation restored, the self-test fails on "fixture present but NOT tracked by git" (expected=FAIL got=PASS), so the regression is genuinely covered rather than accidentally passing. Added a fifth failing check for whole-file Cobertura totals. Review found coverage-analysis/partial-coverage declaring line-rate="0.47" alongside lines-covered="35" lines-valid="60" (58.3%); every method was internally consistent, so the method-level check could not see it. Compares two declared values, so it cannot fire on well-formed input. Also reports, as a warning, file/package/class line-rate disagreeing with the <lines> beneath it. Warning rather than error because the only live instance (coverage-analysis/fixtures/plateau, declaring 75% against a 47% payload) cannot be recomputed: its prompt quotes 75% and its rubric requires CalculateGpa to remain the 0% blocker, which caps the achievable rate at 48.9%. Failing on it would compel rewriting a passing scenario. Extended the push trigger to match the pull_request paths. The gate reads plugins/* and .gitignore, so a merge touching only those could previously land on main without it running. Replaced the three bare open() calls with context managers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1947263a-0ef9-47bd-ac53-5af5afa3ddaa
A grader whose config is absent, null, or missing its required key parses as valid YAML and enforces nothing, so the scenario appears to carry one more assertion than it really does. The usual cause is an indentation slip during an edit, which leaves `- type: output-matches` / `config:` with the pattern attached to the next list item. This is not hypothetical: it shipped in 705b876 on #945 and was caught only by review. It survived YAML validation (the document parses) and a bespoke regex validator, which did `(g.get("config") or {}).get( "pattern")` and silently skipped the broken entry - so the pattern count was 262 both before and after the fix. Verified end to end: restoring the broken eval.yaml from 705b876 into the working tree makes the gate exit 1 with 'Detect time-related statics and recommend TimeProvider' grader[3] (output-matches) has no config; it silently enforces nothing. Check the indentation of the 'pattern:' line. and the fixed tree is clean. The self-test gains a matching case (now 8/8) that injects the same indentation slip. Swept the repo while adding this: 1,906 graders across 97 specs, no other instance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1947263a-0ef9-47bd-ac53-5af5afa3ddaa
Evangelink
force-pushed
the
dev/amauryleve/eval-quality-gate
branch
from
July 28, 2026 13:48
9dad14c to
31646c0
Compare
Evangelink
requested review from
AbhitejJohn
and removed request for
JanKrivanek and
ViktorHofer
July 29, 2026 07:45
Evangelink
merged commit Jul 29, 2026
6ef2aca
into
dev/amauryleve/dotnet-test-eval-followups
33 checks passed
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.
Split out of #945 at review request: this is repo-wide infrastructure (the workflow gates every plugin, and its warnings cover fixtures and skills in plugins that PR does not own), so it has a different blast radius and a different reviewer set from the
dotnet-testcontent fixes.Why
Every defect class here was found the hard way in #945: as a skill mysteriously losing to its own baseline, with the eval specs parsing cleanly and
skill-validatorpassing throughout. An audit of all 97 eval specs quantified the wider picture:line-ratecontradicts own<lines>None of this was visible to any existing check.
What fails the build
All four are structural — they inspect file existence, git state, or YAML keys, never prose — so they cannot fire spuriously on a well-written eval.
A referenced fixture missing on disk.
A referenced fixture not tracked by git.
.gitignorecarriescoverage*.xml(correct for Coverlet output) and silently swallowed a committed Cobertura fixture.git add -Areported success, the eval passed locally, and three scenarios would have failed at setup in CI. Verifying against the working tree cannot catch this — only the git index can.A Cobertura fixture whose declared
line-ratecontradicts its own<lines>. Thecrap-scoreskill documents both parse paths, so the two arms of a comparison can legitimately read different coverage inputs and the eval measures the disagreement instead of the skill. Observed live — a scenario lost −40% with the judge writing:The fixture was wrong, not the response.
A dormancy guard (
expect_activation: false) that also setsreject_skills. That forces the skilled arm skill-free, making it identical to the baseline arm, so the head-to-head score is pure judge noise. The same guard scored −0.4, +0.4, +0.4 and 0 across four evals, and twice cost a skill its pass.What only warns
Statistical power, orphaned fixtures, uncovered skills, and dormancy guards that appear to lack an anti-hijack rubric item.
Two deliberate calls:
system-text-json-net11whose rubric reads "Does NOT load or reference the system-text-json-net11 skill". Detecting that property needs phrase matching over free text and will always have false positives, so it warns. A gate that blocks a PR spuriously is a gate the team switches off.runs) is a CI cost decision — see Eval gate is underpowered: 27% of verdicts flip on byte-identical inputs (A/A test); 53% of evals run at n<=3 #952, which argues against a blanketruns: 3because thedotnet-testjob already takes ~75 min against a 360-min cap.The gate is self-testing
selftest_eval_quality.pyinjects each defect into a scratch tree, asserts the gate rejects it, then asserts a clean tree passes:CI runs the self-test before the gate, so the gate cannot silently stop working.
Checks
eval-qualityworkflow verified green in real CI on Fix dotnet-test findings from the refreshed cross-family eval (#899) #945 before the splitactionlintclean on the new workflowmarkdownlintclean on the READMERefs #899, #952.