From f1b6abb7ca8809288365cd505d8afcb96771f6cb Mon Sep 17 00:00:00 2001 From: uipreliga Date: Wed, 12 Aug 2026 11:30:53 -0700 Subject: [PATCH 001/144] =?UTF-8?q?feat(dataset):=201/3=20=E2=80=94=20add?= =?UTF-8?q?=20Dataset.split=5Ffield=20and=20the=20--split=20row=20filter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Label dataset rows with a split (tune / holdout / …) and select one at run time with `coder-eval run --split `. The filter runs BEFORE either sampler: sampling first would leave an unpredictable (possibly zero) number of rows per split, destroying the comparison the split exists to protect. - `Dataset.split_field` (default "split") mirrors `stratify_field`'s shape. - The filter is inlined in `expand_dataset` rather than extracted: one call site, a one-line comprehension, and a helper would re-declare the missing-field convention `_stratified_sample` already owns. - A row is unlabelled when the field is absent, null, or "". A task whose rows are all unlabelled passes through unfiltered — `--split` is global to the invocation, so an unlabelled suite beside a labelled one must not fail. A labelled task with no matching row raises, naming the splits that exist; `resolve_all_tasks` records that as a skipped task, so a mistyped selector is a zero-task run that still exits 0. Documented rather than papered over. `--split` unset leaves expansion byte-for-byte unchanged (pinned by test). Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 +- docs/AB_EXPERIMENTS.md | 1 + docs/DATASETS.md | 59 ++++- docs/TASK_DEFINITION_GUIDE.md | 2 + docs/USER_GUIDE.md | 1 + src/coder_eval/cli/run_command.py | 17 ++ src/coder_eval/models/tasks.py | 12 + src/coder_eval/orchestration/config.py | 8 + src/coder_eval/orchestration/experiment.py | 1 + src/coder_eval/orchestration/task_loader.py | 36 ++- tests/test_dataset_expansion.py | 232 ++++++++++++++++++++ 11 files changed, 365 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e7fc33e9..fde316ee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -138,7 +138,7 @@ action.yml # Published composite GitHub Action (coder-ev - **Single declarative merge resolver**: All five config layers merge through ONE engine (`orchestration/config_merge.py::resolve_root`) for the three `-D`-reachable roots (`agent`/`run_limits`/`sandbox`). Each field declares *how it merges* once, on the model, via `MergeField(strategy="deep"|"append"|"replace")` (or a type-aware default: nested `BaseModel`/free-form `dict` → `deep`; `list`/scalar → `replace`). `resolve_task_for_variant` (layers 1–4) and `apply_overrides` (layer 5) build `Layer` lists and call the same `resolve_root`, so a field merges identically regardless of which layer supplied it (the unification invariant, enforced by `tests/test_merge_unification.py`). Lint rule CE014 forces every list field to declare its strategy explicitly. - **Generic CLI overrides (`-D`/`--set`)**: Layer 5 is a thin wrapper (`orchestration/overrides.py`) over the resolver above. `coder-eval run -D agent.model=opus -D run_limits.max_turns=30` overrides any field on the resolved `TaskDefinition` (`agent`/`run_limits`/`sandbox` roots), schema-validated with did-you-mean. Only `--model` (→ `agent.model`) and `--driver` (→ `sandbox.driver`) survive as active thin aliases that emit the equivalent `-D` entry; an alias and `-D` targeting the same path is a hard error. `--type` (→ `agent.type`) is a separate, lighter alias that does NOT route through that collision check — `--type` and `-D agent.type=…` last-win rather than hard-error (the `-D` value wins). Tools, plugins, and SDK options are `-D`-only. - **All core models importable from `coder_eval.models`** regardless of submodule -- **Dataset fan-out**: `TaskDefinition.dataset` (inline rows or JSONL path) expands a single task into N row-tasks with `${row.}` substitution in `initial_prompt` and `success_criteria` string fields. Expansion runs in `task_loader.expand_dataset` **before** variant resolution, so variants cannot override the dataset. Row sampling: CLI `--sample N` (fixed-seed uniform-random N over the whole dataset) overrides `--sample-per-stratum N` / `dataset.sample_per_stratum` (stratified random N-per-stratum, keyed on `stratify_field`, default `expected_skill` — for classification suites like activation). Stratified sampling (whether the N-per-stratum count comes from the **CLI** `--sample-per-stratum` flag or **YAML** `dataset.sample_per_stratum`) is **nondeterministic** by default — it re-draws each run (so the nightly activation suite broadens coverage over time). Set `dataset.sample_seed` to pin a reproducible sample; an explicit seed always wins. (Only `--sample N` uses a fixed seed, since a smoke test wants the same N rows each run.) +- **Dataset fan-out**: `TaskDefinition.dataset` (inline rows or JSONL path) expands a single task into N row-tasks with `${row.}` substitution in `initial_prompt` and `success_criteria` string fields. Expansion runs in `task_loader.expand_dataset` **before** variant resolution, so variants cannot override the dataset. Row selection is filter-then-sample: CLI `--split ` (keep only rows whose `dataset.split_field` value matches, default field `split`) runs **first** and is orthogonal to the sampler win-order — a row is unlabelled when the field is absent/`null`/`""` and a task whose rows are all unlabelled passes through unfiltered; partial labelling drops the unlabelled rows. A *labelled* task with no matching row raises a `ValueError` listing the splits that exist — which `resolve_all_tasks` catches into `skipped_tasks` like any load failure, so a mistyped selector yields a zero-task run that still exits 0. Filtering before sampling is a correctness requirement: sampling first would leave an unpredictable (possibly zero) number of rows per split, destroying the tune/holdout comparison. Then sampling: CLI `--sample N` (fixed-seed uniform-random N over the whole dataset) overrides `--sample-per-stratum N` / `dataset.sample_per_stratum` (stratified random N-per-stratum, keyed on `stratify_field`, default `expected_skill` — for classification suites like activation). Stratified sampling (whether the N-per-stratum count comes from the **CLI** `--sample-per-stratum` flag or **YAML** `dataset.sample_per_stratum`) is **nondeterministic** by default — it re-draws each run (so the nightly activation suite broadens coverage over time). Set `dataset.sample_seed` to pin a reproducible sample; an explicit seed always wins. (Only `--sample N` uses a fixed seed, since a smoke test wants the same N rows each run.) - **Per-criterion aggregation**: Each `BaseCriterion` subclass exposes `aggregate(criterion, per_row_results) -> CriterionAggregate | None`. Default emits `count / mean / median / std / min / max` so every criterion is suite-thresholdable for free. Classification-style criteria return `ClassificationCriterionResult` (subclass of `CriterionResult`) and layer accuracy / P/R/F1 / confusion via the shared `overlay_classification_metrics` utility. `BaseSuccessCriterion.suite_thresholds` gates the suite on those metrics; CLI exits non-zero on any gate failure. - **Sub-agent token accounting**: There is NO separate per-sub-agent field. Every sub-agent generation is captured as a `parent_tool_use_id`-tagged `AssistantMessage` in the turn transcript, so per-sub-agent usage is derived by grouping those messages on that id (the evalboard's `aggregateSubAgentUsage` does exactly this). Claude bubbles its sub-agent's intermediate generations into the parent stream natively, and the **terminal** generation (delivered as the Agent tool result, never streamed) is synthesized into one via `_synthesize_subagent_terminal_message` from `tool_use_result.usage`. Codex reconstructs all child generations from the child rollout (`_recover_subagent_tool_calls`). The turn total already includes sub-agent cost — Claude via the SDK's cumulative `model_usage`; Codex via `_fold_subagent_tokens`, which folds the child messages (their real per-generation tokens) into the parent total. `CommandTelemetry.result_summary` is stored **untruncated** (no 200-char cap) so sub-agent returns are preserved whole. Set `CODER_EVAL_RAW_SDK_LOG=1` to dump every raw SDK event to the task log for inspection. - **Reconciliation message (stream self-reconciles to the turn total)**: The per-message stream consistently under-reports the authoritative turn total — a fixed prompt slice (~512 input tokens on Claude) is billed on no SDK-emitted message, and sub-agent input/cache only partially bubbles up. So `EventCollector.build_turn_record` appends one synthetic `ReconciliationMessage` (`role="reconciliation"`, in the `TranscriptMessage` union) per turn, carrying the per-bucket residual = `token_usage` − Σ(assistant message buckets). The invariant: **summing the four token buckets across `TurnRecord.messages` (assistant + reconciliation) equals `token_usage` exactly**, for both Claude and Codex (Codex's stream is already complete after `_recover_subagent_tool_calls`, so its residual is usually 0 and no entry is emitted). This is what lets the evalboard SUM the message stream as the source of truth instead of reading a separate aggregate ("agent tokens"): `selectTokenTotals` returns the stream sum whenever a reconciliation entry is present, and the timeline renders it as its own row. It is agent-agnostic (booked at the single `EventCollector` seam), carries no cost (cost stays on `token_usage`), and is excluded from generation/turn counts and the cost simulator. The LiteLLM open-weight actual-cost join (`litellm_cost.apply_actual_cost`) deliberately writes cost at the TURN level only (`token_usage.total_cost_usd` = the real OpenRouter bill) plus the per-call `TurnRecord.provider_call_costs` audit record; it does NOT touch the message token buckets, so `EventCollector` stays the single writer and this invariant holds on every backend. The Python `token_usage`/`total_token_usage` aggregate is unchanged and still authoritative for budget/judges/reports. diff --git a/docs/AB_EXPERIMENTS.md b/docs/AB_EXPERIMENTS.md index 040d9b97..fc5144a4 100644 --- a/docs/AB_EXPERIMENTS.md +++ b/docs/AB_EXPERIMENTS.md @@ -350,6 +350,7 @@ if any listed metric is below its minimum. | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | | `-e, --experiment ` | Experiment YAML. Bare name → `experiments/.yaml`. | | `--sample N` | For dataset-backed tasks, use a fixed-seed random N-row sample (reproducible, unbiased across paths; cheap smoke test). | +| `--split NAME` | For dataset-backed tasks, keep only rows whose `dataset.split_field` value matches (e.g. `tune` / `holdout`). Applied before `--sample`. Unlabelled tasks unaffected. | | `--repeats N` | Run each `(task, variant)` N times; overrides YAML `repeats`. | | `--driver tempdir\|docker` | Override sandbox driver for all tasks. | | `-j, --max-parallel N` | Run up to N tasks concurrently. | diff --git a/docs/DATASETS.md b/docs/DATASETS.md index 498addab..5a8e1b98 100644 --- a/docs/DATASETS.md +++ b/docs/DATASETS.md @@ -124,11 +124,38 @@ Load-time errors, with their message shapes: | Two rows share an id | `Duplicate dataset row id for task '': ''` | | Both/neither `rows` and `paths` | `Dataset must specify either 'paths' or 'rows'` / `... only one of ...` | | `paths: []` | `Dataset.paths must be a non-empty list` | +| `--split X` on a labelled dataset with no `X` rows | `Dataset for task '' has no rows in split 'X' (split_field='split'); labelled splits present: ['holdout', 'tune']` | -## Sampling a subset +## Selecting a subset -A full dataset is expensive. Two independent mechanisms cut it down, and **`--sample` wins whenever -both apply**: +A full dataset is expensive. Three mechanisms cut it down, in two stages: **`--split` filters +first**, then **one** of the two samplers runs over what survives, with **`--sample` winning +whenever both samplers apply**. + +**Stage 1 — the filter.** `--split` is not in that win-order; it is orthogonal, and always applies +first. + +**`--split ` (CLI only)** — keep only rows whose `dataset.split_field` value (default field: +`split`) equals ``. Exact string match, no case normalization; non-string values compare via +`str()`. + +Three behaviours are worth knowing before you rely on it: + +- **A task whose rows carry no split label at all passes through unfiltered.** `--split` is global to + the invocation, so an unlabelled dataset sitting beside a labelled one in the same run must not + fail. A row counts as unlabelled when the field is absent, `null`, or `""`. +- **Partial labelling drops the unlabelled rows.** If only some rows carry a label, `--split tune` + keeps just the `tune` rows — the unlabelled ones are excluded rather than folded in. That is the + safe direction (an unlabelled row never leaks into a named split), but during an incremental + migration it silently *shrinks* the suite, which moves the aggregate metrics `suite_thresholds` + gates on. Finish labelling before you compare two runs. +- **A labelled task with no row in the requested split is skipped, not fatal.** Expansion raises, + naming the splits that do exist, and the run records it in `run.json`'s `skipped_tasks` and carries + on with whatever else resolved. So a mistyped selector (`--split holdou`) produces a run of **zero + tasks that still exits 0** — check the skipped-task count, not just the exit code, when a split run + comes back suspiciously clean. + +**Stage 2 — the samplers**, over whatever survived the filter: 1. **`--sample N` (CLI only)** — a flat uniform-random N rows over the whole dataset. Fixed seed, so the same N rows come back every run: a reproducible, cheap smoke flavor of a big suite. Unlike a @@ -155,6 +182,32 @@ both apply**: coverage over time matters more than run-to-run comparability. Note the contrast with `--sample N`, which is fixed-seed and reproducible by default. +### Tune and holdout splits + +Label each row with a split and you can develop against one half and confirm on the other, which is +what keeps a measured improvement from being an artifact of the rows you tuned on: + +```jsonl +{"id": "pos-1", "prompt": "review my task files", "expected_skill": "lint-tasks", "split": "tune"} +{"id": "pos-2", "prompt": "are my evals any good?", "expected_skill": "lint-tasks", "split": "holdout"} +``` + +```bash +coder-eval run tasks/skills/activation.yaml --split tune # iterate here +coder-eval run tasks/skills/activation.yaml --split holdout # confirm here, once +``` + +**The filter runs before either sampler, and that ordering is load-bearing.** Sampling first would +leave an unpredictable — possibly zero — number of rows per split, so the two arms of the comparison +would no longer be the same size or the same rows. Filter-then-sample means `--split tune --sample 8` +is always drawn from the tune rows alone — at most eight of them, and all of them if `tune` holds +fewer than eight. + +Two consequences worth planning for. A split **halves each side of the suite**, so a dataset sized +for a single one-shot measurement is undersized once split — budget roughly double the rows you +would otherwise want. And a holdout is only worth what its independence buys: consult it to confirm +a decision already made on `tune`, not to choose between candidates, or it becomes a second tune set. + ## Suite-level scoring Per-row pass/fail is rarely the number you care about on a dataset — the suite metric is. Every diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index e96ac153..b8d6bc55 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -83,6 +83,7 @@ dataset: sample_per_stratum: 5 # optional: keep up to N rows per stratum stratify_field: "expected_skill" # which row field defines the stratum sample_seed: 1234 # optional: pin the stratified draw + split_field: "split" # which row field names the row's split (CLI --split) ``` | Field | Default | Description | @@ -93,6 +94,7 @@ dataset: | `sample_per_stratum` | `null` | Stratified random sample: keep up to N rows per stratum. Overridden by CLI `--sample`. | | `stratify_field` | `"expected_skill"` | Row field whose value defines the stratum for `sample_per_stratum`. | | `sample_seed` | `null` | Seed for the stratified draw. Unset means the sample is **re-drawn every run**; set an integer to pin it. CLI `--sample` is separately fixed-seed and always reproducible. | +| `split_field` | `"split"` | Row field naming the row's split (e.g. `tune` / `holdout`). CLI `--split ` keeps only rows whose value here matches, **before** any sampling. A task whose rows never set this field is unaffected by `--split`. Splits are open strings. | Full guide — row sources, substitution rules, sampling precedence, suite-level scoring, and worked examples: **[Bring Your Own Dataset](DATASETS.md)**. diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index e62d4195..44acff69 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -44,6 +44,7 @@ coder-eval run tasks/hello_date.yaml --stream full # live LLM output | `--resume` | Resume an interrupted run: skip tasks already finalized in `--run-dir` and run the rest, folding prior results into `run.json`. Requires `--run-dir`. A task with *any* final status (incl. FAILED/ERROR) counts as finalized, so resume does **not** retry failures — delete a task's `task.json` to force a re-run. A config mismatch is warned, not refused. | | `--sample N` | For dataset-backed tasks, run a fixed-seed random N-row sample (reproducible; cheap smoke test). See [Bring Your Own Dataset](DATASETS.md). | | `--sample-per-stratum N` | For dataset-backed tasks, keep up to N rows per stratum (`stratify_field`). Overridden by `--sample`. Nondeterministic unless `dataset.sample_seed` is set — see [Bring Your Own Dataset](DATASETS.md). | +| `--split NAME` | For dataset-backed tasks, keep only rows whose `dataset.split_field` value (default field: `split`) matches — e.g. `--split tune` / `--split holdout`. Applied **before** `--sample` / `--sample-per-stratum`. Tasks whose rows carry no split label are unaffected. See [Bring Your Own Dataset](DATASETS.md). | | `--include-skipped` | Also run tasks marked `skip: true` in their YAML (off by default so CI keeps excluding them). | | `--exclude-tags` | Skip tasks matching any of these tags (comma-separated) | | `--tags, -t` | Only run tasks matching any of these tags (comma-separated) | diff --git a/src/coder_eval/cli/run_command.py b/src/coder_eval/cli/run_command.py index 9b22929c..19346451 100644 --- a/src/coder_eval/cli/run_command.py +++ b/src/coder_eval/cli/run_command.py @@ -297,6 +297,17 @@ def run_command( ), min=1, ), + split: str | None = typer.Option( + None, + "--split", + help=( + "For dataset-backed tasks, keep only rows whose dataset.split_field value " + "(default field: split) matches this name — e.g. --split tune / --split holdout. " + "Applied BEFORE --sample / --sample-per-stratum, so a sampled split keeps a " + "predictable size. Tasks whose rows are all unlabelled are unaffected; a " + "labelled task with no row in this split is reported in skipped_tasks." + ), + ), repeats: int | None = typer.Option( None, "--repeats", @@ -409,6 +420,7 @@ def run_command( experiment_path=resolved_experiment, max_rows=sample, sample_per_stratum=sample_per_stratum, + split=split, repeats=repeats, verbose=verbose, resume=resume, @@ -434,6 +446,7 @@ async def _run_all_tasks( experiment_path: Path | None = None, max_rows: int | None = None, sample_per_stratum: int | None = None, + split: str | None = None, repeats: int | None = None, verbose: bool = False, resume: bool = False, @@ -457,6 +470,9 @@ async def _run_all_tasks( from -D/--set and the bespoke flag aliases stream_mode: Optional stream mode ('full' or 'minimal') for real-time output experiment_path: Optional path to experiment YAML (default: experiments/default.yaml) + split: Optional dataset row filter (--split): keep only rows whose + dataset.split_field value matches. Applied before max_rows / + sample_per_stratum; tasks whose rows carry no split label are unaffected. junit_xml: Optional path to write a JUnit XML report to, after the run summary is persisted and before the failure exit-code gate. """ @@ -481,6 +497,7 @@ async def _run_all_tasks( overrides=overrides or {}, max_rows=max_rows, sample_per_stratum=sample_per_stratum, + split=split, repeats=repeats, verbose=verbose, include_skipped=include_skipped, diff --git a/src/coder_eval/models/tasks.py b/src/coder_eval/models/tasks.py index 9614d675..8f121ff1 100644 --- a/src/coder_eval/models/tasks.py +++ b/src/coder_eval/models/tasks.py @@ -258,6 +258,18 @@ class Dataset(BaseModel): default="expected_skill", description="Row field whose value defines the stratum for 'sample_per_stratum' (default: 'expected_skill').", ) + split_field: str = Field( + default="split", + description=( + "Row field naming the row's split (e.g. 'tune' / 'holdout' / 'holdback'). " + "CLI --split keeps only rows whose value for this field matches, and " + "is applied BEFORE any sampling so a sampled split still has a predictable " + "size. A row is unlabelled when this field is absent, null, or ''; a task " + "whose rows are all unlabelled is unaffected by --split, while partial " + "labelling keeps only the matching rows and drops the unlabelled ones. " + "Splits are open strings; nothing here constrains the set of names." + ), + ) sample_seed: int | None = Field( default=None, description=( diff --git a/src/coder_eval/orchestration/config.py b/src/coder_eval/orchestration/config.py index 695b5594..f871bc88 100644 --- a/src/coder_eval/orchestration/config.py +++ b/src/coder_eval/orchestration/config.py @@ -86,6 +86,14 @@ class BatchRunConfig(BaseModel): "stratified dataset without editing the task YAML. Ignored when max_rows is set." ), ) + split: str | None = Field( + default=None, + description=( + "CLI --split: keep only dataset rows whose dataset.split_field value matches. " + "Applied before max_rows / sample_per_stratum. Tasks whose rows carry no split " + "label are unaffected." + ), + ) # Replicate count override repeats: int | None = Field( diff --git a/src/coder_eval/orchestration/experiment.py b/src/coder_eval/orchestration/experiment.py index f75c585e..5562b089 100644 --- a/src/coder_eval/orchestration/experiment.py +++ b/src/coder_eval/orchestration/experiment.py @@ -627,6 +627,7 @@ def resolve_all_tasks( task_file.parent, max_rows=config.max_rows, sample_per_stratum=config.sample_per_stratum, + split=config.split, ) # Narrow set: real load failures only. We deliberately don't catch # AttributeError / TypeError / ImportError — those signal a regression diff --git a/src/coder_eval/orchestration/task_loader.py b/src/coder_eval/orchestration/task_loader.py index bff0c13a..51134cba 100644 --- a/src/coder_eval/orchestration/task_loader.py +++ b/src/coder_eval/orchestration/task_loader.py @@ -343,6 +343,7 @@ def expand_dataset( task_file_dir: Path, max_rows: int | None = None, sample_per_stratum: int | None = None, + split: str | None = None, ) -> list[TaskDefinition]: """Fan out a task with ``dataset:`` into one TaskDefinition per row. @@ -371,13 +372,24 @@ def expand_dataset( Lets a runner cap a stratified dataset without editing the task YAML (the nightly activation suite uses this). Ignored when ``max_rows`` is set. When None, falls back to ``dataset.sample_per_stratum``. + split: Optional CLI row filter (``--split``) — keep only rows whose + ``dataset.split_field`` value equals this. Applied BEFORE either + sampler, so a sampled split still has a predictable size. A row is + unlabelled when the field is absent, ``None``, or ``""``. A task + whose rows are all unlabelled passes through unfiltered (``--split`` + is global to the invocation, so an unlabelled task in a multi-task + run must not fail); partial labelling keeps the matching rows and + drops the unlabelled ones; a *labelled* task with no matching row + raises. Note the raise is caught by ``resolve_all_tasks`` into + ``skipped_tasks``, so at the run level a mistyped split name is a + skipped suite rather than an aborted run. Returns: Expanded list of TaskDefinitions. Length is 1 when dataset is None. Raises: - ValueError: Empty dataset, duplicate row ids, missing id_field, or - malformed row id. + ValueError: Empty dataset, duplicate row ids, missing id_field, + malformed row id, or a labelled dataset with no row in ``split``. FileNotFoundError: Dataset path does not exist. """ if task.dataset is None: @@ -387,6 +399,26 @@ def expand_dataset( if not rows: raise ValueError(f"Dataset for task '{task.task_id}' is empty") + # --split filters BEFORE either sampler below: sampling first would leave an + # unpredictable (possibly zero) number of rows per split, destroying the + # tune/holdout comparison the split exists to protect. + if split is not None: + field = task.dataset.split_field + # Unlabelled means the key is absent, null, or "" — the same "no value here" + # convention _stratified_sample applies to a missing field, extended to the + # explicit null a half-labelled JSONL carries. Any other value, including a + # falsy 0, is a real label and compares via str() (also as _stratified_sample does). + labelled = [r for r in rows if r.get(field) not in (None, "")] + if labelled: + rows = [r for r in labelled if str(r[field]) == split] + if not rows: + raise ValueError( + f"Dataset for task '{task.task_id}' has no rows in split {split!r} " + + f"(split_field={field!r}); labelled splits present: " + + f"{sorted({str(r[field]) for r in labelled})}" + ) + # else: no row in this task carries a split label -> --split does not apply here. + # Row selection precedence: # 1. CLI --sample (max_rows): flat uniform-random N over the whole dataset. # Fixed seed => reproducible across runs, but (unlike a first-N slice) diff --git a/tests/test_dataset_expansion.py b/tests/test_dataset_expansion.py index 08693cfb..98029075 100644 --- a/tests/test_dataset_expansion.py +++ b/tests/test_dataset_expansion.py @@ -284,6 +284,146 @@ def test_dataset_seed_wins_over_cli_arg(self, tmp_path: Path) -> None: assert ids_flag == ids_yaml +class TestExpandDatasetSplit: + """CLI --split: keep only rows whose dataset.split_field value matches. + + The filter runs BEFORE either sampler, so a sampled split still has a + predictable size — sampling first would leave an unpredictable (possibly + zero) number of rows per split and silently destroy the tune/holdout + comparison the split exists to protect. + """ + + @staticmethod + def _split_rows() -> list[dict[str, Any]]: + return [ + {"id": "t1", "prompt": "p", "expected": "e", "split": "tune"}, + {"id": "t2", "prompt": "p", "expected": "e", "split": "tune"}, + {"id": "t3", "prompt": "p", "expected": "e", "split": "tune"}, + {"id": "h1", "prompt": "p", "expected": "e", "split": "holdout"}, + {"id": "h2", "prompt": "p", "expected": "e", "split": "holdout"}, + ] + + def test_keeps_only_matching_rows(self, tmp_path: Path) -> None: + task = _make_task_with_dataset(rows=self._split_rows()) + expanded = expand_dataset(task, tmp_path, split="tune") + assert [t.row_id for t in expanded] == ["t1", "t2", "t3"] + # task_id / row_id rewriting is unchanged by the filter. + assert [t.task_id for t in expanded] == ["suite/t1", "suite/t2", "suite/t3"] + assert all(t.suite_id == "suite" for t in expanded) + + def test_unlabelled_dataset_passes_through(self, tmp_path: Path) -> None: + # --split is global to the invocation. A run containing several + # dataset-backed tasks would otherwise fail every task that does not use + # splits, so a task with NO labelled row at all is left whole. + rows = [{"id": f"r{i}", "prompt": "p", "expected": "e"} for i in range(4)] + expanded = expand_dataset(_make_task_with_dataset(rows=rows), tmp_path, split="tune") + assert [t.row_id for t in expanded] == ["r0", "r1", "r2", "r3"] + + def test_partially_labelled_excludes_unlabelled_rows(self, tmp_path: Path) -> None: + # Safe direction: an unlabelled row never leaks into a named split. + rows = [ + {"id": "a", "prompt": "p", "expected": "e", "split": "tune"}, + {"id": "b", "prompt": "p", "expected": "e"}, + {"id": "c", "prompt": "p", "expected": "e", "split": ""}, + ] + expanded = expand_dataset(_make_task_with_dataset(rows=rows), tmp_path, split="tune") + assert [t.row_id for t in expanded] == ["a"] + + def test_labelled_but_unmatched_raises_listing_available_splits(self, tmp_path: Path) -> None: + task = _make_task_with_dataset(rows=self._split_rows()) + with pytest.raises(ValueError, match="no rows in split 'Tune'") as exc: + expand_dataset(task, tmp_path, split="Tune") + # Exact match, no case normalization — the message names what does exist. + assert "'holdout', 'tune'" in str(exc.value) + + def test_explicit_null_split_counts_as_unlabelled(self, tmp_path: Path) -> None: + # `"split": null` is the natural JSONL shape for "not assigned yet", and it + # matches the null -> "" convention row substitution already uses. It must + # NOT read as the label "None" (which `str(row.get(field, ""))` would make + # truthy), or a half-migrated dataset fails instead of passing through and + # the error advertises a phantom split. + rows = [ + {"id": "a", "prompt": "p", "expected": "e", "split": None}, + {"id": "b", "prompt": "p", "expected": "e", "split": None}, + ] + expanded = expand_dataset(_make_task_with_dataset(rows=rows), tmp_path, split="tune") + assert [t.row_id for t in expanded] == ["a", "b"] + + def test_null_split_rows_are_excluded_from_a_named_split(self, tmp_path: Path) -> None: + # The partial-labelling half of the same rule: a null-split row is unlabelled, + # so it is dropped from a named split rather than joining a "None" split. + rows = [ + {"id": "a", "prompt": "p", "expected": "e", "split": "tune"}, + {"id": "b", "prompt": "p", "expected": "e", "split": None}, + ] + expanded = expand_dataset(_make_task_with_dataset(rows=rows), tmp_path, split="tune") + assert [t.row_id for t in expanded] == ["a"] + + def test_zero_is_a_real_split_label(self, tmp_path: Path) -> None: + # Guards the null fix against an `or ""` implementation, which would make the + # falsy-but-present value 0 unlabelled. Only None/"" mean "no label". + rows = [ + {"id": "a", "prompt": "p", "expected": "e", "split": 0}, + {"id": "b", "prompt": "p", "expected": "e", "split": 1}, + ] + assert [t.row_id for t in expand_dataset(_make_task_with_dataset(rows=rows), tmp_path, split="0")] == ["a"] + + def test_non_string_split_values_compare_by_str(self, tmp_path: Path) -> None: + rows = [ + {"id": "a", "prompt": "p", "expected": "e", "split": 1}, + {"id": "b", "prompt": "p", "expected": "e", "split": 2}, + ] + expanded = expand_dataset(_make_task_with_dataset(rows=rows), tmp_path, split="1") + assert [t.row_id for t in expanded] == ["a"] + + def test_custom_split_field(self, tmp_path: Path) -> None: + rows = [ + {"id": "a", "prompt": "p", "expected": "e", "fold": "train"}, + {"id": "b", "prompt": "p", "expected": "e", "fold": "test"}, + ] + task = _make_task_with_dataset(rows=rows, split_field="fold") + assert [t.row_id for t in expand_dataset(task, tmp_path, split="test")] == ["b"] + + def test_filter_runs_before_max_rows(self, tmp_path: Path) -> None: + # Ordering guard: every sampled row must still carry the requested split. + rows = self._split_rows() + task = _make_task_with_dataset(rows=rows) + expanded = expand_dataset(task, tmp_path, max_rows=2, split="tune") + assert len(expanded) == 2 + assert {t.row_id for t in expanded} <= {"t1", "t2", "t3"} + + def test_filter_runs_before_sample_per_stratum(self, tmp_path: Path) -> None: + # Same assertion on the stratified path: strata are computed WITHIN the + # filtered set, so a holdout row can never be drawn under --split tune. + rows = [ + {"id": "t1", "prompt": "p", "expected": "e", "split": "tune", "expected_skill": "a"}, + {"id": "t2", "prompt": "p", "expected": "e", "split": "tune", "expected_skill": "a"}, + {"id": "t3", "prompt": "p", "expected": "e", "split": "tune", "expected_skill": "b"}, + {"id": "h1", "prompt": "p", "expected": "e", "split": "holdout", "expected_skill": "a"}, + {"id": "h2", "prompt": "p", "expected": "e", "split": "holdout", "expected_skill": "b"}, + ] + task = _make_task_with_dataset(rows=rows, sample_per_stratum=1, sample_seed=0) + expanded = expand_dataset(task, tmp_path, split="tune") + assert len(expanded) == 2 # one per stratum, within tune only + assert {t.row_id for t in expanded} <= {"t1", "t2", "t3"} + + def test_split_none_is_byte_identical_to_today(self, tmp_path: Path) -> None: + # No-regression guard: with no --split, a dataset carrying mixed split + # values expands exactly as it did before the filter existed. + task = _make_task_with_dataset(rows=self._split_rows()) + assert [t.row_id for t in expand_dataset(task, tmp_path)] == ["t1", "t2", "t3", "h1", "h2"] + assert [t.row_id for t in expand_dataset(task, tmp_path, split=None)] == ["t1", "t2", "t3", "h1", "h2"] + + def test_task_without_dataset_unaffected(self, tmp_path: Path) -> None: + task = TaskDefinition(**_base_task_dict()) + assert expand_dataset(task, tmp_path, split="tune") == [task] + + def test_split_field_defaults_to_split(self) -> None: + task = _make_task_with_dataset(rows=[{"id": "a"}]) + assert task.dataset is not None + assert task.dataset.split_field == "split" + + class TestExpandDatasetJsonl: def test_loads_jsonl(self, tmp_path: Path) -> None: ds_path = tmp_path / "rows.jsonl" @@ -514,6 +654,98 @@ def test_max_rows_applies(self, tmp_path: Path) -> None: # --sample is now a random subset (count == max_rows), not a first-N slice. assert resolved[0].task.task_id in {"suite/row-a", "suite/row-b"} + def test_split_applies(self, tmp_path: Path) -> None: + # BatchRunConfig.split threads CLI -> config -> expand_dataset with no + # merge-layer participation (dataset expansion precedes variant resolution). + data = { + "task_id": "suite", + "description": "Test", + "initial_prompt": "Prompt: ${row.prompt}", + "sandbox": {"driver": "tempdir"}, + "success_criteria": [{"type": "file_exists", "path": "out.txt", "description": "File"}], + "dataset": { + "rows": [ + {"id": "row-a", "prompt": "a", "split": "tune"}, + {"id": "row-b", "prompt": "b", "split": "holdout"}, + ] + }, + } + task_file = tmp_path / "suite.yaml" + task_file.write_text(yaml.safe_dump(data)) + default_exp, experiment = self._make_experiment(["v1"]) + config = BatchRunConfig(run_dir=tmp_path / "runs", split="tune") + + resolved, _ = resolve_all_tasks( + task_files=[task_file], + experiment=experiment, + default_experiment=default_exp, + config=config, + ) + assert [rt.task.task_id for rt in resolved] == ["suite/row-a"] + + def _write_split_suite(self, tmp_path: Path, task_id: str, splits: list[str | None]) -> Path: + rows: list[dict[str, Any]] = [] + for i, split in enumerate(splits): + row: dict[str, Any] = {"id": f"row-{i}", "prompt": str(i)} + if split is not None: + row["split"] = split + rows.append(row) + data = { + "task_id": task_id, + "description": "Test", + "initial_prompt": "Prompt: ${row.prompt}", + "sandbox": {"driver": "tempdir"}, + "success_criteria": [{"type": "file_exists", "path": "out.txt", "description": "File"}], + "dataset": {"rows": rows}, + } + p = tmp_path / f"{task_id}.yaml" + p.write_text(yaml.safe_dump(data)) + return p + + def test_split_leaves_an_unlabelled_sibling_suite_whole(self, tmp_path: Path) -> None: + # The actual reason unlabelled datasets pass through: --split is global to the + # invocation, so a run mixing a split-labelled suite with an unlabelled one must + # filter the first and leave the second entirely alone. Asserting this at the + # resolver (not on one isolated expand_dataset call) is what proves the claim. + labelled = self._write_split_suite(tmp_path, "labelled", ["tune", "holdout"]) + unlabelled = self._write_split_suite(tmp_path, "plain", [None, None, None]) + default_exp, experiment = self._make_experiment(["v1"]) + + resolved, skipped = resolve_all_tasks( + task_files=[labelled, unlabelled], + experiment=experiment, + default_experiment=default_exp, + config=BatchRunConfig(run_dir=tmp_path / "runs", split="tune"), + ) + assert not skipped + assert sorted(rt.task.task_id for rt in resolved) == [ + "labelled/row-0", # the one tune row + "plain/row-0", # unlabelled suite survives whole + "plain/row-1", + "plain/row-2", + ] + + def test_unmatched_split_demotes_the_suite_to_a_skipped_task(self, tmp_path: Path) -> None: + # A labelled suite with no row in the requested split raises out of + # expand_dataset, but resolve_all_tasks catches ValueError and records a + # SkippedTask rather than aborting — so the run reports it in run.json's + # skipped_tasks instead of failing. Pinned because the reason string is the + # ONLY place a mistyped --split selector surfaces: nothing else distinguishes + # it from an empty run. + task_file = self._write_split_suite(tmp_path, "labelled", ["tune", "holdout"]) + default_exp, experiment = self._make_experiment(["v1"]) + + resolved, skipped = resolve_all_tasks( + task_files=[task_file], + experiment=experiment, + default_experiment=default_exp, + config=BatchRunConfig(run_dir=tmp_path / "runs", split="holdou"), + ) + assert resolved == [] + assert len(skipped) == 1 + assert "no rows in split 'holdou'" in skipped[0].reason + assert "'holdout', 'tune'" in skipped[0].reason + def test_non_dataset_task_unaffected(self, tmp_path: Path) -> None: task_file = self._write_task_yaml(tmp_path, "plain", with_dataset=False) default_exp, experiment = self._make_experiment(["v1"]) From 1aee44e997df05f898f2800e831a6b75b7430d4e Mon Sep 17 00:00:00 2001 From: uipreliga Date: Wed, 12 Aug 2026 12:07:21 -0700 Subject: [PATCH 002/144] =?UTF-8?q?feat(plugin):=202/3=20=E2=80=94=20add?= =?UTF-8?q?=20the=20optimize-skill=20skill=20and=20split-label=20the=20act?= =?UTF-8?q?ivation=20template?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/coder-eval:optimize-skill` turns an activation suite's confusion matrix into candidate description rewrites, A/B tests them as experiment variants, and promotes only what beats run-to-run noise and then survives a held-out split. Explicit-invocation only: it spends real money across three stages. Two mismeasurements were designed out rather than discovered later: - The sibling-regression gate reads the sibling's `recall.yes`, not its precision. Annexation makes the sibling's criterion expected=yes/observed=no — a false negative — and `precision = tp/(tp+fp)` stays pinned at 1.0 when the sibling never misfires, so a precision gate would gate on a constant. - Each candidate snapshots the WHOLE skills directory, siblings copied unchanged. A variant's `plugins` block replaces the task's, so the snapshot is the arm's only skill source: snapshot one skill and every sibling criterion silently observes `no` in every arm, and the description is tested against a listing it will never face. Supporting changes: the activation template gains `split_field` and per-row tune/holdout labels (both splits carry positives and distractors; no `stop_early:` — that would degrade sibling measurement); run-layout.md documents the suite-rollup path, aggregate shape, `failed_samples` as the only row-identity field, and replicate pooling — the contract that keeps someone from "simplifying" Stage B's three invocations into `--repeats 3`, which pools into one suite.json and leaves the gate nothing to read. Five shipped descriptions trimmed so seven skills fit the listing budget (1,524/1,600) without raising the ceiling: the budget is shared with every skill the user has installed, so growing our own footprint evicts theirs. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/shared/run-layout.md | 41 +++ CLAUDE.md | 2 +- README.md | 6 +- docs/PLUGIN.md | 10 +- docs/USER_GUIDE.md | 2 +- plugins/coder-eval/README.md | 8 +- plugins/coder-eval/reference/run-layout.md | 41 +++ .../reference/templates/activation-rows.jsonl | 12 +- .../reference/templates/activation.yaml | 13 + plugins/coder-eval/skills/analyze/SKILL.md | 2 +- .../coder-eval/skills/check-skill/SKILL.md | 2 +- plugins/coder-eval/skills/init/SKILL.md | 2 +- plugins/coder-eval/skills/lint-tasks/SKILL.md | 2 +- .../coder-eval/skills/optimize-skill/SKILL.md | 335 ++++++++++++++++++ plugins/coder-eval/skills/task/SKILL.md | 2 +- tests/test_custom_lint.py | 73 +++- 16 files changed, 524 insertions(+), 29 deletions(-) create mode 100644 plugins/coder-eval/skills/optimize-skill/SKILL.md diff --git a/.claude/shared/run-layout.md b/.claude/shared/run-layout.md index 57edc4a6..7ed1ff3d 100644 --- a/.claude/shared/run-layout.md +++ b/.claude/shared/run-layout.md @@ -14,6 +14,47 @@ runs/////{task.json, task.log, artifacts/} - `task.json.malformed` — present only on the docker degrade path: when an existing `task.json` fails to parse (schema skew from a stale `:latest` image, or a truncated/torn write), the docker runner moves the unparseable original aside to this sidecar and writes a synthetic `final_status=ERROR` `task.json` in its place. Diagnostic-only; `rglob("task.json")` consumers do not match it. - `task.log` — the human-readable task log; `artifacts/` — files the agent produced. +## Suite rollups (dataset-backed tasks only) + +A task carrying `dataset:` fans out into one row-task per row and additionally writes a +per-suite rollup: + +``` +runs////{suite.json, suite.md} +``` + +`` is the original (pre-fan-out) `task_id`. Nothing is written for a task +without `dataset:`. + +`suite.json` carries the suite's pass counts plus `criterion_aggregates[]` — one entry per +criterion that opted into across-row aggregation, each with: + +- `criterion_type`, and `description` (set when a task stacks several criteria of the same + type, e.g. one `skill_triggered` per skill — that is what distinguishes them); +- `metrics` — a **flat** name → float map. Classification-style criteria emit + `accuracy`, `macro_f1`, and per-label `precision.