Skip to content

Commit 8c9b1bc

Browse files
fix(evals): stop the judge grading mixed runners or skipping a grown condition set
`group_responses()` keyed groups on (case_id, trial) alone and assigned `groups[key][condition] = response`. run_evals.py keys its own resume on (case_id, trial, condition, runner), so one responses file legitimately holds several runners; the later runner's rows overwrote the earlier runner's, and every condition was then graded from whichever runner happened to be written last, with no warning and exit 0. Raise on the collision instead. The judge's own resume key had the same shape: `judged` was a set of (case_id, trial), so a group judged in an earlier pass under a narrower --conditions was skipped forever. Judging baseline+candidate and appending comparator responses afterwards is the documented flow, because run_evals.py writes one condition per invocation; the rerun printed "skip judged" for every group, wrote no comparator rows, and exited 0, after which `run_evals.py score` gated on the two conditions that happened to be present. Record the conditions covered by each written group instead. Re-judging a partially covered group is refused with the missing conditions named, because the scorer rejects duplicate rows: re-judging would fail there anyway, and skipping is what lost the condition silently.
1 parent ff690b6 commit 8c9b1bc

2 files changed

Lines changed: 177 additions & 5 deletions

File tree

scripts/judge.py

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,26 @@ def parse_judge_scores(
143143

144144

145145
def group_responses(rows: list[dict[str, Any]]) -> dict[tuple, dict[str, str]]:
146-
"""Collect responses into {(case_id, trial): {condition: response}} groups."""
146+
"""Collect responses into {(case_id, trial): {condition: response}} groups.
147+
148+
A condition may appear at most once per group. One (case, trial) can carry
149+
rows from several runner invocations -- `run_evals.py` keys its own resume on
150+
`(case_id, trial, condition, runner)`, so a file with two runners is a shape
151+
the harness produces. Collapsing those rows into one dict silently grades one
152+
runner's answers under the other runner's name and reports success, so the
153+
collision is raised instead of overwritten.
154+
"""
147155
groups: dict[tuple, dict[str, str]] = defaultdict(dict)
148156
for row in rows:
149-
groups[(row["case_id"], row["trial"])][row["condition"]] = row["response"]
157+
key = (row["case_id"], row["trial"])
158+
condition = row["condition"]
159+
if condition in groups[key]:
160+
raise ValueError(
161+
f"{row['case_id']}/trial {row['trial']}: two responses for the "
162+
f"{condition} condition. Judge one runner's responses per file "
163+
f"(run_evals.py keys completed rows by runner as well)."
164+
)
165+
groups[key][condition] = row["response"]
150166
return dict(groups)
151167

152168

@@ -280,9 +296,34 @@ def main(argv: Optional[list[str]] = None) -> int:
280296
file=sys.stderr,
281297
)
282298

283-
judged: set[tuple] = set()
299+
# Which conditions each already-written group covers. Keying only on
300+
# (case_id, trial) would skip a group judged in an earlier pass under a
301+
# narrower --conditions, leaving a requested condition ungraded while the
302+
# run still exits 0. A group is either complete for this run (skipped) or
303+
# absent; a partially covered one cannot be re-judged without writing
304+
# duplicate rows for the conditions that are already there, and the scorer
305+
# rejects duplicates, so it stops the run instead.
306+
written: dict[tuple, set[str]] = defaultdict(set)
284307
if args.output.exists():
285-
judged = {(row["case_id"], row["trial"]) for row in run_evals.read_jsonl(args.output)}
308+
for row in run_evals.read_jsonl(args.output):
309+
written[(row["case_id"], row["trial"])].add(row["condition"])
310+
partial = sorted(
311+
key for key in complete if written[key] and not required.issubset(written[key])
312+
)
313+
if partial:
314+
missing = {
315+
key: sorted(required - written[key]) for key in partial
316+
}
317+
raise ValueError(
318+
f"{args.output} already holds scores for "
319+
+ ", ".join(
320+
f"{case_id}/trial {trial} without {', '.join(missing[(case_id, trial)])}"
321+
for case_id, trial in partial
322+
)
323+
+ ". Judge into a fresh --output file, or remove those rows first: "
324+
"re-judging would write duplicate rows for the conditions already present."
325+
)
326+
judged = {key for key in complete if required.issubset(written[key])}
286327

287328
config = json.loads(args.runner_config.read_text(encoding="utf-8"))
288329
runner = config[args.runner]
@@ -295,7 +336,8 @@ def main(argv: Optional[list[str]] = None) -> int:
295336
with args.output.open("a", encoding="utf-8") as destination:
296337
for key in sorted(complete):
297338
case_id, trial = key
298-
if key in judged:
339+
pending = required - written[key]
340+
if not pending:
299341
print(f"skip judged {case_id}/trial {trial}")
300342
continue
301343
if case_id not in cases:

