Skip to content

feat(tasks): persist the raw grader reply in LLM-judged tasks - #51

Merged
ethan-scitix merged 2 commits into
mainfrom
feat/grader-reply
Aug 4, 2026
Merged

feat(tasks): persist the raw grader reply in LLM-judged tasks#51
ethan-scitix merged 2 commits into
mainfrom
feat/grader-reply

Conversation

@ethan-scitix

@ethan-scitix ethan-scitix commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Type

  • feature — new benchmark, task, or capability

Summary

  • Adds grader_reply to the feedback record of the whole LLM-graded family — hle_0shot_gen, browsecomp_0shot_gen, simpleqa_verified_0shot_gen, aa_lcr_0shot_gen — holding the grader's reply verbatim and in full on every attempt.
  • The parsed verdict was persisted; the text it was derived from was not. A report could say "7 replies didn't parse" (judge_unparsed, from feat(hle): add Humanity's Last Exam dataset + 0-shot LLM-judge task #43) with no way to tell truncation from format drift from an API error from a genuine matcher gap. The HLE regexes have been hardened twice, both times from reasoning about likely reply shapes rather than observed ones — because the failures were never recorded.
  • Beyond HLE: parse_grade resolves a reply it cannot read to INCORRECT (browsecomp, aa_lcr) or NOT_ATTEMPTED (simpleqa_verified), so a drifted reply is indistinguishable from a real negative verdict or a real abstention — and the F1 treats abstention very differently from a wrong answer.
  • Grading is not reproducible from artifacts: a grader model version is not pinnable like a Hub revision, so re-running a judge need not reproduce a past verdict. The reply is the only durable evidence of what the grader actually said.
  • Docstrings and reference_impl.notes on all four claimed only the grade + grader model id were persisted — stale, not merely incomplete. Updated, hence the sieval/meta/index.json regen.

Settling the three questions #48 left open:

Question Decision Why
Payload Full, always The record already carries predicted, a full generation typically longer than a judge reply, so the marginal cost is small. Storing only unparsed replies leaves a wrong-but-parsed verdict — the failure that actually moves scores — unauditable.
Gating No knob A record_* knob would put task-specific state in TaskRunnerConfig (a core/ layer that forbids it) and widen --resume strict matching for no scoring benefit.
Field name grader_reply Reads consistently with the existing grader_model.

aa_lcr is now included. #44 merged while this PR was open, so the family-wide pass covers it rather than leaving a fourth task to diverge (plan posted on #44 before it merged). One wrinkle specific to it: the empty-candidate short-circuit grades INCORRECT without calling the checker, so grader_reply = "" is bound explicitly on that path. Leaving the variable unbound there would attribute the previous attempt's real verdict to an ungraded answer when n > 1 — AA-LCR runs n=3. Empty is unambiguous: the branch is exactly not predicted.strip(), so the empty predicted in the same record identifies it. A sentinel string ("<not invoked>") was rejected as a magic value the record does not need.

No behavior change to scores. Purely additive to the artifact; no metric reads the new field.

Related Issues

Closes #48. Refs #43 (which added the judge_unparsed count this makes diagnosable), #44 (whose task this now covers).

Test Plan

Automated

  • Lint/format clean (ruff check && ruff format --check)
  • Type check clean (ty check clean; mypy --strict error count is identical before and after this change on every file touched — all pre-existing unannotated stage overrides, verified by stashing the source changes and re-running)
  • Unit tests pass — pytest tests/unit → 2511 passed; the four task files → 58 passed
  • python scripts/check_preflight.py → all checks pass, including the meta-drift check after regenerating the index

New assertions cover every branch, and are written to fail against the rejected alternatives:

  • Parsed path — a multi-line reply containing reasoning the parse discards is asserted equal in full. This fails a "store only the matched fields" or "store only on parse failure" implementation.
  • Drift path — HLE unparseable (judge_parsed is False), browsecomp no-verdict (INCORRECT default), simpleqa_verified non-matching (NOT_ATTEMPTED default), aa_lcr unreadable (INCORRECT default) each assert the reply survives, which is the motivating audit case.
  • aa_lcr not-invoked pathfeedback(["Rising", ""]) asserts the graded attempt keeps its reply and the short-circuited one is empty. This is the assertion that fails if the reply = "" binding is dropped, so it discriminates the leak described above rather than merely restating the happy path.

Manual

Not applicable — no scoring path changed, so there is no score to compare. The four tasks' existing validation numbers (HLE gpt-oss-20b 12.14/3.61; BrowseComp gemma-3-27b-it 0.316%; SimpleQA Verified gemma-4-31B-it F1 9.95; AA-LCR gpt-oss-120b/20b within ~2–3 pts of the AA leaderboard) are unaffected by an additive feedback field.

Resume compatibility, verified by inspection: records written before this change simply lack grader_reply, and nothing in report() reads it, so resuming an in-flight run does not raise — it yields records mixed on this field, which a cross-version resume already warns about.

Checklist

Required (all PRs)

  • PR title follows conventional format (type(scope): description)
  • No internal paths, credentials, or personal info in committed files
  • AI-generated code has AI-Generated Code - <model> (<provider>) in module docstring — all four modules already carry it; unchanged
  • No new upper-layer dependencies added to core/core/ is untouched, which is the reason there is no config knob
  • Deleted code verified — no remaining call sites depend on it — nothing deleted; change is additive

If: New or Modified Benchmark

  • Reference paper/repo linked in Summary — unchanged from each task's existing reference_impl.url (HLE @ 26dca2e, arXiv:2504.12516, arXiv:2509.07968, AA-LCR card @ bdae010b)
  • Score comparison table — no scoring path changed
  • Dataset loading tested — no dataset change
  • Task registered in package-level __init__.py — already registered; no new task

🤖 Generated with Claude Code

@ethan-scitix

Copy link
Copy Markdown
Collaborator Author

Scope note: aa_lcr_0shot_gen should become the fourth member of this pass, handled here rather than in #44.

Sequencing, since #44 is still open: this PR waits for #44 to merge, then rebases onto the new main (which will have aa_lcr) and adds grader_reply to all four tasks in one place. That keeps one PR as the single owner of the field, the "in full on every attempt, no config knob" convention, and the notes wording — rather than having #44 invent its own copy of the field in parallel.

The cost is that aa_lcr sits on main briefly without the field. That's acceptable: the window is however long this PR takes, and it removes work from a fork PR that still has an outstanding blocker.

Two things to remember when it lands:

  1. The empty-candidate short-circuit. Unlike the other three, aa_lcr grades an empty/whitespace candidate INCORRECT without invoking the checker (aa_lcr_0shot_gen.py:182), so there is no reply to store. Going with option 1 from the feat(aa-lcr): add dataset + 0-shot LLM-equality-checker-graded task #44 discussion: "grader_reply": "" plus a comment on that branch noting the checker was never called. grade == "INCORRECT" with an empty predicted already identifies the branch unambiguously, so a sentinel string isn't worth the magic value.
  2. reference_impl.notes needs the same edit as the other three. aa_lcr's notes currently say "the per-sample grade and grader model id are persisted in the feedback record" — that becomes stale the moment the field is added, and it drives sieval/meta/index.json, so regen and commit it or preflight's meta-drift check fails.

Also worth knowing for this PR: on aa_lcr the stored reply is what makes the grader's phrasing space auditable at all. Two accuracy-inflating parse bugs have already been found there by reading the prompt rather than the replies (negated verdicts in 216614fc, hedged verdicts in 4a8c858f), and neither could be checked against the 1200 real replies from the validation runs because they were never persisted. Those are gone permanently — this field is what stops that recurring.

@ethan-scitix

Copy link
Copy Markdown
Collaborator Author

Heads-up on a conflict, and the agreed resolution — this PR goes first.

#60 (stage-output protocol pilot) independently added grader_reply to simpleqa_verified before I spotted this PR, so the two overlap. A trial merge conflicts in three files:

  • sieval/tasks/simpleqa_verified_0shot_gen.py
  • tests/unit/tasks/test_simpleqa_verified_0shot_gen.py
  • sieval/meta/index.json (both PRs rewrote that task's reference_impl.notes, so both regenerated the index)

The other three judge tasks here — hle, browsecomp, aa_lcr — are untouched by #60, so they conflict with nothing.

Plan: this PR merges as-is (it is the family-wide change and it Closes #48), then #60 rebases on top. In that rebase simpleqa_verified's grader_reply moves from a top-level feedback field to extra.grader_reply, because the protocol puts grader-specific detail under extra while correct / reference / the rollout counts stay at the top. The field, its name and its rationale are unchanged — only its position in the record. The four assertions this PR adds for simpleqa_verified get rewritten to the new shape, not dropped, and #60 additionally asserts that the grader's per-call usage (extra.grader_calls) is persisted.

Nothing to do on this PR beyond an Update branch — it is MERGEABLE, just BEHIND main.

Also worth flagging for the eventual judge-family protocol migration (tracked as a follow-up on #60): the profiler only reads task-supplied TaskStageMeta["model_calls"], and protocol records are returned bare rather than boxed in a TaskStageOutput, so grader spend lands on disk but not in profile.json. That needs a core-side fix before all four tasks migrate, otherwise each one inherits the blind spot.

ethan-scitix and others added 2 commits August 4, 2026 19:55
`grader_reply` joins the feedback record for the whole LLM-graded family —
`hle_0shot_gen`, `browsecomp_0shot_gen`, `simpleqa_verified_0shot_gen`,
`aa_lcr_0shot_gen` — holding the grader's reply verbatim, in full, on
every attempt.

The parsed verdict was persisted; the text it came from was not. So a
report could say "7 replies didn't parse" (`judge_unparsed`, added in #43)
with no way to tell truncation from format drift from an API error from a
genuine matcher gap. The HLE regexes have been hardened twice, both times
from reasoning about likely reply shapes rather than observed ones,
because the failures were never recorded.

Settled the three questions #48 left open:

  - Payload: full, always. The record already carries `predicted`, a full
    generation typically longer than a judge reply, so the marginal cost
    is small. Storing only unparsed replies would leave a
    wrong-but-parsed verdict — the failure that actually moves scores —
    unauditable.
  - Gating: none. A `record_*` knob would put task-specific state in
    `TaskRunnerConfig` (a `core/` layer that forbids it) and widen
    `--resume` strict matching for no scoring benefit.
  - Name: `grader_reply`, reading with the existing `grader_model`.

The default-grade paths are why this matters beyond HLE: `parse_grade`
resolves a reply it cannot read to INCORRECT (browsecomp, aa_lcr) or
NOT_ATTEMPTED (simpleqa_verified), so a drifted reply is
indistinguishable from a real negative or a real abstention — and the F1
treats abstention very differently from a wrong answer.

aa_lcr's empty-candidate short-circuit grades INCORRECT without calling
the checker, so it binds `grader_reply = ""` explicitly: leaving the
variable unbound on that path would attribute the previous attempt's real
verdict to an ungraded answer when `n > 1`. Empty is not ambiguous there —
the branch is exactly `not predicted.strip()`, so the empty `predicted` in
the same record identifies it.

Docstrings and `reference_impl.notes` claimed only the grade and grader
model id were persisted; updated on all four (hence the meta index regen).
Grading is also not reproducible from artifacts — a grader model version
is not pinnable like a Hub revision — which makes the reply the only
durable evidence of what the grader actually said.

Resuming a run started before this change is unaffected: nothing reads
`grader_reply`, so pre-existing records simply lack the field.

Closes #48

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The persisted reply is `ModelOutput.texts` — response content only — so a
judge that spends its whole budget on reasoning records an empty reply,
indistinguishable from an empty API response. `finish_reasons` ("length")
and `reasoning_texts` would separate the two and are not captured. Narrow
the HLE docstring, `reference_impl.notes` and test comment that claimed
the reply tells a *truncated* reply from a matcher gap, and document the
limit on `JudgeFeedback.grader_reply` so the gap is visible to anyone
auditing a `judge_unparsed` count. No behavior change.

Collapse the rationale that was stated three times per file down to one
statement per layer: module docstrings now give the fact and point at the
field comment, which holds the argument; `notes` stays self-contained
because it is what `sieval/meta/index.json` exposes. The "grader model
version is not pinnable like a Hub revision" clause is dropped where the
same docstring or the same `notes` string had already said it.

Also drop the duplicated HLE subset paragraph, folding its one unique
claim (the task stays isomorphic to upstream `run_model_predictions.py`)
into the Subset paragraph above, and rename aa_lcr's
`test_feedback_unrecognized_grader_reply_is_incorrect` to
`..._empty_grader_reply_is_incorrect` — it passes `grader_reply=""`, and
a sibling added in the previous commit covers the genuinely unrecognized
case, matching simpleqa_verified's naming.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ethan-scitix
ethan-scitix merged commit 8875430 into main Aug 4, 2026
9 checks passed
@ethan-scitix
ethan-scitix deleted the feat/grader-reply branch August 4, 2026 12:05
ethan-scitix added a commit that referenced this pull request Aug 5, 2026
…imit

hle, browsecomp and aa_lcr adopt simpleqa_verified's shape from #60: the
grader's whole ModelOutput is persisted as `extra.grader_output` (reply,
reasoning, usage, finish_reasons, model id) instead of #51's flat
`grader_reply`, which was response text only.

That closes the limit #51 documented and #60 could only close for
simpleqa_verified: a reasoning autorater that spends its entire budget thinking
returns empty content, so an empty `grader_reply` was indistinguishable from an
empty API response. `finish_reasons`, in the same output, is what separates
them. HLE is the case #51 flagged as most reachable, since it normally runs a
reasoning judge. Nothing is hand-picked, so no field is silently dropped and a
future ModelOutput field is captured for free -- `grader_model` is subsumed
(model id is in the output) and `predicted`/`gold` move to their protocol homes
(the prediction record, and the judgement's `reference`).

aa_lcr's empty-candidate short-circuit gets strictly better: it grades INCORRECT
without calling the checker, and `grader_output` is now ABSENT on that path
rather than an empty string -- absence is unambiguous where #51's "" meant both
"never called" and "the checker returned nothing". The two are now asserted
distinct. The short-circuit condition is also no longer spelled twice: blank
answers normalize to None in postprocess, so `extracted: false` on the
prediction and the feedback branch are one notion of "no answer".

Grade / confidence / judge_parsed stay in `extra`, not `metrics`: none of them
measures whether the answer was right. HLE's `confidence` in particular is the
raw material report() pools into a calibration error, which a per-sample metric
could not reconstruct.

Module docstrings and reference_impl notes are updated in the same commit rather
than left describing the deleted `grader_reply` / GradeFeedback / JudgeFeedback
types; that rotates sieval/meta/index.json, regenerated here.

Parity: 300 randomized differential trials per task (900 total) against the
pre-migration report() from d622448, over 1-3 attempts, 0-15 samples and 0-4
fails. HLE's trials randomize judge_parsed so the unparsed-drop path (which must
stay out of the calibration arrays) is exercised. 900/900 identical.

Closes the #51 item in #61. Refs #61

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ethan-scitix added a commit that referenced this pull request Aug 5, 2026
…/40) (#62)

* feat(tasks): migrate the pass@k math family to the stage-output protocol

aime_2026 / hmmt_feb_2026 moved in #60 while their six near-identical siblings
did not, so tests/unit/tasks/test_math_pass_at_k_family.py carried a fork that
built two different feedback shapes depending on set membership. All six now
return PromptRecord / PredictionRecord / JudgementRecord, PROTOCOL_TASKS is
gone, and _feedback() builds one shape for the whole family.

The gold reaches disk from preprocess for all eight, which it previously did
only for the two pilots -- raw_sample is never serialized, so a pass@k row that
recorded no reference had no ground truth on disk at all.

imo_answer_bench records the RAW dataset gold as `reference`, not the
normalize_answer() output: normalization returns None when nothing survives
stripping, and a None reference is reserved for "the ground truth is a
procedure". The normalized form is in extra.normalized_reference, where None
correctly reads as "the gold did not normalize".

Correcting #61: it groups hmmt_nov_2025 and imo_answer_bench as pass@k tasks
that "also use an LLM grader". They do not -- both grade deterministically
(math_verify / the vendored verify_math_answer), and imo's own notes say an LLM
grader would break the reproducibility contract. No grader detail is recorded
because there is no grader.

Parity: 300 randomized differential trials per task (1800 total) against the
pre-migration report() loaded from d622448, over k in {1,2,4}, n >= k, 0-12
samples and 0-4 fails, including rollouts < k so the `short` warning path is
exercised. 1800/1800 identical.

Refs #61

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(tasks): migrate the clp/ppl family to the stage-output protocol

The nine conditional-log-prob / perplexity tasks now return the three record
types. #61 already corrected #60's "blocked on a design call" framing; this
confirms it against the code: every one of the nine produces exactly ONE
prediction per sample (an argmax index or a winning option label), so a
one-entry rollouts[] is the honest shape. The candidate fan-out lives in
`infer`, which the protocol deliberately leaves unwrapped.

The migration unit is the family module, not the task: sieval/tasks/_arc.py
holds the record builders (arc_prompt_record / arc_prediction_record /
arc_judgement_record) and arc_report shared by all four ARC tasks
(challenge/easy x ppl/clp), so the four move together and cannot fork.

hellaswag_kshot_ppl is the `metrics` tier's second pilot after ifeval: acc and
acc_norm are two argmax rules over one inference set, so both are named in
`metrics` and `correct` is derived from acc_norm (the headline `score`) rather
than recomputed. The raw-logprob argmax is the other rule's answer, not a second
rollout, so it sits in the prediction record's extra next to the scores.

mmmlu_kshot_clp keeps its TaskStageOutput box on `infer`: infer is not a record
stage, so boxing it does not violate the bare-record rule. Its four flat
prob_A..prob_D keys become one `probs` mapping in the rollout's extra --
mechanism behind the argmax, not metrics measuring the answer.

Two deliberate non-changes, so no denominator moves:
- cmmlu's report still groups by ctx.raw_sample, not the record's extra.subject.
  The two agree whenever both exist, but a final whose raw_sample is absent has
  always been dropped from the macro-average, and switching the source would
  silently change the denominator. The record carries `subject` regardless, so a
  row on disk is self-describing.
- The ppl tasks' "no option could be picked" was the sentinel index -1 and is
  now None, per the protocol. Equivalent downstream: both mismatch the gold and
  both map to "" through choice_text (which now accepts None).

Parity: 300 randomized differential trials per report against the pre-migration
implementations loaded from d622448 -- 1800 total, 1800 identical. The cmmlu
trials deliberately inject raw_sample=None samples to exercise the skip path.

Refs #61

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(tasks): migrate the LLM-judge family, closing #51's documented limit

hle, browsecomp and aa_lcr adopt simpleqa_verified's shape from #60: the
grader's whole ModelOutput is persisted as `extra.grader_output` (reply,
reasoning, usage, finish_reasons, model id) instead of #51's flat
`grader_reply`, which was response text only.

That closes the limit #51 documented and #60 could only close for
simpleqa_verified: a reasoning autorater that spends its entire budget thinking
returns empty content, so an empty `grader_reply` was indistinguishable from an
empty API response. `finish_reasons`, in the same output, is what separates
them. HLE is the case #51 flagged as most reachable, since it normally runs a
reasoning judge. Nothing is hand-picked, so no field is silently dropped and a
future ModelOutput field is captured for free -- `grader_model` is subsumed
(model id is in the output) and `predicted`/`gold` move to their protocol homes
(the prediction record, and the judgement's `reference`).

aa_lcr's empty-candidate short-circuit gets strictly better: it grades INCORRECT
without calling the checker, and `grader_output` is now ABSENT on that path
rather than an empty string -- absence is unambiguous where #51's "" meant both
"never called" and "the checker returned nothing". The two are now asserted
distinct. The short-circuit condition is also no longer spelled twice: blank
answers normalize to None in postprocess, so `extracted: false` on the
prediction and the feedback branch are one notion of "no answer".

Grade / confidence / judge_parsed stay in `extra`, not `metrics`: none of them
measures whether the answer was right. HLE's `confidence` in particular is the
raw material report() pools into a calibration error, which a per-sample metric
could not reconstruct.

Module docstrings and reference_impl notes are updated in the same commit rather
than left describing the deleted `grader_reply` / GradeFeedback / JudgeFeedback
types; that rotates sieval/meta/index.json, regenerated here.

Parity: 300 randomized differential trials per task (900 total) against the
pre-migration report() from d622448, over 1-3 attempts, 0-15 samples and 0-4
fails. HLE's trials randomize judge_parsed so the unparsed-drop path (which must
stay out of the calibration arrays) is exercised. 900/900 identical.

Closes the #51 item in #61. Refs #61

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(tasks): migrate the code-execution family to the stage-output protocol

human_eval (chat + base), mbpp and livecodebench-base adopt the
livecodebench_code_generation_0shot_gen shape from #60.

This retires the ResourceMetrics TypedDict from all four. It was the coupling
#61 named: the evaluator payload was persisted verbatim behind a client-side
type, so when the evaluator gained n_cases/n_passed, #60 had to widen the
TypedDict in lockstep. The payload now lands in the rollout's untyped `extra`
(n_cases / n_passed hoisted, everything else under `resources`), so the next
evaluator field needs no task change. `msg` stays raw and deliberately
un-bucketed -- it is free text from a separately deployed service whose wording
has already drifted once, which is why #60 removed its substring-derived failure
category rather than keep it.

`reference` is None across the family: the ground truth is a test suite, a
procedure rather than a value. Each records what was actually run in the
judgement's sample-level extra -- entry_point for HumanEval, the three
lm-eval-shown asserts for MBPP, the case counts and io_mode for LiveCodeBench.

Unextractable output is None in the record but still "" on the wire, so the
evaluator runs it and reports a compile error. That is the pre-protocol
behaviour: a real verdict, not a skipped rollout.

The "Not evaluated" pre-filled feedback list is gone. It was unreachable -- the
only path that left it in place re-raises -- so appending per rollout is
equivalent and does not pretend a placeholder verdict can reach a report.

Parity: 300 randomized differential trials per task (1200 total) against the
pre-migration report() from d622448. The msg pool mixes casings and the
near-miss "timed out" so the `timeouts` substring counter is exercised in both
directions; rollout counts vary 0..n to cover short returns. 1200/1200
identical.

Refs #61

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(tasks): migrate the remaining generative tasks; protocol adoption complete

The last ten: mmlu, openbookqa, gsm8k (chat + base), hendrycks_math, theoremqa,
drop, ruler, t_eval_before_calling, ifbench. All 39 registered tasks now emit
PromptRecord / PredictionRecord / JudgementRecord, so a consumer can read any
sample's prompt, prediction, ground truth and verdict without knowing which task
wrote the row -- the thing #60 built the protocol for and #61 tracked to done.

The `metrics` tier does the work #61 predicted it would:

- gsm8k_kshot_base_gen: strict and flexible extraction are co-equal published
  metrics; both are named, `correct` is DERIVED from the strict one.
- drop: em -> correct, f1 -> score fits natively, but both are also in `metrics`
  so a generic reader can enumerate them, and report() pools from there.
- t_eval: six co-equal continuous axes and no published headline. All six are
  named; `correct` is the strict reading (every scored axis at 1.0), derived
  from the mapping. Deliberately NOT parse_rate -- a task whose `correct` meant
  "the output parsed" would look near-perfect on the one axis that is supposed
  to be comparable across tasks.
- ifbench: strict + loose, mirroring #60's ifeval work, with loose as the
  headline (ifeval's is strict). Grading moves from report() into feedback(),
  so per-sample verdicts are finally persisted, and report() stops depending on
  raw_sample being present. Its dead _get_report is removed.

ruler gains a per-sample verdict it never had: upstream's string_match_* take
whole lists and average internally, so before this no RULER sample carried a
score of its own. Both metrics decompose exactly, so feedback() records the
per-sample term. report() still calls the VENDORED functions on the whole cell
-- re-deriving the mean here would fork upstream's scoring, and a vendored
metric that no longer runs is a reproduction that drifts silently.

theoremqa's `empty` counter now reads `extracted` off the prediction record
instead of comparing the stored prediction against "": postprocess maps exactly
that empty extraction to None, so it is the same population, spelled once.

Parity: 300 randomized differential trials per task (2700) against the
pre-migration report() from d622448 -- theoremqa's trials inject unextracted
answers to exercise `empty`, ruler's vary reference counts and partial matches
across both metric families, t_eval's vary all six axes. 2700/2700 identical.
ifbench is covered instead by its unit test, whose expected report numbers are
unchanged across the grading relocation.

Refs #61

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(core): route grader model_calls into the profiler

#60 accepted a documented workaround: the profiler reads only task-supplied
TaskStageMeta["model_calls"], which the runner derives from a stage RETURNING a
ModelOutput. A judge task's `feedback` returns a bare record (the protocol
forbids boxing it), so its grader calls had nowhere to go -- grader spend was on
disk inside the record but absent from profile.json. #61 flagged fixing this
before the judge family migrated in bulk; that family has now migrated, so all
four judge tasks would otherwise inherit it.

The record itself is the channel. A judged rollout already persists the grader's
whole ModelOutput under extra.grader_output, so:

- records.iter_grader_outputs() walks a judgement record for those outputs, and
  GRADER_OUTPUT_KEY names the key it reads. Non-judgements and grader-less
  judgements (a string compare, a test suite -- most tasks) return empty; this
  runs for every stage of every task, so it stays cheap and silent.
- build_model_call_meta_from_mapping() rebuilds a ModelCallMeta from that
  already-flattened output. A mapping with no `model` returns None rather than
  contributing a usage-less phantom call.
- build_stage_meta() gains `model_calls=` for calls not represented by an
  `outputs` entry, and the runner passes the recovered grader calls through it.

No task changes: the four judge tasks already write the key. It is documented in
sieval/tasks/CLAUDE.md as load-bearing rather than cosmetic -- spell it
differently and the grader's tokens silently leave the profile.

Verified end-to-end through the real TaskRunner + profiler, not just the
helpers: a bare-record judge task's grader tokens now land in the FEEDBACK
stage's usage (20 in / 1 out per sample from MockJudgeModel), the candidate's
own spend stays attributed to `infer`, and the recorded call names the judge
model. Also covers the aa_lcr case where a short-circuited rollout never called
the checker and must not fabricate a call.

Refs #61

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(tasks): keep failed problems' steps in scicode's sub-problem denominator

scicode reports two accuracies and they disagreed about what a pipeline failure
means. `main_problem_accuracy` counted a failed problem as unsolved;
`sub_problem_accuracy` dropped its steps from the denominator entirely. One
metric was therefore diluted by a failure while the other was silently inflated
by its removal, and a full test split fell below the fixed 288-step denominator
the official sub-problem figures are computed over -- making the number
incomparable to the leaderboard exactly when a run was unhealthy.

The comment justifying it ("their step counts are unknown") was false: a failed
context carries its `raw_sample`, so its tested steps are recoverable. Recover
them -- excluding the three scientist-authored steps, as on the success path --
and leave them in the denominator scoring zero. A new `unevaluated_steps` report
field says how many of `total_steps` never ran, so a reader can tell a model
failure from a pipeline failure; read it alongside `fails`.

A context that failed before its sample was loaded has nothing to recover and is
skipped. It still dilutes main-problem accuracy.

Audited against a prior in-house SciCode port, which cleared the rest of this
task (identical sub-problem formula, byte-identical code extraction, identical
prompt templates, equivalent special-step gold injection under the default
`verbatim` mode). Deliberately not followed there: it filters failures out of
*both* metrics, which hides pipeline failures and makes the denominator depend
on run health.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* feat(tasks): migrate scicode to the stage-output protocol, 40/40

scicode_0shot_gen was the one registered task #61's inventory of 32
missed (#42 landed it just before #60). It is not a mechanical port: a
sample is a chain of *dependent* sub-steps, each generating and
executing code, which is not what rollouts[] means -- independent
attempts at the same thing.

One rollout per problem. The whole step sequence is the attempt; steps
are neither independent nor attempts at the same thing, so they are not
rollouts. Per-step detail lives in rollouts[0].extra.steps, and metrics
carries both axes (main_problem_solved, sub_problem_pass_rate) with the
headline derived from them. The raw counts correct_steps/total_steps sit
in the record's extra because report() *pools* them -- averaging
per-problem rates would give a different, wrong sub_problem_accuracy
(the same reasoning ifeval's instruction-level pooling rests on).

prompt is the first *generated* step's prompt, not the whole problem. It
is knowable at preprocess because every preceding step is a special step
whose gold code is static; infer then *reuses* the recorded prompt for
that step, through a shared _gold_code(), so the record cannot disagree
with what the model saw. Only steps 2..N are rebuilt, since their
prompts embed the model's own prior output. extra.first_generated_step /
n_generated_steps / previous_code say this explicitly, so prompt is not
misread as the whole model input. Persisting all N per-step prompts
would be quadratic text; stuffing the sample in would duplicate
raw_sample.

prediction is None iff *no* tested step produced extractable code. A
partial miss is still a scoreable answer and keeps its prediction, which
keeps the extraction_failure anomaly rule a real signal instead of
firing on every partially-empty problem. reference is None: the ground
truth is a procedure, this problem's per-step test suites. New
StepCode.extracted_code holds the model's per-step answer so the
prediction need not re-run the extractor.

report.json is unchanged: 300 randomized differential trials against the
pre-migration report(), 0 mismatches over 167 solved problems and 4529
graded steps; a deliberately sabotaged variant did produce mismatches,
so the differential discriminates. Tests 34 (was 25), including one that
drives the real feedback() twice and feeds both records to report(), so
report reads exactly what feedback writes.

* fix(tasks): record only the axes t_eval actually scores

feedback() recorded every axis _evaluate returns, and correct is derived as
"every recorded axis came out at 1.0". But _evaluate pre-seeds all six axes to 0
to keep its mapping rectangular, and thought similarity costs an embedding call
so it is opt-in -- so under the default eval_thought=False a retained
thought: 0.0 pinned correct to False on every sample of every run. The one axis
the stage-output protocol exists to make comparable across tasks was
structurally unreachable for this task.

A 0 on an axis nobody measured is a hole, not a measurement. metrics now carries
only the scored axes, from a new _metric_keys() shared with the macro-average, so
the recorded metrics, the derived correct and report() cannot disagree about
which axes are real.

report.json does not move. _post_process already macro-averaged exactly this key
set (same json/str branch, same eval_thought gate), and it is now fed the
narrowed mapping, so the *_parsed variants read defensively -- the str modes
score no args at all, and 0.0 is what they reported before, when _evaluate's
pre-seeded zeros were still on the record.

Also fixes a latent NameError: metric_keys was left unbound for any prompt_type
outside json/str, surfacing deep in the macro-average instead of rejecting the
config. The unsupported branch now raises NotImplementedError where the decision
is made.

Tests 13 (was 1): the scored-axis set per mode, the NotImplementedError, correct
reachable under the default config, a wrong tool name moving only `name`, an
unparseable answer, and the str-mode report path where *_parsed must stay 0.0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(tasks): read ruler's prediction with .get() on the resume-report path

report() indexed rollouts[0]["prediction"], but postprocess normalizes a blank
response to None so `extracted` stays a real signal -- and obj_to_dict drops
None-valued keys, so the field is ABSENT on disk, not null. report() runs over
disk-rehydrated records whenever every sample is complete but report.json is
missing or invalid, which is exactly the resume case: an already-paid-for
long-context run reached the last step and died with KeyError.

Nothing about this is ruler-specific reasoning -- records.py states the rule
("optional fields are read with .get()"); this call site just didn't follow it.

The test helper now round-trips both records through obj_to_dict, so every report
test builds the shape report() actually has to read rather than the strictly more
forgiving in-memory one, and the new case pins the blank sample as a scored miss
(50.0 in a two-sample cell) rather than a silently skipped sample.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(tasks): use GRADER_OUTPUT_KEY instead of the literal in judge tasks

The key name is load-bearing, not cosmetic: the runner reads extra["grader_output"]
back to route grader spend into profile.json, because a feedback stage returning a
bare record has no ModelOutput for the profiler to derive a call from. Spelled as
a literal in four places, a divergence is silent -- the grader's tokens sit on disk
and go missing from the profile, with nothing failing.

core.tasks already exports the constant; the producers just weren't using it.
Docstring occurrences stay as prose, since those describe the on-disk key name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(tasks): default a null evaluator msg in the code-exec timeout counters

The five reports that count timeouts did r["extra"]["msg"].lower(). msg is passed
straight through from the code-eval service response (res["msg"]), so a null makes
a FRESH run raise AttributeError on None.lower() -- the timeout counter takes the
whole report down at the last step, after every sample has already been executed
and paid for. Serialization dropping None-valued keys makes the resumed run fail
the same way for a second reason.

Now (r["extra"].get("msg") or "") -- absent or null both read as "no message",
which is what an evaluator that reported no message means. Nothing else about the
verdict depends on msg; status already carries pass/fail.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(core): type the record predicates as TypeGuards over object

`iter_grader_outputs` and the two `is_*_record` predicates took `Any`, which
passes silently in both directions: a caller may hand them anything, and the
body may do anything with it. `object` is the accurate spelling -- no caller is
affected (everything is an `object`) while the bodies are forced to narrow,
which they already did.

That narrowing then has to reach the caller. `iter_grader_outputs` walks
`value.get("rollouts")` only after `is_judgement_record` says yes, and a plain
`bool` return throws that away -- only the old `Any` made it type-check. Both
predicates now return `TypeGuard[Mapping]`, so the narrowing they perform is the
narrowing callers get, with no redundant `isinstance` at the call site.

`TypeGuard` and deliberately not `TypeIs`: a mapping that is not a record still
returns False, so the negative branch says nothing about the type. `anomaly.py`
depends on exactly that, testing `isinstance(post_result, dict)` after a False.

Two of the three signatures predate this PR (#60); cleaned up here rather than
left inconsistent with their sibling.

Also fixes a non-discriminating assertion. `iter_grader_outputs({"rollouts":
"not a list"})` read as covering the per-rollout `isinstance` guard, but without
`n_rollouts` the value is not a judgement at all: it returned at the first guard
and never reached the one under test, so removing that guard failed no test.
The fixture now carries `n_rollouts`, and a mutant with the guard removed raises
AttributeError on the string's characters.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(t_eval): give the thought-scoring test an embedding key

`eval_thought=True` builds the embedding client in `__init__` with
`api_key=os.getenv("SIEVAL_EMBED_API_KEY", "")`. An explicit empty string
is not the same as `None`: it skips the OpenAI client's `OPENAI_API_KEY`
fallback, and openai >= 2.x raises `OpenAIError: Missing credentials` at
construction rather than failing later on the first call. So the test
could not build the instance in a credential-free environment.

Setting the env var keeps the instance real -- the point of the helper --
and still exercises the `eval_thought=True` branch of `__init__`. No
embedding call is made; the assertion is on `_metric_keys()` alone.

Verified against the locked openai (2.45.0): `api_key=""` raises exactly
the CI error, `api_key="not-a-real-key"` constructs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(tasks): put four stragglers' declared types on the record protocol

The migration moved these four tasks' bodies onto `build_prompt_record` /
`build_prediction_record` / `build_judgement_record`, but left their
`Task[...]` parameterization describing the shapes they no longer return —
`list[ChatCompletionUserMessageParam]` / `str` for the prompt slot, `str` /
`list[str]` / a per-task `Prediction` TypedDict for the prediction slot, and
a per-task `Feedback` TypedDict for the judgement slot.

Nothing caught it: the stage methods carry no return annotations, so the
generic slots are never cross-checked against the bodies, and the five dead
TypedDicts stayed reachable as those slots' arguments — which is also why
the deletion sweep for per-task `Feedback`/`Prediction` names read clean.

On-disk output was already correct, so no report or shard moves. What
changes is that the declared type now matches it, the five unused TypedDicts
are gone, and `gsm8k_0shot_gen` no longer imports `openai` at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(t_eval): name the env var that actually helps when a key is missing

`eval_thought=True` builds the embedding client with
`api_key=os.getenv("SIEVAL_EMBED_API_KEY", "")`. An explicit "" is not
None, so it skips the OpenAI client's own OPENAI_API_KEY fallback and
raises `Missing credentials` naming OPENAI_API_KEY -- a variable that
would not help even if set, because the endpoint is our embedding
service, not OpenAI's. The failure also arrives at construction, before
any run output, so the message is all the operator gets.

Check the key in the task instead and raise a ValueError that names
SIEVAL_EMBED_API_KEY, mentions SIEVAL_EMBED_API for a non-default
endpoint, and says the axis is optional (leave eval_thought unset). The
empty-string case is the reachable one -- an exported-but-blank variable
-- so both None and "" take the new path.

No behaviour change when the key is present, and none at all under the
default `eval_thought=False`, which builds no client. The new test
discriminates: pre-fix the constructor raised OpenAIError, which is not
a ValueError subclass, and its message named OPENAI_API_KEY.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: persist the raw grader reply in LLM-judged tasks

1 participant