feat(tasks): finish the stage-output protocol migration (33 tasks, 40/40) - #62
Conversation
Seventh commit: a scicode metric fix, not part of the 32-task migration
So a failure diluted one metric and silently inflated the other, and a full test split fell below the fixed 288-step denominator the official sub-problem figures are computed over — the number became incomparable to the leaderboard exactly when a run was unhealthy. The comment justifying the drop ("their step counts are unknown") was false — a failed New No user-facing break: scicode (#42) is unreleased, so this pins the definition down before it ships rather than changing it under anyone. Audited against a prior in-house SciCode port — and where I deliberately did not follow itThe rest of the port cleared: identical sub-problem formula, byte-identical code extraction, identical prompt templates, and equivalent special-step gold injection under the default Not adopted from it:
Why it is on this branch: it is the prerequisite for scicode's own protocol migration. Putting Tests: 25 in |
Eighth commit: scicode migrates too — adoption is 40 of 40
One rollout per problem. The whole step sequence is the attempt.
That last row is the load-bearing one, and it is the same call ifeval's instruction-level accuracy already rests on: a per-problem rate cannot reconstruct a pooled one, so the raw counts have to survive to report time. Averaging the Steps-as-rollouts was the alternative and it loses on semantics, not on convenience: step i's prompt embeds the model's own code from steps 1..i−1, so the steps are neither independent nor attempts at the same thing. Recording them as rollouts would make Two details worth a second look: what
|
d805418 (RULER, #11) re-locked with a bare `pdm lock` instead of `pdm lock --update-reuse`, so a 4-package addition arrived with 71 already-locked packages moved, 16 of them across a major. None was requested: all five constraints that commit placed on already-locked packages (numpy, scipy, nltk, transformers, certifi) were already satisfied by the versions then locked, and `requires-python` did not change. The openai 2.9.0 -> 2.45.0 jump in that set is what made #62 CI-red while green locally. Reverted by restoring d805418^'s pdm.lock, inserting the three groups added since (ruler, ruler-gen, scicode) into `[metadata].groups`, and re-locking with `--update-reuse` -- the procedure in .claude/rules/deps.md rule 10. Result: all 71 drifted packages are back on their exact pre-RULER version and none landed on a third value. Notably datasets 5.0.0 -> 4.4.1, transformers 5.14.1 -> 4.57.3, huggingface-hub 1.23.0 -> 0.36.2, openai 2.45.0 -> 2.9.0, mypy 2.3.0 -> 1.19.1, and torch 2.13.0 -> 2.9.1, which swaps the CUDA 13 wheel tree back for CUDA 12 (16 nvidia-*-cu12 packages return, their cu13 counterparts go). The six packages RULER and SciCode genuinely introduced are kept: tiktoken, wonderwords, html2text, beautifulsoup4, soupsieve, h5py. Net 134 packages vs 140 on main and 128 before RULER. Verified against the reverted set, not the shared venv (which had been synced to the drifted lock, so it could not have caught this): `pdm install --check --frozen-lockfile` into a clean venv with CI's light group selection, then pytest tests/unit tests/integration tests/acceptance -m "not stress and not benchmark" -> 2826 passed, 1 deselected, the same count as the drifted lock produces. `pdm lock --check`, `ty check`, `ruff check` and the full preflight are green on the reverted set too. Land this BEFORE the lock-drift gate (feat/lock-drift-gate). The gate FAILs on any version move no requirement change asked for, and a deliberate revert is not distinguishable from accidental drift by that rule -- it reports these 71 as unjustified. Gate first would mean this PR cannot go green without pinning ceilings we do not want. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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>
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>
…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>
…otocol 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>
…n 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>
#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>
…minator
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>
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.
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>
…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>
…e 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>
…nters
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>
`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>
`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>
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>
`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>
ae5d81b to
f7b5f1a
Compare
Type
Summary
Finishes the migration #60 started: all 40 registered tasks now return
PromptRecord/PredictionRecord/JudgementRecord, so a consumer can read any sample's prompt, prediction, ground truth and verdict without knowing which task wrote the row. At 7/40 the protocol paid off nowhere — every consumer still branched per task for the other 33.scicodelast in its own commit.hle/browsecomp/aa_lcradoptextra.grader_output(the grader's fullModelOutput), sofinish_reasonsseparates a reasoning judge that burned its budget thinking from an empty API response — the case feat(tasks): persist the raw grader reply in LLM-judged tasks #51 flagged as most reachable on HLE.PROTOCOL_TASKSfork is gone.test_math_pass_at_k_family.pyno longer builds two feedback shapes by set membership.ResourceMetricsis retired from all four code-exec tasks — the coupling [Feature]: finish the stage-output protocol migration — 32 remaining tasks #61 named, where an evaluator gainingn_cases/n_passedforced a client-side TypedDict widening.profile.json(core-side; feat(core): uniform stage-output protocol, piloted on 7 tasks #60's accepted workaround, resolved — see below).rulergains a per-sample verdict it never had;ifbenchgrades infeedback()rather thanreport(), mirroring feat(core): uniform stage-output protocol, piloted on 7 tasks #60's ifeval relocation.scicodeis the one non-mechanical port. A sample is a chain of dependent sub-steps, each generating and executing code against the previous step's output — which is not whatrollouts[]means (independent attempts at the same thing). So it maps to one rollout per problem: the whole step sequence is the attempt, per-step detail lives inrollouts[0].extra.steps, andmetricscarries both axes with the headline derived from them. [Feature]: finish the stage-output protocol migration — 32 remaining tasks #61's inventory of 32 missed it because feat(scicode): add SciCode dataset + 0-shot generation task #42 landed just before feat(core): uniform stage-output protocol, piloted on 7 tasks #60.Related Issues
Closes #61. Refs #60, #51 (its documented
grader_replylimit closes here), #49.Test Plan
Automated
ruff check→ all passed;ruff format --check→ 368 files already formatted)ty check→ All checks passed)tests/unit+tests/integration+tests/acceptance(was 2827 onmain; +63). CI reports it as2834 passed, 55 skipped, 1 deselected; a local venv with more optional groups installed skips fewer.scripts/check_preflight.pyall-PASS;sync_meta_index.py --checkandsync_package_stubs.py --checkcleanManual — report parity
report.jsonmetrics are unchanged for every task, verified against the pre-migration implementations (loaded fromd622448cwith the task decorator stripped), not only against new tests. 8,700 randomized differential trials, 300 per task, 8,700 identical. Each family's fixtures deliberately exercise its edge paths:k ∈ {1,2,4},n ≥ k, rollouts< kso theshortwarning path runsarc_report)raw_sample=Noneskip; hellaswagacc+acc_normjudge_parsed=Falseso the unparsed-drop stays out of the calibration arraystimeoutssubstring counter in both directions, incl. the near-miss"timed out"empty; ruler across both metric families with partial matches; t_eval all 6 axesTaskRunner+ profiler, not just the helpers: a bare-record judge task's grader tokens land in thefeedbackstage's usage (20 in / 1 out per sample), the candidate's own spend stays attributed toinfer, and the recorded call names the judge model.Corrections to #61
hmmt_nov_2025/imo_answer_benchas pass@k tasks that "also use an LLM grader". They do not — both grade deterministically (math_verify/ the vendoredverify_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.scicode_0shot_gen(feat(scicode): add SciCode dataset + 0-shot generation task #42) landed just before feat(core): uniform stage-output protocol, piloted on 7 tasks #60 and is in no group — it is migrated here too, in its own commit (see Summary).Deliberate non-changes (so no denominator moves)
ctx.raw_sample, not the record'sextra.subject. The two agree whenever both exist, but a final whoseraw_sampleis absent has always been dropped from the macro-average; switching the source would silently change the denominator. The record carriessubjectregardless, so a row on disk is self-describing.ruler's report still calls the vendoredstring_match_*on the whole cell. The per-sample term recorded infeedbackis the exact decomposition, but re-deriving the cell mean in the task would fork upstream's scoring — a vendored metric that no longer runs is a reproduction that drifts silently.mmmlu_kshot_clpandscicode_0shot_genkeep theirTaskStageOutputbox oninfer.inferis not a record stage, so boxing it does not violate the bare-record rule. Both need the box for the same reason: one sample is several model calls, so the box carries the stage value whilemetareports every call's usage to the profiler.-1becomesNone(the protocol's spelling). Equivalent downstream: both mismatch the gold and both map to""throughchoice_text.The one deliberate denominator change (
90981721)scicode's two accuracies disagreed about what a pipeline failure means:main_problem_accuracycounted a failed problem as unsolved, whilesub_problem_accuracydropped its steps from the denominator entirely. One metric was 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. A failed context still carries itsraw_sample, so its tested steps are recoverable; they now stay in the denominator scoring zero, and a newunevaluated_stepsfield says how many oftotal_stepsnever ran.This is the one place
report.jsonmoves, and only whenfails > 0— a clean run is identical. It is a separate commit ahead of the migration so the parity trials above measure the migration alone.Notes on the
metricstiermetricscarries every multi-metric task, and in each the headline is derived from the mapping so the two cannot drift: hellaswag (acc/acc_norm), gsm8k-base (exact_match/flexible_exact_match), drop (em/f1), ifbench (strict/loose), scicode (main_problem_solved/sub_problem_pass_rate), t_eval (the co-equal axes its configuration scores).t_eval needed a judgement call:
correctis "every axis the evaluator scored came out at 1.0", deliberately notparse_rate. A task whosecorrectmeant "the output parsed" would look near-perfect next to every other task on the one axis that is supposed to be comparable across them. "The axes it scored" is load-bearing rather than decorative — see the review round below.Review round — fixes in the latest push
Two of these were live bugs in the migration; the rest are latent or type-level.
t_eval'scorrectwas structurally unreachable.feedbackrecorded every axis_evaluatereturns, including ones the configuration never scores — andcorrectis derived as "every recorded axis is 1.0", so under the defaulteval_thought=Falsea pre-seededthought: 0.0pinnedcorrecttoFalseon every sample of every run, emptying the one axis the protocol exists to make comparable across tasks.metricsnow carries only the scored axes, from a_metric_keys()shared with the macro-average so the recorded metrics, the derivedcorrectand the report cannot disagree.report.jsondoes not move: the same axes were already being macro-averaged, and the*_parsedvariants now read defensively so thestrmodes that score no args still report0.0, as before. Fixes a latentNameErroron the way through —metric_keyswas left unbound for anyprompt_typeoutside json/str, surfacing deep in the macro-average rather than at config time.ruler's report crashed on the resume path.report()indexedrollouts[0]["prediction"], butpostprocessnormalizes a blank response toNoneand serialization omitsNone-valued keys — so on the resume-report path (every sample complete,report.jsonmissing) the key is absent, not null, and an already-paid-for long-context run died at the last step withKeyError. Now.get(...) or "". The test helper round-trips both records throughobj_to_dict, so every report test that builds a final now asserts against the shapereport()actually has to read, not the more forgiving in-memory one.grader_outputas a literal instead ofrecords.GRADER_OUTPUT_KEY. The name is what the runner reads back to route grader spend intoprofile.json, so a divergence is silent — tokens on disk, missing from the profile.r["extra"]["msg"].lower()unguarded.msgis passed straight through from the evaluator response, so a null makes a fresh run raiseAttributeError— not only a resumed one. Now(r["extra"].get("msg") or "").Any.Anypasses silently in both directions — a caller may pass anything, and the body may do anything with it.objectis the accurate spelling, and switching to it surfaced thatis_judgement_record's narrowing never reached its caller: onlyAnymadeiter_grader_outputs'value.get("rollouts")type-check. Both sniffers now returnTypeGuard[Mapping]— deliberately notTypeIs, since a mapping that is not a record still returnsFalse, whichdetect_empty_postprocessrelies on when it testsisinstance(post_result, dict)after aFalse. Two of the three signatures predate this PR. Also replaces a non-discriminating assertion:iter_grader_outputs({"rollouts": "not a list"})read as covering the per-rolloutisinstanceguard, but withoutn_rolloutsthe value is not a judgement at all, so it returned at the first guard and deleting the one under test failed no test.t_evaltest could not construct its instance in CI.eval_thought=Truebuilds the embedding client in__init__withapi_key=os.getenv("SIEVAL_EMBED_API_KEY", ""), and an explicit""is notNone— it skips the OpenAI client's ownOPENAI_API_KEYfallback, soopenairaisesMissing credentialsat construction. The test now sets the env var: still a real instance, still covering theeval_thought=Truebranch, no embedding call. Reproduced rather than inferred from the traceback —api_key=""gives the CI error exactly, and it does so on bothopenai2.45.0 (locked when this branch was cut) and 2.9.0 (restored by chore(deps): revert the unrequested lock drift from d805418a #63, now onmain), so the fix is not version-specific. The whole suite was re-run against the 2.9.0 set before chore(deps): revert the unrequested lock drift from d805418a #63 landed, so this branch is verified on the dependency set it will actually merge into.Missing credentialsnamesOPENAI_API_KEY, which is useless here: the endpoint is our own embedding service (SIEVAL_EMBED_API), not OpenAI's. The failure also lands at construction, before any run output, so that message is all an operator gets.eval_thought=Truenow checks the key itself and raises aValueErrornamingSIEVAL_EMBED_API_KEY, pointing atSIEVAL_EMBED_APIfor a non-default endpoint, and saying the axis is optional (leaveeval_thoughtunset). BothNoneand""take that path —""is the reachable case, an exported-but-blank variable. No new exception class: sieval has no config-error base (ResultDirExistsError/ResumeVersionErrorboth subclass builtins), and tasks must not changecore/. The new test discriminates — pre-fix the constructor raisedOpenAIError, which is not aValueErrorsubclass, and its message containedOPENAI_API_KEY. Defaulteval_thought=Falseis untouched: it builds no client, and nothing in-tree passeseval_thought=True.gsm8k_0shot_gen,gsm8k_kshot_base_gen,hendrycks_math_kshot_base_genandtheoremqa_kshot_base_genhad bodies fully on the builders butTask[...]slots still readinglist[ChatCompletionUserMessageParam]/strfor the prompt,str/list[str]/ a per-taskPredictionTypedDict for the prediction, and a per-taskFeedbackTypedDict for the judgement. 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 exactly why the deletion sweep in the checklist below 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, andgsm8k_0shot_genno longer importsopenaiat all.Checklist
Required (all PRs)
type(scope): description)AI-Generated Code - <model> (<provider>)in module docstringcore/— the core change touches onlycore/tasks/records.py,core/utils/meta.py,core/runners/runner.pyARCFeedback,JudgeFeedback,GradeFeedback,ResourceMetrics,RulerFeedback,StepFeedback,ifbench._get_report, and the per-taskFeedback/Preprocessed/PredictionTypedDicts. Remaininggrader_replyhits are prose explaining what replaced it. The per-task TypedDicts held out longest — five of them survived in four files as arguments to staleTask[...]slots, so a reference grep could not see them; removed in67bd03bd(see the review round above), and the sweep now reads 0 for real.If: Breaking Change
On-disk schema, for the 33 newly-migrated tasks. Result dirs written by 0.7.x hold the old feedback shape and cannot be re-reported by the new
report(). Migration path: start fresh. This rides the 0.8.0 minor bump already staged for #60 and the infer-recipe split, so the--resumeversion gate rejects a 0.7.x → 0.8.x resume with a version message rather than a confusing shape error.report.jsonshapes are unchanged for every task except scicode's addedunevaluated_steps(above), so leaderboards, alignment cards andresolve_model_nameare unaffected.Module-level names removed from
sieval/tasks/*are importable, so their removal is user-visible even though nothing in-tree referenced them (verified above).Follow-ups (not in this PR)
Three of #61's adjacent items concern the code evaluator. None is blocked upstream —
code-evaluatoris vendored in this repo (vendor/code-evaluator/, formerly a submodule), so each is a local patch plus aVENDORED.mdentry, exactly how then_cases/n_passedpatch already in that copy was done. What actually gates two of them is a redeploy: tasks reach the evaluator over HTTP (SIEVAL_CODE_EVAL_API), so a new response field only appears once the image is rebuilt fromvendor/code-evaluator/docker/. The sieval side reads these with.get(), so an unpatched evaluator keeps working with the field absent.n_cases/n_passedpatch upstream and re-vendor. Pure upstream hygiene — it keeps the fork delta from growing. It gates nothing: the patch is in the vendored copy, and all five tasks that call the evaluator for test-case counts already read both fields (the four migrated here pluslivecodebench_code_generation_0shot_genfrom feat(core): uniform stage-output protocol, piloted on 7 tasks #60).n_cases/n_passed— a client-side classifier over free-textmsgdecays silently, which is why feat(core): uniform stage-output protocol, piloted on 7 tasks #60 removed one. Doable in the vendored copy; needs the redeploy.CODE_EVAL_FLOAT_TOLin the existing patch set.Not code-evaluator, and not deferred any more: the
eval_thoughtcredential error is fixed in this PR (see the review round).Still open, unrelated to the above: task-level service env vars are undocumented.
SIEVAL_EMBED_API_KEY,SIEVAL_EMBED_APIandSIEVAL_CODE_EVAL_APIappear only in code — the README and FAQ coverSIEVAL_DATA_DIRalone. Pre-existing and broader than this PR; worth its own small docs change.🤖 Generated with Claude Code