feat(core): persist task identity in run meta.json - #57
Conversation
3f24b7d to
51c15b3
Compare
aa701d3 to
3d57c3c
Compare
|
Self-review follow-up — three commits addressing five findings,
|
| shots rendered | meta.json recorded |
|
|---|---|---|
DROP, 1-row train, n_shot=3 |
1 | 3 |
OpenBookQA, 2-row train, n_shot=5 |
2 | 5 |
Exactly the disagreement n_shot_used exists to remove, one task family over. Reachable through supported config rather than only hand-construction: a dataset operations: slice takes a split, so - slice: {num: 2, split: train} shrinks the few-shot pool while args: {n_shot: 3} stands. Stock configs are safe — DROP's train is ~77k rows and OpenBookQA defaults to n_shot=0.
Both now raise in setup(), before any inference spend, matching gsm8k / hellaswag / arc. DROP additionally rejects a negative n_shot, which previously rendered 0 shots and recorded the negative value.
DROP's exemplar draw also moves from preprocess to setup(): it was re-shuffling the whole train split once per eval sample. The draw is seeded (42), so every per-sample call already returned the same set — prompts verified byte-identical against the pre-change rendering at n_shot 0/1/3/8, so no score moves. preprocess keeps a lazy fallback for callers that skip setup(), and both guards sit after the n_shot == 0 early return and before the lazy drop_eval import, so they cost no optional dependency.
aae59050 — fix(scripts): two coverage holes in check_task_shot_knobs
Both found by mutating the live repo against the new check, and both let through the regression it exists to catch.
- Rule 3 accepted a docstring as evidence.
ast.walkreaches docstringConstants, so a class documenting "pass@1" while computing nothing of the kind flipped a boguskparameter back to PASS — andkmatches no shot-count spelling, so rules 1 and 2 do not catch it either. Class and method docstrings are now excluded; real metric keys (f"pass@{k}"in a report dict) still count. - A decorated task inheriting its
__init__was skipped at both ends — the subclass declares no__init__, the base carries no decorator. A knob-bearing constructor then went unchecked with only a silently lower count as a symptom, and nothing asserts that count. This PR introduces both thearc/_base.pyshared-base layout and theCLAUDE.mdrule counting a shared module toward the ≥5-file threshold, so the pattern is now encouraged. Then_shotrules now bind every constructor undersieval/tasks/; thekrule stays decorated-classes-only, since an undecorated base'spass@kis normally computed by its subclass and judging it from the base's body would be a false positive.
The live repo still PASSes at the same 36 constructors. Two existing tests asserted the old scope — test_undecorated_class_ignored and test_inherited_init_ignored, whose stated expectation ("a subclass with no __init__ of its own has no knob to check") is hole 2 — so they are rewritten to the new contract rather than adjusted around it.
13bb0159 — docs(core,tasks): bound two claims the code makes about its own reach
No behaviour change; both remove a reading a later change would lean on.
gate_resume_identityjustified comparingnameonly with "a wider comparison would refuse a task merely redefined between runs". Sound for the declaration fields, butn_shotescapes that argument — it is the run's value, and the reason the block records it at all. The docstring now says why it is still excluded: a same-task resume under a differentn_shotis a changed invocation, which the CLI's strict--resumeconfig match already refuses by comparing the persisted YAML body,tasks.*.argsincluded, before a runner is built. What is left for this gate is the mismatch that match cannot see at all.- MMLU/CMMLU
setup()claimed a short pool aborts "before any inference spend". True only for subjects the few-shot split contains — one present intestbut absent fromdevhas no prefix to build there, so it still surfaces per sample. The residual is noted in the PR body; the comment is where a reader looks.
Verification
- 2879 → 2896 tests (DROP +7, OpenBookQA +4, preflight +6). Every new test mutation-checked: reverting the corresponding fix fails it, confirmed for all four fixes.
scripts/check_preflight.py20/20 PASS, includingcheck_task_shot_knobsandcheck_meta_index_sync.ruff check+ruff format --check,ty check,scripts/sanitize.sh,scripts/check_layer_imports.py,sync_meta_index.py --check,sync_package_stubs.py --checkall clean.- CI green on 3.12 and 3.13.
Body still needs one Breaking Change line
Not yet covered above the fold; suggested entry:
DROP / OpenBookQA few-shot pool — breaking. Both now abort at
setup()when the few-shot split holds fewer thann_shotexemplars, instead of silently rendering fewer shots thann_shot_usedrecords. Only reachable when the pool is shrunk below the requested count — e.g. a datasetsliceoperation on the few-shot split — so no stock configuration hits it. DROP also now rejects a negativen_shot, which previously rendered 0 shots and recorded the negative value. Recovery: ask for a count the split can supply.
fa6b6e8 to
790b2da
Compare
A finished run directory recorded which sieval version produced it, but not which task produced it or under what evaluation protocol it was scored. `@sieval_task` already builds a full `TaskMeta`, but it only ever reached the in-process registry and the static `sieval/meta/index.json` catalog — nothing survived into the run. The directory name is not a substitute: it comes from `Task.name`, a user-chosen YAML key with no guarantee of matching the registered `@sieval_task(name=...)`. Consumers were left recovering question type and scoring mode by parsing naming conventions, which fails silently. This matters most for plugin-provided tasks, which never appear in `index.json` at all, so persistence is their only channel. `meta.json` now carries an optional `task` block with `name`, `display_name`, `dataset`, `eval_mode`, `n_shot`, `tags`, and `status`. Settling the open questions the issue raised: - No backfill. `write_run_meta` stays absolutely create-if-absent, so `task` is `NotRequired` and absent means "pre-feature run or undecorated class". On a resume the block would describe the resuming process, and the resume gate matches on version alone — never on task identity — so a mismatched resume would stamp a wrong identity onto samples another task produced. Absent is honest; wrong is worse. - No shared finalize-stage envelope. `meta.json` is the run-start handshake, not a finalize-stage file, so a `generated_at` envelope would be wrong for it; the block is purely additive. - Identity is not inherited. `get_task_run_identity` reads `cls.__dict__`, not the MRO, so an undecorated subclass of a decorated task gets no block rather than silently claiming its parent's name. `cls.tags` / `cls.model_type` stay inherited — those are behavioral defaults, not identity — and `get_task_meta` is unchanged. The persisted fields are a chosen subset, not `task_meta_to_dict`: `model_type` is on its way out, `reference_impl.notes` is not frozen within `schema_version=1`, and `description` / `deps_group` describe the task rather than the run. The rationale lives at the projection site so nobody "completes" it later. Closes #49 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`meta.json` took `n_shot` straight off `@sieval_task`, so a task built
with a shot-count override persisted the class's advertisement rather
than what the run rendered: `cmmlu_kshot_clp` under `args: {k: 3}`
recorded `n_shot: 5`. A run directory is read to find out what the run
did, and the file is written once and never rewritten, so a declared
default there is not a smaller answer — it is a wrong one that cannot be
corrected after the fact.
`Task.n_shot_used` is a field every task carries. `None` — the default —
means the declared value stands, so a task with no shot-count knob is
correct with no code of its own; the 15 that have one assign it in
`__init__` from the value they actually store, one line each, after any
normalisation the constructor applies. `get_task_run_identity` now takes
the instance and reads `n_shot` off it. Every other field stays a claim
about the class, which no instance may restate.
The default is `None` rather than `0` on purpose. `0` would look tidier
— every task carrying a real number — but it turns a forgotten
assignment from a still-correct value into a wrong one, and
`hendrycks_math_kshot_base_gen` is exactly that case today: it declares
`n_shot=4` and takes no knob, so it must keep reporting 4 without
touching it.
The value is assigned per task rather than inferred, because the obvious
convention is actively wrong: `self._k` is a few-shot count in the
`*_kshot_*` tasks but the `pass@k` rollout count in several `*_0shot_*`
ones, so reading it by name would record rollouts as shots on exactly
the tasks that declare `n_shot=0`. Verified in both directions — all 15
knob-bearing tasks report their constructor value, all 11 pass@k tasks
still project their declaration, and no task sets the field at class
level.
Passing a class now raises: it would resolve to no metadata, which the
fail-soft `None` path reports as "undecorated" and would have silently
dropped the whole identity block.
Also renames the `TaskSaver` constructor argument `task_meta` to
`task_identity`, matching the name the projection has carried since it
was introduced.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A result directory is matched by path alone, and `Task.name` is a user-chosen YAML key, so two task classes can share one. Under `auto_resume` a valid `report.json` short-circuits `arun()` before any stage runs, so the second task got the first one's report handed back as its own result, having evaluated nothing, while `meta.json` went on naming the first. Nothing caught it: the version gate passes because both runs are the same version, and the session-level YAML strict-match is skipped entirely when no `result_dir` is configured — which is the default `outputs/<task.name>/<timestamp>` path. `gate_resume_identity` sits beside `gate_resume_version` in `__init__`, so it covers that short-circuit. A check further down, where `meta.json` is written, would never run on the very path that costs you a result. Adjudicating a resume is also not the saver's job. It compares the registered `name` only: that is the registry key the rest of the block derives from, and a wider comparison would refuse a task merely redefined between runs, which is the version gate's business. Recovery is the documented pair, with no escape hatch — start fresh, or give the task its own result_dir. Deliberately weaker than the version gate, which fail-closes on a missing version: an absent `task` block is indistinguishable from a pre-feature run or an undecorated producer, so it has to pass. Identity only bites once both sides carry a block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`k` meant two different things across the task tree: a few-shot exemplar
count in the 13 `*_kshot_*` tasks, and the pass@k rollout count in 13
others. That was survivable while nothing read it, but `meta.json`'s new
`task.n_shot` block reads the first while YAML users write both — and a
reader cannot tell from the name which one a given task means.
Rename the few-shot knob to `n_shot` in the 12 `*_kshot_*` tasks plus the
shared `_arc` base, matching `@sieval_task(n_shot=...)`, `Task.n_shot_used`
and the recorded `task.n_shot`. The pass@k tasks keep `k`: it is the right
name there, and leaving it is what makes the name unambiguous again.
BREAKING CHANGE: task kwargs are reachable from YAML `args:`, so any config
passing `args: {k: N}` to one of those 13 tasks must now pass
`args: {n_shot: N}`; the constructor raises TypeError on the old spelling
rather than silently ignoring it. In-tree configs are updated.
`examples/leaderboard-math-sft.yaml`'s `k: 1` is a pass@k rollout count and
is deliberately unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…json `meta.json` records the shot count a run actually used by reading `Task.n_shot_used`. A task that takes a shot-count argument and never assigns that attribute persists the declared `@sieval_task(n_shot=...)` default instead, and nothing at runtime can tell the two apart — the run directory simply reports a number the run never used. A convention that only holds because 15 authors remembered it is not a contract, so check it. `check_task_shot_knobs` parses each task module (AST only, so tasks whose optional deps are absent are still covered) and enforces three rules on every `@sieval_task` class defining its own `__init__`: 1. a shot-count parameter is spelled `n_shot`, nothing else. Anchored, so `fewshot_split` / `fewshot_seed` / `fewshot_as_multiturn` — which name a different noun — do not match; 2. a task accepting `n_shot` assigns `self.n_shot_used`. The source expression is unconstrained: `len(self._examples)` is a legitimate way to report a count the constructor derived; 3. `k` is a pass@k rollout count, so a task accepting `k` must compute a pass@k metric, and `n_shot_used` may never be fed from it. This is rule 1 for the one name that denotes a shot count without containing "shot", and it is what stops `k` from re-acquiring a second meaning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`check_task_shot_knobs` enforced a convention no document stated, which is the dead-rule half of the policy/enforcer chain: a task author hits the check without anywhere to read what it wants, and the rule's reasoning lives only in the enforcer's docstring. State it where the other task-authoring conventions live, including why `n_shot_used` is load-bearing (skip it and meta.json records the declared default) and which lookalikes are deliberately out of scope. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ARC ships four task variants over one shared scoring module, which is the
point at which `sieval/tasks/CLAUDE.md`'s subdirectory rule now bites: the
count includes the extracted shared module, so 4 tasks + `_base.py` is 5.
Reword the rule to say so, and make `arc/` the reference layout. A benchmark
only grows a shared module once its variants have logic worth reusing, so the
shared module is the signal the group has become a unit -- and it is the thing
a flat layout has nowhere good to put.
Filenames keep the full task name inside the subpackage
(`arc/arc_easy_kshot_ppl.py`), so a grep for a registered task name still
finds its file. `sieval/tasks/_arc.py` becomes `arc/_base.py`, per the
existing private-shared-module convention.
`get_task_class()` had to learn this layout, not just tolerate it: it resolved
a name by importing `sieval.tasks.{name}` and nothing else, so all four ARC
tasks would have raised `KeyError` from `sieval task show` and from any by-name
task resolution -- the module imports fine, the lookup just cannot find it.
It now falls back to scanning `sieval.tasks` subpackages for `{subpkg}.{name}`,
one directory listing paid only on a cache miss for a subpackage-hosted task.
Flat tasks still resolve in a single import.
`tests/unit/core/tasks/test_meta.py` pinned the flat layout as a hard
invariant, and its docstring named this exact blocker in advance. Its
assertion now checks the eponymous-*filename* convention without constraining
depth, which is what `get_task_class()` actually resolves; a task whose
defining file is named something other than the task stays banned, since no
amount of walking can find it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…plars Six tasks slice a per-subject exemplar pool to `n_shot`. Four (C-Eval, MMMLU, TheoremQA, MBPP) raise when the pool is shorter than asked for; MMLU and CMMLU silently capped. With `n_shot=8` against a 5-row subject they render 5 shots, and against a subject absent from `dev` they render 0 -- while `meta.json`'s `n_shot_used` records 8, with nothing on disk saying otherwise. That is the same class of drift this PR's `n_shot_used` field exists to remove: a recorded count that the prompts do not back. Guard `_select_examples`, and pre-build every per-subject prefix in `setup()` so the failure aborts the run before any inference spend rather than failing thousands of samples one at a time (`sieval/core/CLAUDE.md`: failed samples are per-sample and retryable). This makes both tasks behaviourally identical to their C-Eval sibling, whose `setup()` already does exactly this. Upstream-safe: both `dev` splits are a uniform 5 rows per subject, so the guard can only fire for `n_shot > 5`, already a non-upstream configuration. Sweeping the whole dev pool rather than only the evaluated subjects is deliberate, and inherited from C-Eval: an under-populated subject the run never reaches still aborts. Strictness is the house default, and it is unreachable at the shipped 5/subject anyway. The CMMLU test fixture gained a second `logical` row for this reason; the one-row version lives on as the guard's own fixture. A subject present in the eval split but absent from `dev` entirely is invisible to that sweep, so it still surfaces per sample -- pinned by its own test rather than left as an undocumented gap. BREAKING CHANGE: `mmlu_kshot_clp` and `cmmlu_kshot_clp` now abort at setup when any subject in the few-shot split holds fewer than `n_shot` exemplars, instead of quietly rendering fewer shots. Only reachable with `n_shot > 5`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The layout rule credited only the top-level `__init__.py`'s lazy export for making a nested benchmark work. That is the assumption this PR's ARC move falsified: export alone got four importable tasks that `get_task_class()` could not resolve, because the lookup tried the flat path and stopped. Anyone reading the rule would have concluded, as the rule said, that nesting was already handled end to end. Name both halves, mark them load-bearing, and record the symptom of losing either one, so the next reader can tell an incomplete mechanism from a working one without rediscovering it through a `KeyError`. Split the empty-`__init__.py` rationale into its own bullet while here. It justified itself with "the registry resolves it through the top-level package", which is not how a nested task resolves at all -- `get_task_class()` imports the nested module directly. The actual reason is that both mechanisms key on the module, not on a subpackage attribute: importing it runs `@sieval_task`, which registers the class. Same conclusion, a premise that survives nesting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comments and docstrings accumulated restatement as the branch grew: the same fact argued twice, closing sentences that re-derived a rule already stated, and rhetorical tails that read as persuasion rather than reference. Cut those and keep every load-bearing claim. Trimmed: `get_task_run_identity` (37 -> 25 lines), `Task.n_shot_used` (19 -> 11), `gate_resume_identity`, `get_task_class`, `TaskRunIdentity`, the `meta.py` module note, `check_task_shot_knobs`, both MMLU/CMMLU `setup()` comments, three `sieval/tasks/CLAUDE.md` bullets, and the short-pool test comments. Nothing about behaviour, rationale, or the contracts these describe changes -- this is the same content at a lower word count. 2879 tests still pass, ruff and ty clean, full preflight PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same drift the MMLU/CMMLU commit removes, reached by a different mechanism
and so missed by that sweep: it covered the six per-subject-pool tasks,
while these two draw through `Dataset.retrieve_samples`, which truncates to
the split length (`k = min(k, len(ds))`) and returns `[]` for a missing
split. DROP at `n_shot=3` against a 1-row `train` rendered 1 shot and
recorded 3; OpenBookQA at `n_shot=5` against 2 rows rendered 2 and recorded
5 — exactly the disagreement `n_shot_used` exists to remove.
Reachable through supported config, not only hand-construction: dataset
`operations:` `slice` takes a `split`, so `- slice: {num: 2, split: train}`
shrinks the pool while `args: {n_shot: 3}` stands. Defaults are safe.
Both now raise in `setup()`, before any inference spend, like
gsm8k/hellaswag/arc. DROP also rejects a negative `n_shot`, which used to
render 0 shots and record the negative value.
DROP's draw moves from `preprocess` to `setup()`: it was re-shuffling the
whole train split once per eval sample. The draw is seeded, so every call
already returned the same set — prompts verified byte-identical against the
pre-change rendering at n_shot 0/1/3/8, so no score moves. `preprocess`
keeps a lazy fallback for callers that skip `setup()`.
Guards land after the `n_shot == 0` early return and before the lazy
`drop_eval` import, so they cost no optional dependency.
AI-Generated Code - Claude Opus 4.6 (Anthropic)
Both found by mutating the live repo against the new check, and both let
through the regression the check exists to catch.
1. Rule 3 accepted a *docstring* mention of `pass@` as evidence of a pass@k
metric, because `ast.walk` reaches docstring Constants. A class
documenting "pass@1" while computing nothing of the kind flipped a bogus
`k` parameter back to PASS — and `k` matches no shot-count spelling, so
rules 1 and 2 do not catch it either. Class and method docstrings are now
excluded; real metric keys (`f"pass@{k}"` in a report dict) still count.
2. The scan keyed on `@sieval_task`, so a decorated task inheriting its
`__init__` from an undecorated base was skipped at both ends: the
subclass declares no `__init__`, the base carries no decorator. A
knob-bearing constructor then went unchecked with only a silently lower
count as a symptom — and nothing asserts that count. This PR introduces
both the `arc/_base.py` shared-base layout and the CLAUDE.md rule that
counts a shared module toward the >=5-file threshold, so the pattern is
now encouraged. The `n_shot` rules bind every constructor under
`sieval/tasks/`; the `k` rule stays decorated-only, since an undecorated
base's pass@k is normally computed by its subclass and judging it from
the base's body would be a false positive.
Live repo still PASSes at the same 36 constructors. Two existing tests
asserted the old scope — `test_undecorated_class_ignored` and
`test_inherited_init_ignored`, whose stated expectation ("a subclass with no
`__init__` of its own has no knob to check") is the hole — so they are
rewritten to the new contract rather than adjusted around.
AI-Generated Code - Claude Opus 4.6 (Anthropic)
Neither changes behaviour; both remove a reading a later change would rely on. `gate_resume_identity` justified comparing `name` only by "a wider comparison would refuse a task merely redefined between runs" — sound for the declaration fields, but `n_shot` escapes it, being the run's value and the reason the block records it at all. Say why it is still excluded: a same-task resume under a different `n_shot` is a changed invocation, which the CLI's strict `--resume` config match already refuses by comparing the persisted YAML body, `tasks.*.args` included, before a runner is built. What is left for this gate is the mismatch that match cannot see at all. MMLU/CMMLU `setup()` claimed a short pool "aborts before any inference spend". True only for subjects the few-shot split contains — one present in `test` but absent from `dev` has no prefix to build, so it still surfaces per sample. The PR body notes the residual; the comment is where a reader looks. AI-Generated Code - Claude Opus 4.6 (Anthropic)
The new shot-knob rule got `k`'s own semantics wrong in the five places it
states them, and one of the two is a user-facing error message. `k` is the
metric's parameter: `pass@k` is estimated from `n` samples per problem, with
`k <= n` enforced in the constructor. The sampling budget is `n` — it is what
reaches `agenerate(n=self._n)`; `self._k` appears only in `_pass_at_k(...)`
and the `f"pass@{self._k}"` metric key. Verified across mbpp, human_eval and
livecodebench_code_generation, which agree.
Calling `k` a "rollout count" therefore names `n`'s job. Worth fixing rather
than leaving, because this PR exists to remove a knob that meant two things,
and `sieval/tasks/CLAUDE.md` is where task authors go to learn which is which
— a wrong definition there reintroduces the confusion one level up.
Five sites: the CLAUDE.md rule, `check_task_shot_knobs`' docstring and both
of its violation messages, and the `Task.n_shot_used` docstring, whose
`*_0shot_*` example was describing the same conflation. No behaviour change;
the one test asserting the old message text is updated with it.
AI-Generated Code - Claude Opus 4.6 (Anthropic)
`n_shot_used` mirrored `self._n_shot` into a second attribute in all 15
knob-bearing tasks — 15 byte-identical lines whose only job was to move a
value the task had already stored. It also needed a `None`-means-declared
sentinel, a paragraph explaining why the default could not be `0`, and a
fallback in `get_task_run_identity`.
`@sieval_task` now seeds `cls.n_shot` beside `cls.tags` / `cls.model_type`,
and a task with a knob assigns `self.n_shot`, shadowing it for that instance.
Ordinary attribute lookup does what the sentinel and the fallback did:
"n_shot": meta.n_shot if n_shot_used is None else n_shot_used
-> "n_shot": task.n_shot
A knobless task now needs no code at all to be correct, which is what the
`None` default was protecting by hand. `"n_shot" in task.__dict__` still
distinguishes an override from a declaration, so nothing is lost.
Plain class attribute, NOT a ClassVar like its two neighbours: those are
never set per instance, and `ClassVar` makes the shadowing assignment a type
error ("Cannot assign to ClassVar `n_shot` from an instance"). The decorator
also assigns after the class body, so a subclass cannot advertise a count
different from its declaration — pinned by a test.
The docstring's reason for keeping the two apart was that `self._k` held a
shot count in some tasks and pass@k's `k` in others, making any name-based
rule unsafe. This PR's own rename removed that collision, so the reason no
longer held.
check_task_shot_knobs rule 2 is retained, retargeted to `self.n_shot`: a task
storing its knob privately still works, and only `meta.json` goes silently
wrong, which is the failure the rule exists for. Verified by mutation, and on
the real registry: all 40 tasks still project their catalog row, the four
knob-bearing ones honour a constructor override while leaving the class
declaration intact, and the pass@k tasks leak no `k`.
AI-Generated Code - Claude Opus 4.6 (Anthropic)
790b2da to
53a96a7
Compare
Type
Summary
A finished run recorded which sieval version produced it, but not which task or under what evaluation protocol it was scored.
@sieval_taskalready builds a fullTaskMeta; it just never left the process — reaching only the in-process registry and the staticsieval/meta/index.jsoncatalog. Consumers recovered question type and scoring mode by parsing directory names, which fails silently, and that name comes fromTask.name— a user-chosen YAML key with no guarantee of matching the registered one.meta.jsonnow carries an optionaltaskblock, read off the instance viaget_task_run_identity(task)rather than looked up by name, so it works identically for plugin tasks — which never appear inindex.jsonat all, making persistence their only channel. Additive on disk; absent means pre-feature run or undecorated class, never backfilled.n_shotis the run's, not the class's. Every other field is the task's declaration and matches its catalog row.@sieval_taskseeds the declared count ontoTask.n_shot, besidecls.tags/cls.model_type; the 15 knob-bearing tasks assignself.n_shotin__init__, shadowing it for that instance, soargs: {n_shot: 3}stops recording5. A task with no shot knob needs no code of its own to be correct, and"n_shot" in task.__dict__still separates an override from a declaration after the fact.n_shoteverywhere now, and a preflight check keeps it that way. Recording it exposed thatkmeant two unrelated things in the task tree — a few-shot count in 13 tasks, thekofpass@kin 13 others (the metric's own parameter, not the sampling budget; that isn, andk <= n) — the same silent convention this PR exists to remove, one layer down. Newcheck_task_shot_knobs(AST-only). Breaking for YAMLargs:.auto_resumeshort-circuits on a validreport.jsonbefore any stage runs, so a task pointed at another task's directory was handed that report as its own result, having evaluated nothing. Neither existing guard caught it. Breaking.n_shot=8, 0 for a subject absent fromdev, while the recorded count said 8. DROP/OpenBookQA reached the same drift by a different mechanism, and so were missed by the sweep that found the first two: they draw throughDataset.retrieve_samples, which truncates to the split length (k = min(k, len(ds))) and returns[]for a missing split, so DROP atn_shot=3against a 1-rowtrainrendered 1 shot and recorded 3. All four now abort insetup(), before any inference spend, like gsm8k/hellaswag/arc. Breaking.sieval/tasks/arc/, which turned out to be load-bearing: it exposed thatget_task_class()could not resolve a subpackage-hosted task at all, so all four ARC tasks would have raisedKeyErrorfromsieval task show. Now falls back to scanning subpackages.{ "version": "0.7.0", "deterministic": true, "task": { "name": "cmmlu_kshot_clp", "display_name": "CMMLU (few-shot, base CLP)", "dataset": "cmmlu", "eval_mode": "clp", "n_shot": 5, "tags": ["chinese", "multiple-choice", "base-model"], "status": "stable" } }Decisions, with the rationale at the code
Each of these is argued where it binds — the docstring or rule that a later change would have to edit — rather than only here, where nobody re-reads it:
meta.json(TaskRunMeta) — it would stamp the resuming process's identity onto samples another task produced. Absent is honest; wrong is worse.get_task_run_identity) — readscls.__dict__, not the MRO likeget_task_meta. An undecorated subclass is a different task. No task-to-task subclassing exists in-tree, so this only bounds plugins.get_task_run_identity) —model_type,reference_impl,description,deps_group. The block is a chosen subset, nottask_meta_to_dict().tagsis persisted but advisory: the one field whose values aren't frozen withinschema_version=1.Task.n_shot) — before the renameself._kheld a few-shot count in*_kshot_*tasks andpass@k'skin several*_0shot_*ones, so any name-based rule would have recorded that metric parameter as shots on exactly the tasks declaringn_shot=0. Task filenames are no better a proxy:openbookqa_kshot_gendeclaresn_shot=0.n_shotis a plain class attribute, not aClassVar(Task.n_shot) —tagsandmodel_typeareClassVarbecause nothing ever assigns them per instance.n_shotis the one field a run changes, andClassVarmakes the shadowing assignment a type error underty("Cannot assign to ClassVarn_shotfrom an instance"). Not a property either: there is no second source to derive it from — the decorator assigns after the class body, so a class-bodyn_shotcannot advertise a count different from its declaration. Pinned by a test.nameonly (gate_resume_identity) — a wider comparison would refuse a task merely redefined between runs, which is the version gate's job.n_shotis excluded for a different reason, now written down at the gate: a same-task resume under a differentn_shotis a changed invocation, which the CLI's strict--resumeconfig match already refuses by comparing the persisted YAML body,tasks.*.argsincluded, before a runner is built. What is left for this gate is the mismatch that match cannot see at all. It passes when either side has no block, so it can never reject a pre-feature resume.@sieval_task__init__by AST:k(26),n(17),fewshot_split(10), then singletons.nis the sampling count in all 17, matching the OpenAI field.kwas the only collision, split exactly 13/13.Related Issues
Closes #49
Refs #25 — the task-side capability declaration will extend
@sieval_task, touching the same metadata surface. This PR deliberately does not persistmodel_typefor that reason.Test Plan
Automated
ruff check && ruff format --check)ty checkonsieval/ scripts/ tests/;mypydiffed before/after on touched files — no new errors)pytest tests/unit tests/integration tests/acceptance→ 2959 passed, re-run after rebasing ontomain@66d00482(on top of feat(tasks): finish the stage-output protocol migration (33 tasks, 40/40) #62 and chore(deps): revert the unrequested lock drift from d805418a #63). All 15 commit subjects preserved; this branch touches neitherpdm.locknorpyproject.toml.feat(tasks): finish the stage-output protocol migration (33 tasks, 40/40) #62 migrated DROP and OpenBookQA to the stage-output protocol, which overlaps this branch two ways. Two commits conflicted (
openbookqa_kshot_gen.py, and the HellaSwag/TheoremQA tests) — resolved as orthogonal overlays:main's record protocol plus this branch'sk→n_shotrename, neither reverted. Four further sites were conflict-free but semantically broken:preprocessnow returns aPromptRecord, so this branch's new pool-guard tests had to readpre["prompt"][0]["content"], notpre[0]["content"](KeyError: 0— caught by running the full suite, not by the rebase). Those four are folded into the commit that introduced the tests rather than appended as a fixup, so every commit from that point toHEADstill passes its own suite (verified per commit).Stated plainly because it bounds the claim above: the local run used the pre-chore(deps): revert the unrequested lock drift from d805418a #63 dependency set, since chore(deps): revert the unrequested lock drift from d805418a #63 reverts 71 packages, 16 across a major (
datasets5.0.0 → 4.4.1,transformers5.14.1 → 4.57.3,openai2.45.0 → 2.9.0,mypy2.3.0 → 1.19.1,torch2.13.0 → 2.9.1). Themypydiff above also predates that revert. CI's clean--frozen-lockfileinstall on 3.12 and 3.13 is the authority for the reverted set; a shared venv synced to the drifted lock structurally cannot check it, which is the trap chore(deps): revert the unrequested lock drift from d805418a #63 itself documents.scripts/check_preflight.py20/20 PASS, including the newcheck_task_shot_knobs("all 36 task constructor(s) spell and wire the shot knob correctly") andcheck_meta_index_sync—index.jsonis touched: the rename edits sixreference_impl.notesstrings embedded in the catalog, regenerated viascripts/sync_meta_index.pyin the same commit. NoTaskMetafield changes.scripts/sync_package_stubs.py --check,check_layer_imports.py, andscripts/sanitize.shclean.New coverage — the regression-critical files first:
tests/unit/core/runners/test_runner.pyarun()persists registry identity whileTask.namediffers; a run rendering 7 shots against a class declaring 2 persists 7.TestResumeIdentityGate: refuses a directory another task produced (verified to fail with the gate call removed), allows same-task / pre-feature / undecorated / malformed blockstests/unit/core/tasks/test_meta.pytags,Nonefor an undecorated subclass withget_task_metastill MRO-reading,n_shotfollowing the instance, class-argument raising. Plus: a class-bodyn_shotcannot shadow the declaration (the decorator assigns last), and the subpackage fallback with the flat path asserted to genuinely miss first, so it can't pass vacuouslytests/unit/scripts/test_check_preflight.pyTestCheckTaskShotKnobs— one per rule and exemption: every misspelling flagged,fewshot_*not flagged,n_shot = len(...)allowed, the knob fed fromk(the regression the check exists for), subpackage tasks scanned, plus a live-repo assertion. Both closed holes are pinned: apass@mention in a class or method docstring no longer buys an exemption, and a decorated task inheriting__init__from an undecorated base is now checked at the basetests/unit/tasks/test_drop_kshot_gen.py,test_openbookqa_kshot_gen.pyn_shot, split missing entirely, negativen_shot) fire atsetup(), not mid-run; DROP's lazydrop_evalimport discipline preserved — the guards land before it, so they cost no optional dependency; and prompts byte-identical with and withoutsetup(), which is what makes thepreprocess→setup()hoist score-neutraltests/unit/core/tasks/test_saver.py,test_context.pytaskgenuinely optionaltests/unit/tasks/test_mmlu_kshot_clp.py,test_cmmlu_kshot_clp.pysetup()(not mid-run), and the residualsetup()structurally cannot see — a subject absent fromdeventirely, which still surfaces per sampleEvery fix in the review round was mutation-checked: revert the fix, the new test fails.
Manual
index.jsonrow field-for-field. Both directions of the shot hook — all 15 knob-bearing tasks report the constructor value while the class declaration stays intact, all 11pass@k-only tasks project their declaration with nokleaking in as shots. Re-run unchanged aftern_shot_usedcollapsed intoTask.n_shot, which is what establishes that refactor as behaviour-preserving.kin tracked YAML and docs before renaming, since task kwargs are user-facing. Exactly two were shot counts (leaderboards/sft_fast_202511.yaml,docs/guide/configuration.md) and are updated; the four inexamples/leaderboard-math-sft.yamlarepass@kand deliberately left. Prose kept wherekisn't this knob (top-k,k-shot, upstreamdev_df[:k]).cmmlu_kshot_clpthroughget_task_run_identity+write_run_metainto a temp dir: block matches its catalog row, and a second write leaves the file byte-identical.n_shot=8against the live MMLU/CMMLU classes. The follow-up sweep of the six per-subject-pool tasks confirmed those two were the only ones of that mechanism not already fail-closed — it did not coverretrieve_samples-drawn pools, which is how DROP/OpenBookQA were missed and then found separately. Both reproduced live (DROPn_shot=3on a 1-rowtrain→ 1 shot, recorded 3; OpenBookQAn_shot=5on 2 rows → 2, recorded 5) before fixing.n_shot0/1/3/8 before and after moving the few-shot draw out ofpreprocess: byte-identical, so no score moves. The draw was seeded, so it had been re-shuffling the whole train split once per eval sample to return the same set.sieval.tasks.arc.*from a cold cache, flat tasks still resolve in one import,sieval task show arc_easy_kshot_pplrenders.check_task_shot_knobsheld at 36 tasks across the move, which is what proves its recursive scan works on real data.Checklist
Required (all PRs)
type(scope): description)scripts/sanitize.shpasses)AI-Generated Code - <model> (<provider>)in module docstring — the only new files are the two emptyarc/__init__.pymarkers, which carry no docstring by convention and no code to attribute; every ARC module keeps its attribution through the movecore/— new imports arecore.runners→core.tasks.meta/core.tasks.context, alongside thecore.tasks.*importsrunner.pyalready had. The subpackage scan adds no static edge: it goes throughpkgutil.iter_moduleson an already-dynamically-importedsieval.tasks, the same call-time indirectionimport_all_tasks()usesTask.n_shot_usedand the preflight helper that read it, both introduced earlier in this same branch. Swept afterwards:n_shot_usedappears nowhere in tracked filesIf: Breaking Change
Four parts; the first is additive, the other three break.
On-disk schema — additive.
taskisNotRequired; readers ignoring unknown keys are unaffected and the version handshake is untouched. Pre-existing runs never gain the block, so consumers must treat it as optional indefinitely — the documented contract, not an oversight.Resume behaviour — breaking. Resuming into a directory produced by a different registered task raises
ResumeIdentityError. What it replaces was not a working workflow: that resume returned the first task's report as the second's result with nothing evaluated. Recovery is the documented pair, no escape hatch — remove theresult_dirand start fresh, or give the task its own. Fires only when both sides carry a block, so it can never reject a pre-feature resume.Constructor + YAML
args:— breaking.k→n_shoton 13 tasks:arc_challenge/arc_easy(×clp,ppl, viaarc/_base.py),c_eval,cmmlu,drop,gsm8k-base,hellaswag,mmlu,mmmlu,openbookqa,theoremqa-base.args: {k: N}becomesargs: {n_shot: N}; the constructor raisesTypeErroron the old spelling, so this breaks loudly at startup, never as a wrong score. The 13pass@ktasks keepk. In-tree configs updated in the same commit.Few-shot pool — breaking, four tasks. All four now refuse a pool that cannot supply the count they will record, at
setup(), before any inference spend. Recovery in every case: ask for a count the split can supply.n_shotexemplars. Only reachable withn_shot > 5, since both shippeddevsplits are a uniform 5 rows/subject — no upstream-faithful configuration hits it. Sweeping the wholedevpool rather than only the evaluated subjects is deliberate and inherited fromc_eval— stricter than strictly necessary, and unreachable at 5/subject regardless. Residual, documented at the code: a subject present intestbut absent fromdevhas no prefix to build insetup(), so that case still surfaces per sample.n_shot, or missing entirely; DROP additionally rejects a negativen_shot, which used to render 0 shots and record the negative value. Reachable through supported config, not only hand-construction: a datasetoperations:slicetakes asplit, so- slice: {num: 2, split: train}shrinks the pool whileargs: {n_shot: 3}stands. Defaults are safe. DROP's draw also moves frompreprocesstosetup()— prompts verified byte-identical,preprocesskeeps a lazy fallback for callers that skipsetup().Minor API break for direct callers:
get_task_run_identitytakes aTaskinstance, not a class (raisingTypeErrorrather than silently reporting "undecorated").Task.n_shot, no released break. The field started this branch asTask.n_shot_usedand was collapsed into a publicTask.n_shotbefore merge, so no release ever carried the old name. Stated for anyone building on the branch, because the failure mode is silent: a task still assigningself.n_shot_usednow writes an attribute nobody reads, andmeta.jsonrecords the declared default as though the run had used it. Assignself.n_shotinstead.check_task_shot_knobscatches this for in-tree tasks; it scanssieval/tasks/only, so an out-of-tree plugin gets no warning.