Repository navigation
Conversation
Every path out of _grade_with_llm that cannot reach a verdict returned False, the same value the judge returns when it decides against the skill. A missing CLI, a spent deadline, a judge timeout and a reply carrying no verdict were therefore all published as the skill having failed the expectation it was asked about. The polarity is what makes this worse than a mislabelled failure. For unexpected_behavior, False asserts the prohibited behaviour happened, on no evidence at all, which inverts the default the function documents: absence of evidence is supposed to count as the requirement being satisfied. One reported run recorded a skill as claiming measured results it never produced, on the strength of a dropped connection, with every other expectation in that case passing. Those four paths now return None, and a Check records whether it was graded. An ungraded check still refuses to call the case green, so the run stays red, but it is reported as an error against the run, listed apart from unmet expectations, and left out of the rate. An evals/hooks.py assertion the judge could not answer raises Ungradable rather than AssertionError so that it lands in the same place, instead of being recorded as a check the skill failed. Routing already draws this line, and usage.md already documents it for that half: a case whose own run errored counts towards neither side of the bar. Signed-off-by: John Lybeck <john.lybeck@amd.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.
Fixes #25.
Summary
A behavioral run could report a skill as having violated an expectation that was never actually checked. A dropped connection produced this line, and the skill's author had nothing to debug:
Root cause.
_grade_with_llmhas four paths that cannot produce a verdict, and all four returnedFalse, which is the same value the judge returns when it decides against the skill: noclaudeCLI on PATH, the command deadline already spent, the judge timing out, and no JSON verdict in the reply. The last one swallows every API error, because the error text arrives in theresultfield where a verdict was expected. With a return type oftuple[bool, str], "the judge said no" and "the judge could not answer" were indistinguishable to the caller.The polarity is what makes this worse than a mislabelled failure. For
unexpected_behavior,Falseasserts the prohibited behavior happened, on no evidence at all, which inverts the default the function's own docstring sets:In the reported run every other expectation in that case passed, 5/5 expected, 3/3 files, 2/2 logs. A second skill had one graded expectation in its only case, so a judge outage took it to 0%.
What changed. Those four paths return
None, and aCheckrecords whether it wasgraded. An ungraded check is not a pass and not a failure. It still refuses to call the case green, so the run stays red and still exits 1, but it is reported as an error against the run, listed under its own heading rather than among unmet expectations, and left out of the rate the report publishes.Non-obvious decisions
--min-accuracy, anddocs/usage.mdalready documents that. Behavioral had no equivalent. The wording for the new carve-out deliberately mirrors the existing one.gradedis a second field rather thanpassed: bool | None. The report is uploaded as a CI artifact, so"passed": nullwould be a shape change for anything reading it, andall(c.passed ...)keeps failing the case with no edit at the call sites.evals/hooks.pyassertion raises a newUngradablerather thanAssertionError.run_caseconvertsAssertionErrorinto a check failed by the skill, which is the attribution this change exists to prevent.Ungradableis caught separately and recorded as an ungraded check, so the hook path lands in the same place as the dataset path. Teardown still runs.Expectationscolumn is nowGraded, soMet / Gradedis a real rate and matches the headline's denominator. Ungraded has its own column.claude_env()disables the CLI's retry loop on purpose so an unreachable API fails fast instead of hanging, and reversing that is a separate argument from reporting the outcome honestly.Scope
Not included: the same three sites in
engine/judge.pyandengine/scorers.pyadded by #19, which reproduce this (parse_verdictalready returnsNoneandgradecollapses it back toFalse; theListingFailedbranch there carries a comment describing exactly this problem). That is a follow-up against #19's engine once it lands, so this PR does not conflict with it. The report layer this change touches is the half #19 preserves.Test plan
Reproduced before fixing, with no tokens spent: stub
shutil.whichtoNone, and stub the CLI to return anECONNRESETpayload. Both printed the[FAIL]line above onmain; both print[----]withgraded=Falseafter.python -m unittest discover -s tests -t ., 242 passing. The nine new tests were confirmed to fail against unfixed source, and that run showed the ungraded check listed under Unmet expectations, which is the bug.Mutation-tested rather than assumed. Each of these is caught by at least one test: every one of the four paths reverted to
False,_verdict_labellosing its third branch, the headline denominator reverting to all checks, the per-skill ungraded count forced to 0, and the hook path recording a graded failure instead of an ungraded check.Risk: low. No behavior change when the judge answers, which the tests pin for both a true and a false verdict. Pipe escaping, truncation and row ordering in
render_markdownare unchanged.check_api_reachablehas a near-identical deadline guard that is deliberately untouched, since it is a preflight rather than a judge.