feat(plugin): optimize-skill's execution track, measured end to end — plus the skill_triggered and --split fixes it surfaced

.claude/shared/run-layout.md

@@ -14,6 +14,82 @@ runs/<run_id>/<variant_id>/<task_id>/<NN>/{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/<run_id>/<variant_id>/<suite_id>/{suite.json, suite.md}
+```
+
+`<suite_id>` 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);
+- `rows_total` and `rows_excluded` — the denominator and what was dropped from it. A row
+  that errored before criteria ran (a timeout, say) is **excluded** rather than scored, so
+  metrics are computed over `rows_total - rows_excluded`;
+- `metrics` — a **flat** name → float map. Classification-style criteria emit
+  `accuracy`, `macro_f1`, and per-label `precision.<label>` / `recall.<label>` /
+  `f1.<label>` (for `skill_triggered` the labels are `yes` / `no`). It also carries
+  `completion_rate` — the surviving fraction of the denominator above — which is an
+  ordinary metric, so `suite_thresholds: {completion_rate: 1.0}` gates a run whose sample
+  eroded;
+- `details` — `labels`, `per_label`, `confusion`, `total_pairs`;
+- `threshold_checks` + `passed`, from the criterion's `suite_thresholds`.
+
+Alongside the aggregates, `failed_samples[]` lists failed/errored rows **by `row_id`** with
+their failure reasons, capped at a fixed limit. It is the only place in `suite.json` that
+carries row identity — `metrics` and `details` are counts only — so a consumer that needs
+to know *which* rows failed reads `failed_samples`, then falls back to the per-row
+`<variant_id>/<suite_id>/<row_id>/<NN>/task.json` for anything past the cap.
+
+**Read `metrics`; never recompute a metric from `details.confusion`.** The criterion layer
+owns that arithmetic, including its division-by-zero convention, and a consumer that
+re-derives F1 will disagree with the gate the run already applied.
+
+**Rollups pool replicates.** The grouping key is `(variant_id, suite_id)` — the replicate
+index is *not* part of it. So `--repeats N` over an M-row suite writes **one** `suite.json`
+per variant, with `rows_total: N × M` and a single pooled confusion matrix. There is no
+per-replicate metric in that file. A consumer that needs replicate-to-replicate spread must
+invoke the run N times and read N run directories; `--repeats` cannot serve that purpose.
+(Contrast `experiment.md`'s `## Paired Comparison` block, which averages replicates per row
+before pairing — there `--repeats` is exactly the right tool.)
+
+**Row-selection provenance.** `run.json` carries `row_selection` — the `split` / `max_rows` /
+`sample_per_stratum` this run executed under (`--split` / `--sample` / `--sample-per-stratum`).
+Its **absence** means the run predates the field, which is deliberately NOT the same as a
+recorded `{"split": null, ...}`: the first says nothing about what was selected, the second
+says no selector was passed on the command line.
+
+It records what was **requested on the CLI**, not the effective row set. A task's own
+`dataset.sample_per_stratum` still narrows the run without appearing here — and that draw is
+re-drawn every invocation unless `dataset.sample_seed` is pinned — so a recorded all-`null`
+selection is *not* a promise that every row ran. Read the task YAML for that half.
+
+A consumer comparing two runs must not pair different `split` values — a train run and a test
+run of one suite are two different row sets, and reporting their difference as one measurement
+is the failure this field exists to prevent. `optimize_gate.activation_gate` enforces exactly
+that: it REFUSES a pair whose arms recorded different splits, and NOTES an unrecorded one rather
+than assuming it matched. The refusal leaves `promoted` at `null` like every other activation
+verdict — `holm_promote` is what forces it to `false` and what makes the block headline
+`NOT A RESULT`; before that the headline is `UNDECIDED` and the reason prints on its own line. `measure_noise_floor` refuses to
+measure across them at all, and a floor measured over any unrecorded run dir is never cached.
+
+One caveat on `--resume`: the field records the selection of the invocation that WROTE the
+summary, so a resumed run stamps its own selectors over results produced by an earlier one. The
+run-config fingerprint warns about that drift separately; the provenance field itself does not
+distinguish the two.
+
+Scope: only `split` is compared by the gate. `max_rows` / `sample_per_stratum` are recorded but
+not gate-checked — a mismatch there shows up as a small `rows_paired` beside a large
+`rows_excluded` rather than as a refusal.
+
 **Scope-marker files** (used to detect what a given path represents):
 
 - `run.json` at the run root → **run scope**. If `experiment.json` (+ `experiment.md`) is also present → multi-variant experiment.

