feat(plugin): optimize-skill's execution track, measured end to end — plus the skill_triggered and --split fixes it surfaced by uipreliga · Pull Request #109 · UiPath/coder_eval

uipreliga · 2026-08-12T21:36:21Z

What

Adds Dataset.split_field + a --split row filter, ships a seventh plugin skill /coder-eval:optimize-skill, and dogfoods the whole loop in a new tutorial — which is where most of the value in this PR came from.

Implements c/2026-08-12-skill-optimize-p0-p1.md, phase by phase.

The feature

--split (P0). Label dataset rows train / test and select one at run time. 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. Unlabelled tasks pass through untouched, so --split is safe in a multi-task run; a labelled task with no matching row raises.

/coder-eval:optimize-skill (P1). A/B tests candidate edits to a skill as experiment variants, promoting only what beats run-to-run noise and then survives a held-out split. Two tracks:

Activation — the frontmatter description, measured against an activation suite. Does it fire when it should?
Execution — the skill body, measured against an outcome suite with real success criteria. Having fired, does it do the job?

Explicit-invocation only — it spends real money.

Why the two gates differ (worth a reviewer's eye)

The tracks share splits, snapshots, reachability, replicate discipline and the ledger. What differs is the instrument, and it is load-bearing:

Activation compares F1, which a pooled suite.json cannot report per replicate — hence three separate invocations.
Execution compares per-row weighted_score, which paired_comparison already computes correctly over replicates it averages per row before pairing — hence --repeats 3 on exactly two variants.

So the paired block that is mere corroboration on the activation track is the primary instrument on the execution track: tested code instead of arithmetic by hand. Unifying the gates would swap in an instrument that cannot see the metric, so both halves are pinned by sensors.

Two smaller inversions, also sensored: activation rows must never name the skill (that tests obedience, not activation) while execution rows must invoke it by slash command — /plugin:skill at the top of initial_prompt — to hold activation constant; and disable-model-invocation: true now routes to the execution track rather than hard-stopping, so init and ci are optimizable after all.

The slash form is not a stylistic preference. A disable-model-invocation skill is not offered to the model at all, so asking in prose returns "no such skill is available" and the row measures nothing; the slash form loads it, emits a real Skill tool call, and is detected by skill_triggered. Both halves verified live (see below).

Validation

Beyond the test suite, the skill itself was live-tested in three layers, because most of its failure modes are silent rather than loud:

Static — every ${CLAUDE_PLUGIN_ROOT} reference, slash command and CLI flag it emits resolves; the experiment YAML in its body validates against ExperimentDefinition.
Cold agents — two agents with no context executed the skill end to end, one per track, on a zero-run budget, building real snapshots and experiment YAML for inspection.
Live probe — one real agent run to settle whether skill_triggered detects a slash-command invocation (it does; scored 1.0).

That surfaced 15 problems, fixed in 9ac72f6. The two worth a reviewer's attention were silent confounds in the snapshot step: the diagram showed only skills/ while the prose said "everything that source contained" (a diagram-faithful snapshot mounts skills whose bundled reference/ files are missing — invisible on the activation track), and omitting .claude-plugin/plugin.json makes the namespace default to the arm's directory name, so arms would differ in the listing name as well as the text under test. Neither would have errored; both would have produced confident, wrong numbers.

It also produced one piece of guidance now in the skill: before proposing an A/B, check whether a lint rule already answers the question for zero runs. The complaint that prompted the ci test — "emits workflows that miss a step" — overlaps CE026 almost exactly, and ~300 agent runs is the wrong way to learn something a static rule catches permanently.

Three bugs caught before shipping

Two were errors in the plan, found by review; the third was already shipped on main.

The sibling-regression gate read the wrong metric. Annexation makes the sibling's criterion expected=yes, observed=no — a false negative. Since precision = tp/(tp+fp), it stays pinned at 1.0 however many requests are stolen. The gate would have been gating on a constant. Now reads recall.yes.
A one-skill candidate snapshot empties the sandbox. 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 — the sibling gate "passes" by measuring nothing. Snapshots now carry the whole skills tree.
The shipped reachability guidance was wrong (pre-existing). A local plugin path must be a plugin root holding skills/; the template told users to point at the bare skills directory, which loads nothing. Proven by probing three layouts against the real CLI. Every suite check-skill generated would have reported recall 0.0 — exactly what the template's own comment calls "reads exactly like a broken skill". Corrected in 7 surfaces, including ci, which was writing the broken path into users' CI workflows.

The tutorial (docs/tutorials/08)

Runs the loop for real against this repo's own skills, on Sonnet, and reports what happened rather than a tidied version:

lint-tasks measured at ceiling (F1 1.000 both splits), so the loop declined to spend ~224 runs chasing an unreachable number. That also closed the plan's open residual: the earlier 66-character description trim is now measured safe.
A task misfire that reproduced on both splits turned out to be 2-in-3 variance under replicates. Two agreeing runs were not evidence.
The real headroom was a sibling: analyze missed "what regressed" deterministically. Full three-stage A/B → a-regression promoted, gated on the train split (1.000 vs 0.667, non-overlapping, three invocations) and confirmed on the test split (1.000 vs 0.909).
Two comparisons were invalidated mid-run and thrown out — one candidate ranked on an eroded denominator, and a confident p = 0.038 that was a billing limit eating one arm harder than the other. completion_rate is what caught both.
Bare-name collision hazard: skill_triggered strips plugin: prefixes, and Claude Code ships its own unscoped init. A skill_name: "init" criterion would silently score a different skill. Documented in check-skill.

Notes for review

Listing budget held flat at 1,574/1,600 with a seventh skill added — five descriptions trimmed rather than raising the ceiling, since the budget is shared with every skill the user has installed.
analyze's description change is the one behavioural edit to an existing skill, and it is measured.
3 new lint sensors (mutation-tested); 1 harness gap deferred to .claude/harness-candidates.md — an all-skipped run exits 0, now reachable by a one-character --split typo. Fixing it changes exit semantics for every skipped-task path, so it wants a decision rather than a drive-by.

make verify green: 4071 passed, coverage 91.57%.

🤖 Generated with Claude Code

Late changes (after the first review pass)

Two follow-ups from using the thing, both pushed since the PR opened:

Split values renamed tune/holdout → train/test. Docs and content only — split values are open strings, so no schema change and nothing to migrate; split_field still defaults to "split". 215 replacements, followed by a pass disambiguating bare "test" back to "test split" wherever it named the data half rather than a check, since in a testing framework "without a test" parses exactly wrong. (Pedantic but recorded: nothing is trained here — you hand-select a candidate, which is a form of fitting, so "train" is defensible in the ML sense and far more recognisable than "tune".)

The skill now labels the splits itself rather than asking the user to. It previously offered to add labels, leaving a mechanical JSONL edit and a balance judgement to the reader. It now writes them and shows the counts for objection, under two rules that are easy to get wrong by eye and expensive when you do: stratify within each polarity so both halves carry positives and distractors, and assign deterministically so the split never re-rolls between rounds — a reshuffled split is not a test split, because rows already tuned against leak into it.

Not done, deliberately: a hash-based auto-split (--split train --split-ratio 0.6, membership derived from row_id, no labels in the file) would remove the last of the friction, but it conflicts with a load-bearing behaviour — unlabelled tasks currently pass through unfiltered, which is what makes --split safe in a multi-task run. Auto-splitting them would silently change that, so it needs an opt-in flag and its own tests. Left as a follow-up rather than smuggled in here.

Late changes (second pass): the execution track measured for real — and a criterion bug it uncovered

The execution track had never been run end to end. Doing that turned up a bug in skill_triggered that invalidates two claims made earlier in this description, so those are corrected below rather than quietly edited.

⚠️ Behaviour change reviewers should look at

skill_triggered no longer counts an errored Skill call as engagement (src/coder_eval/criteria/skill_triggered.py).

Any suite where a Skill call failed previously scored yes and will now score no. That is the correction — a refused call means the skill's body never loaded — but an existing green activation suite that was passing on refused calls will go red, and correctly so. The file-read signal is deliberately not gated the same way: a refused call loaded nothing, whereas a path reference means the SKILL.md was actually opened. Both cases are pinned by tests.

What the run found

An outcome suite was built to A/B ci's body: 4 arms differing only in that body, 24 rows. All four tied exactly on every criterion — which is a bug report, not a result.

ci sets disable-model-invocation: true, and the Skill tool refuses such a call outright:

<tool_use_error>Skill coder-eval:ci cannot be used with Skill tool
due to disable-model-invocation</tool_use_error>

24 of 24 calls failed this way, result_status: "error" on every one, and no row read the SKILL.md off disk either. The body never entered context; the agent answered from its own knowledge of GitHub Actions, plausibly enough that nothing downstream looked wrong. The tell was in the output the whole time:

body NOT loaded:  uses: anthropics/coder-eval-action@v1   <- does not exist
body loaded:    - uses: UiPath/coder_eval@v0             <- what the body specifies

Re-scoring those same artifacts with the fixed criterion gives 0/6 engagement per arm where the run reported 6/6.

Corrections to claims made above

"asking in prose returns no such skill is available" — wrong. Measured across the same six rows, by how often the model attempted the call: slash form alone 3/6, prose instruction alone 5/6, slash + explicit imperative 6/6. Prose works better than the slash form. Nothing in coder-eval expands a slash command; it arrives as plain text the model may ignore.
"Live probe — … whether skill_triggered detects a slash-command invocation (it does; scored 1.0)" — that 1.0 was the false positive. The call had errored.
"init and ci are optimizable after all" — true, but only with the snapshot fix: delete the disable-model-invocation: line in every arm's snapshot, which is what a real user's slash command effectively does. Verified — the same rows then score 1.000 with the action reference correct.

Also in this pass

Verdict on ci: no promotion, a verified ceiling. With the body loaded, the train split scores 1.000 on every row and every criterion (18/18 assertions, completion_rate 1.0). The three candidates were solving a problem that did not exist. Stages B and C were correctly never reached. ~55 Sonnet runs, ~$20.
The outcome-suite template (reference/templates/outcome.yaml) + a checked-in worked example (tasks/skills/ci-outcome.yaml), runnable in two documented lines.
Engagement is now actually gated (recall.yes: 1.0) in both — they described it as a gate and carried no threshold, which is how a zero-engagement run exited 0.
CE035 (partly-labelled split datasets) and CE036 (a row's prompt must not contain the string its criteria grade — four scenarios here had that leak).
Tutorial 09, written from the real numbers, plus tutorial 08 made reproducible (snapshot layout, per-stage experiment YAML, the -e command lines) and restructured into two parts.
docs/TASK_DEFINITION_GUIDE.md updated — it documented the pre-fix detection rule.

make verify green: 4074 passed, coverage 91.57%.

Third pass — the review's P0/P1/P2 fixes, then a tutorial audit

Implements c/2026-08-13-optimize-skill-review-v2-fixes.md, one commit per phase.

Two behaviour changes to look at first

skill_triggered now requires the signal to have actually delivered. The early-stop watcher evaluates on the tool call, where result_status is still None, so an in-flight or crash-force-closed Skill call live-passed while the frozen check scored it no — live and final verdicts disagreed on the one criterion that decides whether a paid round measured the skill at all. Engagement is now an allowlist: a Skill call counts only on "success" (for that tool the body is the tool result), and a Read/Glob/Grep that errored or has not resolved does not count — the path is in its parameters, but nothing loaded. Bash stays ungated, because cat … | grep exits non-zero after genuinely reading the file. Historical activation P/R/F1 is not directly comparable if a run contained those shapes; two golden-corpus entries were appended to force that acknowledgement rather than editing existing ones.

This reverses a documented design decision, so it is worth a reviewer's eye. Seven tests in tests/test_early_stop.py pinned the opposite semantics as deliberate; each is inverted, not deleted, with the reasoning in its comment, plus a Bash twin for the path that still decides on the call. The cost is stated in the test that pays it: a Skill call whose result never arrives no longer stops the run, so that run continues to its turn cap. The old behaviour stopped it and scored yes — crediting a skill whose body never reached the agent.

A mistyped --split now exits 2 instead of 0. It used to raise, get caught like any load failure, and land in skipped_tasks: one yellow line, zero evaluations, exit 0 — a CI gate reporting success for a one-character typo. SplitSelectorError separates a malformed file (still demoted, so one bad task cannot abort a suite) from a malformed invocation (re-raised — the same selector applies to every task in the run). No new CLI code was needed: the existing except ValueError -> typer.BadParameter supplies the exit.

The rest

coder-eval plan expands datasets and takes --split — prints total/selected row counts and catches a bad ${row.*} at plan time rather than per-row after the sandbox is built. The pre-spend check now costs nothing. It is also louder: a task with a missing dataset JSONL now fails a plan it previously passed.
Partial split labelling stops being silent — a WARNING with the drop count from expand_dataset, a yellow ⚠ from plan, and a check in lint-tasks for adopters CE035 cannot reach. All three call row_split_label, so the rule cannot drift even though the messages differ.
Row-id validation covers the whole dataset, before --split and sampling narrow it. A malformed row in the test half used to validate under every --split train run and surface at promotion time — the most expensive moment to learn it.
The bundled activation.yaml caps and isolates (run_limits, setting_sources: []), mirroring the checked-in worked example key for key. It preached both and shipped neither, so a user who copied it got the opposite of the advice they had just read.
CE036 gets real fixtures and a skill_name exemption. Its only test was a repo scan that passed whether or not the rule could still detect anything; detection now lives in a helper the scan and six fixtures share. CE036_LOCATOR_FIELDS is the source and CLAUDE.md the derived surface, with a both-directions parity sensor — that list had already drifted once.
reference/optimize-method.md — the track-invariant method extracted from an 859-line SKILL.md as a byte-identical move (verified against HEAD), the three named seams fixed, and the prose sensor split into procedure-vs-method sets that state which file each token belongs to and why.
Method caveats the review asked for: promotion verdicts apply to the flag-removed configuration, and Stage B now requires a predeclared primary criterion plus guardrails — scanning every per-criterion aggregate post hoc is uncorrected multiple testing in the rejection direction, so noisy criteria veto real wins.
ci-outcome's both-triggers row asked for two triggers and graded one, so a schedule-only workflow scored 1.000 — and it graded the same string as its sibling, discriminating nothing. Fixed without touching any train row, so the file's own measured "the train split scores 1.000 on every row" claim stays true by construction.

Tutorial audit

All nine checked mechanically — every path, relative link, cross-doc anchor, CLI flag against live --help, and every complete YAML snippet against the real models. All clean. By content, three defects, every one the same shape: a hand-maintained number describing a set the code derives.

"14 criterion types" (×3) against a registry of 15. CLAUDE.md's heading was right, which is why nothing noticed — it was the only guarded surface.
"six commands" in tutorial 07, omitting optimize-skill entirely. A skill-count sensor already existed; this file was not one of the four surfaces it read, and the tutorial's phrasing ("commands") was a fourth the matcher did not know.
Tutorial 09 quoting ci-outcome.yaml excerpts this PR had just changed — a reader copying the shown row would have built a suite that raises at expansion.

Each is now derived rather than asserted, and tutorial 07 joins SKILL_DOC_SURFACES.

Tutorial 08 Part 1 was re-measured rather than annotated — 62 Sonnet runs, three replicates of the train split plus one of the test split. lint-tasks still holds at F1 1.000 on a suite three rows larger. analyze reads 1.000 where Part 1 recorded 0.000, because the description this PR promotes is committed (4c7481c): the baseline is unreproducible precisely because the loop worked. And hard-3 is still unstable at exactly the 2-in-3 Step 6 measured — months later, on a different revision of the suite, which is the page's own "two agreeing runs are not evidence" lesson arriving unprompted. The page now states what a reader gets today and why it differs, instead of quietly refreshing the numbers.

Notes for review

6 new sensors + 6 CE036 fixtures, all mutation-verified — each was confirmed to fail when the property it guards is reverted.
1 harness gap deferred: "the ToolStart seam decides" is now a per-criterion property rather than a global invariant. Not mechanically detectable — it is about what a live_verdict reads, which no AST rule can infer.
No new Pydantic model, criterion, agent, config key or MergeField; one new exception type (SplitSelectorError), one module logger, one CLI option on an existing command.

make verify green: 4115 passed, 390 lint checks, coverage 91.63%.

Label dataset rows with a split (tune / holdout / …) and select one at run time with `coder-eval run --split <name>`. 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) <noreply@anthropic.com>

…activation template `/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) <noreply@anthropic.com>

…lity guidance it disproved Runs the optimization loop end-to-end against this repo's own `lint-tasks` skill and reports what actually happened, including the parts that did not go to plan. The shipped reachability guidance was wrong. Both the activation template and `check-skill` said a local plugin `path` is "the directory containing the skill's directory" — `.claude/skills`. Probing three layouts against the real CLI shows a plugin path must be a PLUGIN ROOT holding a `skills/` subdirectory (`<path>/skills/<name>/SKILL.md`; a manifest is optional, the namespace then defaults to the directory name). A bare directory of skill directories loads nothing at all. So the correct root for `.claude/skills/my-skill/SKILL.md` is `.claude`. Every suite `check-skill` generated would have reported recall 0.0 — precisely the symptom the template's own comment warns "reads exactly like a broken skill". Corrected in the template, `check-skill` and `optimize-skill`, and pinned by a new sensor. The run itself: `lint-tasks` scores F1 1.000 on both splits, so the loop stops at the diagnosis step rather than spending ~130 further runs chasing a number the gate makes unreachable. That result closes an open question from the previous commit — the 66-character description trim is now measured safe rather than assumed. The headroom turned out to be in the sibling matrix: `task` annexes setup requests on both splits, and `analyze` never fires on "what regressed". The tutorial ends on that, because a walkthrough that only works when the result is positive teaches the wrong lesson about A/B testing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ing --split Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The reachability correction reached only half its surfaces. `ci`'s workflow snippet still exported `SKILL_SOURCE_PATH=.../.claude/skills` — a shipped skill writing the broken path straight into users' CI, where the failure mode is a permanent red that looks exactly like the drift the schedule exists to detect. Same stale form in docs/PLUGIN.md and tutorial 07, plus a self-contradicting sentence in optimize-skill's own Step 8. All corrected, and the sensor now covers seven surfaces and asserts the specific layout: its previous `"skills/" in text` check passed on the pre-fix text via `.claude/skills/my-skill/SKILL.md`, so it guarded nothing. `check-skill` was never told about splits, so following it to the documented row count produced a PARTLY labelled suite — the one state where `--split` silently drops rows and shrinks the denominator the thresholds gate on. It now labels every row it writes, and optimize-skill gained the missing branch. Two rows of the new repo suite asserted `expected_skill: "init"`, a skill that sets `disable-model-invocation: true` and so can never be engaged: an unsatisfiable criterion that made every run of that suite exit 1. Relabelled to `""`, which asks the question that has an answer. Also: duplicate row ids are now rejected across the whole dataset before `--split` narrows it, so a duplicate in an unselected split can no longer validate under every split and surface only on a full run; run-layout documents `rows_total` / `rows_excluded` / `completion_rate`; and the claim that an excluded row shrinks a suite "silently" was wrong — the rollup reports it and it is gateable. Re-ran both splits after the label fix. `lint-tasks` holds F1 1.000. The `task`-annexes-setup finding, which had reproduced on both splits, did NOT survive the third run on byte-identical prompts — so the tutorial now teaches that instead, which is a better lesson about why the gate demands replicates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… two open findings Both follow-ups from the previous round are now settled by measurement rather than left as open questions. **The `task` misfire was intermittent, not phantom.** Three tune invocations show `hard-3` engaging `task` in two of three runs on byte-identical prompts. The earlier pair of agreeing runs was luck. This is the clearest argument yet for the replicate gate, and the tutorial now teaches it with the real trace. **The `analyze` gap was real, stable, and fixable.** Recall 0.500 in all three runs with precision 1.000 throughout — one row ("what regressed") missed every time, no over-claiming. The suite gained analyze rows on both splits so the gap could be measured at all, then the full loop ran: - Stage A (68 runs): three candidates, all above the incumbent. `b-results` looked competitive but lost a row to an error (completion 0.941), so its recall came from 3 rows not 4 — ranked out rather than compared. - Stage B (153 runs, three separate invocations): `a-regression` at F1 1.000 in every run against a rock-steady incumbent 0.667. Non-overlapping, no sibling regression, precision never off 1.000. One incumbent invocation dropped a row and was excluded rather than averaged in. - Stage C: not obtained. The first holdout was uninformative — a flat tie, because every regression-phrased row had been put in tune. Fresh holdout rows were authored and the re-run then hit the org's monthly spend limit, which removed 11 rows from one arm and 6 from the other and produced a confident p = 0.038 pointing the WRONG way. A p-value over an asymmetrically eroded sample is not evidence. `a-regression` is therefore promoted **gated on tune, unconfirmed on holdout** — stated plainly in the tutorial rather than dressed up. Listing budget holds at 1,574/1,600. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…d on holdout Re-ran the holdout confirmation now that budget is available. Erosion was one row against the candidate and none against the incumbent — near-symmetric, and pointing the conservative way — so unlike the spend-limited attempt this run is interpretable. incumbent analyze recall 0.833 precision 1.000 F1 0.909 a-regression analyze recall 1.000 precision 1.000 F1 1.000 The direction reproduces on rows the candidate was never tuned against, which is what Stage C is required to show. One row separates the arms, and it is one of the fresh rows authored at promotion time: an-6 "Which of my tasks got worse after I switched the model?" incumbent 1 of 3 a-regression 3 of 3 No sibling regressed in either arm; the incumbent shows the intermittent `task` misfire once more, consistent with the 2-in-3 rate measured earlier. The paired comparison reads exactly zero (p = 1.000). That is the documented limit of the block rather than a contradiction: it pairs per-row weighted_score across all three criteria, so a gain confined to one criterion on one row out of eleven is diluted below what 11 pairs resolve. F1 is the promotion metric; the tutorial now reports both and says which one the decision rested on. `a-regression` therefore ships gated AND confirmed, and the earlier "unconfirmed" framing is gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The Stage C per-row data showed `init` being engaged on the setup rows in both arms — which contradicted a comment claiming coder-eval's `init` "can never be engaged" because it sets `disable-model-invocation: true`. Both halves were wrong in an instructive way. Claude Code ships its own unscoped `init` skill, and `skill_triggered` matches on the BARE name (it strips any `plugin:` prefix), so the two are indistinguishable to the criterion. A `skill_name: "init"` criterion would have silently scored a different skill's activation as though it were the plugin's. That is a second, independent reason not to have written one — beyond the unsatisfiability already fixed. Documented where suite authors will hit it: `check-skill` (which already covers bare-name matching), the tutorial's caveat section, and the suite's own comment. A collision does not error; it measures the wrong skill. Also marks the plan file complete and ticks its 52 acceptance criteria, with a header recording the four deviations the implementation required. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-08-12T21:37:14Z

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

Code Review in Progress

I'm reviewing PR #109 following the guidelines in .github/code_review.md.

Todo List

Read .github/code_review.md for review standards
Read CLAUDE.md for project conventions
Run git diff origin/main...HEAD to see full changes
Review changed files with full context
Perform cross-file consistency checks
Analyze what's missing
Provide comprehensive review feedback

Starting review now...

… body, not just the description A skill can fail two independent ways: it never gets reached, or it gets reached and gives bad instructions. optimize-skill only measured the first. Both tracks now live in the one skill, because they share almost everything — splits, snapshots, reachability, replicate discipline, the ledger, the stop rule. What differs is the instrument, and that difference is load-bearing: `skill_triggered` is a binary one-turn probe that says nothing about the quality of the work that follows, so an activation suite cannot grade a body. The execution track uses an ordinary outcome suite with real success criteria. Three consequences worth calling out, each pinned by a sensor: - **The two gates differ on purpose.** Activation compares F1, which a pooled suite.json cannot report per replicate — hence three separate invocations. Execution compares per-row weighted_score, which `paired_comparison` already computes correctly over replicates it averages per row before pairing. So the paired block that is mere corroboration on the activation track is the PRIMARY instrument on the execution track: tested code instead of hand arithmetic. Unifying the gates would swap in an instrument that cannot see the metric. - **The prompt rule inverts.** Activation rows must never name the skill (that tests obedience, not activation). Execution rows must, to hold activation constant so the body is the only variable. - **`disable-model-invocation: true` now routes instead of stopping.** Such a skill's description never enters the activation decision, but its body still determines whether it does its job — so `init` and `ci` are optimizable after all, just not on the track that was previously the only one. Also: one variable per round (never both tracks at once), regression coverage for rows that already pass (a body edit breaks things silently, where the confusion matrix shows activation regressions for free), and trajectory-based diagnosis with the five failure modes a body edit actually addresses. Tutorial 08 now says which track it walks. Listing budget holds at 1,577/1,600. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two agents executed the skill with no prior context and a zero-run budget, one per track, building real artifacts. Between them they found 15 problems. The substantive ones: **The execution track never said how to engage the skill it routes to.** Step 2 sends every `disable-model-invocation` skill there, and Step 4 then said "name the skill in the prompt" — which does not work: such a skill is not offered to the model at all, so prose gets "no such skill is available" and the row measures nothing. Verified live, and verified the fix: a slash command in `initial_prompt` DOES load it, emits a real `Skill` tool call, and is detected by `skill_triggered` (probe scored 1.0). The routing was right; the mechanism was missing. Now specified, with a worked `initial_prompt`. **Two silent confounds in the snapshot step.** The diagram showed only `skills/` while the prose said "everything that source contained" — a diagram-faithful snapshot mounts skills whose `${CLAUDE_PLUGIN_ROOT}/reference/` files are gone, invisibly on the activation track. And dropping `.claude-plugin/plugin.json` makes the namespace default to the arm's directory name, so arms differ in the name shown in the listing as well as the text under test — on the one track where activation is a competition between listings. **A version string is not a capability check.** Two binaries reported the same 0.9.6 and differed in whether `--split` exists at all; the pinned-version rule said "carry on" while every run would fail at load. Step 1 now requires `coder-eval plan <suite>` to exit 0 before spending. **The skill writes descriptions and never mentioned the length ceiling** every natural fix pushes against. Now budgeted before candidates are written. Also: the baseline is now a line item in the cost table with its non-redundancy explained (it validates the snapshot wiring against the task's own source); `completion_rate` is documented as per-criterion-aggregate rather than top-level; the execution track's no-suite stop matches the activation track's force and carries the split-label and slash-invocation requirements into the hand-off; suite sizing resolves to a number rather than contradicting itself; Step 3 gains a "check whether a lint rule answers this for zero runs" pre-check; and repo-layout.md now prunes `tmp/` and any already-found run store, which had been returning hundreds of agent-produced SKILL.md files. Fixes two bugs this branch introduced: an off-by-one step reference from the renumbering, and tutorial 08 claiming the skill hard-stops where it now routes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…vention Tutorial 08 shipped without the YAML front-matter every other tutorial carries, and with an H1 that did not follow `# Tutorial NN — Title Case`. The front-matter is not decorative: mkdocs renders `description:` into the page's `<meta name="description">`, so the page had no search or social-preview summary at all. - adds the folded `description: >-` block, matching 01–07's shape and voice - retitles to `# Tutorial 08 — Optimizing a Skill Description` - title-cases the nav and tutorials/README entries, which were the only lowercase ones in either list - regenerates docs/llms.txt via `make docs-indexes` (verified idempotent) Audited the rest of the tree while here: 24 of 24 docs pages now carry a front-matter description; 08 was the only one missing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…, and label them for the user Terminology: `train`/`test` is the vocabulary everyone already has, and split values are open strings, so this is docs and content only — no schema change, no migration. `Dataset.split_field` still defaults to `"split"`; only the values this project's own suites and guidance use have changed. 215 replacements across 19 files, then a pass to disambiguate bare "test" back to "test split" wherever it named the data half rather than a check — in a testing framework "without a test" reads as "without a unit test", which is exactly the wrong parse. One honest imprecision worth noting: nothing is *trained* here. You hand-select a description from candidates, which is a form of fitting, so "train" is defensible in the ML sense of "the data you made your choices on" — and it is far more recognisable than "tune". Keeping the familiar word. Also removes the labelling friction that prompted this. Step 5 previously told the skill to *offer* to add labels, which left the user with a mechanical JSONL edit and the balance to get right by eye. It now does the labelling itself and shows the resulting counts for objection, under two rules that are easy to get wrong manually: stratify within each polarity so both halves carry positives and distractors, and assign deterministically so the split never re-rolls between rounds — a reshuffled split is not a test split, because rows already tuned against leak into it. Verified end to end: `--split train` -> 17 rows, `--split test` -> 11, unsplit -> 28. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`!!! note` / `!!! danger` are mkdocs-material syntax. GitHub does not understand them: it renders the marker as literal text and turns the indented body into an accidental code block, so both callouts looked broken when read from the repo or a PR diff. Tutorials 01-07 use plain `>` blockquotes, which render correctly in both places — I had introduced the admonitions without checking the convention. Converted both callouts, and added a lint guard so the next tutorial cannot reintroduce them (mutation-tested: it fires on an injected `!!! note`). Scoped to tutorials deliberately. The one admonition under docs/ is in DATASETS.md, is site-first reference material, and predates this branch. Also drops `<sub>` tags from the skill's cost formula — a SKILL.md is read as plain text by a model, so HTML there is noise rather than markup. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…table The execution track's instructions produced silently-wrong measurements. Every fix here is prose; no evaluation behaviour changes. - The outcome suite must be ONE dataset-backed task, one row per scenario. suite.json is written only for tasks the dataset expander touched (rollups group on suite_id), and --split filters dataset ROWS — so a directory of separate task files gives Stage A no rollup to rank and makes Stage C's `--split test` silently re-run the train rows. - Two consequences that decide how rows are written: criteria are copied to every row with ${row.*} substituted into every string leaf (so per-scenario assertions are parameterized, never written per scenario), and substitution never reaches sandbox.template_sources — every row shares ONE fixture, so variation lives in the prompt and the fixture must clear the skill's own hard stops or every arm ties at zero. - Step 4's worked example described a repo with no .github/, which is exactly the hard stop `ci` refuses on; it now shows a repo that clears the precondition and names its output path. - There is no --variant flag, so the arm set changes by authoring a file per stage: round<N>-triage/gate/confirm.yaml. Re-passing the triage file at Stage B/C costs (N+1)/2x the budgeted runs and renders no Paired Comparison block at all. - The paired mean diff subtracts in variant declaration order, so with incumbent declared first a candidate win reads NEGATIVE. Stated in both Stage B and Stage C. - Pin the tool policy under `defaults: agent:` and size the allowlist to the union of every arm's tools, or a candidate whose hypothesis is "use a different tool" is scored on the prohibition. - Name the per-row max_usd brake, and say plainly that an activation suite's tight caps must not be carried over — a truncated outcome row scores as a body failure that never happened. - Cost table: M_tune/M_holdout -> M_train/M_test, matching the split rename. The deletion sensor gains one entry per new instruction, each with its why, plus a counted assertion that the sign rule survives in BOTH stages and a guard that the stale cost-table symbols stay gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… track optimize-skill's execution track had no artifact to hand over: Step 4 pointed users at /coder-eval:task with a list of requirements attached, which came back half-applied. It now hands over a template, the way the activation track has always handed over activation.yaml. outcome.yaml is the execution twin of activation.yaml and carries the two constraints that are expensive to discover by running: - ONE fixture serves every row (${row.*} never reaches sandbox:), so scenario variation lives in the prompt and the fixture must clear whatever precondition the skill checks before it will act — else every arm ties at the floor and reads as three bad candidates. - Criteria are copied to every row, so per-scenario assertions are parameterized by row fields (path: "${row.expected_path}"). Its run_limits are deliberately unlike an activation suite's: an outcome row is a full task run, so max_turns is 20 rather than 2 and max_usd is the per-row brake. The run_command slot ships commented out — file_check already asserts what it would, and a row-supplied shell command would drag quoting and sandbox-tool-availability into every copy. Tests are loader-backed, mirroring the activation template's. The structural contract lives in ONE shared helper because Phase 4's checked-in sample asserts the same four properties; the threshold vocabulary is the union of a real aggregate() call and a real _attach_row_accounting() call, since completion_rate comes from the latter and a check against aggregate() alone would fail on the very template this ships. Also closes a harness gap found reviewing 1/8: nothing verified that a ${CLAUDE_PLUGIN_ROOT}/... pointer in a skill resolves to a file that exists, so the pointer at this template shipped one commit before the template did, past 344 green lint tests. Now 28 such references are checked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…issed them docs/PLUGIN.md said "All six skills read it" while seven ship, and both READMEs named three of the four skills that shell out to the CLI. The count sensor was supposed to catch the first and did not: it substring-matched raw file text, and the offending line reads "All six\n skills read it", so the newline sat exactly between the two words the check needed adjacent. 91 lint tests passed over it. - docs/PLUGIN.md: six -> seven; both READMEs name all four members of SKILLS_REQUIRING_THE_CLI in the install paragraph; both `templates/` blurbs now name the outcome suite alongside the activation suite. - Extract `_normalized()` and read every prose surface through it. The idiom was already hand-copied at three sites, all now converted. - Extract the count matcher into `_wrong_skill_count_offenders` so its wrapped-phrase self-test can run the REAL matcher against a built file. Asserting only that `_normalized()` collapses whitespace left the sensor free to be reverted with every test still green — mutation-checked: reverting it now fails the self-test, and previously did not. - New `test_cli_driving_skills_are_named_in_the_install_prose`, derived from SKILLS_REQUIRING_THE_CLI with no names in the assertion, so a fifth CLI-driving skill cannot ship with the prose still listing four. Scoped to the install paragraph, since every skill name appears somewhere in both files. Note the strengthened sensor immediately caught this commit's own first draft ("the four skills that shell out to the CLI"). Reworded rather than exempted, as the sensor is deliberately blunt about any count word next to "skills". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…orked example Stands to the content track as tasks/skills/lint-tasks-activation.yaml stands to the trigger track: a real, commented, runnable suite built from the bundled outcome template. `ci` is the subject because it sets disable-model-invocation: true (so the rows MUST use the slash form, dogfooding the mechanism), its output is a real workflow file, and it needs no extra credentials in the sandbox. 10 rows, 6 train / 4 test. Because row substitution never reaches `sandbox:`, all ten run against ONE fixture, so each row is a different REQUEST against the same repository and each names its own output path — which takes the agent's filename choice out of the measurement without hinting at the content graded. The fixture is not scenery. It carries `.github/workflows/lint.yml` because `ci` stops outright on a repo with no `.github/` (an empty fixture ties every arm at zero and reads as three bad candidates), and that workflow avoids the string `coder_eval` or `ci` takes its don't-clobber branch instead. Its eval tree sits at `evals/` rather than `tasks/`, at two depths, with one task interpolating $SKILL_SOURCE_PATH, one experiment and a version pin — each making one load-bearing body rule observable rather than unreachable. The fixture lives under templates/, NOT tasks/ as drafted: tasks/**/*.yaml is swept recursively by CE034, test_yaml_migration, test_tags and the CLI's own default task discovery, all of which load every match as a TaskDefinition — and the fixture must contain an experiment file. templates/ is the repo's existing home for sandbox template dirs and is swept by nothing. One row was smoke-run for real ($0.43, SUCCESS, skill_triggered observed 'yes', file_check resolving a file the agent actually wrote) and it paid for itself twice: - max_usd was drafted at 0.50, which the measured row came within 15% of. A slightly longer row would have aborted COST_BUDGET_EXCEEDED and scored as a body failure that never happened. Raised to 2.00. - The emitted workflow invented `anthropics/coder-eval-action@v1` and still scored 1.0. The real action reference is now asserted as its OWN criterion rather than a second `includes` entry: file_check scores found/total, so folding a constant sub-check into the gated criterion would put a fixed contribution in every row of every arm and quietly weaken `mean: 0.7` on the one dimension that discriminates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

optimize-skill calls a partly-labelled dataset "the dangerous state, because it does not look like one", and it is right: --split keeps the rows whose label matches and SILENTLY DROPS the unlabelled ones, so the run succeeds, the report renders, and every metric is computed over a smaller suite than the file suggests. Nothing in the output says how many rows went missing. That is mechanically detectable, so per CLAUDE.md it becomes a rule rather than a paragraph. - Extract `row_split_label()` in task_loader.py as the single definition of the SPLIT-FILTER convention, and rewrite expand_dataset's inline predicate to use it in one pass. Behaviour-preserving: the old `x not in (None, "")` and the new `x is None or x == ""` were checked over None/""/0/0.0/False/True/[]/{}/ b""/"0"/" "/"None"/missing-key and agree on every one, so a falsy 0 remains a real label compared via str(). - _stratified_sample keeps its DIFFERENT convention on purpose (it folds a missing key into the "" stratum, which turns an explicit None into "None") and now carries a comment naming the divergence — the old comment read as though the two agreed. - CE035 as a @pytest.mark.lint class, mirroring CE034: every tasks/**/*.yaml with a dataset: block must label every row or none. Both legal states pass; only the mixture is a finding. It keys on the CONFIGURED split_field, not the literal "split", and covers inline rows as well as paths. Four dataset-backed repo tasks are exercised for real, not skipped. Also closes a second definition of "labelled" that had survived in the harness: the two template row tests used truthiness (`r.get("split")`), which would have reported a legitimate `"split": 0` as unlabelled — the exact disagreement this phase exists to remove. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…rects the slash-form claim it disproved Ran the execution track against `ci` for real. Four baselines on --split train, ~24 runs, ~$9. Stages A/B/C were NOT spent: the round never cleared its own wiring check, and the method says a baseline that cannot be trusted is a stop, not a starting point. Full ledger in .optimize-skill/ci/history.json. Engagement never reached 1.0: 4/6, 4/5, 3/6, 4/6 across four runs, failing on DIFFERENT rows each time. Three silent slips, all observed: - the model answers the slash command by dispatching a sub-agent, which reads the skill in the child so no Skill call reaches the parent stream; - it ignores the command and does the work itself, emitting no Skill call; - the scenario's wording routes it to a sibling — "so we find out if a skill quietly stops triggering" sent one row to check-skill instead of ci. At that rate 20-50% of every arm would measure the absence of the thing under test, and Stage B's own promotion rule ("the skill actually engaged on every scored row") could not have been satisfied. Cost avoided: ~84 runs, ~$36. On rows where `ci` DID engage the weighted score is 1.000 — a ceiling. The emitted workflows carry the per-depth globs with their rationale, the real UiPath/coder_eval action, the version pin, extra-args, both runtime prerequisite steps and both hardening lines. So both no-go conditions hold at once. The round's real output is a correction to shipped guidance. SKILL.md and the bundled template both asserted the slash form "loads it, emits a real Skill tool call, and is detected by skill_triggered" — presented as reliable. Measured at 50-80%. Both surfaces now say so, name the three failure modes, and make engagement a GATE on the baseline rather than an afterthought. They also record that skill_triggered counts READING the skill's SKILL.md as engagement, not only a Skill call — one row reported engaged while the command it issued named a different skill. Suite and template fixes the run paid for: - `Skill` was missing from allowed_tools though it is the mechanism under test; it worked anyway, which is why the omission was invisible. - disallowed_tools: [Agent, Task] — an allowlist cannot suppress delegation, since those stay available whatever allowed_tools says. This one setting moved engaged-row scores from 0.333 to 1.000: with delegation on, the sub-agent did the work WITHOUT the skill and every row scored engagement only. The tool policy was not a detail, it decided the result. - turn_timeout 300 -> 900: 300 turned a row into an ERROR, which is excluded from the aggregate and surfaces only as completion_rate < 1.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… a null Written from the run in 5/8, which produced no promotion. Tutorial 08 already tells a "measure first, then decline to spend" story on the activation track; this is its execution-track counterpart, and the reason it stopped is more useful than a promotion would have been. The page covers the outcome suite's shape (one dataset-backed task, rows as scenarios), the one-fixture rule and why the fixture must clear the skill's own preconditions, criteria copied per row with row-parameterized assertions, split labelling, reachability, and the three checks to run before reading any score. Then the go/no-go, which was no-go on two grounds at once: engagement never reached 1.0 (4/6, 4/5, 3/6, 4/6 across four runs, failing on different rows each time), and where `ci` did engage the score was already 1.000. The sharpest exhibit is the same suite either side of one setting. With sub-agent delegation available, every row scored 0.333 and the emitted workflows named an action that does not exist, used the recursive glob the body forbids, and misspelled the score-floor input. Denying delegation moved engaged rows to 1.000. The tool policy was not a detail; it decided the result. It also carries the parts of the mechanics the round never reached but a reader still needs: the snapshot layout as a full plugin root (from the real incumbent snapshot, including why plugin.json is a trap), the per-stage experiment files, and the paired mean diff's sign — it subtracts in variant declaration order, so with incumbent declared first a candidate win reads negative. Every figure traces to a preserved artifact; the cost line is the measured $7.47, not an estimate. Two lint rules caught the draft and both were answered by rewriting rather than exempting: the recursive-glob guard (which is blunt on purpose, since integrators copy-paste out of docs) and CE026's runtime-prereq check on the emitted-workflow excerpt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…le it The page reported ~350 runs of A/B results without ever showing how the arms were wired: no snapshot layout, no experiment_id, no variants block, no `-e` invocation. A reader could follow the reasoning and not reproduce any of it. Adds, for the `analyze` round it already describes: - the .optimize-skill/analyze/1-<slug>/ snapshot tree, with the siblings and .claude-plugin/plugin.json, and why each is load-bearing — without the manifest the arms compete under different namespaces, and since a variant's plugins block REPLACES the task's, a snapshot missing its siblings makes every sibling criterion observe `no` in every arm; - the experiment YAML that mounts each snapshot by absolute path; - the exact command lines for Stages A, B and C, plus the rule that no flag selects a subset of an experiment's variants, so each stage needs its own file. Restructures into Part 1 (`lint-tasks`, a ceiling result and when to stop) and Part 2 (`analyze`, a full A/B that promotes), with the shared machinery under Reference. `lint-tasks`'s test-split confirmation previously sat between `analyze`'s Stage B and Stage C, so the page alternated subjects mid-argument; it now closes Part 1. Every finding survives verbatim — the recall-0.0 wiring trap, the 2-in-3 misfire, the billing-limit p = 0.038 artifact, and the uninformative test split. Now that the commands are shown, collapsing Stage B's three invocations into `--repeats 3` is a one-line edit, so a sensor pins it: three distinct --run-dir values, no --repeats in the fenced commands, and the warning itself retained. Scoped to the code fences, since the section legitimately says "not --repeats 3" in prose. Also corrects "the other six skills" to seven — tutorials are not in SKILL_DOC_SURFACES, so no count sensor covered that line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Cross-phase review (two external models plus an Opus reader over the whole diff) found three defects living in the seams between phases, where no single-phase review could see them. HIGH — the tool-policy instruction was shadowed by the artifacts shipped alongside it. SKILL.md said to declare allowed_tools/permission_mode in the experiment's `defaults: agent:`, but those fields merge by REPLACE and the task layer outranks experiment defaults — so the template and the sample, both of which declare them on the task, silently override it. Following the instruction was a no-op, and it broke precisely the case the paragraph exists to enable: widening the allowlist so a candidate whose hypothesis is "use a different tool" is scored on the instruction rather than the prohibition. All three surfaces now say to declare it on the suite, and note that a variant can still override deliberately. HIGH — four of the ten scenarios in the checked-in sample stated the behaviour they graded ("listed explicitly rather than with a recursive wildcard" scoring the explicit glob; naming the skill-source requirement and scoring its passthrough). An arm whose body DELETED that rule would still have scored 1.0, because the prompt supplied it — the answer-leak this repo's own task rubric and lint-tasks flag. Rewritten to describe the situation and leave the method to the body. Tutorial 09 now carries the caveat that its reported figures predate the fix, which makes its ceiling reading softer rather than firmer. MEDIUM — the template still shipped max_usd: 0.50, the cap a measured $0.43 row came within 15% of, while the sample had been raised to 2.00 from that same evidence; a comment claiming allowed_tools is a claude-code field when it is on BaseAgentConfig (only setting_sources needs the discriminator); SKILL.md listing "keep Skill in allowed_tools" as an engagement mitigation when tutorial 09 measured that it works either way; and SKILL.md conflating the two cap failures — max_turns exhaustion scores low (a fabricated body failure) while a timeout ERRORS and is excluded, surfacing only as completion_rate < 1.0. Test harness: the fixture depth assertion counted evals/experiments/ as a depth, so deleting the file it was entirely about still passed (mutation-verified fixed); _normalized's docstring asserted a repo-wide invariant eight sensors do not follow; _outcome_metric_vocabulary was documented as universal while derived from file_check alone, which would have failed the next gate the docs ask for. Also records why the fixture lives under templates/ rather than beside its suite, and names reference/templates/ in CLAUDE.md's plugin inventory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… grade Closes the harness gap behind one of the final review's High findings. Four scenarios in the checked-in outcome sample stated the behaviour they graded, so an A/B arm whose body DELETED that rule would still have scored 1.0. This repo's task rubric and the `lint-tasks` skill both call that out — for a USER's files. Nothing applied it to this repository's own, so it shipped. Guards the verbatim form: a criterion's content-asserting strings must not appear in the prompt the row expands to. Location fields (path, agent_file, command) are exempt, and that distinction is the rule — a prompt MAY say where to write, which takes the agent's filename choice out of the measurement without revealing anything graded; it may not say what the artifact must contain. Scope is stated honestly in the docstring: this cannot catch a SEMANTIC leak, where the prompt describes the graded behaviour in different words. That is what actually happened here and it needs a reader. Guarding the blunt form is still worth it — it is the easy mistake and it is silent. Mutation-verified: injecting `minimum-task-score` into a scenario that grades it fails the rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`--split` / `--sample` / `--sample-per-stratum` had no test that started at the command line. `tests/test_dataset_expansion.py` hand-builds `BatchRunConfig(split=...)` and `tests/test_plan_command.py` passes `split=` as a Python kwarg — both begin downstream of the two hops where the wiring can break silently: 1. `run_command` -> `_run_all_tasks`, where the parameter is RENAMED (`sample` -> `max_rows`); and 2. `_run_all_tasks` -> `BatchRunConfig`, where a dropped keyword leaves the selector at its `None` default and runs the whole suite while the report still claims the subset the user asked for. Both are now mutation-verified: deleting `split=split` at run_command.py:424 fails three of these tests, deleting it at :501 fails one. Also pins the documented `SplitSelectorError` -> `typer.BadParameter` conversion — the thing that stops a mistyped selector producing a green run over zero rows — including its exit code 2 and the fact that the message still names the splits that do exist. The patch targets `coder_eval.orchestration.experiment. resolve_all_tasks` because `_run_with_experiment` imports it lazily inside its own body, so the `cli.run_command` attribute does not exist. Test-only: no production file changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The three dataset row selectors were three flat fields on BatchRunConfig and nowhere else. A train run and a test run of the same suite therefore produced byte-indistinguishable artifacts, so anything downstream pairing two run dirs was comparing two different row sets and reporting the difference as one measurement. They become one model, `models/row_selection.py::RowSelection`, embedded by BatchRunConfig (the request) and by RunSummary (the record). The two defaults differ on purpose and the difference is load-bearing: a config always HAS a (possibly empty) selection, while `RunSummary.row_selection is None` means NOT RECORDED — a run predating this field — which is deliberately not the same as a recorded `split: null`. `coder-eval aggregate` carries the value over from a prior run.json, degrading to None on a malformed one rather than aborting. RowSelection deliberately declares NO extra="forbid". It nests under a container that does and one that does not, and reports read run.json back from directories possibly written by a NEWER coder-eval — a forbid here would turn a future fourth selector into a hard parse failure of the whole report instead of an ignored key. The config-side typo risk is covered by pyright at the single construction site. Rendered in run.md (`**Rows**:`) and as per-variant JUnit <properties>, both gated on `requested`, so a full-suite run and an unrecorded run emit exactly what they did before. The field records what was REQUESTED ON THE CLI, not the effective row set — a task's own dataset.sample_per_stratum still narrows a run without appearing here — and both run-layout.md copies say so. NOTE FOR run.json CONSUMERS (coder-eval-uipath / eval-runner, evalboard): this adds one additive top-level key, `row_selection`. Unknown keys inside it are ignored, and no existing key changes shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`plan` is sold as the pre-spend preview of a run, but it offered only `--split` while `run` offered three selectors — so two thirds of the ways a run is scoped could not be previewed at all. Worse, its accounting line named whichever selector was SET rather than whichever one narrowed the set, so a reduction caused by a task's own `dataset.sample_per_stratum` was reported as `(--split train)`. Both come from the same root: the selector precedence was expressed once in `expand_dataset`'s body and restated, badly, in the preview. It now has ONE declaration — `task_loader.select_rows`, returning a `RowSelectionOutcome` carrying the selected rows and the causes that actually removed one. `plan` prints those causes verbatim and never re-derives the win-order. `--sample 99` over four rows names nothing, because it narrowed nothing. `expand_dataset`'s signature and return type are unchanged (it delegates to `expand_dataset_with_selection`), so every existing caller compiles untouched. `_validate_row_ids` deliberately stays outside the selection: a malformed row in an unselected split must still fail the load. Also: a per-stratum breakdown grouped through the sampler's own rule, now extracted as `task_loader.stratum_key` — a preview that invented its own grouping would print counts for strata the sampler does not draw from; a warning when an unseeded stratified draw means `run` will execute this many rows but not necessarily these ones; and the three help strings shared via `cli/row_selectors.py`. Ships CE043, which pins that both commands declare the same selectors and describe each with the same help-string object (`is` identity, so an inlined copy is caught). Its docstring states what it does not catch. Stratum values are user data and are now `rich.markup.escape`d: an `expected_skill: "[/dim]"` previously raised MarkupError, which the per-task handler swallowed into a non-zero exit on a valid suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…st run `activation_gate` takes two arms from SEPARATE `coder-eval run` invocations, so they can genuinely have scored different row sets. Nothing checked. A train arm gated against a test arm produced a confident interval, a p, and a promotion decision about a difference that is not an effect — the two arms never saw the same rows. It now reads each run root's `run.json` before any bootstrap and REFUSES the pair outright, with every statistic None. That makes it the second `gate_refusal` setter on this track, and the two are told apart by the p rather than by a new field: a discreteness refusal is a statement about the suite's RESOLUTION and is only reachable inside holm_promote's `p_value is not None` branch, while a wiring refusal says no comparison was made and never carries one. Hence the renderer's fifth headline, NOT A RESULT, and the guard that stops the outside-the-family note printing under a refusal. Missing provenance is a NOTE, never a refusal: old run dirs stay gatable, and the one state where a cross-split pair is undetectable must not also be the one state that says nothing. `execution_gate` only notes its split — it takes ONE run_dir holding both arms, so a mismatch is unrepresentable there, and that asymmetry is stated in code. `NoiseFloor` gains `split`, DERIVED from the run dirs rather than passed (a defaulted `split=` would reintroduce this bug on the first snippet that forgot it). It joins the cache key for free — `_floor_key` reads `model_fields` — and on the shipped outcome.yaml template it is the ONLY key field that differs between the train and test measurements. A floor measured over any provenance-less run dir carries `UNRECORDED_SPLIT` and is never written; a floor cached BEFORE this field existed is skipped rather than served, because its `split=None` default would assert "no --split was passed" about a measurement that may have used one. Also fixes a message that was wrong in exactly one state: a zero-discordant refusal said "adding rows cannot change it", which is false for an all-negative subset where adding POSITIVE rows is the fix. That state now gets its own note, and `--split` is how it arises in practice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…re-measured `skill_triggered._YES` was a second `"yes"` literal beside `models.TARGET_LABEL`, which the optimize gate reads as `f1.yes`. Two equal constants agree today and diverge silently the moment either moves — the gate would then report an F1 for a class the criterion had stopped emitting. It now imports the one declaration. The dependency points criterion -> models because `models/optimize.py` is a cycle-free leaf the gate imports and `models` cannot import `criteria`. Also states the blast radius the rule carries, in the docstring of the module that owns it: requiring the skill body to have been DELIVERED re-baselines every suite authored before it DOWNWARD, since traces that scored `yes` for an errored `Skill` call now score `no`. A suite from before that rule fails without anything in the report saying the CHECKER moved rather than the skill, and `framework_version` is the only attribution a trend line across the boundary gets. RE-MEASURED both suites on claude-sonnet-4-6, 2026-08-15, and NO threshold moved: lint-tasks-activation 28 rows x 2 recall.yes 1.000 precision.yes 1.000 ci-outcome 10 rows x 2 runs recall.yes 1.000 mean 1.000 The backlog predicted these had drifted below their gates. They had not. Recorded as the negative result it is, with full provenance above every suite_thresholds block — measured value, date, model, row and replicate counts, run dir — because a threshold with no measurement beside it is the state this phase exists to end. Two things worth knowing for the next re-measure. `ci-outcome` CANNOT be measured by mounting plugins/coder-eval directly: `ci` sets `disable-model-invocation: true`, so the Skill tool refuses the call and every row scores 0 — a void run, not a regression. It needs a prepared copy with that line stripped, as the suite's own header documents. And ci-outcome's `recall.yes` is deliberately NOT lowered to 0.90 even though `measured - 0.10` sits below it: the re-baseline rule exists to stop a stricter checker flipping a gate red, this gate is green, and 0.90 over ten rows would admit a round where one row never engaged the skill — the mixture that gate exists to reject. That reasoning is recorded beside the number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A CI gate that runs the TRAIN rows scores a skill partly on its own training data, and drifts optimistic exactly as the skill improves — the direction that hides a regression rather than revealing one. The `ci` skill emits workflows that users copy into their own repos, so it saying nothing about `--split` produces a fleet of gates quietly measuring the wrong half of every split-labelled suite. It now tells the agent to grep the discovered datasets for a split label and pass `--split test` through `extra-args`, with the reason stated, plus the two cases that are not errors: an unlabelled suite is unaffected, and `--split test` on a labelled suite with no test row aborts loudly, which is intended. `docs/USER_GUIDE.md`'s `plan` flag table listed only `--experiment`. It was already missing `--split`, which `plan` has shipped for a while, so this adds three rows rather than the two Phase 3 created — and says that `plan` previews exactly what `run` selects, which CE043 enforces. `docs/DATASETS.md` states the row-id validation behaviour B6 asked us to document: it runs over the WHOLE dataset, before `--split` and before either sampler, so a malformed row in a split you did not select still fails the load. That is intended — "the dataset is well-formed" must not depend on what an invocation selected, or a bad row in `test` validates under every `--split train` run and surfaces at promotion time. Two prose sensors, both mutation-verified to fail when their sentence is deleted. The USER_GUIDE one parses table ROWS rather than substrings: every selector is also named in its siblings' prose, so a token test stayed green after the row documenting a flag was removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…d it `extra="forbid"` on a container does not propagate into a nested model, and CE038 exists to catch exactly that. It scanned only models reachable from `coder_eval.models` — so `BatchRunConfig`, which declares `extra="forbid"` and now nests the deliberately-lenient `RowSelection`, was the rule's precise shape sitting outside its reach. The rule stayed green because of WHERE the container lives, not because of anything about the container. Adds an explicit `EXTRA_ROOTS` list and an `EXEMPT` map that stores the REASON a pair is deliberate rather than just its name, plus two companion tests: one failing if a listed root stops forbidding extras (a root that checks nothing), one failing if an exemption stops describing a real violation (a stale licence the next lenient model would inherit silently). Verified the extension is not vacuous: clearing EXEMPT makes CE038 fail on `BatchRunConfig.row_selection -> RowSelection`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Deferred rather than guarded: catching it needs a notion of the PREVIOUS fingerprint schema that nothing in the tree carries, which is a design question rather than a test. Recorded with the reproduction so the next reader does not have to rediscover it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Five real findings the per-phase reviews could not see, because each is a disagreement BETWEEN phases. 1. The all-negative note fired on the wiring path. `any()` over an empty iterable is False, so a mistyped `criterion_index` — which scores nothing on either arm and already has its own note — also got "no row expects or observes 'yes', check expected_skill and your --split". One block, two contradictory remedies, on the commonest wiring error this gate has a dedicated message for. It also falsified the note's own justification: with no pairs `n_discordant` is None, so the zero-discordant path does NOT refuse and the "already refused anyway" argument does not hold. Guarded on there being pairs, with a test. 2. `run.md` printed "**Rows**: subset", the exact claim `RowSelection.requested`'s docstring forbids reading into that field — `--sample 99` over four rows requests a sample and narrows nothing. Now "requested". 3. `plan` and `run.md` described one invocation two ways, and `plan` had regressed: a selector that was HONOURED but removed nothing printed nothing at all, so the line became byte-identical to passing no selector and a user could no longer confirm the flag was read. `plan` now reports the narrowing causes when there are any and the requested selectors otherwise. USER_GUIDE's claim that parity is "enforced by a lint rule" is corrected: CE043 pins the flag set and help text; the row sets are pinned by the preview-parity test. 4. The cross-split refusal said the arms recorded "DIFFERENT row selections" while comparing only `split`. `max_rows` / `sample_per_stratum` are recorded in the same object and not compared. Message narrowed to what it checks, with the gap and the reason stated where the check lives. 5. Both layering docstrings still said `optimize_gate` imports "exactly two names" from `optimize_store`. It imports three. CLAUDE.md and the confining test were updated in phase 4; the modules' own statements of their boundary were not. Plus: the threshold provenance now records that both suites were measured with NO --split (the ci skill tells users to gate on `--split test`, which those numbers do not cover); the run-layout mirrors no longer state the post-Holm `promoted=False` as if the gate produced it directly, and note the --resume and sampler-scope caveats. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Recorded rather than fixed: widening the preflight beyond `--split` is a behaviour change the phase was not scoped for, and it needs a decision about whether an intentional `--sample` difference should be gateable at all. The refusal message and a comment beside the check now state the scope, so the gap is documented rather than claimed away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`computed_claims.evaluate_expression` ended in `case _: return lhs / rhs`, so widening `_ALLOWED_OPS` by one line would have made the one sensor class whose purpose is catching arithmetic that lies compute a cost cell with DIVISION and report it as true. The wildcard now raises and CE044 pins the parity between the whitelist and the dispatch. `test_existing_history_json_is_left_alone` guarded on `.optimize-skill/`, which is gitignored — so it passed locally and skipped in CI, every time, taking a whole-package AST scan with it. That half now runs unconditionally, and CE045 reports any test that skips itself on a path no clone has. Its scanner resolves the guarded path back through a local assignment: the real subject's condition is `not history.exists()` and carries no string literal at all, so the naive version of this rule is blind to the only thing it was written for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

CE046 extends the CE030 doc-parity family from model fields to CLI flags: every long `--flag` a visible command declares must reach `docs/USER_GUIDE.md`. Four were absent — `--version`, `--junit-xml`, and `_run-task-internal`'s two. The last pair needs no exemption list, because deriving the scan set from Typer's `hidden=True` leaves "this command is not part of the user surface" declared exactly once, in `cli/__init__.py`. The decl is split on `/` before matching: `--preserve/--no-preserve` arrives as one unspaced string while the guide writes it spaced, so a raw-substring test would fail on a correctly documented flag and demand an edit making the guide worse. CE047 forbids a bare `assert` under `cli/`, where `python -O` turns argument validation into no validation at all — the shape `plan_command.py` shipped and a later plan removed without a guard. Scoped to one directory on purpose: `src/` holds 77 asserts and nearly all are internal invariants, so the rule states that count in its boundary and a test recomputes it. `long_flags` / `help_for` move to `tests/lint/cli_flags.py`, where CE043 now reads them too — two rules reading `param_decls` two ways is the drift CE043 exists to prevent, one level up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…olds it `model_copy(update={...})` does not check the update's KEYS. A mistyped one is set as a bare instance attribute: absent from `model_dump()` entirely, with the field it was meant to set left at its default, nothing raised and nothing logged. `extra="forbid"` does not help — that governs validation, and `model_copy` skips validation by design. Both optimize-gate verdicts were written that way, on the models whose entire job is to say what a promotion decision rests on, where a `mean_dif` typo renders a block reporting no difference at all. `copy_with(model, /, **updates)` raises on an unknown field name and CE048 forbids the bare call shape in `src/`. All 21 call sites converted, with one documented exemption (`agent_judge` — a dict variable from user YAML plus `deep=True`). The first parameter is positional-only because `NoiseFloor` has a field literally named `model`. Four of those sites were invisible to the single-line grep this work was scoped from, because they wrap across lines — one of them `holm_promote`'s main promotion write. Two claims are stated rather than assumed. `**updates: object` means pyright checks nothing at the call site, so the runtime raise is the whole enforcement; literal keywords buy source visibility, not a static check. And the exempt site genuinely does still drop keys — `AgentConfig` is a four-way union while the judge's defaults are always claude-code — which is now recorded as a behaviour question rather than waved away in a comment. The verdict pin reads `__dict__`, not `model_dump()`: under `extra="forbid"` a stray key never serializes, so a dump-based check cannot fail even with the defect present. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The blocked-render assertion `"ACCEPT" not in block.replace("DO NOT ACCEPT", "")` was vacuous on its own fixture: the headline there is CANNOT COMPARE, so the strip removed nothing and the absence assertion could not fail while reading as a strong guard. Both headlines are now asserted as PRESENT on the input that produces each — by first line, not via `_headline`, which returns the first `**`-prefixed line and would raise on an `###`-headed block. `early_stop.py` claimed `command_executed`'s verdict is fully determined by the call's inputs. It is not, under `require_success` — the configuration CE034 mandates for an armed, live-passable one: `_matching_commands` drops the in-flight call, whose `result_status` is None, and ToolEnd decides. The claim appeared twice in that module, the second copy line-wrapped past the grep that found the first, plus once in the backlog; all three corrected, and the module docstring now defers to the method's rather than restating it. The optimize-skill snippet sensor asserted only that the imported names EXIST, so a renamed keyword argument was invisible to it — the tokens stay present and the snippet raises in the user's terminal after three invocations are paid for. It now parses every fence and binds each call's keywords against the real signature. Also records that the shipped-template SHA-pin finding was investigated and refuted: that file is a graded eval fixture mounted into a sandbox, GitHub never runs it, and editing it would move an A/B baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`optimize-method.md` promises that `execution_gate` reports candidate minus incumbent whichever order the experiment declared its variants in, and says why that single behaviour is the reason the gate exists: a reversed reading promotes the arm that lost, and every subsequent number in the ledger corroborates it. Nothing checked that the sentence was still true of the code. `TestExecutionGateSign` pins the behaviour and no sensor pins the prose, so the file could have been reworded into a lie about a gate that still worked — or left intact over a gate that had stopped. This claim runs the gate under both declaration orders and asserts the sign, the agreement and the interval order, beside asserting the sentence is still there. Both halves are needed; neither test replaces the other, and both say so where a future reader would delete one as redundant. Its self-test drives the real claim function against a monkeypatched sign-blind gate, rather than asserting something that cannot fail — which is the defect this whole plan started from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A confidence interval can move 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. `docs/REPORT_SCHEMA.md` tells consumers to recompute CIs from `per_replicate_scores`; this adds the section that tells them when the recomputation's answer stepped, and a pull_request-only job that demands a row whenever a PR moves a watched constant or modifies a pinned rendered-number fixture. It is a row COUNT across the merge base, not a touched-file test — the ledger lives in a busy page, where a typo three sections away would otherwise clear the gate. Both watch lists are pinned by parity tests, because the failure that matters here is not a false red but a silent green: a renamed constant or a moved fixture directory makes the scan match nothing forever, and `git diff --name-only` reports only a rename's post-image path. The fixture half deliberately covers `optimize_renders/` as well as `report_snapshots/`. The module's stated blind spot is an estimator FORM change — `bootstrap_p_floor` went from `1/m` to `2/(m+1)` once already — and that floor is rendered into those fixtures, so watching them is what keeps the boundary from being total. The ledger's last column is a PR number rather than a version or a bare SHA: semantic-release assigns the version at merge, and main is squash-merged, so a branch SHA does not survive. The seeded row records `b306a99`'s real step, both intervals it moved, marked as the pre-squash branch commit it is. `parse_markdown_tables` moves to `tests/lint/markdown_tables.py` on this, its second consumer — stdlib-only, because the job installs nothing, which a subprocess import under `-S` now pins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A mutation review of the six new sensors reverted each guarded thing and re-ran. Every sensor fired except one: CE046 matched a flag as a bare substring, so it could not fail for `--sample` — a prefix of the real `--sample-per-stratum`, which satisfied it on its longer sibling's behalf. Measured: deleting every mention of `--sample` from the guide left the check green. Matching at a word boundary closes it, and a test now pins that exact blind spot on the real guide. The estimator protocol watched six statistical constants and missed two live ones in the same file: `FLOOR_RESOLUTION`, which decides whether an MDE counts as measurable at all and therefore whether the execution gate REFUSES, and `NEAR_FLOOR_MULTIPLE`, which gates the resolution-floor note. Both are watched now, and a test binds the documented list to the code so the two cannot drift. Four claims this plan itself introduced were not true. `plugin-validate` does not "install nothing" — it installs the Claude Code CLI and the package. The ruff `external` list is inert; ruff accepts an unlisted noqa code silently. The ledger module's boundary quoted a regex its code does not use. And the verdict-copy pin's docstring claimed it notices a call site leaving the seam, when it notices only a call site that leaves the seam AND gets a key wrong. CE047's docstring count is no longer an equality anchor: an assert added anywhere in `src/` would have turned `make lint` red for a rule scoped to `cli/` and never touched. The claim that matters is the bound, plus the exact fact that `cli/` itself is clean. Three backlog entries had been falsified by this very plan and still read as current — a "live instance" of a vacuous assertion that Phase 4 fixed, a cross-reference to CE046 before Phase 2 spent that number, and a closed entry whose superseded design conclusion ("a rule that flags the CALL is wrong") was left standing next to the rule that flags the call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`runner.py`'s import-time uniqueness assert covers `ALL_RULES`, and roughly a third of the CE rules are not `BaseRule`s at all — CE026-CE031, CE033-CE036, CE038-CE039 and CE043-CE046 are `@pytest.mark.lint` classes, because their subject is Markdown, YAML, a resolved Typer signature or the whole `src/` tree rather than one `.py` AST. Those ids live only in a class name, so a class-wired rule could claim a number a `BaseRule` already had and nothing would fail. A `# noqa` keys on the id string, so one suppression would quietly disarm both. The subject is the `TestCE<NNN>` class names, where every rule surfaces: a BaseRule has one testing it and a class-wired rule is one, so two rules claiming one number appear as two classes claiming it. This run's own near-miss is why it is worth the ten lines: a backlog entry had reserved "CE046" for a different rule, Plan D Phase 2 spent that number on the CLI-flag rule, and the only thing that noticed was a reviewer reading prose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

uipreliga

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

Scope: pr:109 (120 files) axis:1,2,3,4,5,6,7,8 · branch feat/plugin-optimize-skill · fa69cdf · 2026-08-16T17:17Z · workflow variant

Change class: complex — introduces a new statistical promotion-gate subsystem (optimize_gate/optimize_store/reports_optimize, ~3.8k new src lines) plus row-selection semantics, criterion-label changes and 6 new lint rules; correctness requires reasoning about bootstrap estimators, Holm correction, refusal conditions and split provenance

Architecturally this is a strong codebase — 8.9/10 overall, with error handling, tests, types and security all ≥9.3 and the new decision/presentation/storage split of the optimize gate genuinely pinned by layering tests — but the real risks are concentrated in two places: one live harness regression where an armed skill_triggered criterion now latches a fail on the very tool call that would satisfy it (flipping score and final_status for identical agent output), and a cluster of silent-wrong-number paths in the new promotion gate (negative criterion indices, stale-run-dir split provenance, a promotion veto that only exists in the renderer) sitting inside a 3010-line module with eight D-grade functions; fix the harness regression and the input-guard bugs before merge, and treat the remaining optimize_gate decomposition and the zero-coverage CE041 lint rule as fast-follow debt.

Summary

Axis	Score	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	7.7 / 10	0	4	3	Copy-pasted logic and diagnostic text duplicated across sites in optimize_gate.py despite the module's own factoring helpers
2. Type Safety	9.3 / 10	0	1	2	CE041 never consults `node.level`, so relative `from ..models import` is invisible — the rule catches 0 of the 8 `**`-splat sites in src/, all of which use the relative form
3. Test Health	9.4 / 10	0	1	1	Changed behaviour shipped without a direct test (plan dataset-row resolution, copy_with boundaries, widened replicate glob)
4. Security	9.4 / 10	0	1	1	Proposer prompt spec (`reference/proposal-prompt.md`) directs the proposer to read agent trajectories with no untrusted-data framing; the skill's one such sentence is confined to its execution-track section and never reaches the activation track or this file
5. Architecture & Design	8.4 / 10	1	1	1	optimize_gate.py ships as a 3010-line, six-concern module with eight D-grade (CC>20) functions
6. Error Handling & Resilience	9.9 / 10	0	0	1	A negative criterion index is silently accepted as a Python negative index and grades the LAST criterion instead of failing loudly
7. API Surface & Maintainability	8.8 / 10	0	2	2	`coder-eval plan` prints ✓ and then ✗ for the SAME task file when dataset expansion fails
8. Evaluation Harness Quality	7.9 / 10	1	2	1	skill_triggered is no longer decidable at ToolStart, so an armed instance with `decide_within: N` latches an effective FAIL on the very Skill call that would satisfy it — score/final_status changes for identical agent output

Overall Score: 8.9 / 10 · Weakest Axis: Code Quality & Style at 7.7 / 10
Totals: 🔴 0 · 🟠 2 · 🟡 12 · 🔵 12 across 8 axes.

Blockers

[Axis 5] optimize_gate.py ships as a 3010-line, six-concern module with eight D-grade (CC>20) functions (src/coder_eval/optimize_gate.py:1743) — The PR's own three-way split (decision / presentation / storage) is real and well-pinned, but the residue is still 6x the Axis-5 god-module anchor (">500 lines mixing 3+ concerns"). Measured on the working tree at PR HEAD: wc -l = 3010; grep -c '^def \|^class ' = 57; uv run radon cc -s -n C reports 13 functions at C or worse, 8 of them D (_load_and_pair D(30) @1104, _execution_diagnostics D(29) @2195, holm_promote D(28) @1646, execution_gate D(27) @1881, cost_latency_guardrails D(24) @382, _sibling_checks D(23) @1007, activation_gate D(23) @1242, holm_promote_execution D(22) @2425) against a whole-repo radon average of B (5.56). The module carries exactly ONE structural marker for all of it — line 1743-1745, # ---... / # The execution track's gate - the same decision, on the reporter's own statistic - so 1742 lines above it are an unsegmented run of 40 functions. The seams are already visible and non-overlapping: (a) the whole execution track, lines 1748-2510 (763 lines: _completion_rates, _integrity_checks, execution_gate, _execution_diagnostics, holm_promote_execution) shares nothing with the activation track but _balance_pair, _row_score, cost_latency_guardrails and the two Holm note constants; (b) the three fronts, lines 2778-3010 (_finite_scores, _dominates, pareto_front, instance_best_front, CostQualityPoint, cost_quality_points, cost_quality_front) operate only on ArmRowScores and import nothing from either gate; (c) the search loop + leak preflight, lines 2513-2777 (_row_score, arm_row_scores, candidate_leaks, SearchComparison, lineage_head_scores, search_compare); (d) run-directory loading, lines 139-196 (TASK_JSON_GLOB, _task_json_pattern, load_suite_rows, _pool, load_arm_rows); (e) noise-floor measurement, lines 676-920. Split at least (a) into optimize_gate_execution.py and (b)+(c) into optimize_fronts.py / optimize_search.py on the exact precedent reports_optimize.py and optimize_store.py already set in this PR, and extend the existing layering test (tests/test_optimize_gate.py:810 _OPTIMIZE_MODULES) to cover the new members so the edges stay pinned. Note this is a structure finding, not a correctness one: every import edge CLAUDE.md claims was verified and holds (gate does NOT import reports_optimize; gate imports exactly {UNRECORDED_SPLIT, UNRESOLVED_MODEL, lookup_noise_floor} from optimize_store at line 66; optimize_store imports only coder_eval.models at line 33; reports_optimize's runtime coder_eval imports are only models + reports_stats.bootstrap_p_floor at lines 32-33 with optimize_gate deferred under TYPE_CHECKING at line 41; no typer/rich/coder_eval.cli anywhere in the three).
[Axis 8] skill_triggered is no longer decidable at ToolStart, so an armed instance with decide_within: N latches an effective FAIL on the very Skill call that would satisfy it — score/final_status changes for identical agent output (src/coder_eval/criteria/skill_triggered.py:103) — skill_triggered.py:103 now reads if cmd.result_status != "success":, so an in-flight Skill call (result_status is None at ToolStartEvent) contributes nothing to _engaged_skill_names and live_verdict returns "undecided". early_stop.py:580 then reads budget_expired = verdict == "undecided" and budget is not None and tool_call_index >= budget, and _evaluate_impl reports an in-flight call as self._tool_call_index + 1 (early_stop.py:614). So for a criterion armed with decide_within: N, the ToolStart of tool call N converts to an effective FAIL and fires the stop BEFORE the matching ToolEnd can decide it as a pass — the criterion is unsatisfiable at exactly its boundary step. Verified by execution against the repo's own test helpers: feeding [_agent_start(), _turn_start(), _skill_start("date-teller")] to _watcher([_skill_crit("date-teller", "date-teller", stop_on_pass=True, max_steps_to_decide=1)]) yields should_stop = True, reason = decision_budget_exceeded on this branch, and should_stop = True, reason = criterion_passed with the pre-PR _engaged_skill_names monkeypatched in — identical event stream, opposite armed-gate outcome. The module docstring (skill_triggered.py:11-18) documents the downward re-baseline of suite_thresholds, and early_stop.py:436-439 documents that skill_triggered is never decidable at ToolStart, but neither states this budget consequence. Fix: either exempt a criterion whose live_decidable_polarities() cannot resolve at ToolStart from budget expiry on an in-flight round (_collect_verdicts already knows in_flight is not None), or defer the budget check to resolved rounds only; add a test asserting an armed skill_triggered with decide_within: N still passes when its Skill call lands at step N and succeeds. No shipped suite or bundled template arms skill_triggered with decide_within, which is why this is 🟠 rather than 🔴.