tests/test_judge.py

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,43 @@ def test_groups_missing_a_condition_are_partitioned_out_not_dropped(self):
5858
self.assertEqual([("direct-answer", 1)], sorted(complete))
5959
self.assertEqual([("casual-message", 1)], sorted(incomplete))
6060

61+
def test_two_responses_for_one_condition_are_rejected_not_overwritten(self):
62+
# run_evals.py keys its own resume on (case, trial, condition, runner),
63+
# so one responses file legitimately holds several runners. Collapsing
64+
# them into one dict would grade the last runner's answers under the
65+
# condition name and report success.
66+
rows = [
67+
{
68+
"case_id": "direct-answer",
69+
"trial": 1,
70+
"condition": condition,
71+
"runner": runner,
72+
"response": f"{condition}-from-{runner}",
73+
}
74+
for runner in ("claude", "codex")
75+
for condition in ("baseline", "candidate")
76+
]
77+
78+
with self.assertRaisesRegex(ValueError, "two responses for the baseline condition"):
79+
judge.group_responses(rows)
80+
81+
def test_the_same_condition_from_one_runner_still_groups(self):
82+
rows = [
83+
{
84+
"case_id": "direct-answer",
85+
"trial": 1,
86+
"condition": condition,
87+
"runner": "claude",
88+
"response": condition,
89+
}
90+
for condition in ("baseline", "candidate")
91+
]
92+
93+
self.assertEqual(
94+
{"baseline": "baseline", "candidate": "candidate"},
95+
judge.group_responses(rows)[("direct-answer", 1)],
96+
)
97+
6198

6299
class ParseJudgeScoresTest(unittest.TestCase):
63100
@staticmethod
@@ -411,6 +448,99 @@ def test_runner_failure_skips_its_group_and_continues(self):
411448
self.assertEqual(2, len(rows))
412449
self.assertNotEqual(0, exit_code, "skipped groups must not report success")
413450

451+
def test_a_group_judged_under_narrower_conditions_is_not_silently_skipped(self):
452+
# run_evals.py writes one condition per invocation, so judging
453+
# baseline+candidate and appending comparator responses afterwards is
454+
# the documented flow. Skipping on (case_id, trial) alone would drop the
455+
# comparator with exit 0, and `run_evals.py score` would then gate on
456+
# the two conditions that happen to be present.
457+
with tempfile.TemporaryDirectory() as tmp:
458+
tmp_path = Path(tmp)
459+
responses = tmp_path / "responses.jsonl"
460+
responses.write_text(
461+
"".join(
462+
json.dumps(
463+
{
464+
"case_id": "direct-answer",
465+
"trial": 1,
466+
"condition": condition,
467+
"runner": "stub",
468+
"response": f"{condition} answer",
469+
}
470+
)
471+
+ "\n"
472+
for condition in ("baseline", "candidate")
473+
)
474+
)
475+
verdict = tmp_path / "verdict.json"
476+
verdict.write_text(
477+
json.dumps({"A": self.VERDICT, "B": self.VERDICT, "C": self.VERDICT})
478+
)
479+
runner_config = tmp_path / "runners.json"
480+
runner_config.write_text(
481+
json.dumps(
482+
{
483+
"stub": {
484+
"command": ["sh", "-c", f"cat >/dev/null; cat {verdict}"],
485+
"response_format": "text",
486+
}
487+
}
488+
)
489+
)
490+
output = tmp_path / "scores.jsonl"
491+
common = [
492+
"--responses",
493+
str(responses),
494+
"--cases",
495+
str(ROOT / "evals" / "cases.jsonl"),
496+
"--rubric",
497+
str(ROOT / "evals" / "rubric.md"),
498+
"--runner-config",
499+
str(runner_config),
500+
"--runner",
501+
"stub",
502+
"--output",
503+
str(output),
504+
]
505+
506+
self.assertEqual(0, judge.main(common))
507+
508+
with responses.open("a") as handle:
509+
handle.write(
510+
json.dumps(
511+
{
512+
"case_id": "direct-answer",
513+
"trial": 1,
514+
"condition": "comparator",
515+
"runner": "stub",
516+
"response": "comparator answer",
517+
}
518+
)
519+
+ "\n"
520+
)
521+
522+
with self.assertRaisesRegex(ValueError, "without comparator"):
523+
judge.main(
524+
[
525+
"--responses",
526+
str(responses),
527+
"--cases",
528+
str(ROOT / "evals" / "cases.jsonl"),
529+
"--rubric",
530+
str(ROOT / "evals" / "rubric.md"),
531+
"--runner-config",
532+
str(runner_config),
533+
"--runner",
534+
"stub",
535+
"--conditions",
536+
"baseline",
537+
"candidate",
538+
"comparator",
539+
"--output",
540+
str(output),
541+
]
542+
)
543+
414544

415545
if __name__ == "__main__":
416546
unittest.main()

0 commit comments

Comments
 (0)