.github/workflows/pr-checks.yml

@@ -207,6 +207,37 @@ jobs:
         working-directory: evalboard
         run: pnpm verify
 
+  estimator-protocol:
+    # A rendered statistic can step for IDENTICAL data when an estimator or a resample
+    # count changes, and nothing in a run artifact distinguishes that from a real change
+    # in the thing being measured. So a PR touching a watched statistical constant or a
+    # rendered-number snapshot fixture must add a row to `## Estimator changes` in
+    # docs/REPORT_SCHEMA.md. Diff-based, so it cannot live in `make verify` — a working
+    # tree has no base ref — and `pull_request`-only, because without a base ref
+    # $GITHUB_BASE_REF is empty and the fetch below would turn every push red for the
+    # wrong reason. Installs nothing: the checker is stdlib-only by construction.
+    name: Estimator Change Protocol (statistical constants ↔ ledger)
+    if: github.event_name == 'pull_request'
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+        with:
+          fetch-depth: 0
+
+      - name: Set up Python 3.13
+        uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0
+        with:
+          python-version: "3.13"
+
+      # No `git fetch`: `fetch-depth: 0` already populates refs/remotes/origin/*, and re-fetching
+      # would advance origin/BASE past the merge commit this run checked out — which is how a
+      # racing PR's ledger row gets credited to this one. The checker resolves the merge base
+      # itself, so both sides of the row count share one baseline.
+      - name: Check the estimator ledger
+        run: python -m tests.lint.estimator_ledger "origin/$GITHUB_BASE_REF"
+
   plugin-validate:
     # Proves the Claude Code plugin marketplace is installable and that the suite
     # `check-skill` scaffolds is real: the manifests pass strict validation, and the

.gitignore

@@ -69,3 +69,8 @@ refs/
 
 # SkillsBench tasks for testing
 /resources/
+
+# /coder-eval:optimize-skill working tree: candidate skill snapshots,
+# per-stage experiment files and the round ledger. A snapshot is a full plugin
+# root copied per arm, so a round writes several megabytes of duplicated skills.
+.optimize-skill/

CONTRIBUTING.md