Non-blocking, but please consider before merge

src/coder_eval/optimize_gate.py:1677 — Copy-pasted logic and diagnostic text duplicated across sites in optimize_gate.py despite the module's own factoring helpers. holm_promote lines 1677-1692 and holm_promote_execution lines 2464-2478 carry the same eleven lines, differing only in the decided annotation's element type: family = [(i, v.p_value) for i, v in enumerate(verdicts) if v.p_value is not None] rejections =…
src/coder_eval/optimize_gate.py:2208 — Vestigial parameter states kept for call forms that cannot occur (refused_already, instance_best). The parameter is declared at line 2208 (refused_already: bool,) and passed at the sole call site, line 2178, as refused_already=gate_refusal is not None.
src/coder_eval/cli/plan_command.py:205 — plan_command runtime type-sniffs its own Typer options for direct test callers, contradicting its annotations. Lines 201-209 add production branching whose stated purpose is a test calling convention: # Tests call plan_command(...) directly rather than through CliRunner, so an unpassed # option arrives as a typer OptionInfo rather than its declared default ...
src/coder_eval/optimize_store.py:209 — UNRECORDED_SPLIT is an in-band sentinel in an open-string field; the "can never collide with a real split name" comment (optimize_store.py:201-202) is an overclaim, and the test that names the guarantee does not establish it. UNRECORDED_SPLIT = "(unrecorded)" (line 209) is documented at lines 201-202 as "It can never / collide with a real split name." That is not true: --split takes an unconstrained string (src/coder_eval/cli/run_command.py:283 declares `split: str | None = t…
tests/lint/rules/ce041_no_model_dict_splat.py:59 — CE041 never consults node.level, so relative from ..models import is invisible — the rule catches 0 of the 8 **-splat sites in src/, all of which use the relative form. _imports_models never consults node.level, so a relative import is invisible: ```python def _imports_models(node: ast.ImportFrom) -> bool: """True for from coder_eval.models import ... and its submodules.""" module = node.module or "" return module ==…
src/coder_eval/cli/plan_command.py:305 — Changed behaviour shipped without a direct test (plan dataset-row resolution, copy_with boundaries, widened replicate glob). The PR changes plan's per-variant resolution from the raw task to the first EXPANDED row: ``` 303: # shape a run actually resolves, and it is where a criterion that only 304: # becomes invalid after ${row.*} substitution shows up.
plugins/coder-eval/reference/proposal-prompt.md:15 — Proposer prompt spec (reference/proposal-prompt.md) directs the proposer to read agent trajectories with no untrusted-data framing; the skill's one such sentence is confined to its execution-track section and never reaches the activation track or this file. CVSS:3.1/AV:L/AC:H/PR:N/UI:R/S:C/C:L/I:L/A:N proposal-prompt.md is the file optimize-skill/SKILL.md:659 ("Generate them against ${CLAUDE_PLUGIN_ROOT}/reference/proposal-prompt.md") points at to define what the proposer is handed.
src/coder_eval/optimize_gate.py:2486 — The promotion veto is applied in the presentation layer, not the decision layer — promoted=True can coexist with a failed guardrail, contradicting the module's own "the gate decides, this renders the decision" contract. reports_optimize.py's module docstring (line 5) states the split as "the gate decides, this renders the decision", and optimize_gate.py's (line 19) as "What stays here is everything that DECIDES".
src/coder_eval/cli/plan_command.py:265 — coder-eval plan prints ✓ and then ✗ for the SAME task file when dataset expansion fails. This PR moved dataset expansion into plan, but placed it AFTER the success banner.
src/coder_eval/cli/plan_command.py:322 — Untrusted text interpolated into Rich markup without escape() on CLI paths (plan and aggregate). console.print(f" [red]Error: {e}[/red]") (plan_command.py:322) passes the exception text straight into Rich's markup parser.
src/coder_eval/optimize_gate.py:1312 — run.json's row_selection describes the last invocation while the optimize gate loads rows from the directory tree — a reused --run-dir yields confident but false split provenance, silently defeating the cross-split refusal. activation_gate reads provenance from run.json (optimize_gate.py:1312: incumbent_provenance = read_split_provenance(incumbent_run_dirs)) but loads rows from the filesystem (optimize_gate.py:168: ` for task_json in sorted(suite_dir.glob(TASK_JSON_GLOB)…
templates/ci-outcome-fixture/evals/activation.yaml:29 — The ci-outcome fixture repo ships an eval task that fails TaskDefinition validation, and nothing loader-tests the fixture. templates/ci-outcome-fixture/evals/activation.yaml declares suite_thresholds: / recall.yes: 0.7 (lines 29-30) with no dataset: block.

Nits

src/coder_eval/optimize_gate.py:1363 — optimize_gate.py carries roughly as many comment/docstring lines as code, including ~14 changelog-style comments that record PR history rather than the code. Measured on the file at HEAD: 3010 total lines = 1345 code, 419 # comments, ~884 docstring lines, 362 blank — i.e.
src/coder_eval/optimize_gate.py:1970 — execution_gate tests the same-variant condition twice, 91 lines apart, and runs three 20,000-draw bootstraps on the path it has already refused. if incumbent_variant == candidate_variant: appears at line 1970 (which only calls _refuse(...)) and again at line 2061 (return _verdict());
src/coder_eval/optimize_store.py:198 — optimize_store.py defines its two public sentinels at the bottom of the file, after the functions that reference them. UNRESOLVED_MODEL = "(unresolved)" (line 198) and UNRECORDED_SPLIT = "(unrecorded)" (line 209) are the last two statements in the module, but they are read at line 112 (("model", UNRESOLVED_MODEL) if floor.model == UNRESOLVED_MODEL) and line 114, and re-e…
src/coder_eval/models/copy_with.py:46 — copy_with(**updates: object) gives no static or runtime VALUE checking on a new coder_eval.models public export that CE048 now routes all 20 src/ update sites through. def copy_with[T: BaseModel](model: T, /, **updates: object) -> T: — **updates: object means pyright accepts every keyword and every value type, and the body is return model.model_copy(update=updates), which pydantic does not validate.
src/coder_eval/optimize_gate.py:1764 — _completion_rates carries a promotion-gating count in a stringly-keyed dict of positional 2-lists, where neither the key nor the slot index is type-checked. The helper backing the execution track's completion_rate integrity check builds totals = {"incumbent": [0, 0], "candidate": [0, 0]} (line 1764), accumulates via totals[arm][0] += ... / totals[arm][1] += ... (lines 1769-1770) and returns `return (totals…
tests/test_custom_lint.py:5110 — Guards whose scope is narrower than the property they claim (CE036 tasks/-only scan, denylist-based layering pin). CE036's own docstring justifies the rule with "a checked-in worked example shipped with four such rows", yet its repo scan is parametrized over this repo's tasks/ tree alone: ``` 5110: sorted(p for p in (Path(file).parent.parent / "tasks").rglob("*.yaml"…
src/coder_eval/optimize_store.py:52 — _atomic_write's stated symlink limit is backwards, and the one symlink exposure it does have (a symlinked parent directory) is unstated. CVSS:3.1/AV:L/AC:H/PR:L/UI:R/S:U/C:N/I:L/A:N The docstring at optimize_store.py:51-52 states as an accepted limit: "And os.replace follows a symlink at path, replacing the link rather than its target." The module docstring (optimize_store.py:22-23) and CLAUDE.md's optimize_store.py entry…
src/coder_eval/optimize_gate.py:49 — optimize_gate.py is the only module in src/ that imports a package-private symbol across a package boundary (criteria._classification_aggregate). Line 49 is from coder_eval.criteria._classification_aggregate import classification_metrics.
src/coder_eval/optimize_gate.py:211 — A negative criterion index is silently accepted as a Python negative index and grades the LAST criterion instead of failing loudly. Both row extractors guard only the upper bound: - _label_pairs, optimize_gate.py:211-213 — if criterion_index >= len(result.success_criteria_results): continue / criterion_result = result.success_criteria_results[criterion_index] - _row_score, optimize…
src/coder_eval/cli/plan_command.py:320 — plan and run classify an unmatched --split differently: task-file error / exit 1 / keep going vs invocation error / exit 2 / abort. SplitSelectorError's own docstring (task_loader.py:38-55) argues the distinction is load-bearing: it "describes a malformed INVOCATION" and "there is no per-task isolation argument for it, because the same selector is applied to every task in the run".
src/coder_eval/optimize_store.py:75 — optimize_store.load_measurements raises a bare pydantic error with no path context when the sidecar is not under a <skill>/ directory. skill = path.parent.name (line 75) feeds OptimizeMeasurements(skill=skill) at line 77, and the field is skill: str = Field(min_length=1, ...) (models/optimize.py:561-563).
src/coder_eval/orchestration/config.py:80 — BatchRunConfig's max_rows / sample_per_stratum were removed with no alias, and the model forbids extras — an out-of-tree caller of run_batch now fails at construction. orchestration/config.py:80 replaces the two flat fields with row_selection: RowSelection = Field( while BatchRunConfig keeps model_config = ConfigDict(extra="forbid"), so BatchRunConfig(run_dir=..., max_rows=5) — the pre-PR spelling — is now a hard…

What's Missing

Parallel paths

🟠 The delivered-body rule was applied to skill_triggered only, but command_executed.py:173 drops any command whose result_status != "success" for the same reason and is therefore equally undecidable at ToolStart — and tasks/early_stop_decision_budget_exceeded.yaml already arms it with require… (src/coder_eval/criteria/skill_triggered.py`)
🟡 RunSummary.row_selection is rendered by reports.py:340 (run.md) and reports_junit.py:374 (XML <properties>), but the A/B surface a human actually reads to compare two arms — reports_experiment.py's experiment.md — shows no split/sample at all, and ExperimentResult (models/experiment.py:2… (src/coder_eval/models/row_selection.py)
🟡 plan_command.py:123-127 warns when a stratified draw is unseeded ("run will execute this many rows but not necessarily these ones"), but run itself emits no such warning — and the activation track's two arms are two separate coder-eval run invocations, so an unpinned dataset.sample_seed si… (src/coder_eval/cli/plan_command.py)
🟡 execution_gate performs no split-provenance read at all (its justification is that one run dir implies one split "by construction"), so on that track a reused --run-dir is not merely un-refused but undetectable — it cannot even emit the note activation_gate emits. The invariant the justificati… (src/coder_eval/optimize_gate.py)
🟡 The three command examples added at docs/DATASETS.md:210-219 (coder-eval run tasks/skills/activation.yaml --split train, --split test, and the plan preview) name a task file that exists neither on origin/main nor at HEAD — the suites this PR actually ships are tasks/skills/lint-tasks-acti… (docs/DATASETS.md`)

Tests

🟠 Nothing asserts the contract plan is now sold under — that its previewed row set equals what run would execute. CE043 pins flag declaration and help-string identity and its own docstring concedes it pins "never that the flag does anything", and every selector test in tests/test_plan_command.py… (src/coder_eval/cli/row_selectors.py)
🟠 test_real_decision_within_budget_wins_over_budget_check (tests/test_early_stop.py:1818) is the test that should have caught the regression; it passes only because its helper _skill_events() (:293-296) emits AgentStart + TurnStart + ToolEnd and omits the ToolStartEvent a real agent always sen… (tests/test_early_stop.py)
🟡 test_aggregate_cli_degrades_a_malformed_row_selection_rather_than_aborting (tests/test_aggregate.py:423) parametrizes four malformed values — "train", ["train"], 3, {"max_rows": "not-an-int"} — none of them markup-shaped, which is exactly why the MarkupError at aggregate_command.py:160… (tests/test_aggregate.py)
🟡 Every CE041 fixture in tests/test_custom_lint.py:524-588 uses the absolute from coder_eval.models import ... form, including the boundary test test_does_not_claim_to_catch_an_alias_or_a_factory — so the rule's whole blind spot (19 of 78 model-importing files under src/, holding all 8 real sp… (tests/lint/rules/ce041_no_model_dict_splat.py)
🟡 templates/ci-outcome-fixture/ is the only directory under templates/ shipping eval YAML and is referenced by nothing in tests/, src/, Makefile or .github/, while the plugin's reference/templates/{activation,outcome}.yaml are loader-backed at tests/test_custom_lint.py:1973/2040/2084. A… (templates/ci-outcome-fixture/evals/activation.yaml)
🟡 The plugin has an extensive prose-sensor family (_PROPOSAL_TOKENS, the optimize-method.md / task-rubric.md pointer sensors in tests/test_custom_lint.py:2584-3005) yet none asserts the untrusted-agent-output convention that analyze/SKILL.md:110, llm_judge.py:250, agent_judge.py:414 and j… (plugins/coder-eval/reference/proposal-prompt.md`)

