feat(cli): plan expands datasets and takes --split, plus four correctness fixes

[uipreliga]

PR 3 of 3 extracted from #109. Based on pr/split-execution-track (#130), not main, so
the diff above is its own slice rather than a cumulative one. Merge order is #129 → #130 → this.
Afterwards #109's remaining work is Waves 2-4 (PRs 4-12); it stays open as the reference tree.

⚠️ No CI until rebased

Gating workflows filter on the base branch, so a PR based on another PR's branch runs
no CI. This tree was verified locally against the full gate instead: 376 files already formatted, ruff check clean, pyright 0 errors, 0 warnings, 4802 passed, 0 failed.
Before merging it is rebased onto main and force-pushed, which is what triggers the
required checks. Do not merge until they are green.

This is the behaviour-carrying PR of the three. Please read the left column closely.

Review map

.github/CODEOWNERS maps * to four owners with no per-path rules, so the repo cannot route the
behaviour half of this PR to a specific reviewer. This table is the routing.

Read closely — behaviour	Skim — prose
criteria/skill_triggered.py	plugins/coder-eval/reference/optimize-method.md (new, 205 lines)
models/criteria.py	docs/tutorials/08-optimizing-a-skill-description.md (renamed)
orchestration/early_stop.py	docs/tutorials/09-…, 07-…, 04-…
orchestration/task_loader.py	plugins/coder-eval/skills/optimize-skill/SKILL.md
cli/plan_command.py	plugins/…/templates/*.yaml, tasks/skills/ci-outcome*
orchestration/experiment.py, cli/run_command.py	CLAUDE.md, docs/DATASETS.md, docs/PLUGIN.md

The four fixes

All four were found by running the optimize-skill method against this repo's own skills.

1. skill_triggered's live and final verdicts agree again (d3d0f5b). The live verdict now
   stays undecided until ToolEndEvent, because the skill body is delivered as the tool
   result — deciding at ToolStartEvent latched a verdict the final check then contradicted. This
   touches EarlyStopWatcher's per-criterion decision seam; the reason is in the docstring and
   tests/test_early_stop.py covers it. Please do not simplify it back to a global rule.
2. A --split that matches nothing aborts instead of exiting 0 (2483f5a). A labelled task
   with no matching row raises SplitSelectorError, and resolve_all_tasks re-raises rather
   than demoting it to skipped_tasks: it describes a malformed invocation applied to every
   task, not a malformed file. Previously an all-skipped run exited 0 and rendered a report over
   zero rows.
3. Every row id is validated, not just the selected ones (38efbce). Validating after
   selection meant a duplicate or malformed id in an unselected row stayed invisible until someone
   changed the selector.
4. plan expands datasets and previews what run will execute (b23b6a1). It takes all
   three row selectors, names which selector narrowed the set rather than re-deriving the
   win-order, prints a per-stratum breakdown, and warns on partial labelling and on an unseeded
   stratified draw.

Two corrections applied during the split

• The CE060/CE061 renumber is carried through this range, including the surface this range
  itself introduces: CE061_LOCATOR_FIELDS, CE061_MIN_LEAK_CHARS and
  test_ce061_exemption_list_matches_claude_md. That test binds the constant to a CLAUDE.md
  sentence in both directions, so the constant and the prose have to move in one edit — a
  half-applied rename fails it either way. PR2's collision guard is what proves the renumber
  survived this rebase.
• A shipped source comment named the wrong rule. cli/plan_command.py said the split
  convention is shared with CE035 — the branch's number for it. That file arrives in this PR,
  so PR2 could not have renamed it, and the plan's edit list did not list it. A whole-tree sweep
  found it; the file list would not have.
• _load_dataset_rows becomes the public load_dataset_rows. cli/plan_command.py imports
  it across a module boundary, and a leading underscore on a cross-module import is a claim the
  code does not honour. Six call sites, one signature, no behaviour change — pyright reports
  0 errors.

Notes for reading the diff

• docs/tutorials/08-optimizing-a-skill.md is renamed, not rewritten, to
  08-optimizing-a-skill-description.md — it optimizes a skill's description, and tutorial 09
  optimizes its body. mkdocs.yml, docs/llms.txt, docs/tutorials/README.md, docs/PLUGIN.md
  and 07-…md move with it, and CE028 gates those index surfaces. If GitHub renders the 789-line
  file as delete+add rather than a rename, try ?w=1.
• plan_command.py also imports expand_dataset and row_split_label; both were already public.
  Only _load_dataset_rows moves.

Verification

ruff format --check (376 files), ruff check clean, pyright 0 errors, 0 warnings, and
4802 passed, 0 failed — including test_ce061_exemption_list_matches_claude_md (the two-way
binding) and the collision guard from PR2, which was run first after the rebase precisely
because resolving its conflict the wrong way would have reintroduced the id collision green.

[github-actions]

Claude finished @uipreliga's task in 1m 28s —— View job

---

Code Review for PR #131

Review Checklist

• Read code review guidelines (.github/code_review.md)
• Read project conventions (CLAUDE.md)
• Examine full diff from base branch (origin/pr/split-execution-track)
• Review the four correctness fixes in detail
• Check behavior-critical files (left column in PR description)
• Verify cross-file consistency
• Perform "what's missing" analysis
• Complete design-level scrutiny
• Provide comprehensive review feedback

Starting review now...

[uipreliga]

Review: coder_eval — pr:131 (34 files) axis:1,2,3,4,5,6,7,8

Scope: pr:131 (34 files) axis:1,2,3,4,5,6,7,8 · branch pr/split-dogfood-fixes · bcd2317 · 2026-08-21T04:09Z · workflow variant

Change class: complex — introduces a new SplitSelectorError re-raised run-wide, reorders dataset id validation to whole-set, changes skill_triggered engagement semantics from an error-denylist to a success-allowlist across two tool families, and adds a CLI --split selector to plan

Architecture, typing, error handling and security are in excellent shape (9.7-10 on four axes, zero critical or high findings across all eight), so the real risk is concentrated in measurement trust rather than code health: the skill_triggered engagement gate still credits a bare path MENTION in a Write/Edit/Bash parameter and two of its three gated read tools have no test, one committed outcome suite cannot pass under default task discovery, and the partial-labelling row drop plus the CLI-sampler-blind plan preview leave the row denominator unrecorded and the pre-spend count unfaithful — fix the five items below and the harness is releasable as-is.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	9.3 / 10	0	0	1	2	Hand-rolled JSONL reader pasted at 3 new sites in tests/test_custom_lint.py (6 total), while the same file already routes one site through the runtime reader the PR just made public
2. Type Safety	9.8 / 10	0	0	0	2	plan_command's `split: str
3. Test Health	8.4 / 10	0	0	3	1	Two of the three members of the new _FILE_READ_TOOLS scoring gate (Glob, Grep) are exercised by no test
4. Security	9.8 / 10	0	0	0	2	plan's new dataset expansion routes dataset-row content into an unescaped Rich markup sink — a row containing [/red] aborts the whole plan run with a MarkupError traceback, and [link=…] markup renders spoofed console output
5. Architecture & Design	9.7 / 10	0	0	0	3	SplitSelectorError's docstring claims the CLI converts it to typer.BadParameter, but the new plan surface demotes it to a per-task file error
6. Error Handling & Resilience	10 / 10	0	0	0	0	—
7. API Surface & Maintainability	9.2 / 10	0	0	1	3	plan's dataset row preview applies no CLI sampler, so the printed "N selected" count can disagree with what run executes
8. Evaluation Harness Quality	9.4 / 10	0	0	1	1	The partial-labelling row drop reaches no persisted artifact: only logger.warning (task_loader.py:540) and a plan console print (plan_command.py:52-57) — SuiteRollup.rows_total is the KEPT count with no denominator, and RunSummary records no row selection

Overall Score: 9.5 / 10 · Weakest Axis: Test Health at 8.4 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 6 · 🔵 14 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

1. [Axis 1] Hand-rolled JSONL reader pasted at 3 new sites in tests/test_custom_lint.py (6 total), while the same file already routes one site through the runtime reader the PR just made public (tests/test_custom_lint.py:1787) — The same four-line comprehension is pasted again:

        rows = [
            json.loads(line)
            for line in (sample.parent / "ci-outcome-rows.jsonl").read_text().splitlines()
            if line.strip()
        ]

git show origin/pr/split-execution-track:tests/test_custom_lint.py | grep -c json.loads = 3; at HEAD it is 7 — the PR adds copies at lines 1787, 2469 and 2647, bringing the file to six independent JSONL readers (1723, 1787, 1956, 1975, 2469, 2647). None handles a malformed line or a non-object row, unlike the real reader. The same tests deliberately route through row_split_label "the runtime's single definition of labelled" (comment at tests/test_custom_lint.py:1727-1729) while re-deriving the reader by hand — and this PR just made task_loader.load_dataset_rows public (task_loader.py:331) for exactly this kind of consumer. Extract one module-level def _jsonl_rows(path: Path) -> list[dict] in the test file (or export task_loader._load_jsonl) and call it from all six sites.
2. [Axis 3] Two of the three members of the new _FILE_READ_TOOLS scoring gate (Glob, Grep) are exercised by no test (src/coder_eval/criteria/skill_triggered.py:52) — _FILE_READ_TOOLS = frozenset({"Read", "Glob", "Grep"}) (line 52) is a scoring-relevant closed set: membership decides whether a failed/in-flight file read counts as skill engagement, i.e. whether a row scores yes or no. git show pr-131:tests/test_skill_triggered.py | grep -n 'Glob\|Grep' returns NOTHING, and in tests/test_early_stop.py Glob/Grep appear only inside comments (lines 2197 and 2237). Every gate test uses tool_name="Read" (test_skill_triggered.py:152, 162, 184). Failure scenario: a refactor drops "Glob" from the frozenset (or misspells it); antigravity's tool map renames find_file -> Glob and search_directory -> Grep, so a FAILED antigravity glob/grep whose path parameter contains skills/<name>/ then scores yes for a skill whose body never loaded — the exact false positive the PR exists to remove — and the entire suite stays green. Parametrize the existing test_failed_read_of_a_skill_path_is_not_engagement / test_pending_read_of_a_skill_path_is_not_engagement / test_unknown_read_still_counts over sorted(_FILE_READ_TOOLS) so the set is derived rather than sampled.
3. [Axis 3] plan's new dataset preview has no test for a suite carrying dataset.sample_per_stratum: the unseeded draw makes the printed "N selected" count, the set of rows whose ${row.*} substitutions get validated, and the plan exit code all nondeterministic (src/coder_eval/cli/plan_command.py:29) — _preview_dataset calls expanded = expand_dataset(task, task_file.parent, split=split_name) (line 29) passing no sampler args, but expand_dataset still applies the YAML-level sampler: n_per_stratum = sample_per_stratum if sample_per_stratum is not None else ds.sample_per_stratum with stratum_seed = ds.sample_seed (task_loader.py:567, 572), which is deliberately unseeded by default. Verified in the PR worktree: four successive expand_dataset calls on an 8-row, sample_per_stratum: 2 dataset returned ['r1','r2','r5','r6'], ['r0','r2','r5','r7'], ['r0','r3','r5','r6'], ['r2','r3','r4','r5']. Consequences with no test: (a) line 47 prints f" [dim]Dataset: {len(rows)} rows -> {len(expanded)} selected{suffix}[/dim]" where suffix attributes the narrowing solely to --split (line 46: suffix = f" (--split {split_name})"), so YAML sampling is silently folded into a line that names a different cause; (b) line 207 validates expanded[0] — "Resolve the FIRST expanded row" — which is a RANDOM row on such a suite, so coder-eval plan can exit 0 on one invocation and 1 on the next for an unchanged repo. Every new test in tests/test_plan_command.py::TestPlanCommandDatasetPreview builds inline rows via _make_dataset_task with no sampler, so this path is untested. Add a test with Dataset(rows=..., sample_per_stratum=N) pinning the intended behaviour (preferably: plan reports the unsampled count and states that sampling is redrawn at run time, or validates all rows rather than expanded[0]).
4. [Axis 3] The "CLI seam" test for the SplitSelectorError abort reimplements the handler in-test, so its assertion is a tautology and its exit-code comment pins nothing (tests/test_dataset_expansion.py:863) — test_unmatched_split_surfaces_as_a_cli_bad_parameter (line 863) is labelled "The CLI seam", but its body is try: resolve_all_tasks(...) except ValueError as e: # exactly what cli/run_command.py does / raise typer.BadParameter(str(e)) from e (lines 874-882). It never imports or invokes run_command, so it proves only that raise typer.BadParameter raises typer.BadParameter. Its own comment asserts an exit code it never checks: "typer.BadParameter exits 2, not 1 — do not 'fix' a later assertion to 1" — there is no exit-code assertion anywhere in the test. Failure scenario: someone changes cli/run_command.py:688-689 (except ValueError as e: raise typer.BadParameter(str(e)) from e) to log-and-continue, or moves the split filter ahead of that try block; the run then goes back to the exit-0/zero-rows outcome that motivates this whole change (see .claude/harness-candidates.md's "Narrow case CLOSED" note, which credits the CLI conversion) and this test still passes. git grep -n '\-\-split' tests/ finds no CliRunner-level run test at all. Replace the in-test mirror with a CliRunner invocation of the real run command (or at minimum call the module-level function under patch) asserting a non-zero exit and the splits-present message.
5. [Axis 7] plan's dataset row preview applies no CLI sampler, so the printed "N selected" count can disagree with what run executes (src/coder_eval/cli/plan_command.py:29) — _preview_dataset calls the expander with the split only:

29:    expanded = expand_dataset(task, task_file.parent, split=split_name)

run calls the same function with all three selectors (src/coder_eval/cli/run_command.py:681-687 -> orchestration/experiment.py:630-637: max_rows=config.max_rows, sample_per_stratum=config.sample_per_stratum, split=config.split), and plan exposes no --sample / --sample-per-stratum option at all. So the line printed at

47:    console.print(f"  [dim]Dataset: {len(rows)} rows -> {len(expanded)} selected{suffix}[/dim]")

can disagree with the run in both directions. Verified on the in-repo suite tasks/skills/lint-tasks-activation.yaml (28 rows) at pr-131 HEAD:

• plan --split train -> 28 rows -> 17 selected (--split train)
• run --split train --sample-per-stratum 2 -> 8 rows executed
• run --split train --sample 5 -> 5 rows executed

The reverse (preview UNDER-reports) is reachable too, because a CLI --sample-per-stratum N overrides dataset.sample_per_stratum (task_loader.expand_dataset line n_per_stratum = sample_per_stratum if sample_per_stratum is not None else ds.sample_per_stratum): a YAML sample_per_stratum: 2 is applied by the preview, and run --sample-per-stratum 10 then executes up to 5x the previewed rows.

The claim is explicit, not incidental:

18:    Runs at the one surface that costs nothing, so a suite's resolved row count — the
19:    number every cost estimate and every A/B comparison depends on — is knowable before

and the flag help asserts parity with run:

79:            "matching `coder-eval run --split`. Tasks whose rows are all unlabelled are "

The shipped plugin depends on that number: plugins/coder-eval/skills/optimize-skill/SKILL.md:45-47 tells the user to "read the printed row count, which is the number every cost estimate below depends on".

Fix, cheapest first: (a) add --sample / --sample-per-stratum to plan and pass all three through, so the preview is the run's selection by construction; or (b) if the extra flags are unwanted, stop claiming parity — drop "matching coder-eval run --split" and the "every cost estimate" sentence, and label the number as pre-sampling (e.g. 28 rows -> 17 selected (--split train; --sample/--sample-per-stratum not applied)). Also worth stating on the same line that a run multiplies by variants and --repeats, which the printed row count does not include.
6. [Axis 8] The partial-labelling row drop reaches no persisted artifact: only logger.warning (task_loader.py:540) and a plan console print (plan_command.py:52-57) — SuiteRollup.rows_total is the KEPT count with no denominator, and RunSummary records no row selection (src/coder_eval/orchestration/task_loader.py:540) — The new mitigation is logger.warning("Task '%s': --split %r kept %d of %d rows; %d row(s) carry no %r label and were DROPPED. Every metric below is computed over the smaller set.", ...) at line 540, plus the matching console.print in cli/plan_command.py:48-53. Neither reaches an artifact a consumer parses. SuiteRollup (models/results.py:876-912) carries rows_total / rows_passed / pass_rate, where rows_total is the KEPT count only — no dataset size, no drop count. RunSummary (models/results.py:1043-1093) has skipped_tasks but no row-selection record at all (there is no models/row_selection.py and no row_selection field on this branch). The --split value itself lands on disk only in resume_fingerprint.json via compute_run_fingerprint (batch.py:470-489, config.model_dump(mode="json")), which its own docstring calls "best-effort and informational only" for the --resume drift warning and which is not part of the run.json spine in docs/REPORT_SCHEMA.md. docs/DATASETS.md (in scope) already states the hazard — "during an incremental migration it silently shrinks the suite, which moves the aggregate metrics suite_thresholds gates on" — so the gap is acknowledged but left unrecorded. FAILURE SCENARIO: the nightly runs a mid-labelling suite with --split test. Night 1, 3 of 10 rows labelled and 1 is test: suite.json records rows_total: 1, pass_rate: 1.0. Night 2, labelling finishes and 4 rows are test: rows_total: 4, pass_rate: 0.5. The external coder-eval-uipath dashboard sees a 50-point drop with nothing in either artifact distinguishing "the skill regressed" from "the denominator quadrupled because 6 rows stopped being dropped" — the explanatory log line lives in run.log on the runner, not in the record the consumer reads. FIX: add the pre-selection dataset size and the dropped-row count to SuiteRollup (e.g. rows_available / rows_dropped_unlabelled) plus a row-selection block on RunSummary, and document both in docs/REPORT_SCHEMA.md.

Nits

1. [Axis 1] Defensive scaffolding in _preview_dataset: an assert used as the runtime guard on a user-facing CLI path (and placed after its own consumer), plus an over-broad return and a second full dataset read (src/coder_eval/cli/plan_command.py:58) — Three small items in the new helper, all removable:
2. assert expanded, "expand_dataset returned no rows (the empty-dataset raise should have fired)" (line 58) is unreachable by construction — expand_dataset raises ValueError(f"Dataset for task '{task.task_id}' is empty") at task_loader.py:516 — and it sits after line 47 already consumed len(expanded), so it guards nothing its own function did not already do. It is also stripped under python -O.
3. The helper returns list[TaskDefinition] (potentially hundreds of full models) for a single call site that reads only expanded[0] (line 207). Returning the first expanded task would delete both the list and the assert.
4. The docstring at lines 30-32 explains avoiding a second expand_dataset call, but line 33 rows = load_dataset_rows(task.dataset, task_file.parent) re-reads the JSONL that expand_dataset already read one line earlier — expand_dataset could return or expose the pre-filter count instead.
   Separately, split_name = split if isinstance(split, str) else None (line 116) shapes production code around tests calling plan_command(...) directly; it mirrors the pre-existing experiment guard at line 132, but there are now two, and switching tests/test_plan_command.py to CliRunner would let both go.
5. [Axis 1] Dead split_field parameter on the new _make_dataset_task test helper restates the model default and is never overridden (tests/test_plan_command.py:320) — split_field: str = "split", has exactly one use (line 331, dataset=Dataset(rows=rows, split_field=split_field)) and no caller in the file passes it — every call site is _make_dataset_task([...]) or _make_dataset_task([...], prompt=...). It also restates a default the model already owns: Dataset.split_field is Field(default="split", ...) at src/coder_eval/models/tasks.py:311-312. Drop the parameter and the split_field= argument; Dataset(rows=rows) is equivalent.
6. [Axis 2] plan_command's split: str | None annotation is untrue at runtime and the compensating isinstance guard exists only because the tests bypass Typer, so neither pyright nor a test covers the option wiring (src/coder_eval/cli/plan_command.py:116) — The new option is annotated split: str | None = typer.Option(...) (line 73) but the declared type is a lie for direct callers, so line 116 compensates at runtime:

split_name = split if isinstance(split, str) else None

with the comment "Tests call plan_command(...) directly rather than through CliRunner, so an unpassed option arrives as a typer OptionInfo". tests/test_plan_command.py confirms this (plan_command(task_files=[task_file], **kwargs)). Pyright sees split: str | None, so the else None branch looks like dead narrowing and pyright cannot flag the next option added without a guard — the type system has been opted out of exactly where the invariant lives. Evidence the convention does not hold: two lines earlier, resolved_task_files = task_files if task_files else discover_default_tasks() (line 112) has no guard, and typer.Argument(None, ...) returns an ArgumentInfo that is truthy and not iterable (verified: bool(ArgumentInfo) is True, list(...) raises TypeError: 'ArgumentInfo' object is not iterable), so plan_command() with no arguments crashes rather than discovering tasks.

Fix: make the tests go through CliRunner, or extract a typed inner _plan(task_files: list[Path], experiment: Path | None, split: str | None) -> None that the typer-decorated wrapper calls with real values, and delete both isinstance shims. That restores the annotation as the contract and lets pyright, not a hand-written guard per option, hold it.
4. [Axis 2] New --split accepts an empty string, which the label convention makes unsatisfiable — a value that can never match any row (src/coder_eval/cli/plan_command.py:73) — split: str | None = typer.Option(None, "--split", ...) (line 73) puts no non-empty constraint on the value, while its numeric siblings do (--sample / --sample-per-stratum carry min=1 in run_command.py, and the model fields carry ge=1). "" is inside str | None but outside the domain: row_split_label maps an absent/None/"" field to None (src/coder_eval/orchestration/task_loader.py:392, return None if value is None or value == "" else str(value)), so no row's label can ever equal "". Verified against the PR head:

expand_dataset(labelled_task, split='')   -> SplitSelectorError: has no rows in split '' (split_field='split'); labelled splits present: ['test', 'train']
expand_dataset(unlabelled_task, split='') -> ['t2/a', 't2/b']   # runs everything

Because this PR promotes that raise to a run-wide abort (experiment.py:637 re-raises, run_command.py:688 turns it into typer.BadParameter), --split "" now aborts a labelled suite outright while silently running an unlabelled one whole. Fix: reject an empty/whitespace-only value at the flag (a callback= raising typer.BadParameter, or min_length=1 on BatchRunConfig.split) so a malformed selector is refused where it is typed, rather than reported as a missing split.
5. [Axis 3] The committed outcome suite's duplicate-includes scoring claim (load-bearing for the mean: 0.7 gate's variance) has no assertion anywhere (tasks/skills/ci-outcome.yaml:201) — The new second slot - "${row.expected_snippet_2}" (line 201) ships with a numeric claim at lines 190-193: "two identical entries score 0/2 or 2/2, i.e. numerically identical to a single include, so no constant sub-check is introduced and the variance the mean: 0.7 gate reads is untouched" — and nine of the ten rows in tasks/skills/ci-outcome-rows.jsonl do set expected_snippet_2 equal to expected_snippet. The claim currently holds (found = sum(1 for inc in criterion.includes if inc in content); includes_score = found / total in src/coder_eval/criteria/file_check.py:71-74, no dedup), but nothing asserts it: grep -n includes tests/test_file_check.py shows cases for 1, 2 and 5 DISTINCT includes only, never a repeated value. Failure scenario: FileCheckChecker is changed to de-duplicate or to weight includes, the nine identical pairs stop being score-neutral, and every outcome row's score shifts against an unchanged mean: 0.7 suite threshold with no test failing. Add one test_duplicate_includes_score_identically_to_one in tests/test_file_check.py asserting includes=["a","a"] scores the same as includes=["a"] for both the found and not-found cases.
6. [Axis 4] plan's new dataset expansion routes dataset-row content into an unescaped Rich markup sink — a row containing [/red] aborts the whole plan run with a MarkupError traceback, and [link=…] markup renders spoofed console output (src/coder_eval/cli/plan_command.py:228) — The new call expanded = _preview_dataset(task, task_file, split_name) (plan_command.py:187) makes JSONL row bodies reach the pre-existing sink console.print(f" [red]Error: {e}[/red]") (plan_command.py:228), because raise ValueError(f"Dataset row {i} for task '{task.task_id}' missing id_field '{id_field}': {row}") (orchestration/task_loader.py:412) interpolates the ENTIRE row dict, and SplitSelectorError's message interpolates row-derived split labels. Reproduced at PR HEAD: a single-row JSONL {"prompt": "harmless [/red] and [link=file:///etc/passwd]click-me[/link]"} with no id field gives uv run coder-eval plan tmp/t.yaml tmp/z-second.yaml -> MarkupError: closing tag '[/red]' at position 145 doesn't match any open tag, an unhandled traceback raised from INSIDE the except Exception handler, so the loop aborts and z-second.yaml is never validated (the per-file all_valid = False path is bypassed entirely). A well-formed row is enough for the spoofing half: [link=file:///etc/passwd]click-me[/link] renders as a clickable hyperlink whose visible text is row-chosen. This matters because the bundled skills (plugins/coder-eval/skills/optimize-skill/SKILL.md, check-skill) have the AGENT author dataset JSONL rows, so the content is not operator-typed. Fix: pass the untrusted half through rich.markup.escape() (or console.print(..., markup=False)) at plan_command.py:228, 221 and 224 — e.g. console.print(f" [red]Error: {escape(str(e))}[/red]"); the same escape is already used in streaming/renderers.py:103. Consider also trimming the whole-row dump at task_loader.py:412 to the row index plus its id-field keys, since that message is persisted into skipped_tasks in run.json. CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:N/A:L
7. [Axis 4] skill_triggered still credits engagement from a MENTION of skills/<name>/ in any Write/Edit/Bash parameter, so the evaluated agent can satisfy the new outcome suite's recall.yes: 1.0 engagement gate without the skill body ever loading (src/coder_eval/criteria/skill_triggered.py:118) — The rewritten gate is if not (cmd.tool_name in _FILE_READ_TOOLS and cmd.result_status in ("error", None)): (skill_triggered.py:118) followed by the scan over cmd.parameters.values() (119-121), with _FILE_READ_TOOLS = frozenset({"Read", "Glob", "Grep"}) (line 52). Everything else — Write, Edit, Bash, TodoWrite — is scanned unconditionally, and claude_code_agent.py:361 stores the tool input dict untruncated, so Write.content is scanned whole. Reproduced at PR HEAD against the real function: Write with content="# see plugins/coder-eval/skills/ci/SKILL.md" and result_status="error" -> _engaged_skill_names returns {'ci'}; Bash with command="echo skills/ci/" and result_status="error" -> {'ci'}. This PR ships tasks/skills/ci-outcome.yaml, whose gate is suite_thresholds: {recall.yes: 1.0} on skill_triggered and whose graded artifact is an agent-written .github/workflows/*.yml that plausibly contains a plugin path (the skill-source-path row's scenario is about SKILL_SOURCE_PATH), so a row can pass the engagement gate on a mention while the body never loaded — exactly the false yes the rest of this diff was written to remove. Note the resulting asymmetry: Read with result_status="unknown" counts (line 118 excludes only "error"/None) while Skill with "unknown" does not (line 89), and the module docstring's claim that 'a failed or unresolved Read/Glob/Grep loaded nothing and are not engagement' (lines 7-9) is therefore not what the code does. Fix: restrict the path scan to parameters that are plausibly READ targets (a _PATH_PARAMS allowlist such as file_path/path/pattern/command, mirroring LEAK_LOCATOR_FIELDS) rather than every string value, and make the status gate an allowlist (cmd.result_status in ("success", "unknown")) to match the Skill branch's stated allowlist-not-denylist rule. CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:L/A:N
8. [Axis 5] SplitSelectorError's docstring claims the CLI converts it to typer.BadParameter, but the new plan surface demotes it to a per-task file error (src/coder_eval/orchestration/task_loader.py:52) — The class docstring asserts one CLI behaviour - lines 51-53: A ``ValueError`` subclass so every existing ``except ValueError`` caller keeps working; ``resolve_all_tasks`` re-raises it explicitly and the CLI turns it into a ``typer.BadParameter``. That holds for run (run_command.py:688-689, except ValueError as e: raise typer.BadParameter(str(e)) from e, exit 2) but not for the second CLI consumer this PR adds: _preview_dataset's raise at plan_command.py:29 lands in the per-task-file except Exception as e: at plan_command.py:226-229, printing ✗ <file> / Error: ..., setting all_valid = False, continuing with the remaining task files, and exiting 1 - the per-task demotion lines 40-45 of the same docstring argue against for this error class, and a different exit code from run for the identical user error. The behaviour is deliberate and tested (tests/test_plan_command.py::test_plan_reports_an_unmatched_split_as_an_error_and_exits_1), so fix the contract statement: say the class is run-wide fatal on run and per-file on plan, or catch SplitSelectorError explicitly in plan_command and raise typer.BadParameter so both surfaces agree.
9. [Axis 5] The new except-branch lands inside resolve_all_tasks's 240-line, CC-37 try block instead of narrowing it (src/coder_eval/orchestration/experiment.py:637) — resolve_all_tasks spans lines 538-777 and is the tree's second-worst function by radon (E (37), tmp/code-review-260820-2109/automated/radon.txt:981 F 538:0 resolve_all_tasks - E (37)); this PR adds except SplitSelectorError: / raise at 637-641 as a fourth exception arm on a try: (line 613) that already covers load_task, the skip: true short-circuit and expand_dataset together. The god-function pre-dates this change, so the worsening is marginal, but the cheap fix keeps it from growing: pull the load+skip+expand block into a _load_and_expand(task_file, config) helper that owns the two exception policies (re-raise SplitSelectorError, demote (FileNotFoundError, OSError, ValueError, yaml.YAMLError) at 647-651), leaving the loop body to the per-file resolution it is actually about.
10. [Axis 5] A new per-criterion contract dimension (which event seam a live verdict is decidable at) is documented in prose only, next to an existing declared mechanism for the analogous dimension (src/coder_eval/orchestration/early_stop.py:425) — Lines 420-426 introduce a requirement on every future member of the live-criterion extension point: Whether a given criterion can actually decide there is **per-criterion, not global** ... A new live criterion must state which seam its verdict is decidable at. Nothing carries that statement: the sibling dimension has a real declaration point, BaseSuccessCriterion.live_decidable_polarities() (models/criteria.py:306, overridden at 1078 and 1215) which EarlyStopWatcher reads and which CE036 covers, whereas the seam is only implicit in each checker's live_verdict body. Nor can CE036's replay reach it: tests/_fixtures/live_criteria.py:33 defaults result_status="success", so no ContractCase exercises an in-flight (None) or force-closed call. Fix: either express the seam the same way the polarity is expressed (a declared method/attribute the watcher consults, so "stating it" is code rather than a docstring), or say explicitly in that paragraph that the seam is expressed only by returning undecided from live_verdict and add a contract case with result_status=None so the boundary is exercised.
11. [Axis 7] New plan --split flag never reaches the canonical CLI reference: docs/USER_GUIDE.md's coder-eval plan flag table still lists only --experiment, while the sibling run table documents all three row selectors (src/coder_eval/cli/plan_command.py:73) — The PR adds

73:    split: str | None = typer.Option(
74:        None,
75:        "--split",

but leaves docs/USER_GUIDE.md untouched (it is not in the PR's 34-file diff). At pr-131 HEAD that file's ### \coder-eval plan` — validate taskssection (line 62) carries a two-row flag table whose only entry is line 73:| `--experiment, -e` | Experiment definition YAML to resolve variants against (default: `experiments/default.yaml`). |— while therun table above it documents all three selectors on lines 45-47 (--sample N, --sample-per-stratum N, --split NAME). Nothing mechanical catches this: there is no CLI-flag/doc-parity lint rule at this HEAD (tests/lint/cli_flags.pydoes not exist inpr-131`).

Add one row to that table — | \--split NAME` | For dataset-backed tasks, preview only rows whose `dataset.split_field` value matches. ... |— and consider a CE rule that resolves everytyper.Optiondeclared name incli/to a row in the matchingUSER_GUIDE.md flag table, which would also have caught this. (docs/DATASETS.md:158-160does mentioncoder-eval plan --split , which is why this is Low rather than higher.) 12. **[Axis 7] --split help text is a second hand-authored literal in plan_command, already drifted from run_command's, with nothing pinning them together** (src/coder_eval/cli/plan_command.py:76) — This PR writes a second, hand-authored description of the same flag. plan_command.py:76-82`:

            "For dataset-backed tasks, preview only rows whose dataset.split_field value "
            "(default field: split) matches this name — e.g. --split train / --split test, "
            "matching `coder-eval run --split`. Tasks whose rows are all unlabelled are "
            "unaffected; a labelled task with no row in this split is an error naming the "
            "splits that exist."

and run_command.py:304-310, edited in the same PR:

            "For dataset-backed tasks, keep only rows whose dataset.split_field value "
            "(default field: split) matches this name — e.g. --split train / --split test. "
            "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 aborts the run with an error naming "
            "the splits that exist."

The two are ~85% duplicated prose, and the drift is already load-bearing rather than cosmetic: only the run copy carries the "Applied BEFORE --sample / --sample-per-stratum" sentence, and that omission is the same gap as finding #1. Extract the shared sentences into one module (e.g. src/coder_eval/cli/row_selectors.py) that both commands import, and pin identity with a lint rule asserting the two help= values come from the same object, so a future edit to one surface cannot describe the flag two ways. (Note for the verifier: neither src/coder_eval/cli/row_selectors.py nor rule CE065 exists at pr-131 HEAD — git show pr-131:src/coder_eval/cli/row_selectors.py reports "exists on disk, but not in 'pr-131'" and git grep -n CE065 pr-131 is empty — so this PR is what creates the duplication those later land to fix.)
13. [Axis 7] skill_name's schema description says engagement requires "a successful file read", contradicting the implementation's deliberate ungating of Bash — and it is rendered verbatim into the shipped plugin criteria reference (src/coder_eval/models/criteria.py:1210) — The field description reads:

1208:    skill_name: str = Field(
1209:        description=(
1210:            "Only count SUCCESSFUL Skill invocations whose 'skill' parameter matches this name "
1211:            "(or a successful file read under skills/<name>/)."
1212:        ),

But src/coder_eval/criteria/skill_triggered.py:46-48 states the opposite for the file-read half, on purpose:

46: # File-read tools whose FAILURE means nothing was loaded. ``Bash`` is deliberately absent:
47: # ``cat skills/x/SKILL.md | grep foo`` exits 1 after genuinely reading the file, so a status
48: # gate there would drop real Codex engagement.

and line 118 gates only Read/Glob/Grep, and only on result_status in ("error", None) — so "unknown" counts there too. Concretely: a Codex row whose Bash step is cat skills/my-skill/SKILL.md | grep nothing exits non-zero and still scores observed="yes", which a task author reading the schema would predict as "no". This is not an internal docstring: make plugin-reference renders the string verbatim into the shipped plugins/coder-eval/reference/criteria.md:260, which for plugin users is the only description of the criterion. docs/TASK_DEFINITION_GUIDE.md gets it right ("Bash is deliberately left ungated"), so only the schema SSOT is wrong. Reword to match, e.g. "...or a file read under skills// (a failed or in-flight Read/Glob/Grep does not count; Bash is ungated)."
14. [Axis 8] tasks/skills/ci-outcome.yaml is committed without skip: true even though it cannot pass against the in-repo plugin, so any full-discovery run reds and spends ~$4.30 (tasks/skills/ci-outcome.yaml:41) — The file's own header states the two blocking conditions: ci sets disable-model-invocation: true, so "mounting the plugin as-is loads no body" and "Mount plugins/coder-eval directly instead and engagement is 0/6"; and $SKILL_SOURCE_PATH must point at a hand-prepared copy with that frontmatter line stripped (sed -i '' '/^disable-model-invocation: true$/d' tmp/ci-arm/skills/ci/SKILL.md). Its skill_triggered criterion carries suite_thresholds: recall.yes: 1.0, so with the in-repo plugin (or an unset path) the gate fails, failed_suite_gates > 0, and run_command.py:561 exits 1. cli/run_helpers.py:41 discovers tasks with DEFAULT_TASKS_DIR.rglob("*.yaml"), which reaches tasks/skills/, and this file declares tags: [outcome, skills] (line 41) with no skip: key. FAILURE SCENARIO: a contributor or a nightly entrypoint runs coder-eval run with no task arguments; discovery includes this suite, the Skill call is refused on all 10 rows, recall.yes is 0, the recall.yes: 1.0 gate fails, and the run exits 1 after roughly $4.30 and ~25 minutes of agent time, with no way to pass short of the manual sed the comments document. Repo CI is unaffected — .github/workflows/pr-checks.yml uses the non-recursive tasks/*.yaml glob plus --tags smoke-* — but default discovery is not. FIX: add skip: true; models/tasks.py:423 documents that flag plus --include-skipped as exactly the opt-in-suite mechanism, and the sibling tasks/skills/lint-tasks-activation.yaml does not need it because it passes when SKILL_SOURCE_PATH=plugins/coder-eval.

What's Missing

Parallel paths:

• 🟡 plan validates only ONE row of a dataset per variant (plan_command.py:207, expanded[0]), while run validates EVERY expanded row × variant (experiment.py:665-684 loops for expanded_task in expanded_tasks: for variant in ... and calls validate_early_stop(resolved_task) on each). A criterion that only becomes invalid after ${row.*} substitution on row 7 — or a stop_early: guardrail violation that only that row's substituted criterion trips — prints "All tasks are valid!" at the free pre-spend surface and then hard-errors at resolution once money is committed, which is exactly the divergence the new preview exists to close. Either loop over expanded (cheap: resolution is in-process) or state in the printed line that only the first row was resolved. (trigger: src/coder_eval/cli/plan_command.py)
• 🟡 --split on a task with NO dataset: block is a silent no-op at both new surfaces: expand_dataset returns [task] before any split handling (task_loader.py:511) and _preview_dataset returns [task] with nothing printed (plan_command.py:26-27) — even though the same function prints an explicit "selector did not apply" note for the neighbouring unlabelled-ROWS state ((--split train: not labelled, all rows kept), line 44). The shipped skill names this as a live hazard in its own words — plugins/coder-eval/skills/optimize-skill/SKILL.md:167-169: "A task carrying no dataset: block is untouched by it, so Stage C's --split test silently re-runs the train rows and reports a confirmation" — so a held-out confirmation can be run on training data with no signal at either surface. There is also no test: test_plan_leaves_a_non_dataset_task_unannotated (tests/test_plan_command.py:399) calls _run(...) WITHOUT split=, so the state is unexercised. Print the same style of note for a dataset-less task under --split. (trigger: src/coder_eval/cli/plan_command.py)
• 🟡 plan gained --split but not the other two row selectors, so its printed "N selected" is CLI-sampler-blind while run passes all three (experiment.py:629-635); the shipped skill reads that number as the basis of every cost estimate (plugins/coder-eval/skills/optimize-skill/SKILL.md:45-47). (trigger: src/coder_eval/cli/plan_command.py) (restates: Axis 7: plan's dataset row preview applies no CLI sampler, so the printed "N selected" count can disagree with what run executes)
• 🔵 Three of the four surfaces that describe skill_triggered engagement were updated for the new "the signal must have DELIVERED" rule (models/criteria.py:1172-1215, docs/TASK_DEFINITION_GUIDE.md — which adds two explicit paragraphs — and the regenerated plugins/coder-eval/reference/criteria.md), but CLAUDE.md's own criterion table still reads "Claude's Skill tool call, or (Codex) reading the skill's files off disk" (line 168) and the tree comment still reads "(Skill tool / file read)?" (line 64), i.e. the pre-change semantics. Nothing mechanical covers this: CE030's doc/schema parity tracks TaskDefinition/RunLimits/Dataset/SimulationConfig fields, not the criterion table. (trigger: CLAUDE.md)

Tests:

• 🟡 Two of the three members of the new scoring-relevant _FILE_READ_TOOLS gate (Glob, Grep) are exercised by no test — every gate test uses tool_name="Read", so an antigravity find_file->Glob / search_directory->Grep failure that names a skills/<name>/ path could score yes with the suite green. (trigger: src/coder_eval/criteria/skill_triggered.py) _(restates: Axis 3: Two of the three members of the new FILE_READ_TOOLS scoring gate (Glob, Grep) are exercised by no test)
• 🟡 The shipped activation template now ships train/test labels on all six rows plus split_field: "split" (activation.yaml:68-70, 4/2), and the plugin instructs coder-eval run <this file> --split train (line 11), but the CI step that proves the template works OUTSIDE the source tree (.github/workflows/pr-checks.yml:299-308, "Scaffold assert") still runs only plan activation.yaml and asserts len(expand_dataset(task, Path("."))) == 6 — the unsplit count. No test or CI step anywhere exercises plan/expand_dataset with --split train against the shipped template, so a template whose labels were misspelled (trian) or dropped from one row would pass every gate and only fail in a user's repo, mid-workflow. Extend that assert with a --split train -> 4 case. (trigger: plugins/coder-eval/reference/templates/activation.yaml)
• 🟡 No test drives --split through the real run command: the test labelled "the CLI seam" re-implements run_command.py's except ValueError -> typer.BadParameter conversion inside the test body, so its pytest.raises is a tautology and the exit code its own comment argues about is never asserted. git grep -n '\-\-split' tests/ finds no CliRunner-level run invocation. (trigger: tests/test_dataset_expansion.py) (restates: Axis 3: The "CLI seam" test for the SplitSelectorError abort reimplements the handler in-test, so its assertion is a tautology)

Downstream consumers:

• 🟡 The engagement classification was tightened (a refused / in-flight / force-closed Skill call and a failed or unresolved Read/Glob/Grep now score no where they scored yes), which moves observed_label for IDENTICAL agent output — and every downstream number re-derives from that label through overlay_classification_metrics: the recall.yes: 0.7 / precision.yes: 0.7 gates in tasks/skills/lint-tasks-activation.yaml:59-61 and plugins/coder-eval/reference/templates/activation.yaml:83-85, the recall.yes: 1.0 gate in tasks/skills/ci-outcome.yaml:171-172 and outcome.yaml:189-190, and the measured recall/precision/F1 tables the two new tutorials publish as reproducible (docs/tutorials/08-*.md lines 233, 319, 344, 474, 590, 652, and its "Re-running Part 2's Stage A today measures the description this page shipped: F1 1.000" at line 760). None of those thresholds or numbers is re-derived, re-measured or annotated with the scoring version that produced it, so any A/B or tutorial re-run that straddles this commit compares two different instruments. State which committed numbers were measured under which semantics, or re-measure them. (trigger: src/coder_eval/criteria/skill_triggered.py)
• 🔵 The new Dataset: N rows -> M selected line is a per-suite, pre-fan-out count: a run multiplies it by variants and --repeats (experiment.py:695-700 fans out fan_count = n_trials if n_trials > 1 else effective_repeats), and the optimize skill's Stage B/C recipes use --repeats 3 with two or three variants (SKILL.md:662-665) while telling the user that printed number is "the number every cost estimate below depends on". Say what the number does and does not include. (trigger: src/coder_eval/cli/plan_command.py) (restates: Axis 7: plan's dataset row preview applies no CLI sampler, so the printed "N selected" count can disagree with what run executes)

Display & mapping dicts:

• 🟡 CommandTelemetry.result_status is a CLOSED domain of four values (Literal["success","error","unknown"] | None, models/telemetry.py:372) and this PR adds two more partitions of it, inline and inconsistent with the three already in the tree: the Skill branch is an allowlist (!= "success", skill_triggered.py:88) while the file-read branch is a DENYLIST (in ("error", None), line 118), so "unknown" is engagement for a Read and not for a Skill; analysis.py:55-57 buckets unknown/None together against success/error; reports_html.py:655 and command_executed.py:173 use success-vs-everything-else. A fifth status added to the Literal defaults to "counts as engagement" in the one place that decides a score. Give the closed domain one shared predicate (e.g. delivered_result(cmd)) and make the scoring branch an allowlist like its sibling. (trigger: src/coder_eval/criteria/skill_triggered.py) (restates: Axis 4: skill_triggered still credits engagement from a MENTION of skills// in any Write/Edit/Bash parameter)

Daily/nightly:

• 🟡 The PR adds a SECOND opt-in-only suite to recursive tasks/ discovery, and the Axis-8 finding on ci-outcome.yaml explicitly excuses it — but tasks/skills/lint-tasks-activation.yaml is equally unrunnable by default: it carries no skip: true, mounts path: "$SKILL_SOURCE_PATH", and its own header says "An unset or wrong path is only a warning and leaves every skill unreachable", so with that variable unexported (the default for any contributor or nightly entrypoint that uses discover_default_tasks' rglob("*.yaml")) all 28 rows score recall 0, the recall.yes: 0.7 gate fails and the run exits 1 after paying for 28 agent rows — a red run caused by ambient environment, not by an agent regression. Both new suites want skip: true (+ --include-skipped in whatever wrapper is meant to run them). (trigger: tasks/skills/ci-outcome.yaml) (restates: Axis 8: tasks/skills/ci-outcome.yaml is committed without skip: true even though it cannot pass against the in-repo plugin)
• 🟡 Nothing in the PR states the nightly / cross-repo blast radius of the two behaviour changes, and no artifact carries it either: --split and the dropped-row count reach only a logger.warning and a plan console line (never run.json / suite.json), and the scoring tightening ships with NO version bump (pyproject.toml stays 0.11.0), so run.json's framework_version — the only provenance chip in the documented schema, docs/REPORT_SCHEMA.md:52 — is identical either side of the change. The external coder-eval-uipath dashboard therefore sees a metric step (moved denominator, or a row flipping yes->no) with nothing in the record able to distinguish a real regression from an instrument change, and reports_junit.py's testcase count silently shrinks under --split for CI test-report ingestion. Say in the PR what the nightly does on the first run after merge, and persist the selection + a scoring-semantics marker. (trigger: src/coder_eval/orchestration/task_loader.py) (restates: Axis 8: The partial-labelling row drop reaches no persisted artifact)

Harness & Lint Improvements

Static checks (lint / type):

• [ruff] Enable PT012 (flake8-pytest-style, pytest-raises-with-multiple-statements) in [tool.ruff.lint] select — currently only E/F/I/N/W/UP/B/SIM/RUF + C90 are selected. Measured cost on the tree today: exactly 4 violations (tests/test_cli_empty_glob.py:34 and :74, tests/test_dataset_expansion.py:874, tests/test_logging.py:378), so this is a same-day flip, not a campaign. Optionally add PT011 (pytest.raises without match=, 34 sites) as a second, slower ratchet — the tautological test's only real assertion was a substring check done after the block, which match= would have forced inside it. Prevents: Finding A3 "the CLI seam test reimplements the handler in-test" — tests/test_dataset_expansion.py:874 is one of the 4 PT012 hits: with pytest.raises(typer.BadParameter): try: resolve_all_tasks(...) except ValueError as e: raise typer.BadParameter(str(e)) from e. A multi-statement raises block is the mechanical signature of a test that re-raises the exception it asserts, i.e. of a tautology. Still live at HEAD.
• [ce-lint] CE067 — a Typer-registered command function must not be called directly from tests/; drive it through typer.testing.CliRunner. Class-wired @pytest.mark.lint (it needs a resolved Typer app plus an AST sweep of tests/, not one .py), reusing the existing reader tests/lint/cli_flags.py, which already resolves the app's registered commands and their OptionInfos — wire it beside TestCE065RowSelectorParity in tests/lint_tests/test_lint_harness_meta.py. Detection: collect the command callables registered on coder_eval.cli.app, then flag any ast.Call in tests/** whose func resolves to one of those names. tests/test_cli_set_overrides.py, test_cli_row_selectors.py and eight other suites already use CliRunner, so the pattern is the majority one; the ~10 direct plan_command(...) call sites (tests/test_plan_command.py:77,102,128,159, tests/test_early_stop.py:1092) are the exception the rule closes. Prevents: Findings A1/A2/A6 on cli/plan_command.py — the isinstance shims that exist only because tests bypass Typer, and which make the declared annotations untrue for the one caller shape that is tested. This has gotten worse since the reviewed PR, not better: HEAD now carries four of them (plan_command.py:339 split, :340 sample, :342 sample_per_stratum, :359 experiment). CE047 already covers the assert-as-guard half of the same finding (its docstring names plan_command.py explicitly), so this is the remaining half of that class.
• [ce-lint] CE068 — every field in ROW_SELECTOR_FLAGS must declare a value-domain constraint: ge/gt for numeric fields, min_length=1 for string fields. Class-wired next to CE065 (same module, same derived source: set(ROW_SELECTOR_FLAGS) == set(RowSelection.model_fields) is already asserted there), reading RowSelection.model_fields[...].metadata. CE065's own docstring declares it "pins DECLARATION and HELP TEXT, never behaviour", so the domain is an explicit hole in the sibling rule rather than an overlap. A fourth selector then cannot land without a domain decision. Prevents: Finding A2/A7 "--split accepts an empty string, which the label convention makes unsatisfiable" — row_split_label maps "" to None, so no row's label can ever equal "", yet --split "" aborts a labelled suite and silently runs an unlabelled one whole. The numeric siblings already carry ge=1/min=1; only the str selector has no floor.
• [ce-lint] CE069 — under src/coder_eval/criteria/, a scan over a tool call's parameters must key on a declared parameter-name allowlist, not on parameters.values(). AST detection: any attribute chain ending .parameters.values() (and .parameters.items() used only for its values). The intended fix shape already exists in-tree as leak_detection.LEAK_LOCATOR_FIELDS — a named, reviewable set of path-bearing field names — so the rule points at a precedent rather than inventing one. One live site: src/coder_eval/criteria/skill_triggered.py:157. Prevents: Finding A4 "skill_triggered credits engagement from a MENTION of skills/<name>/ in any Write/Edit/Bash parameter". Write.content is stored untruncated and is scanned whole, so an agent-authored workflow file that merely names a plugin path scores observed=yes for a skill whose body never loaded — a false positive on the exact metric (f1.yes / recall.yes) the activation gate promotes on. CE054 (one result_status comparison site per criterion module) deliberately does not cover this: its docstring records the denylist-shape mirror as considered and not built.
• [ce-lint] CE070 — an in-repo task that default discovery reaches must not gate on a skill it cannot engage. For every tasks/**/*.yaml reachable by cli/run_helpers.py's DEFAULT_TASKS_DIR.rglob("*.yaml"): if it declares a skill_triggered criterion for skill S and plugins/coder-eval/skills/S/SKILL.md frontmatter sets disable-model-invocation: true, the task must declare skip: true. Class-wired over YAML + frontmatter, on the CE052/CE060 precedent (both already reason over tasks/ YAML). Derived from the shipped frontmatter, so it cannot rot into an allowlist. Live violation at HEAD: tasks/skills/ci-outcome.yaml (no skip: key anywhere under tasks/skills/; the only skip: true in the whole tree is tasks/agents/codex_disallowed_tools_test.yaml:12). Prevents: Finding A8 "ci-outcome.yaml is committed without skip: true even though it cannot pass against the in-repo plugin" — a bare coder-eval run reds after roughly $4.30 and 25 minutes, with recall.yes: 1.0 unreachable by construction. A generic "env var ⇒ skip" rule was rejected as over-broad: 10 in-repo task files reference an env var and the sibling lint-tasks-activation.yaml passes with SKILL_SOURCE_PATH=plugins/coder-eval, so the frontmatter condition is what actually separates the two.
• [ce-lint] CE071 — a module-private helper's defaulted parameter that no call site in the same module ever supplies is dead. Extend CE037's shape (no dead private helper) with a second violation class, and give it a tests/-scoped sweep: BaseRules only see src/, and this defect class lives in test fixtures. Detection is module-local AST: for each def _helper(..., p=<default>), collect calls to _helper in the file; flag p if no call passes it positionally or by keyword. Note the gap this fills: ruff's ARG family cannot see it, because the parameter is used inside the body — it is the call sites that never vary it. Prevents: Finding A1 "dead split_field parameter on _make_dataset_task" (tests/test_plan_command.py:320) — a knob that restates Dataset.split_field's own Field(default="split") and that no caller turns, which reads as coverage of a configurable field that nothing configures.
• [ce-lint] CE072 — one shared reader for dataset-row JSONL under tests/. Add tests/lint/jsonl.py::jsonl_rows(path) (or export task_loader._load_jsonl, which already raises a located error on a malformed line and on a non-object row) and forbid json.loads applied to an element of a read_text().splitlines() iterable in tests/**. AST-detectable: a json.loads call inside a comprehension whose iter is a .splitlines() call, or subscripted directly. Live at HEAD: 4 dataset-row sites — tests/lint/outcome_prompt_leak.py:84, tests/lint_tests/plugin_base.py:84, tests/lint_tests/test_lint_plugin_docs.py:162, tests/test_classification_match.py:571. Scope the rule to dataset rows: the three tests/test_litellm_cost_logger.py hits read a cost log, a different artifact (though they also drop encoding=, which the existing read_text_explicit_encoding rule would catch if its sweep reached tests/). Prevents: Finding A1 "hand-rolled JSONL reader pasted at 3 new sites (6 total)". The reviewed PR's own file has since been split, which is why the count is now 4 rather than 6 — the class survived the refactor untouched, which is the argument for a gate rather than another cleanup. Note the verifier's correction: load_dataset_rows(dataset, dir) is not the drop-in (it takes a Dataset, these sites hold bare paths); the reusable primitive is _load_jsonl(path).
• [ruff] Ratchet [tool.ruff.lint.mccabe] max-complexity from 30 to 29 now — measured: with max-complexity = 28 the tree is clean, and the single binding function is _build_argv at 28 (src/coder_eval/isolation/docker_runner.py:1279). The comment at pyproject.toml:242-243 already names that constraint; this makes the ratchet actually tight against it (the CLAUDE.md rule is "one above the worst function", and today it sits two above). Prevents: Finding A5 "the new except-branch lands inside resolve_all_tasks's 240-line try block" — partially, and the measurement matters for scoping the fix: mccabe scores resolve_all_tasks at 19, not radon's E (37), so the C901 ceiling can never fire on it at any plausible setting. The ratchet is worth taking on its own merits; the actual finding needs the statement-cap rule below.
• [ce-lint] CE073 — a try: body under src/coder_eval/orchestration/ may not exceed N statements (set N as a ratchet one above the current worst, same discipline as max-complexity). Precedent in-tree: CE022 is already a statement cap (ce022_dialog_loop_statement_cap.py), so this is that rule's shape one directory over, and it is a BaseRule (one .py, one AST) wired in tests/lint/runner.py. What it forces is the fix the finding asks for: a try: covering load_task + the skip: true short-circuit + expand_dataset under four except arms with two different policies (re-raise SplitSelectorError, demote (FileNotFoundError, OSError, ValueError, yaml.YAMLError)) has to become a _load_and_expand(...) helper that owns those policies. Prevents: Finding A5 on orchestration/experiment.py:637 — a fourth exception arm bolted onto a try block long enough that the two exception policies inside it are no longer readable together, in the tree's second-worst function by radon.
• [ce-lint] CE074 — every LiveSuccessCriterion subclass must declare the event seam its verdict is decidable at, the same way it already declares polarity. Today BaseSuccessCriterion.live_decidable_polarities() (models/criteria.py:306, overridden at 1078 and 1215) is a real declaration point that EarlyStopWatcher reads and CE036 covers; the seam is prose only (orchestration/early_stop.py:420-426). Class-wired via runtime subclass introspection over the criterion registry, beside CE036. If a declared method is judged too heavy, the honest alternative is to narrow the docstring to say the seam is expressed solely by returning undecided — but then say so, rather than requiring future authors to "state" something with no place to state it. Prevents: Finding A5 "a new per-criterion contract dimension is documented in prose only, next to an existing declared mechanism for the analogous dimension" — a requirement placed on every future member of an extension point, with nothing to carry it.
• [ce-lint] Make skill_name's schema description DERIVED from the gate constants (CE040's "derived rather than respelled" pattern, and CE042/CE062's one-declaration family): move _FILE_READ_TOOLS to a cycle-free leaf both models/criteria.py and criteria/skill_triggered.py can import, interpolate the gated tool names into the Field(description=...), and pin with a lint-marked test that the rendered description names exactly the gated tools and the ungated exception (Bash). CE033 already regenerates plugins/coder-eval/reference/criteria.md from that string, so making the description derived fixes the schema SSOT and the shipped plugin reference in one move. Claim the next free id after the above (verify first with grep -rhoE "CE[0-9]{3}" tests/ .claude/ — runner.py's uniqueness assert covers ALL_RULES only, and CE049/CE055/CE056/CE061-CE065 are reserved, CE044 retired). Prevents: Finding A7 "skill_name's description says engagement requires a successful file read, contradicting the deliberate ungating of Bash" — wrong in the one surface plugin users read, while docs/TASK_DEFINITION_GUIDE.md gets it right. A derived string cannot describe a gate the code does not enforce.
• [ce-lint] No new rule for four of the findings — they are already mechanically enforced; spend the effort on the boundaries those rules declare instead. Recorded so a green make lint is not mistaken for a proof: the Rich-markup injection finding (A4, plan_command.py:228) is CE050; the missing plan --split row in docs/USER_GUIDE.md (A7) is CE046; the duplicated/divergent --split help literal and the CLI-sampler parity gap (A7/A8) are CE065 + cli/row_selectors.py; the untested _FILE_READ_TOOLS members (A3) are now derived (tests/test_skill_triggered.py:251 parametrizes over sorted(_FILE_READ_TOOLS), and :381/:394 pin it against the Codex and antigravity tool maps). The two residual holes each rule states about itself are worth closing: CE050 never resolves a name to its value, so msg = f"[red]{e}[/red]" followed by console.print(msg) is invisible to it (add the one-hop follow in the unsafe direction, mirroring the escape(...)-assignment hop it already does); and CE065 "pins DECLARATION and HELP TEXT, never behaviour", so plan could accept --sample and ignore it — which is what the preview-parity test below must cover. Prevents: Findings A4 (markup), A7 (USER_GUIDE flag row), A7/A8 (help-text duplication, sampler parity), A3 (_FILE_READ_TOOLS coverage) — all landed since the reviewed PR. The proposal is the delta: the two declared blind spots, which is where the same defect classes will reappear.

Harness improvements (not statically reachable):

• A determinism guard on plan, and a preview-parity test that covers the YAML-declared sampler. Two assertions: (1) two consecutive plan invocations on a dataset carrying an unseeded dataset.sample_per_stratum must print the same selected count and return the same exit code; (2) extend tests/test_cli_row_selectors.py's preview-parity test — the test CE065's docstring names as the behaviour cover — with a suite whose sampler comes from YAML rather than from a flag. Fix the surface too: the count line should report the pre-sampling size (or state that sampling is redrawn at run time), since expand_dataset applies ds.sample_per_stratum with ds.sample_seed (nondeterministic by default) and validates ${row.*} over only the drawn rows. Why not static: The defect is a property of an RNG draw at run time: no AST or type check can see that two identical invocations select different rows. Reproduced by the verifier — 12 identical plan invocations on a 4-row stratum with one unsubstitutable row gave exit 0 seven times and exit 1 five times, via expand_dataset's own substitution validation reaching the outer except at plan_command.py:226-229. Prevents: Finding A3/A7 "plan's dataset preview has no test for dataset.sample_per_stratum" — a pre-spend surface whose printed row count, validated row set and exit code are all nondeterministic for an unchanged repo, and which the shipped optimize-skill SKILL.md tells the user is "the number every cost estimate below depends on".
• One cross-surface error-policy parity test for an unmatched --split, driven through CliRunner on BOTH commands, asserting each surface's documented outcome (run → non-zero via typer.BadParameter, exit 2; plan → exit 1 with the per-file error) and the splits-present message in both. Then delete the in-test mirror of the handler at tests/test_dataset_expansion.py:864-883, and fix SplitSelectorError's docstring (task_loader.py:52), which claims one CLI behaviour while the second consumer demotes it to a per-file error with a different exit code. tests/test_plan_command.py:383 is the in-repo precedent for asserting the exit code. Why not static: Both halves are behaviour: which handler catches an exception and what exit code the process returns are only observable by invoking the CLI. A lint rule can see that the docstring and the code exist; it cannot see that they disagree about an exit code. Prevents: Findings A3 ("the CLI seam test's assertion is a tautology and its exit-code comment pins nothing") and A5 ("SplitSelectorError's docstring claims the CLI converts it to typer.BadParameter, but plan demotes it") — one test closes both, and it is the test the dangling comment was written for.
• Persist the row-selection accounting into an artifact a consumer parses. Add the pre-selection dataset size and the unlabelled-drop count (e.g. rows_available / rows_dropped_unlabelled) beside SuiteRollup.rows_total / rows_excluded, document both in docs/REPORT_SCHEMA.md, and add a run-level test on a partially labelled suite + --split asserting the drop reaches suite.json. RunSummary.row_selection has landed since the reviewed PR (it records which selectors ran), so the remaining hole is the denominator: at HEAD the drop exists only as logger.warning(... DROPPED ...) at task_loader.py:560 and a plan console print. Why not static: The counts do not exist until a run expands a dataset — there is nothing in the tree for an AST check to read. CE060 covers the in-repo half only (tasks/** must be fully labelled or not at all) and its own docstring concedes the gap in the same words: "Nothing in the output says how many rows went missing." Datasets under plugins/coder-eval/reference/templates/ and in user repos are outside every CE. Once the field exists, CE030-style doc-schema parity can keep it documented — that half is static. Prevents: Finding A8 "the partial-labelling row drop reaches no persisted artifact" — the failure it describes is an external dashboard reading a 50-point pass-rate drop with nothing in either artifact distinguishing a regression from a quadrupled denominator.
• A scoring-invariance test for duplicate includes: includes=["a","a"] must score identically to includes=["a"], in both the found and not-found cases (tests/test_file_check.py currently covers 1, 2 and 5 distinct includes only). Consider also widening CE064's computed-claims surface set to task-YAML comment blocks, so a numeric claim written next to a suite gate is checked by computing it. Why not static: The claim is arithmetic over the scorer's behaviour (found / total in criteria/file_check.py:71-74, no dedup), not over text: a static check can locate the sentence but cannot evaluate whether a scoring change made it false. Prevents: Finding A3 "the committed outcome suite's duplicate-includes scoring claim has no assertion anywhere" — nine of ten rows in ci-outcome-rows.jsonl set expected_snippet_2 == expected_snippet on the stated ground that duplicates are score-neutral, and the mean: 0.7 gate's variance depends on it staying true.
• Exercise the live-criterion seam boundary in CE036's replay: add a ContractCase to tests/_fixtures/live_criteria.py with result_status=None (in-flight) and one with a force-closed "unknown" call, since the fixture defaults to "success" at line 33 and therefore no contract case reaches the seam the new prose paragraph is about. Why not static: The seam is defined by what live_verdict returns when it is handed a partial event stream; that requires replaying a stream, which no static check can synthesize. Prevents: Finding A5 on early_stop.py:425 — the runtime half of the same finding whose declaration half is CE074 above. It is also the exact status value that produced CE054's original false positive (a crash force-closing a Read to "unknown"), so the boundary has already bitten once.
• A default-discovery guard in CI: run coder-eval plan (zero-cost) over the full default discovery set — DEFAULT_TASKS_DIR.rglob("*.yaml"), which repo CI's non-recursive tasks/*.yaml glob never touches — and fail if any discovered suite is unsatisfiable with in-repo assets and not marked skip: true. Pair it with the printed cost estimate so the spend a bare coder-eval run would incur is visible in CI output. Why not static: "Can this suite pass at all?" depends on resolving template sources, mounted fixtures and environment, i.e. on running the resolver — CE070 above catches only the one shape (disable-model-invocation) that is visible in committed frontmatter. Prevents: Finding A8 on tasks/skills/ci-outcome.yaml — the discovery/CI divergence is the root cause: .github/workflows/pr-checks.yml uses tasks/*.yaml --tags smoke-*, so nothing in CI ever exercises the set a contributor's bare coder-eval run actually resolves.
• A false-positive golden for skill_triggered engagement: replay a transcript whose only skills/<name>/ mention is inside Write.content (and a second whose mention is inside a Bash command string) and assert observed="no", plus keep the existing cat skills/x/SKILL.md | grep foo case asserting yes so the deliberate Bash ungating stays covered. The two cases together are what pin the intended behaviour that CE069's shape check cannot express. Why not static: CE069 forbids the over-broad parameters.values() fan-out, but the scoring consequence — whether a given transcript yields yes or no — is a runtime property of the checker over an event stream, and it is a metric the activation gate promotes on, so it needs a value assertion rather than a shape assertion. Prevents: Finding A4 "skill_triggered still credits engagement from a MENTION of skills/<name>/" — verified live at HEAD (skill_triggered.py:157), and the same false yes the rest of that diff was written to remove.

Top 5 Priority Actions

1. Restrict the path scan in src/coder_eval/criteria/skill_triggered.py:118 to plausible read-target parameters (a _PATH_PARAMS allowlist, mirroring LEAK_LOCATOR_FIELDS) and make the status test an allowlist ("success"/"unknown"), because today a Write body or echo skills/<name>/ scores observed="yes" with the skill body never loaded — a false positive that directly satisfies tasks/skills/ci-outcome.yaml's recall.yes: 1.0 gate for identical agent output.
2. Add skip: true to tasks/skills/ci-outcome.yaml:41, since cli/run_helpers.py:41 discovers it through rglob("*.yaml") and the suite cannot pass without the hand-prepared SKILL_SOURCE_PATH copy its own header documents, so any argument-free coder-eval run fails the gate and exits 1 after roughly $4.30 of agent time.
3. Parametrize the three gate tests at tests/test_skill_triggered.py:146/156/177 over sorted(_FILE_READ_TOOLS) so Glob and Grep are derived rather than sampled — src/coder_eval/criteria/skill_triggered.py:52 is a scoring-relevant closed set that no test touches, and antigravity's find_file -> Glob / search_directory -> Grep rename (agents/antigravity_agent.py:152-154) means a dropped member reintroduces the exact false yes this PR removed, with the suite green.
4. Persist the row denominator the warning at src/coder_eval/orchestration/task_loader.py:540 only logs — add rows_available / rows_dropped_unlabelled to SuiteRollup and a row-selection block to RunSummary, documented in docs/REPORT_SCHEMA.md — so a downstream dashboard can tell a real regression from a denominator that changed when rows stopped being dropped.
5. Make plan honest about what run will execute: either accept --sample / --sample-per-stratum and pass all three selectors at src/coder_eval/cli/plan_command.py:29, or drop the "every cost estimate depends on" claim (line 19 and plugins/coder-eval/skills/optimize-skill/SKILL.md:45-47) and label the count CLI-sampler-blind; in the same pass wrap the error sinks at lines 221/224/228 in rich.markup.escape(), because one dataset row containing [/red] raises MarkupError from inside the handler and aborts validation of every later task file.

---

Stats: 0 🔴 · 0 🟠 · 6 🟡 · 14 🔵 across 8 axes reviewed.