@@ -113,7 +113,11 @@ jobs that execute PR-supplied code (`quality-gate`, `no-uipath-extra`, `evalboar
 fall back to it **for fork PRs only**, since this repo is public and untrusted code
 should not run on the shared pool image — any new job running PR-supplied code needs
 the same carve-out. `action-dogfood` always uses it, because it is the executable proof
-behind the published Action and must exercise the image integrators actually use.
+behind the published Action and must exercise the image integrators actually use, and
+`plugin-validate` and `estimator-protocol` always use it too — the safe
+direction, taken unconditionally rather than only on fork PRs. (`estimator-protocol`
+installs nothing at all; `plugin-validate` installs the Claude Code CLI and the
+package, and still takes the stock image rather than the shared pool.)
 
 ## Adding Tasks or Criteria
 

README.md

@@ -107,9 +107,9 @@ results — runs inside the agent:
 /plugin install coder-eval@coder-eval
 ```
 
-That adds six slash commands: `/coder-eval:init`, `/coder-eval:check-skill`,
-`/coder-eval:task`, `/coder-eval:lint-tasks`, `/coder-eval:analyze` and
-`/coder-eval:ci`. They drive the `coder-eval` CLI, so install it too
+That adds seven slash commands: `/coder-eval:init`, `/coder-eval:check-skill`,
+`/coder-eval:optimize-skill`, `/coder-eval:task`, `/coder-eval:lint-tasks`,
+`/coder-eval:analyze` and `/coder-eval:ci`. They drive the `coder-eval` CLI, so install it too
 (`uv tool install coder-eval`). See [Claude Code Plugin](docs/PLUGIN.md).
 
 ## Use as a GitHub Action

docs/AB_EXPERIMENTS.md

@@ -350,6 +350,7 @@ if any listed metric is below its minimum.
 | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ |
 | `-e, --experiment <path\|name>`                                                                                                                                   | Experiment YAML. Bare name → `experiments/<name>.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. `train` / `test`). 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.                                                                        |

docs/DATASETS.md

@@ -124,11 +124,52 @@ Load-time errors, with their message shapes:
 | Two rows share an id | `Duplicate dataset row id for task '<task_id>': '<x>'` |
 | Both/neither `rows` and `paths` | `Dataset must specify either 'paths' or 'rows'` / `... only one of ...` |
 | `paths: []` | `Dataset.paths must be a non-empty list` |
-
-## Sampling a subset
-
-A full dataset is expensive. Two independent mechanisms cut it down, and **`--sample` wins whenever
-both apply**:
+| `--split X` on a labelled dataset with no `X` rows | `Dataset for task '<task_id>' has no rows in split 'X' (split_field='split'); labelled splits present: ['test', 'train']` |
+
+**Row-id validation runs over the WHOLE dataset, before `--split` and before either sampler.** So
+a malformed, id-less or duplicate row sitting in a split you did not select still fails the load.
+That is intended, not an oversight: "the dataset is well-formed" must not depend on what a given
+invocation happened to select. Split the checks — duplicates whole-set, malformed per-selected-row
+— and a bad row in the `test` split validates cleanly under every `--split train` run and surfaces
+only at promotion time, which is the most expensive moment to learn about it. The `Dataset row <i>`
+index therefore counts over the whole file, which is also the more useful number: it points at the
+line.
+
+## Selecting a subset
+
+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 <name>` (CLI only)** — keep only rows whose `dataset.split_field` value (default field:
+`split`) equals `<name>`. 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 train`
+  keeps just the `train` 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 aborts the run.** Expansion raises
+  `SplitSelectorError`, naming the splits that do exist, and — unlike every other dataset error —
+  `resolve_all_tasks` re-raises it rather than recording a `skipped_tasks` entry. So a mistyped
+  selector (`--split holdou`) fails loudly with a non-zero exit instead of producing a green run of
+  zero tasks. The distinction is deliberate: the other errors describe a malformed *file*, and one
+  bad task must not abort a suite; this one describes a malformed *invocation*, and the same
+  selector applies to every task in the run.
+- **A partly labelled dataset logs a WARNING** naming how many rows were dropped, because that run
+  is legitimate but is measuring a smaller suite than the file suggests. `coder-eval plan --split
+  <name>` shows the same thing before you spend anything.
+
+**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 +196,40 @@ 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.
 
+### Train and test 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 trained on:
+
+```jsonl
+{"id": "pos-1", "prompt": "review my task files", "expected_skill": "lint-tasks", "split": "train"}
+{"id": "pos-2", "prompt": "are my evals any good?", "expected_skill": "lint-tasks", "split": "test"}
+```
+
+```bash
+coder-eval run tasks/skills/activation.yaml --split train      # iterate here
+coder-eval run tasks/skills/activation.yaml --split test   # confirm here, once
+```
+
+`coder-eval plan` takes the same three selectors, so the exact invocation you are about to pay for
+can be previewed for free — it prints the selected row count, names which selector narrowed the
+set, and breaks the selection down per stratum:
+
+```bash
+coder-eval plan tasks/skills/activation.yaml --split train --sample-per-stratum 2
+```
+
+**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 train --sample 8`
+is always drawn from the train rows alone — at most eight of them, and all of them if `train` 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 test split is only worth what its independence buys: consult it to confirm
+a decision already made on `train`, not to choose between candidates, or it becomes a second train set.
+
 ## Suite-level scoring
 
 Per-row pass/fail is rarely the number you care about on a dataset — the suite metric is. Every