Downstream consumers

🟠 skill_triggered's own docstring says the delivered-body rule re-baselines every prior suite downward and that run.json's framework_version is the only attribution a trend line gets — but optimize_gate never reads that field, even though it already opens both arms' run.json in read_split_p… (src/coder_eval/criteria/skill_triggered.py`)
🟡 _floor_key (optimize_store.py:94) derives the cache key from every NoiseFloor field except mde and computed_at, so nothing — not a version, not an age — invalidates a floor measured under an older checker; lookup_noise_floor will serve an activation floor measured before the delivered-body… (src/coder_eval/optimize_store.py)
🔵 The two shipped in-tree suites were re-measured against the delivered-body rule (their YAML comments cite the runs), but the bundled template a user copies still carries recall.yes: 0.7 / precision.yes: 0.7 unchanged since #82, and templates/ci-outcome-fixture/evals/activation.yaml copies them… (plugins/coder-eval/reference/templates/activation.yaml)
🔵 execution_gate resolves the paired-diff sign to candidate − incumbent whichever order the experiment declared its variants in, while the two renderers of the same reports_stats.paired_comparison — reports_experiment.py:531 and the untouched reports_html.py:1253 — keep declaration order. Bo… (src/coder_eval/reports_stats.py)

Display & mapping dicts

