fix(evals): a pairwise judge that errored is not a tie - #1731
Open
asdf8675309 wants to merge 1 commit into
Open
Conversation
PairwiseComparison passes when its judge is completely down. compare() catches every judge error and returns `winner: 'tie'`. With position_swap on (the default), a judge that never answers produces two ties, which the aggregator scores as (0 + 2*0.5)/2 = 0.5 — and `passed = score >= 0.5` makes that a PASS. The failure is quiet in the worst way: the run reports every pairwise task as passing, and the per-position reasoning strings say "Comparison error: ..." while the score says the output beat the reference. LLMRubric and NaturalLanguageAssert both already return 0 when their judge throws; pairwise is the one model-based grader that does not. A comparison result gains an `errored` flag, so a judge that never answered stops being indistinguishable from one that answered TIE, and the scoring moves into an exported pure aggregatePairwise() that returns 0 when any comparison errored. One errored comparison fails the whole grade rather than scoring on the survivor: the swap exists to cancel position bias, so half of it is not a debiased result, it is a biased one carrying a debiased result's score. Why it's safe: genuine ties are untouched — a judge that answers TIE twice still scores 0.5. The only movement is outage -> 0, so nothing can newly pass. `errored` is optional, so any existing ComparisonResult literal is unaffected. Verified by mutation, not just by a green suite. Deleting each load-bearing line in turn, against the 11 tests: aggregator short-circuit removed -> 7 failures `errored: true` in the catch -> 4 failures `errored` on the swap arm -> 1 failure That last one matters: an earlier draft tested only the pure function, and both of the wiring mutations left it fully green — the extracted predicate was covered while the wiring that feeds it was not. Four of the eleven tests drive the real grader through a mocked judge for exactly that reason, and one of them fails the FIRST comparison only, since an all-calls-fail case short-circuits before the swap arm is ever read. Adds the skill's first test file (11 tests) and a `test` script. Bare `bun test` rather than an enumerated directory list, so a colocated test added later is discovered rather than silently skipped.
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.
PairwiseComparison passes when its judge is completely down.
compare() catches every judge error and returns
winner: 'tie'. With position_swap on (the default), a judge that never answers produces two ties, which the aggregator scores as (0 + 2*0.5)/2 = 0.5 — andpassed = score >= 0.5makes that a PASS.The failure is quiet in the worst way: the run reports every pairwise task as passing, and the per-position reasoning strings say "Comparison error: ..." while the score says the output beat the reference. LLMRubric and NaturalLanguageAssert both already return 0 when their judge throws; pairwise is the one model-based grader that does not.
A comparison result gains an
erroredflag, so a judge that never answered stops being indistinguishable from one that answered TIE, and the scoring moves into an exported pure aggregatePairwise() that returns 0 when any comparison errored. One errored comparison fails the whole grade rather than scoring on the survivor: the swap exists to cancel position bias, so half of it is not a debiased result, it is a biased one carrying a debiased result's score.Why it's safe: genuine ties are untouched — a judge that answers TIE twice still scores 0.5. The only movement is outage -> 0, so nothing can newly pass.
erroredis optional, so any existing ComparisonResult literal is unaffected.Verified by mutation, not just by a green suite. Deleting each load-bearing line in turn, against the 11 tests:
aggregator short-circuit removed -> 7 failures
errored: truein the catch -> 4 failureserroredon the swap arm -> 1 failureThat last one matters: an earlier draft tested only the pure function, and both of the wiring mutations left it fully green — the extracted predicate was covered while the wiring that feeds it was not. Four of the eleven tests drive the real grader through a mocked judge for exactly that reason, and one of them fails the FIRST comparison only, since an all-calls-fail case short-circuits before the swap arm is ever read.
Adds the skill's first test file (11 tests) and a
testscript. Barebun testrather than an enumerated directory list, so a colocated test added later is discovered rather than silently skipped.