From ec03e2b1ae269923f0d6737ccf47b8a45a049294 Mon Sep 17 00:00:00 2001 From: Ethan Date: Sat, 8 Aug 2026 16:27:57 +0800 Subject: [PATCH] docs(tasks): scope the record protocol to both sides of the contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not a compression change, despite moving 462 words -- for a task author the loaded total goes 1264 -> 1239, which is noise. Three separate things: **The record protocol was visible to only one party.** `sieval/tasks/CLAUDE.md` held the vocabulary for types defined in `sieval/core/tasks/records.py`, and CLAUDE.md loads by directory proximity -- so someone editing `records.py`, `profiler.py` or `runner.py` never saw it. That matters concretely for `grader_output`: the name is a contract the runner reads back to route grader spend into `profile.json`, and the side most likely to rename it is the side that could not see the rule. Moved to `.claude/rules/records.md`, whose frontmatter globs `sieval/tasks/**` *and* `sieval/core/tasks/**`. Task authors load the same content as before; core authors now load it too. **`## Data Flow — Async & Concurrency` was byte-identical in `.claude/rules/tasks.md`**, which globs the same `sieval/tasks/**/*.py`. Not the intended rules-are-a-checklist / CLAUDE.md-is-principles overlap -- the same three bullets twice, loaded together. Dropped here, kept in the rules file, which is where checklist-shaped text belongs. **`Constructor knobs` restated what an enforcer already checks.** The spellings are machine-verified by `check_task_shot_knobs` across every constructor under `sieval/tasks/`; repeating the accepted-name list in prose adds nothing a violation would not surface. Trimmed 234 -> 172 words, keeping only what the checker cannot say: assigning `self._n_shot` instead of `self.n_shot` leaves the class default standing, so `meta.json` reports the declared shot count rather than the one the run used. Net effect on `sieval/tasks/CLAUDE.md` is a coherent charter -- how a task is declared and where it lives -- at 727 words instead of 1264. No coupled site breaks: the inbound references from `.claude/rules/tasks.md` and `sieval/tasks/arc/_base.py` point at the "Task Metadata" and "ppl vs clp" sections, both kept, and `CONTRIBUTING.md` does not mirror the protocol. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/rules/records.md | 36 +++++++++++++++++++++ sieval/tasks/CLAUDE.md | 70 +++++++++++----------------------------- 2 files changed, 55 insertions(+), 51 deletions(-) create mode 100644 .claude/rules/records.md diff --git a/.claude/rules/records.md b/.claude/rules/records.md new file mode 100644 index 00000000..e206d23e --- /dev/null +++ b/.claude/rules/records.md @@ -0,0 +1,36 @@ +--- +paths: + - "sieval/tasks/**/*.py" + - "sieval/core/tasks/**/*.py" +--- + +# Stage-Output Record Protocol (opt-in) + +Per-stage record types so a sample's answer / ground truth / verdict is readable without knowing which task produced it. **Schema, serialization rules and builder contracts live in `sieval/core/tasks/records.py`** — authoritative, and deliberately not repeated here. This file is the vocabulary and the traps. Opt-in per task; legacy shapes keep working, so migrate deliberately. + +Scoped to `sieval/tasks/` *and* `sieval/core/tasks/` because the contract has two +parties: tasks write these records, and the runner and profiler read them back. +A rule visible to only one side is how `grader_output` gets renamed. + +Records are named by **content**, not by the stage that emits them — a shard line reads `prediction` / `judgement`, not `postprocess` / `feedback`: + +| stage | record | +| --- | --- | +| `preprocess` | `PromptRecord` | +| `infer` | `ModelOutput` — already uniform, deliberately **not** wrapped | +| `postprocess` | `PredictionRecord` | +| `feedback` | `JudgementRecord` | + +Vocabulary — these denote **different layers**, keep them distinct: + +- **judgement** — the verdict record, mechanism-agnostic: string-compare, math-verify, test-suite *or* LLM verdicts all produce one. +- **prediction** — the model's extracted answer. `None` means "could not extract" — never `""` or `-1`. **Read it with `.get("prediction")`, never `["prediction"]`**: a `None` value is dropped by serialization, so on resume the key is *absent* and `[]` raises `KeyError` for exactly the samples whose extraction failed — on a fresh run the same line is fine, which is why in-memory tests never see it. `extracted` is the durable companion flag. Enforced by `scripts/check_preflight.py --check check_record_key_access`; neither `ty` nor `mypy --strict` reports it. +- **reference** — ground truth; `None` when it is a *procedure* (test suite, rubric). +- **correct** / **score** — the headline verdict. `correct` is the only axis comparable across tasks. +- **metrics** — every metric measured, by name. A task with *co-equal* metrics (IFEval strict + loose, HellaSwag `acc` + `acc_norm`) records them all here and derives the headline from them; a metric parked in `extra` is hidden from any reader that doesn't already know the task. +- **grade** — an LLM autorater's categorical output (CORRECT / INCORRECT / NOT_ATTEMPTED). A judgement *contains* a grade — grade sits **below** judgement, not beside it. +- **grader** — the LLM actor (the `grader` task arg / model). Not every judgement has one; persist its whole `ModelOutput` in `extra` rather than hand-picked fields, under exactly the key **`grader_output`** (`records.GRADER_OUTPUT_KEY`). The name is load-bearing, not cosmetic: the runner reads it back to route grader spend into `profile.json`, since a `feedback` stage returning a bare record has no `ModelOutput` for the profiler to derive a call from. Spell it differently and the grader's tokens are on disk but missing from the profile. +- **judge** — HLE only, upstream's synonym with its own `parse_judge` contract; do not introduce it in new tasks. +- **extra** — mechanism detail, *not* metrics: a grader's `ModelOutput`, a code runner's failure message, per-constraint results. + +Constructors are `build_*` (matching `build_model_call_meta` / `build_stage_meta`); structural sniffs are `is_*`. Records are returned **bare**, never wrapped in `TaskStageOutput`. diff --git a/sieval/tasks/CLAUDE.md b/sieval/tasks/CLAUDE.md index d0e7d924..de8f0837 100644 --- a/sieval/tasks/CLAUDE.md +++ b/sieval/tasks/CLAUDE.md @@ -42,27 +42,20 @@ answers** (a `datasets/` concern — see `sieval/datasets/CLAUDE.md`). ### Constructor knobs: `n_shot` vs `k` -Two counts that read alike and are not interchangeable. Enforced by -`scripts/check_preflight.py --check check_task_shot_knobs`: - -- **`n_shot`** — the few-shot exemplar count. The only accepted spelling (not - `k`, `shots`, `num_shots`, `fewshot`, …). Store it as **`self.n_shot`**, the - public field `Task` declares and `@sieval_task(n_shot=...)` seeds on the - class: assigning it shadows the class value for that instance, and that is - what `meta.json` records as the count the run used. Store it anywhere else - (`self._n_shot`, …) and the class value stands, so the run directory reports - the declared default. A task with **no** knob needs no code at all — the - seeded class value is already right. -- **`k`** — the `k` in `pass@k`, nothing else: the metric's parameter, **not** - the sampling budget. That is `n`, forwarded to `agenerate(n=...)`, and - `k <= n` — `pass@k` is estimated from `n` samples per problem. A task taking - `k` must compute a `pass@k` metric, and `self.n_shot` may never be fed from it. - -The `n_shot` rules bind **every constructor under `sieval/tasks/`**, including an -undecorated shared base in a subpackage (`arc/_base.py`) — a decorated task can -inherit its `__init__`, and checking only decorated classes would leave that -knob unchecked. The `k` rule is decorated-classes-only: an undecorated base's -`pass@k` is usually computed by the subclass. +Two counts that read alike and are not interchangeable. The spelling rules are +machine-checked (`check_preflight.py --check check_task_shot_knobs`, over every +constructor under `sieval/tasks/`); what the checker cannot tell you is why: + +- **`n_shot`** — the few-shot exemplar count. Store it as **`self.n_shot`**, not + `self._n_shot`: assigning the public field shadows the class value + `@sieval_task(n_shot=...)` seeded, and *that* is what `meta.json` records as + the count the run used. Store it privately and the class value stands, so the + run directory reports the declared default rather than what ran. A task with + no knob needs no code — the seeded value is already right. +- **`k`** — the `k` in `pass@k`, and nothing else: the metric's parameter, not + the sampling budget. That is `n`, forwarded to `agenerate(n=...)`, with + `k <= n`. A task taking `k` must compute a `pass@k` metric, and `self.n_shot` + may never be fed from it. `fewshot_split` / `fewshot_seed` / `fewshot_as_multiturn` name a different noun each and are unaffected. In prose, `k-shot` (the file-naming genre) and `top-k` @@ -87,30 +80,11 @@ each and are unaffected. In prose, `k-shot` (the file-naming genre) and `top-k` ## Stage-Output Protocol (opt-in) -Per-stage record types so a sample's answer / ground truth / verdict is readable without knowing which task produced it. **Schema, serialization rules and builder contracts live in [`sieval/core/tasks/records.py`](../core/tasks/records.py)** — authoritative, and deliberately not repeated here. This section is the vocabulary. Opt-in per task; legacy shapes keep working, so migrate deliberately. - -Records are named by **content**, not by the stage that emits them — a shard line reads `prediction` / `judgement`, not `postprocess` / `feedback`: - -| stage | record | -| --- | --- | -| `preprocess` | `PromptRecord` | -| `infer` | `ModelOutput` — already uniform, deliberately **not** wrapped | -| `postprocess` | `PredictionRecord` | -| `feedback` | `JudgementRecord` | - -Vocabulary — these denote **different layers**, keep them distinct: - -- **judgement** — the verdict record, mechanism-agnostic: string-compare, math-verify, test-suite *or* LLM verdicts all produce one. -- **prediction** — the model's extracted answer. `None` means "could not extract" — never `""` or `-1`. **Read it with `.get("prediction")`, never `["prediction"]`**: a `None` value is dropped by serialization, so on resume the key is *absent* and `[]` raises `KeyError` for exactly the samples whose extraction failed — on a fresh run the same line is fine, which is why in-memory tests never see it. `extracted` is the durable companion flag. Enforced by `scripts/check_preflight.py --check check_record_key_access`; neither `ty` nor `mypy --strict` reports it. -- **reference** — ground truth; `None` when it is a *procedure* (test suite, rubric). -- **correct** / **score** — the headline verdict. `correct` is the only axis comparable across tasks. -- **metrics** — every metric measured, by name. A task with *co-equal* metrics (IFEval strict + loose, HellaSwag `acc` + `acc_norm`) records them all here and derives the headline from them; a metric parked in `extra` is hidden from any reader that doesn't already know the task. -- **grade** — an LLM autorater's categorical output (CORRECT / INCORRECT / NOT_ATTEMPTED). A judgement *contains* a grade — grade sits **below** judgement, not beside it. -- **grader** — the LLM actor (the `grader` task arg / model). Not every judgement has one; persist its whole `ModelOutput` in `extra` rather than hand-picked fields, under exactly the key **`grader_output`** (`records.GRADER_OUTPUT_KEY`). The name is load-bearing, not cosmetic: the runner reads it back to route grader spend into `profile.json`, since a `feedback` stage returning a bare record has no `ModelOutput` for the profiler to derive a call from. Spell it differently and the grader's tokens are on disk but missing from the profile. -- **judge** — HLE only, upstream's synonym with its own `parse_judge` contract; do not introduce it in new tasks. -- **extra** — mechanism detail, *not* metrics: a grader's `ModelOutput`, a code runner's failure message, per-constraint results. - -Constructors are `build_*` (matching `build_model_call_meta` / `build_stage_meta`); structural sniffs are `is_*`. Records are returned **bare**, never wrapped in `TaskStageOutput`. +The record vocabulary and its traps live in `.claude/rules/records.md`, scoped to +this tree *and* `sieval/core/tasks/` — the contract has two parties, and a rule +only the writing side can see is how `grader_output` gets renamed. Schema and +builder contracts are authoritative in +[`sieval/core/tasks/records.py`](../core/tasks/records.py). ## `infer_args` — Per-Task Inference Override @@ -123,9 +97,3 @@ YAML-level `infer_args` overrides model inference parameters via `EvalSession` c - `TaskMeta.dataset` (the FK to a registered Dataset) is resolved automatically from the Task's first generic arg (its sample `TypedDict`); do not pass `dataset=` explicitly. The referenced Dataset class must already be `@sieval_dataset`-decorated (see `.claude/rules/datasets.md`) and its `source` is the authoritative origin consumed by `sieval dataset download`. - Per-run knobs (`k`, `n`, `temperature`, `seed`) stay in runner config, not TaskMeta. - `sieval/meta/index.json` is auto-generated; full field reference lives on the decorator docstring. - -## Data Flow — Async & Concurrency - -- Understand the framework's staged execution data flow before implementing. -- All intermediate state must flow through the framework's persistence layer (record/shard storage) — do NOT use external files, temp caches, or module-level mutable state to pass data between stages. -- Never introduce shared mutable state without proper locking.