🟡 NOT A RESULT means two different things across the two ledgers. On the activation track it is defined by p_value is None ("no comparison was made"), with CANNOT SEPARATE AT THIS SIZE reserved for the p-bearing refusal; (src/coder_eval/reports_optimize.py)

Daily/nightly

🟡 Two new agent-run suites land with no runner: nothing in .github/workflows/, the Makefile or docs/ invokes tasks/skills/*, and ci-outcome.yaml cannot be run unattended at all — its header requires a hand-prepared plugin copy (cp -R plugins/coder-eval tmp/ci-arm, strip disable-model-invo… (tasks/skills/ci-outcome.yaml`)
🟡 The cross-repo contract moves on two axes in one release and neither is stated: BatchRunConfig drops max_rows/sample_per_stratum under extra="forbid", so an out-of-tree run_batch caller (coder-eval-uipath / eval-runner) fails at construction rather than degrading; and run.json gains row… (src/coder_eval/orchestration/config.py`)
🟡 The delivered-body rule steps every activation trend line at this merge for identical agent behaviour, and there is no per-criterion version stamp — only run.json's framework_version, which no dashboard query is stated to key on. The PR does not say whether the external dashboard's historical ac… (src/coder_eval/criteria/skill_triggered.py)

Harness & Lint Improvements

Static checks (lint / type)

Enable mccabe in [tool.ruff.lint]: add "C90" to select and set [tool.ruff.lint.mccabe] max-complexity = 20 (the radon C/D boundary). — prevents: A5 high / A1 high (optimize_gate.py: 8 D-grade functions — _load_and_pair D(30)@1104, _execution_diagnostics D(29)@2195, holm_promote D(28)@1646…
CE050 — untrusted text interpolated into Rich markup must be escaped. New BaseRule in tests/lint/rules/ce050_escape_rich_markup.py, wired in tests/lint/runner.py. — prevents: A7 medium / A4 low (merged): plan_command.py:322 (MarkupError raised inside the except handler, aborting the whole per-task validation loop —…
CE051 — no cross-package import of a leading-underscore module. New BaseRule (tests/lint/rules/ce051_no_cross_package_private_import.py): flag an ImportFrom whose resolved module has a _name final component and whose package prefix is not the importing file's own package (relative or abs… — prevents: A5 low: optimize_gate.py:49 from coder_eval.criteria._classification_aggregate import classification_metrics — the only cross-package private impo…
CE052 — an index guard must bound below as well as above. New BaseRule (ce052_index_guard_bounds_both_ends.py): inside one function, flag a Subscript xs[i] where i is a parameter, guarded by a Compare of the form i >= len(xs) / i > len(xs) (or i < len(xs) as the positive form)… — prevents: A6 low: optimize_gate.py:211-213 (_label_pairs) and :2526-2528 (_row_score) — criterion_index=-1 grades the last criterion;
CE053 — a CLI command may not isinstance-sniff its own declared parameters. New BaseRule scoped to src/coder_eval/cli/ (ce053_no_option_type_sniff.py): flag isinstance(p, T) where p is a parameter of the enclosing function and T is exactly the non-None member(s) of p's own anno… — prevents: A1 medium / A2 low (cross-axis): plan_command.py:205-209 — three shim statements, 5 radon decision points, provably 100% of plan_command's D(23)->…
CE041 fix + a meta-rule that no lint rule reads node.module without node.level. Two parts. — prevents: A2 medium: CE041 as shipped catches 0 of 8 real **-splat sites in src/ (all 8 live in relative-import files: task_loader.py:85,688, `experim…
CE054 — every YAML in the repo that declares task_id: must load through load_task. Class-wired @pytest.mark.lint test (CE026/CE033/CE036 family), scan set DERIVED by globbing the whole repo for task_id: per reference/repo-layout.md's own discovery policy — never a hardcoded directory l… — prevents: A8 medium: templates/ci-outcome-fixture/evals/activation.yaml:29-30 declares suite_thresholds: {recall.yes: 0.7} with no dataset: block and rais…
CE055 — one shared task-file discovery helper for every tasks/-scanning rule. Add tests/lint/task_files.py::all_task_yaml_paths() (derived: repo-wide glob for task_id:, minus metadata.yaml) and a meta-assertion that CE034, CE035, CE036 and the new CE054 all parametrize over it rather tha… — prevents: A3 low: CE036's docstring justifies the rule with "a checked-in worked example shipped with four such rows" while scanning only this repo's tasks/ —…
CE056 — a plugin prose surface that directs the reader at agent-produced artifacts must carry the untrusted-output framing. Class-wired doc-surface rule over plugins/coder-eval/**/*.md (CE026's family): if a file mentions task.json, trajectory, transcript, stdout, iterations/command… — *prevents:* A4 medium (CVSS:3.1/AV:L/AC:H/PR:N/UI:R/S:C/C:L/I:L/A:N): plugins/coder-eval/reference/proposal-prompt.md:15` hands the proposer per-row trajectories…
Extend CE039's ComputedClaim registry to documented runtime-behaviour claims (tests/lint/computed_claims.py), and add the covers entry for optimize_store._atomic_write's accepted-limits paragraph. — prevents: A4 low (CVSS:3.1/AV:L/AC:H/PR:L/UI:R/S:U/C:N/I:L/A:N): the shipped sentence "os.replace follows a symlink at path, replacing the link rather than…
CE057 — the renderer may not branch on a decision input. Extend the existing layering pins in tests/test_optimize_gate.py (which already reason over _OPTIMIZE_MODULES at :809 via AST, not substrings): forbid verdict.guardrails / verdict.integrity_checks / verdict.promoted from appearin… — prevents: A5 medium: the promotion veto is decided in the presentation layer — holm_promote_execution computes promoted at optimize_gate.py:2486 with guar…
CE058 — a module-level constant must be declared before its first use in the file. Cheap BaseRule: flag a module-level UPPER_CASE assignment whose line number is greater than the first Name load of it in the same module. — prevents: A1 low: optimize_store.py:198 (UNRESOLVED_MODEL) and :209 (UNRECORDED_SPLIT) are the last two statements in the module but are read at :112…
CE059 — structurally identical statement runs inside one module (the CODE twin of the prose rule already reserved as CE049 in .claude/harness-candidates.md; — prevents: A1 medium: holm_promote (optimize_gate.py:1677-1692) and holm_promote_execution (:2464-2478) carry the same 11 executable lines, differing onl…
Enable FBT001/FBT002 (flake8-boolean-trap) in [tool.ruff.lint] select, at least via per-file-ignores inversion for src/coder_eval/optimize_gate.py and src/coder_eval/reports_optimize.py. — prevents: A1 medium (partial): optimize_gate.py:2208 refused_already: bool — one call site (:2178) where gate_refusal is provably None (every reachabl…
CE060 — no changelog-style comments in src/. Narrow token grep-rule over comment text only (never docstrings, never prose that carries a measured number): forbid this comment is the only record, used to be, used to annotate, it used to, before this guard existed, which used to assig… — *prevents:* A1 low: ~14 comments in optimize_gate.pyrecording the PR's own edit history rather than the code —:1363` ("this comment is the only record of the…

Harness improvements

Add a pull_request-only diff-coverage CI job (diff-cover against the merge base, or pytest-cov + diff_cover.diff_cover_tool), requiring changed src/ lines to be covered by a test that does not patch the seam under test. — not static because: Needs the merge base and a coverage run — no AST scan can tell that a changed line is executed only through a mock of itself.
One shared event-stream builder for every early-stop test, emitting a ToolStartEvent before every ToolEndEvent, and a parametrization of the existing suite over both "with ToolStart" and "resolved-only" streams. Then fix the seam itself at _collect_verdicts/_evaluate_impl (exempt an in-f… — not static because: The defect is a two-event ordering property of a live stream against a latching state machine;
Bind the gate's split provenance to the tree it actually globs. Either refuse to write into a non-empty run dir without --resume (cli/run_helpers.py:60-66 is currently a bare mkdir(exist_ok=True), and the fingerprint diff is gated behind if resume:), or stamp the row's split into each t… — *not static because:* The mismatch is between a per-invocation artifact (run.json`) and an append-only directory tree;
Validate only the updated fields in copy_with — TypeAdapter(type(model).model_fields[name].annotation).validate_python(value) per key, O(len(updates)) rather than the O(model) re-validation the docstring rejects on cost grounds — or, if that is declined, extend the module docstring's "three… — not static because: **updates: object is deliberately un-narrowable — pyright accepts any keyword by construction (that is the documented trade), an…
A public-API golden snapshot for the library surface: a test that pins BatchRunConfig.model_fields (and the other run_batch-facing signatures) against a checked-in JSON, so a removal fails with a diff and forces either a deprecated alias or an explicit line in the PR description. — not static because: Needs the PREVIOUS version's schema, which nothing in the tree carries — the identical reason the fingerprint_diff "key that MOV…
A CLI-output golden test for plan's per-file status banner: assert exactly one ✓/✗ per task file across each failure class (SplitSelectorError, duplicate/missing row id, malformed dataset path, unresolved ${row.X}), by buffering the per-task lines and emitting the banner once after `_previ… — not static because: It is an ordering property of console output — both prints are individually correct;
A derived run/plan parity test over ROW_SELECTOR_FLAGS asserting the two commands classify the same bad invocation the same way — same exit code and a message naming the FLAG, not the file. — not static because: CE043 reads resolved Typer signatures;
Carry the split tri-state out of band and property-test the sentinels. SplitProvenance already models recorded/unrecorded/mismatched correctly (optimize_gate.py:576-614); — not static because: "Can never collide with a real split name" is a claim about an unconstrained open string supplied at runtime by a CLI flag;
A "test-only branch" report in CI: run the suite twice — once whole, once with the tests that call module-private helpers directly deselected — and diff branch coverage; — not static because: Proving refused_already is always False at optimize_gate.py:2178 requires interprocedural constant propagation across six ea…
A derived diagnostics test over optimize_store's failure modes: parametrize every way load_measurements and _atomic_write can fail and assert each message names the path and the expected layout. — not static because: "Does this message tell the caller where to look" is a semantic judgment about message content;

Top 5 Priority Actions

Fix the armed-early-stop regression: skill_triggered.py:103 now ignores in-flight Skill calls, so with decide_within: N the ToolStart of call N latches decision_budget_exceeded at early_stop.py:580 before the matching ToolEnd can pass it — exempt in-flight rounds from budget expiry (the same shape already reaches shipped tasks/early_stop_decision_budget_exceeded.yaml via command_executed), and fix tests/test_early_stop.py:1818, whose helper omits the ToolStart a real agent always emits.
Bound the criterion index at both extractors (optimize_gate.py:211 and optimize_gate.py:2526 guard only the upper bound), since criterion_index=-1 silently grades the LAST criterion and, on the execution track where engagement_criterion_index is never persisted on the verdict, can pass or fail a promotion on a criterion the caller never asked for.
Close the cross-split provenance hole at optimize_gate.py:1312: provenance comes from run.json (last invocation) while rows come from the globbed tree, so a reused --run-dir pools train and test rows under a single confidently-recorded split and the cross-split refusal never fires — refuse to write into a non-empty run dir without --resume, or stamp the split per row in task.json.
Wrap untrusted text in rich.markup.escape at cli/plan_command.py:322 and cli/aggregate_command.py:160/:135 — a markup-shaped dataset row ([/red], [/INST]) raises MarkupError inside the except handler, aborting the whole per-task loop so remaining task files are never validated; while in plan_command.py, move the ✓ banner (line 265) below _preview_dataset (line 279) so a file is no longer reported as both valid and invalid.
Give the promotion veto one home in the decision layer: holm_promote_execution sets promoted=True with a failed guardrail (optimize_gate.py:2486) and only the renderer blocks it (reports_optimize.py:202), so add a gate-computed shippable/blocked_by (preserving BLOCKED-headline reachability), and continue the module split — 3010 lines, 8 D-grade functions, with the execution track (1748-2510), fronts (2778-3010) and search loop (2513-2777) as clean seams — while fixing CE041's _imports_models (tests/lint/rules/ce041_no_model_dict_splat.py:59), which ignores node.level and therefore catches 0 of the 8 real **-splat sites.

Stats: 0 🔴 · 2 🟠 · 12 🟡 · 12 🔵 across 8 axes reviewed.

Trimmed for GitHub's 65,536-char comment limit. Full narratives, verifier notes and per-axis findings: tmp/code-review-260816-1017/ (00-summary.md, 01-code-quality.md … 08-harness-quality.md).

`_collect_verdicts` expired a criterion's `decide_within` budget on the IN-FLIGHT round (a ToolStart with no result yet). `skill_triggered` can never decide there — for the Skill tool the body IS the result — and `command_executed` cannot under `require_success`, which CE034 mandates for an armed live-passable instance. So the budget failed those criteria on the very call that would have satisfied them, one event before the result arrived. The fix is one conjunct: `in_flight is None`, the same discipline the verdict latch on the next line already applies. It can only ever REMOVE an expiry, and the matching ToolEnd evaluates at the same `tool_call_index`, so `decide_within: N` keeps its meaning — now stated as N *completed* tool calls in CLAUDE.md, the task guide and the `StopEarlyPolicy.decide_within` field description. `_budget_drove`'s transient re-derivation arm is deleted: with the budget expiring on resolved rounds only, a fail is latched before `_evaluate_impl` reads it, and the one remaining unlatched shape is excluded by the arm's own final conjunct. Measured rather than argued — the arm raised on entry across four suites and was never reached. Its docstring records both the argument and the invariant it rests on that CE025 does not enforce. The test helper is the other half of the finding. `_skill_events` emitted a bare ToolEnd, so no test in the file had ever seen a ToolStart precede its result and the bug was invisible for the life of the feature. It now emits the realistic pair, `_rounds()` builds either shape, and the three budget tests are parametrized over both. `test_an_in_flight_budget_expiry_would_defeat_the_recall_deferral` is the independent pin: every other budget test passes with only the conjunct reverted, because a transient fail never latches. That one does not — a distractor's native fail cuts a positive row whose own signal never got to appear, which is the recall truncation the deferral exists to prevent, reached through the budget instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…il gets a veto Two phases in one commit: they share `optimize_gate.py` and `tests/test_optimize_gate.py`, and splitting them would mean committing an intermediate state that was never tested. Both are verified together. 2/9 — `criterion_index` is bounded below as well as above. Selection is positional, so `criterion_index=-1` did not fail: it graded the LAST criterion of every row and returned a confident number for a criterion nobody asked about. The internal guards bound only above, which is right for the overflow case (rows legitimately differ in criteria count, so an over-long index skips the row) and blind to this one. `_require_valid_criterion_index` raises at the six public entry points that take the parameter. Raised rather than clamped: the skill drives this from an inline python snippet, so a wrong index is an authoring error that must be loud. The plan also asked for `ge=0` on the verdict models — both already had it at `fa69cdf`, so that half needed no edit. 3/9 — a failed integrity check or guardrail now forces `promoted = False`. It used to be reported and gated only in the render, so a caller reading `promoted` could promote a candidate whose own block said BLOCKED. Folding the veto in is only safe because the statistical half now has its own name: `ExecutionGateVerdict.separated`, a computed property (never stored, on `SearchComparison.accepted`'s precedent) that the renderer keys its BLOCKED headline on. Read `promoted` there instead and that headline dies the moment the fold lands — silently, degrading a blocked winner to the ordinary NOT PROMOTED rung, which is the one thing it must never be confused with. `separated` alone turned out to be the trap on the other side, found in review and reproduced: it excludes the FAMILY decision, so at m > 1 a p between alpha/m and alpha leaves the interval excluding zero while Holm rejects nothing. Two candidates at p = 0.03 in a family of two, identical in every statistic, rendered BLOCKED and NOT PROMOTED purely because one carried a failing cost check. Nothing on the verdict could tell them apart — `holm_alpha` records the family-wide alpha, never the rank threshold — so the verdict now records `holm_rejected` and the rung requires it. The activation track was read, not assumed: it already folds its sibling checks in and deliberately leaves cost/latency advisory for the skill's prose to gate. That asymmetry is documented rather than "fixed" — folding activation's guardrails in would make ITS blocked headline unreachable, the same defect one track over. Making the checks gating exposed two latent fixture defects in the MDE test: a shifted score of 1.01 failed validation so the replicate was silently dropped, and 0.46 labelled its row `no` so the arm never engaged the skill. Both were invisible while a failed check was advisory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… it does not describe `activation_gate` read split provenance from `run.json` but loaded rows from the DIRECTORY TREE. `run.json` is written per INVOCATION; the tree beneath it is APPEND-ONLY. So a second `coder-eval run --run-dir <same dir> --split test` left the earlier split's rows on disk while rewriting `row_selection` to say `test` — the cross-split refusal then read one honest, single-valued split and never fired, and the gate pooled train and test rows into one arm. The contamination is symmetric: both arms are subdirectories of the same run dir, so the stale rows pair on both sides, producing no `rows_excluded` bump and no unpaired-rows note. Nothing anywhere flagged it. `reconcile_tree_against_run_json` asks the one question that matters — does this run.json describe this tree? — from data that already exists. It matches `(row id, replicate dir)` pairs rather than counting: a fanned row's `task_id` IS `f"{suite_id}/{row_id}"` and `build_task_run_dir` uses that same string as the path segment, so the two correspond by construction. Counting would be blind to two invocations of equal size and would need a fixed glob depth the tree does not have — a dataset row sits four levels below the run dir, a plain task three, and one dir can hold both. The replicate half of the key was added after review: row ids alone are blind to a stale `<NN>` inside a row run.json DOES record, which re-using a run dir with a smaller `--repeats` produces. `load_suite_rows` pools every replicate, `_balance_pair` trims symmetrically and so not at all, and the gate returns a confident interval over contaminated clusters. Both preflight refusals now build their verdict through one `_refuse_activation` closure — three refusal CAUSES, still two `gate_refusal` setters, and a third cause cannot drift from the first two. Degradation is deliberate everywhere it is possible: a missing, unreadable or `task_results`-less run.json is a NOTE, an unreadable suite directory is a NOTE, and an entry missing `variant_id` or `replicate_index` is read permissively. The one state where the fault is undetectable must not also be the one state that refuses everything, and a false refusal blocks a real promotion. `--resume` and `coder-eval aggregate` are separately verified not to false-positive, each through its own real code path rather than a hand-written run.json — they are different paths, which the plan had conflated. The skill and the method file both defined `NOT A RESULT` unconditionally as the cross-split cause, so an agent reading its own guide would have handed the user a false diagnosis and a remedy that fixes nothing. Both now name the two causes and their opposite remedies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…plan prints one banner Rich reads `[...]` in the string it is handed as MARKUP. So a value carrying brackets — a task id, an exception message, a `run.json` fragment, a path — does not render as itself: `[bold]` disappears, an unclosed `[` swallows the rest of the line, a stray `[/red]` closes a tag the program opened. The failure is a corrupted or missing diagnostic, and it lands exactly when something has already gone wrong and the message matters most. 63 interpolations across 7 CLI files are now escaped. Five carry a reasoned `# noqa: CE050`: three integer counts, and two values that are **already escaped at construction** — `escape` is not idempotent, so a second pass prints a literal backslash before every bracket. That second case is the one an automated sweep gets wrong, so it is named at both sites. CE050 is the guard. It flags an f-string passed to `console.print` that carries both a markup tag and an unescaped interpolation, scoped to `src/coder_eval/cli/` on CE047's precedent — everything else in `src/` reports through `logging`, where markup is never interpreted. It exempts provably numeric interpolations (numeric format spec, `len()`/`round()`/…, numeric literal) so the remediation was 63 real sites rather than 73 with noise, and its docstring states what it cannot see: it matches f-strings at the CALL SITE, so a pre-built `msg` variable is invisible, and it makes no attempt to decide whether a value is actually untrusted. A trusted one takes a noqa with a reason, which is the rule working — it forces the author to answer "could this carry a bracket?" rather than never asking. `plan` also printed ✓ and then ✗ for the same file. The banner went out the moment `load_task` returned, but `_preview_dataset` and the per-variant loop both run after that and both can raise, and the outer handler then printed ✗ for a file already marked valid. Per-file output is now buffered — `_preview_dataset` and `_print_strata` write through an `emit` sink — and one banner prints after everything that could fail has run, with the detail lines under it so a multi-file plan keeps every heading attached to its own detail. The soft/hard distinction is preserved exactly: a generic per-variant resolution failure stays soft (red line, ✓, exit 0), and an `EarlyStopConfigError` still flips the exit code while keeping ✓ — the banner reports whether the FILE is loadable, and the red line beneath names the variant that is not. Both are now asserted rather than assumed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…gets a ratchet 6/9 — the relative-import blindness. Five rules under tests/lint/rules/ match on an import's module string. Every one read `node.module`; none read `node.level`. For `from ..models import X` — how most of src/ imports models — `node.module` is the bare string "models", so every `coder_eval.`-anchored pattern failed to match. What made that survive is the DIRECTION of the failure. A broken import rule fails OPEN: it reports zero violations, byte-identical to a clean tree. So four rules were broken simultaneously while `make lint` printed a clean bill of health, and CE041 — added by an earlier plan precisely because two optimize-gate verdicts were built by splatting — reported 0 against 8 real model-constructor splats and had never fired in its life. `tests/lint/import_resolution.py::resolved_module` is now the one resolver, and CE001/CE004/CE017/CE023/CE041 route through it. CE004's exposure had been inconclusive; re-tested, it IS affected and is fixed. Fixing the rules surfaced the two live CE001 violations the blindness had been hiding, both now importing from `coder_eval.models` — which is what adds `UnknownTaskFieldWarning` to that module's exports. The resolver requires the package root it picks to really BE a package. Without that check it did not decline for a file outside src/coder_eval — it fabricated: `…/coder_eval/tests/lint/rules/foo.py` resolved `from ..models` to `coder_eval.tests.lint.models`. A wrong string is strictly worse than None, since it can make a rule fire on innocent code or stay silent on a real violation — the exact failure this helper exists to end. CE051 keeps a sixth rule from joining the class. It keys on "calls the resolver" rather than "mentions node.level" — the DRY rule and the correctness rule at once, unsatisfiable by a stray mention. It scans all of tests/, not just the rules package: `test_optimize_gate.py::_coder_eval_imports` had the identical blindness, and it is the helper behind the layering pin CLAUDE.md calls out as held "by a test, not by this sentence". A sibling-module `from .optimize_gate import …` would have left both layering tests green over a broken boundary. The 8 CE041 sites carry reasoned `# noqa: CE041` rather than the `model_validate` conversion, by explicit decision: converting changes the raised exception type on YAML-parsing paths. The exemptions are honest and checked — every target but `TaskDefinition` declares extra="forbid" so a mistyped key RAISES, and `TaskDefinition` warns through `_warn_on_unknown_fields`. Conversions recorded as a follow-up. The parity test — every import rule fed BOTH spellings of one violation — is what would have caught all four at once. 7/9 — ruff C90. Enabled at max-complexity = 30: a ratchet one above the worst function, so it costs no refactor and still fails a new one above it. The plan assumed `optimize_gate.py` would need the single exemption; by the time this landed the module split had brought it to 18, so no exemption exists at all. Four other functions sit above 20 and are tracked for decomposition. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…the next one does `templates/ci-outcome-fixture/evals/activation.yaml` declared `suite_thresholds: {recall.yes: 0.7}` with no `dataset:` block, which `check_suite_thresholds_require_dataset` rejects outright. Nothing in the tree had ever loaded it — `grep -rn ci-outcome-fixture tests/ src/ Makefile .github/` returns nothing — unlike the plugin's own templates, which are loader-backed. The damage is not a red build. `resolve_all_tasks` isolates a malformed task into `skipped_tasks` and the run prints a yellow warning, so a workflow generated from this fixture would run GREEN while silently skipping the one suite that motivates its `SKILL_SOURCE_PATH` passthrough — the exact failure the fixture's own header comment warns about. The file gets its `dataset:` back rather than losing its thresholds: a dataset-less activation suite is not what the `ci` skill is meant to demonstrate. Rows carry both polarities, because `recall.yes` over positives alone is satisfiable by a skill that fires on everything. CE052 DISCOVERS task YAML under `templates/` (any file with a top-level `task_id:`) rather than enumerating it — which is the property that would have covered this file, and the one that covers the next fixture tree on arrival. Non-task YAML is skipped by content, never by filename, and the skip is asserted so the check cannot quietly start skipping real tasks. The discovery set is asserted non-empty, so a moved directory reports a GAP instead of passing vacuously. Verified the sensor actually fires: restored against the pre-fix YAML it fails three of its five assertions, and passes on the fixed one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ors for one more release `max_rows` and `sample_per_stratum` used to be fields on `BatchRunConfig`. Collapsing them into one `RowSelection` is right — it is the same declaration `run.json` records — but this model declares `extra="forbid"`, so the old spelling went from working to a hard `ValidationError` with no deprecation step. `run_batch` is a public API and the nightly/dashboard consumer lives in a separate repo, so that break lands out of tree. A `mode="before"` validator folds the flat keys into `row_selection`. `before` is required rather than stylistic: `extra="forbid"` rejects an unknown key during validation, so a field validator never runs and a `mode="after"` one never sees it. Folding pre-validation is also what keeps `model_dump()` — and therefore `compute_run_fingerprint`, which dumps the whole config — identical either way, verified against the real function so no existing `--resume` sees drift. Three things review caught, each of which would have quietly undermined the point of the change: It mutated the caller's dict. On `model_validate(payload)` pydantic hands in the caller's own object, and popping rewrote it in place — a caller building kwargs from JSON got their `max_rows` replaced by a `RowSelection` and a later `json.dumps` raised. The raise path mutated too, so catching the error and retrying saw a silently altered payload. It copies now, on both paths. `stacklevel=2` attributed the warning to `pydantic/main.py`. Python's default filter only surfaces a `DeprecationWarning` raised from `__main__`, so an ordinary script saw NOTHING while `pytest.warns` still passed — the alias would have worked and the migration signal never arrived, making 0.11.0 a hard break nobody was warned about. A `mode="before"` validator sits under a variable number of pydantic frames, so the level is now computed out to the first non-pydantic caller, and a test asserts the attribution rather than merely that a warning fired. The stated reason for excluding `split` was false, in three surfaces. `split` WAS a flat field here, between f1b6abb and d5301a6 — both on this unreleased branch. The exclusion is still correct, but for the other reason: v0.9.6 is what every out-of-tree caller has and it never carried a flat `split`, so advertising an alias for it would be churn. Worth correcting rather than filing, since CE039 exists precisely because untrue prose shipped past ~50 presence sensors. `extra="forbid"` is intact — the anti-over-fix test passes an unknown key and still expects a raise. `RowSelection` carries no aliases: one deprecated spelling, in one place, on the container that broke. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

CE025 checks a live criterion DECLARES its polarities but not that its checker AGREES — a checker returning a polarity it never declared produces a verdict the early-stop watcher classifies as neither a native nor a budget fail, so a doomed ceiling fires no stop at all. Pre-existing; recorded in _budget_drove's docstring. Deferred because agreement is behavioural, not an AST shape. And nothing requires a new CExxx rule to ship with a test proving it FIRES. Three guards written in this run were themselves broken fail-open and only found by review; the mechanical version of the check would have caught none of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Three Mediums from the integration review, all of which would have quietly undone part of what the phases bought. The C90 ratchet comment was false in two places. It credited "the module split" for `optimize_gate.py` needing no exemption and put its peak at 18. Measured: the peak is 17, it was ALREADY 17 at fa69cdf so nothing in this change moved it, and the split is still undone — 3250 lines, with radon reporting one E-grade and seven D-grade functions. The only reason no exemption exists is that the ceiling is 30. In a repo that has CE039 because untrue prose shipped past ~50 presence sensors, a false claim newly written into the config and duplicated into the backlog is worth correcting rather than filing — a future reader would have read it as evidence the split landed and stopped looking. CLAUDE.md documented `separated` as what keeps the BLOCKED headline reachable and never mentioned `holm_rejected` at all — which is the design the renderer's own docstring records as the SECOND trap, since `separated` excludes the family decision and fires BLOCKED for a candidate Holm never rejected. Someone reimplementing from CLAUDE.md would have reintroduced exactly what Phase 3 fixed. Both fields are now documented as the pair they are, with the measured failure each exclusion prevents. CE050 was blind to the buffered sink Phase 5 introduced in the same commit. Keying on `console.print` meant `plan`'s move to an `emit` sink and a `detail.append` list took five markup-bearing f-strings out of the rule's view on the very file it was written for — a sensor failing open on its own subject, which is the shape Phase 6 exists to end. It now matches any call in `cli/`, because the sink is an implementation detail. To keep that from costing suppressions it also follows one hop toward trust: a local assigned directly from `escape(...)` counts as escaped, which is what removed the three `detail.append(variant_id)` sites rather than papering them over. Lows: `derive_sibling_indices` was the one public entry point taking a criterion index without validating it — and it fails quietly the other way, since `found - {-1}` removes nothing and the gated criterion stays in its own sibling set. `_DEPRECATED_ROW_SELECTORS` derived a HISTORICAL API from the current field set, so a fourth selector would have silently become an alias for a spelling that never shipped; it is frozen now, with a parity test. `holm_promote`'s docstring named one of `activation_gate`'s two preflight refusal causes. The skill's execution-track BLOCKED sentence predated `holm_rejected`. The two regenerated verdict pins are re-encoded to match their siblings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The two regenerated verdict pins were re-serialized with ensure_ascii=False to match their siblings. Bytes change, values do not — said in the row rather than left for someone to rediscover from a diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

An adversarial pass tried to break each of the eight guards this change added or repaired. It broke six. Two of those are behavioural. **The decide_within fix did not survive parallel dispatch.** Claude emits every ToolStartEvent of one assistant message before any result, and `in_flight` names only the call being evaluated — so a message dispatching `Read, Read, Read, Skill` resolves its three Reads with the Skill still outstanding and invisible. Measured at `decide_within: 3`, the value the one shipped task uses: the budget expired on the third Read while the Skill that was about to satisfy it sat dispatched. That is a false FAIL on a positive activation row, a depressed `recall.yes`, and corrupted input to the promotion gate the rest of this change exists to protect. The watcher now tracks every dispatched-but-unresolved call, not just the one in hand — and the docstring that called parallel dispatch safe said the opposite of what it did. **`execution_gate` read the same append-only tree and never reconciled it.** Phase 4 wired the check into `activation_gate` only, on the reasoning that this track shares one run.json by construction — true for the SPLIT half, false for the TREE half, and Phase 3 had meanwhile folded the integrity checks and guardrails into `promoted`. So contamination stopped being reported and started deciding: measured on an identical winning candidate, four unrecorded incumbent replicates moved `completion_rate` from 1.0 to 0.667 and `promoted` from True to False, with no refusal and no note. Both arms are reconciled now. The remaining unreconciled readers — `measure_noise_floor`, `noise_floor_mde`, `arm_row_scores`, `cost_quality_points` — return floats and vectors with nowhere to put a refusal, so that gap is recorded rather than half-closed. **CE050 could not see `+`-concatenation**, which is this file's dominant style because pyright forbids the implicit form. Rich parses the joined result as one markup string, so a tag in one operand governs an interpolation in another — and two LIVE unescaped sites were sitting behind exactly that: the `--resume` config-drift warning and the `aggregate` summary line. Its tag pattern was also wrong in both directions, verified against the installed rich rather than inferred: it accepted `[Errno 66]` (which Rich prints literally) and missed `[#ff0000]` and `[@handler]` (which it does not). Note an unmatched closing tag RAISES MarkupError, so an unescaped `[/whatever]` crashes the command rather than merely corrupting its output. Also: `sibling_indices` was a public entry point taking criterion indices that `_require_valid_criterion_index` did not cover — and `holm_promote` folds `sibling_checks` into `promoted`, so a negative one silently vetoes on the wrong criterion. The failed-check note was not guarded by `if not refused`, so a refused verdict asserted a headline the block does not carry. CE051 was blind to a lambda-based matcher, and because its reader set was then empty the anti-vacuity GAP did not fire either — a silent double miss in a rule whose job is preventing silent misses. CE052 missed `.yml` entirely, invisible to both discovery and the completeness assertion; fixing it surfaced a second legitimate non-task, now named rather than silently subtracted. CE050's package scan was CWD-relative, the hazard its own sibling documents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

uipreliga and others added 9 commits August 12, 2026 11:30

style: apply ruff format to the reachability lint assertion

fdae123

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs(harness): record the all-skipped-run-exits-0 gap found while add…

2dd5e14

…ing --split Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

uipreliga requested review from akshaylive, bai-uipath and tmatup as code owners August 12, 2026 21:36

uipreliga and others added 5 commits August 12, 2026 14:46

This comment was marked as outdated.

Sign in to view

uipreliga and others added 11 commits August 13, 2026 08:41

docs(harness): defer three gaps found reviewing the optimize-skill plan

278e4a6

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

uipreliga and others added 18 commits August 15, 2026 20:27

uipreliga commented Aug 16, 2026

View reviewed changes

uipreliga and others added 11 commits August 16, 2026 14:00

Navigation Menu

Uh oh!

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

feat(plugin): optimize-skill's execution track, measured end to end — plus the skill_triggered and --split fixes it surfaced#109
uipreliga wants to merge 127 commits into
mainfrom
feat/plugin-optimize-skill

uipreliga commented Aug 12, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 12, 2026 •

edited

Loading

Uh oh!

This comment was marked as outdated.

Uh oh!

uipreliga left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

uipreliga commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

The feature

Why the two gates differ (worth a reviewer's eye)

Validation

Three bugs caught before shipping

The tutorial (docs/tutorials/08)

Notes for review

Late changes (after the first review pass)

Late changes (second pass): the execution track measured for real — and a criterion bug it uncovered

⚠️ Behaviour change reviewers should look at

What the run found

Corrections to claims made above

Also in this pass

Third pass — the review's P0/P1/P2 fixes, then a tutorial audit

Two behaviour changes to look at first

The rest

Tutorial audit

Notes for review

Uh oh!

github-actions Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review in Progress

Todo List

Uh oh!

This comment was marked as outdated.

Uh oh!

uipreliga left a comment

Choose a reason for hiding this comment

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

Summary

Blockers

Non-blocking, but please consider before merge

Nits

What's Missing

Harness & Lint Improvements

Top 5 Priority Actions

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

uipreliga commented Aug 12, 2026 •

edited

Loading

github-actions Bot commented Aug 12, 2026 •

edited

Loading